test(amber): pin the previous run's storage-registry clean-up

[aglinxinyuan]

What changes were proposed in this PR?

clearExecutionResources -- the method that wipes the previous run's storage registry before a new execution starts -- was entered by no test. It was unreachable only by accident: SyncExecutionResourceSpec:145-151 documents parking its fixtures under a second computing unit precisely to avoid this path, so the recipe existed and merely needed inverting.

Tests 8 -> 10. Lines 76/96 (79.2%) -> 84/96 (87.5%), branches 9/14, measured with only this spec running.

The new test drives the real initExecutionService path under MockTexeraDB (zonky EmbeddedPostgres, no Docker), steering executeWorkflow() into a compile failure so it returns before any runtime is created, and asserts the registry rows for the previous execution are deleted -- scoped to that workflow and that computing unit, for the latest execution only.

This PR is small on coverage and I would rather say so than dress it up: 8 lines. What makes it worth landing is the contract, which is currently untested and easy to break silently, and the two defects the verification turned up.

Verification

Review proposed 20 mutations. Every one survived the baseline suite (one exception noted below). 19 are now killed, each verified red on the named test; one is recorded as unpinnable.

The recurring cause was a degenerate fixture -- every id was the same number:

Surviving mutation	Why nothing noticed
WID.eq(cuid).and(CUID.eq(wid)) transposed	wid == uid == cuid == 9411 made the mutated SQL byte-identical
drop the WID leg	only one workflow ran on the unit
Some(executions.max) -> .min	the unit owned exactly one execution
delete .eq -> .le on execution id	no execution existed below the one under test
req.computingUnitId -> the service's own field	request and service carried the same id
executionName / engineVersion at the insert site	the inserted row was never read back
drop registerCleanUpOnStateChange	the stubbed lifecycle manager made it unobservable

Fixed by giving every domain its own literal (wid 9411, otherWid 9412, uid 9413, cuid 9414, otherCuid 9415), seeding a second workflow on the same unit, an older execution below the one under test, and reading the newly inserted row back.

Two defects the verification exposed

The test was cementing a bug. Guarding the clean-up on the previous execution being terminal -- a defensible production fix -- killed the pre-repair test, because the fixture persisted a non-terminal previous execution and asserted it was wiped. That test would have blocked the fix. The fixture now uses a terminal previous execution, so the guard can be added without this spec fighting it.

The suite was doing a real S3 delete. clearExecutionResources calls LargeBinaryManager.deleteByExecution, which reaches S3StorageClient against http://localhost:9000. With SERIAL ids the fixture's execution got eid 1 -- LargeBinaryManager's DEFAULT_EXECUTION_ID sentinel -- so on a dev box running bin/local-dev.sh up the test recursively deleted objects/1/ in MinIO. The baseline log shows it. Explicit non-1 eids fix the blast radius; the call itself still attempts a connection (now to a nonexistent prefix), which cannot be avoided without a production seam.

One mutation is left alive

Deleting line 406 (LargeBinaryManager.deleteByExecution) survives, and is recorded in the spec header rather than counted as covered. The injectable overload is private[util], so the call site cannot use it, and asserting the S3 effect would mean asserting the ambient environment.

Deliberately not included

The fault-tolerance block: gated on ApplicationConfig.faultToleranceLogRootFolder, a val in a Scala object read at object-init with no seam, which ApplicationConfigSpec:90 asserts is None. The replay block: its values are consumed only past createAmberRuntime, which the compile failure guarantees is never reached, so it is pinnable only by reflecting into a private field.

Also reported, not tested: lastCompletedLogicalPlan (line 163) is a write-only var -- a repo-wide grep finds only its declaration and its single write, no readers.

Worth recording for future coverage work on this file: JaCoCo's SyntheticFilter drops Scala's $anonfun$ methods, so every lambda body here -- the lifecycle cleanup callback, the completion diff-handler, the errorHandler, both cleanup loops, the replay foreach -- is invisible to the coverage number. Roughly 30 lines where a test moves it by exactly zero.

No production file is touched.

Any related issues, documentation, discussions?

Closes #7711

How was this PR tested?

STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly org.apache.texera.web.service.WorkflowServiceSpec"

[info] Total number of tests run: 10
[info] Tests: succeeded 10, failed 0, canceled 0, ignored 0, pending 0

Test/scalafmtCheck and Test/scalafix --check both pass.

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

Generated-by: Claude Code (Opus 5)

[Copilot]

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

[github-actions]

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.

[github-actions]

⚠️ Benchmark changes need a look

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

Compared against main 1c4662b 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	379	0.231	25,658/31,488/31,488 us	🔴 +9.2% / 🔴 +103.2%
🔴	bs=100 sw=10 sl=64	799	0.488	122,064/177,490/177,490 us	🔴 +36.3% / 🔴 +68.1%
⚪	bs=1000 sw=10 sl=64	993	0.606	1,012,737/1,087,122/1,087,122 us	⚪ within ±5% / 🔴 +7.4%

Baseline details

Latest main 1c4662b from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	379 tuples/sec	398 tuples/sec	784.16 tuples/sec	-4.8%	-51.7%
bs=10 sw=10 sl=64	MB/s	0.231 MB/s	0.243 MB/s	0.479 MB/s	-4.9%	-51.7%
bs=10 sw=10 sl=64	p50	25,658 us	23,487 us	12,626 us	+9.2%	+103.2%
bs=10 sw=10 sl=64	p95	31,488 us	34,454 us	16,169 us	-8.6%	+94.7%
bs=10 sw=10 sl=64	p99	31,488 us	34,454 us	18,986 us	-8.6%	+65.8%
bs=100 sw=10 sl=64	throughput	799 tuples/sec	890 tuples/sec	1,023 tuples/sec	-10.2%	-21.9%
bs=100 sw=10 sl=64	MB/s	0.488 MB/s	0.543 MB/s	0.625 MB/s	-10.1%	-21.9%
bs=100 sw=10 sl=64	p50	122,064 us	110,925 us	99,185 us	+10.0%	+23.1%
bs=100 sw=10 sl=64	p95	177,490 us	130,261 us	105,616 us	+36.3%	+68.1%
bs=100 sw=10 sl=64	p99	177,490 us	130,261 us	113,681 us	+36.3%	+56.1%
bs=1000 sw=10 sl=64	throughput	993 tuples/sec	1,020 tuples/sec	1,057 tuples/sec	-2.6%	-6.0%
bs=1000 sw=10 sl=64	MB/s	0.606 MB/s	0.623 MB/s	0.645 MB/s	-2.7%	-6.0%
bs=1000 sw=10 sl=64	p50	1,012,737 us	976,278 us	965,435 us	+3.7%	+4.9%
bs=1000 sw=10 sl=64	p95	1,087,122 us	1,110,122 us	1,012,135 us	-2.1%	+7.4%
bs=1000 sw=10 sl=64	p99	1,087,122 us	1,110,122 us	1,042,088 us	-2.1%	+4.3%

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,527.44,200,128000,379,0.231,25658.12,31488.35,31488.35
1,100,10,64,20,2503.42,2000,1280000,799,0.488,122064.26,177489.51,177489.51
2,1000,10,64,20,20141.28,20000,12800000,993,0.606,1012736.54,1087121.59,1087121.59

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.95%. Comparing base (1c4662b) to head (726c4cf).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7712      +/-   ##
============================================
+ Coverage     90.93%   90.95%   +0.01%     
- Complexity     4450     4452       +2     
============================================
  Files          1174     1174              
  Lines         47139    47139              
  Branches       5284     5284              
============================================
+ Hits          42867    42873       +6     
+ Misses         2579     2574       -5     
+ Partials       1693     1692       -1     

Flag	Coverage Δ		*Carryforward flag
access-control-service	81.00% <ø> (ø)		Carriedforward from 1c4662b
agent-service	98.62% <ø> (ø)		Carriedforward from 1c4662b
amber	87.43% <ø> (+0.03%)	⬆️
computing-unit-managing-service	73.67% <ø> (ø)		Carriedforward from 1c4662b
config-service	86.73% <ø> (ø)		Carriedforward from 1c4662b
file-service	68.90% <ø> (ø)		Carriedforward from 1c4662b
frontend	92.43% <ø> (ø)		Carriedforward from 1c4662b
notebook-migration-service	83.74% <ø> (ø)		Carriedforward from 1c4662b
pyamber	97.57% <ø> (ø)		Carriedforward from 1c4662b
workflow-compiling-service	77.19% <ø> (ø)		Carriedforward from 1c4662b

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