Skip to content

refactor(amber): remove the unused BackpressurePause - #8688

Merged
aglinxinyuan merged 1 commit into
apache:mainfrom
aglinxinyuan:refactor/backpressure-pause
Sep 26, 2026
Merged

aglinxinyuan merged 1 commit into
apache:mainfrom
aglinxinyuan:refactor/backpressure-pause

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Deletes BackpressurePause. It is a PauseType that no production code has passed to PauseManager since flow control moved onto ActorMessage. There is no behaviour change: +11/−19 lines.

History

Introduced by #1636 (2022-08-20), "Introduce types of pause in Amber". Backpressure paused the worker through PauseManager under its own pause type
Usage removed by #2237 (2023-12-02), "Use ActorMessage for flow control". It deleted the pauseManager.pause(BackpressurePause) / resume(BackpressurePause) calls

It has been dead for nearly three years. Backpressure still works, but it now bypasses PauseManager: Backpressure(enabled) arrives as an ActorCommand and flips DPThread.backpressureStatus. #4533 removed the sibling SchedulerTimeSlotExpiredPause for the same reason.

Reviewer note: the specs change in two places, and both are fixture swaps, not lost coverage.

  • In PauseTypeSpec, the singleton / identity / pattern-match / Set cases now cover the remaining three kinds.
  • The two WorkerManagersSpec PauseManager cases used BackpressurePause only as "some other pause type". They now use what production actually passes: OperatorLogicPause for a global pause (as DataProcessor does) and ECMPause for a per-channel pause (as ECM alignment does).

Any related issues, documentation, discussions?

Closes #8686

How was this PR tested?

No new tests. The two existing specs keep their cases with the fixtures swapped.

Locally, from the repo root with Java 17:

  • sbt "WorkflowExecutionService/Test/compile": success.
  • sbt "WorkflowExecutionService/testOnly *PauseTypeSpec *WorkerManagersSpec": 27 tests, all pass.
  • sbt "WorkflowExecutionService/scalafmtCheckAll" "WorkflowExecutionService/scalafixAll --check": clean.

To re-check:

git grep -n BackpressurePause   # no hits

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5.5)

🤖 Generated with Claude Code

BackpressurePause has had no production caller since apache#2237 moved flow
control onto ActorMessage: backpressure now toggles a flag in DPThread
and never goes through PauseManager. Only the PauseType and
WorkerManagers specs still referenced it; their PauseManager cases now
use the pause types production actually passes (OperatorLogicPause as a
global pause, ECMPause as a per-channel pause).
Copilot AI lite review requested due to automatic review settings September 26, 2026 08:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang, @Ma77Ball
    You can notify them by mentioning @Yicong-Huang, @Ma77Ball in a comment.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 4 better · 🔴 3 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 33bd07b benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 383 0.233 24,993/31,659/31,659 us 🔴 +10.6% / 🔴 +89.7%
🟢 bs=100 sw=10 sl=64 805 0.491 124,684/136,774/136,774 us 🟢 -5.8% / 🔴 +22.0%
⚪ bs=1000 sw=10 sl=64 909 0.555 1,101,138/1,137,272/1,137,272 us ⚪ within ±5% / 🔴 +6.7%
Baseline details

Latest main 33bd07b from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 383 tuples/sec 415 tuples/sec 736.99 tuples/sec -7.7% -48.0%
bs=10 sw=10 sl=64 MB/s 0.233 MB/s 0.253 MB/s 0.45 MB/s -7.9% -48.2%
bs=10 sw=10 sl=64 p50 24,993 us 22,593 us 13,174 us +10.6% +89.7%
bs=10 sw=10 sl=64 p95 31,659 us 34,954 us 16,900 us -9.4% +87.3%
bs=10 sw=10 sl=64 p99 31,659 us 34,954 us 19,889 us -9.4% +59.2%
bs=100 sw=10 sl=64 throughput 805 tuples/sec 813 tuples/sec 946.47 tuples/sec -1.0% -14.9%
bs=100 sw=10 sl=64 MB/s 0.491 MB/s 0.496 MB/s 0.578 MB/s -1.0% -15.0%
bs=100 sw=10 sl=64 p50 124,684 us 120,867 us 105,257 us +3.2% +18.5%
bs=100 sw=10 sl=64 p95 136,774 us 145,177 us 112,150 us -5.8% +22.0%
bs=100 sw=10 sl=64 p99 136,774 us 145,177 us 125,962 us -5.8% +8.6%
bs=1000 sw=10 sl=64 throughput 909 tuples/sec 914 tuples/sec 973.64 tuples/sec -0.5% -6.6%
bs=1000 sw=10 sl=64 MB/s 0.555 MB/s 0.558 MB/s 0.594 MB/s -0.5% -6.6%
bs=1000 sw=10 sl=64 p50 1,101,138 us 1,085,086 us 1,032,217 us +1.5% +6.7%
bs=1000 sw=10 sl=64 p95 1,137,272 us 1,158,909 us 1,072,497 us -1.9% +6.0%
bs=1000 sw=10 sl=64 p99 1,137,272 us 1,158,909 us 1,102,623 us -1.9% +3.1%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,522.84,200,128000,383,0.233,24992.72,31659.45,31659.45
1,100,10,64,20,2483.69,2000,1280000,805,0.491,124683.61,136773.92,136773.92
2,1000,10,64,20,21997.44,20000,12800000,909,0.555,1101137.88,1137272.09,1137272.09

@aglinxinyuan
aglinxinyuan requested a review from kunwp1 September 26, 2026 08:25
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.72%. Comparing base (33bd07b) to head (c5e5436).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8688      +/-   ##
============================================
- Coverage     92.72%   92.72%   -0.01%     
  Complexity     4947     4947              
============================================
  Files          1243     1243              
  Lines         52682    52681       -1     
  Branches       6520     6520              
============================================
- Hits          48850    48849       -1     
  Misses         2214     2214              
  Partials       1618     1618              
Flag Coverage Δ *Carryforward flag
access-control-service 77.38% <ø> (ø) Carriedforward from 33bd07b
agent-service 99.16% <ø> (ø) Carriedforward from 33bd07b
amber 88.45% <ø> (-0.01%) ⬇️
computing-unit-managing-service 60.41% <ø> (ø) Carriedforward from 33bd07b
config-service 87.37% <ø> (ø) Carriedforward from 33bd07b
file-service 81.53% <ø> (ø) Carriedforward from 33bd07b
frontend 96.53% <ø> (ø) Carriedforward from 33bd07b
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 33bd07b
pyamber 98.56% <ø> (ø) Carriedforward from 33bd07b
workflow-compiling-service 74.09% <ø> (ø) Carriedforward from 33bd07b

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved review issues remain.

Review effort: Lite
Findings: None

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Sep 26, 2026
Merged via the queue into apache:main with commit d102e26 Sep 26, 2026
26 checks passed
@aglinxinyuan
aglinxinyuan deleted the refactor/backpressure-pause branch September 26, 2026 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine refactor Refactor the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove the unused BackpressurePause

4 participants