Skip to content

test(amber): cover workflow lifecycle cleanup - #7151

Merged
aglinxinyuan merged 2 commits into
apache:mainfrom
aglinxinyuan:test-workflow-lifecycle-manager
Jul 31, 2026
Merged

test(amber): cover workflow lifecycle cleanup#7151
aglinxinyuan merged 2 commits into
apache:mainfrom
aglinxinyuan:test-workflow-lifecycle-manager

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds WorkflowLifecycleManagerSpec for the lifecycle manager's externally observable cleanup behavior.

Scenario Observable contract
Multiple users Cleanup waits until the final user disconnects.
Workflow state A RUNNING update cancels cleanup; a terminal update schedules it again.
Repeated terminal updates A later update replaces the older deadline.

The suite uses an isolated Pekko actor system and restores AmberRuntime's shared actor-system reference after the suite, so other Amber suites retain their runtime setup.

The assertions were mutation-tested against temporary production changes, all reverted before commit:

Temporary mutation Result
userCount == 0 to userCount < 0 1 test failed
status == RUNNING to status != RUNNING 2 tests failed
Remove cancellation of an active cleanup deadline 1 test failed

Any related issues, documentation, discussions?

Closes #7150

How was this PR tested?

"C:/Program Files (x86)/sbt/bin/sbt" -java-home "C:/Users/linxi/.jdks/jbr-17.0.14" "WorkflowExecutionService/testOnly org.apache.texera.web.WorkflowLifecycleManagerSpec"
Tests: succeeded 3, failed 0
"C:/Program Files (x86)/sbt/bin/sbt" -java-home "C:/Users/linxi/.jdks/jbr-17.0.14" "WorkflowExecutionService/Test/scalafmtCheck" "WorkflowExecutionService/Test/scalafix --check"

Both checks completed successfully.

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

Generated-by: Codex (GPT-5)

@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:

  • No candidates found from git blame history.

@aglinxinyuan
aglinxinyuan marked this pull request as ready for review July 31, 2026 02:51
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 6 worse · ⚪ 9 noise (<±5%) · 0 without baseline

Compared against main 91ed98d 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 565 0.345 17,086/25,570/25,570 us 🔴 +11.7% / 🔴 +57.9%
🔴 bs=100 sw=10 sl=64 1,190 0.727 80,445/111,486/111,486 us 🔴 +13.5% / 🟢 +23.4%
🔴 bs=1000 sw=10 sl=64 1,416 0.864 698,985/817,941/817,941 us 🔴 +9.6% / 🟢 +42.7%
Baseline details

Latest main 91ed98d from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 565 tuples/sec 577 tuples/sec 765.25 tuples/sec -2.1% -26.2%
bs=10 sw=10 sl=64 MB/s 0.345 MB/s 0.352 MB/s 0.467 MB/s -2.0% -26.1%
bs=10 sw=10 sl=64 p50 17,086 us 16,633 us 12,703 us +2.7% +34.5%
bs=10 sw=10 sl=64 p95 25,570 us 22,893 us 16,199 us +11.7% +57.9%
bs=10 sw=10 sl=64 p99 25,570 us 22,893 us 19,557 us +11.7% +30.7%
bs=100 sw=10 sl=64 throughput 1,190 tuples/sec 1,243 tuples/sec 965.22 tuples/sec -4.3% +23.3%
bs=100 sw=10 sl=64 MB/s 0.727 MB/s 0.759 MB/s 0.589 MB/s -4.2% +23.4%
bs=100 sw=10 sl=64 p50 80,445 us 77,795 us 103,830 us +3.4% -22.5%
bs=100 sw=10 sl=64 p95 111,486 us 98,235 us 110,778 us +13.5% +0.6%
bs=100 sw=10 sl=64 p99 111,486 us 98,235 us 118,713 us +13.5% -6.1%
bs=1000 sw=10 sl=64 throughput 1,416 tuples/sec 1,446 tuples/sec 992.33 tuples/sec -2.1% +42.7%
bs=1000 sw=10 sl=64 MB/s 0.864 MB/s 0.883 MB/s 0.606 MB/s -2.2% +42.7%
bs=1000 sw=10 sl=64 p50 698,985 us 691,759 us 1,016,959 us +1.0% -31.3%
bs=1000 sw=10 sl=64 p95 817,941 us 746,141 us 1,062,501 us +9.6% -23.0%
bs=1000 sw=10 sl=64 p99 817,941 us 746,141 us 1,082,611 us +9.6% -24.4%
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,354.22,200,128000,565,0.345,17085.53,25569.89,25569.89
1,100,10,64,20,1680.00,2000,1280000,1190,0.727,80445.13,111485.97,111485.97
2,1000,10,64,20,14122.59,20000,12800000,1416,0.864,698984.87,817941.21,817941.21

@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.53%. Comparing base (91ed98d) to head (784191b).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7151      +/-   ##
============================================
+ Coverage     79.58%   80.53%   +0.94%     
- Complexity     3837     3900      +63     
============================================
  Files          1160     1160              
  Lines         46188    46188              
  Branches       5145     5145              
============================================
+ Hits          36761    37199     +438     
+ Misses         7795     7311     -484     
- Partials       1632     1678      +46     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 4f24bb6
agent-service 77.42% <ø> (ø) Carriedforward from 4f24bb6
amber 75.67% <ø> (+2.57%) ⬆️
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from 4f24bb6
config-service 65.97% <ø> (ø) Carriedforward from 4f24bb6
file-service 66.80% <ø> (ø) Carriedforward from 4f24bb6
frontend 83.08% <ø> (ø) Carriedforward from 4f24bb6
notebook-migration-service 78.89% <ø> (ø) Carriedforward from 4f24bb6
pyamber 97.31% <ø> (ø) Carriedforward from 4f24bb6
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from 4f24bb6

*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.

@mengw15 mengw15 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.

LGTM

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.

Pull request overview

Adds a new Amber unit test suite to validate WorkflowLifecycleManager’s externally observable cleanup contract (user-count gating, cancellation while RUNNING, and deadline refresh on repeated terminal updates), using a suite-local Pekko ActorSystem wired into AmberRuntime for scheduler-driven assertions.

Changes:

  • Introduces WorkflowLifecycleManagerSpec with three scenarios covering user-count tracking and cleanup scheduling behavior.
  • Uses ExecutionStateStore state updates to drive the lifecycle manager’s state-change subscription path.
  • Temporarily swaps AmberRuntime’s global actor-system reference for deterministic scheduling during the suite, then restores it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
@aglinxinyuan
aglinxinyuan enabled auto-merge July 31, 2026 23:26
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 31, 2026
Merged via the queue into apache:main with commit 0783470 Jul 31, 2026
21 checks passed
@aglinxinyuan
aglinxinyuan deleted the test-workflow-lifecycle-manager branch July 31, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for workflow lifecycle cleanup

4 participants