Skip to content

test(amber): trim LoopIntegrationSpec nested cases to 2x2 and tighten the run deadline - #7053

Merged
aglinxinyuan merged 1 commit into
apache:mainfrom
Yicong-Huang:test/loop-spec-speedup
Jul 30, 2026
Merged

test(amber): trim LoopIntegrationSpec nested cases to 2x2 and tighten the run deadline#7053
aglinxinyuan merged 1 commit into
apache:mainfrom
Yicong-Huang:test/loop-spec-speedup

Conversation

@Yicong-Huang

@Yicong-Huang Yicong-Huang commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

LoopIntegrationSpec is the most expensive suite in the amber-integration CI job. Two changes, both spec-only (no engine change):

Change Before After
Nested-loop input rows (iteration counts are data-driven via i < len(table)) 3 rows → 3 outer x 3 inner 2 rows → 2 outer x 2 inner
Per-workflow completion deadline 3 minutes 90 seconds

Every loop iteration re-executes its region and respawns every worker in it (~2s per Python worker), so the two nested cases dominate the suite's cost; 2x2 halves their iterations while keeping the coverage that matters — nesting depth drives the loop_counter/loop_start_id envelope, and each loop level still takes its back-edge at least twice across the run. The single-loop cases keep the 3-row input so one loop takes the same back-edge twice consecutively.

The deadline matters on macOS: the suite intermittently hangs mid-run there (observed in 3 of 4 attempts across two 2026-07-29 runs, e.g. this failed job), burning the whole deadline before withRetry re-runs the test. 90s is ~2.5x the slowest healthy case on the 3-core macOS runner, so a hang now costs ~1.5 min less per occurrence. Root-causing the hang itself is tracked in #7056.

Any related issues, documentation, discussions?

Closes #7052. Suite added in #5700; the macOS hang is tracked in #7056.

How was this PR tested?

  • Existing LoopIntegrationSpec assertions, updated for the 2x2 counts (outer 9→4, inner 3→2), pass in this PR's amber-integration CI job on both ubuntu and macOS.
  • sbt WorkflowExecutionService/scalafmtCheckAll passes locally.

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

Generated-by: Claude Code (Claude Fable 5)

… the run deadline

The two nested-loop cases used a 3-row input, so each ran 3 outer x 3
inner iterations; every iteration re-executes its region and respawns
every worker in it (~2s per Python worker), making the two nested cases
dominate the suite's CI cost. A 2-row input (2x2) keeps the coverage
that matters -- nesting depth drives the loop_counter/loop_start_id
envelope, and each loop level still takes its back-edge at least twice
across the run -- while roughly halving the suite's worker respawns.

Also tighten the per-workflow completion deadline from 3 minutes to 90
seconds (~2.5x the slowest healthy case on the 3-core macOS runner). On
macOS the suite intermittently hangs mid-run and burns the entire
deadline before withRetry re-runs the test, so the loose deadline was
costing ~3 minutes per occurrence on top of the retry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@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.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 791d7df 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 393 0.24 23,826/35,436/35,436 us 🔴 +10.1% / 🔴 +124.6%
bs=100 sw=10 sl=64 795 0.485 125,414/147,122/147,122 us ⚪ within ±5% / 🔴 +37.0%
bs=1000 sw=10 sl=64 903 0.551 1,110,980/1,155,097/1,155,097 us ⚪ within ±5% / 🔴 +13.3%
Baseline details

Latest main 791d7df from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 393 tuples/sec 403 tuples/sec 786.12 tuples/sec -2.5% -50.0%
bs=10 sw=10 sl=64 MB/s 0.24 MB/s 0.246 MB/s 0.48 MB/s -2.4% -50.0%
bs=10 sw=10 sl=64 p50 23,826 us 24,571 us 12,305 us -3.0% +93.6%
bs=10 sw=10 sl=64 p95 35,436 us 32,176 us 15,774 us +10.1% +124.6%
bs=10 sw=10 sl=64 p99 35,436 us 32,176 us 18,978 us +10.1% +86.7%
bs=100 sw=10 sl=64 throughput 795 tuples/sec 816 tuples/sec 999.71 tuples/sec -2.6% -20.5%
bs=100 sw=10 sl=64 MB/s 0.485 MB/s 0.498 MB/s 0.61 MB/s -2.6% -20.5%
bs=100 sw=10 sl=64 p50 125,414 us 122,191 us 100,616 us +2.6% +24.6%
bs=100 sw=10 sl=64 p95 147,122 us 140,649 us 107,356 us +4.6% +37.0%
bs=100 sw=10 sl=64 p99 147,122 us 140,649 us 113,255 us +4.6% +29.9%
bs=1000 sw=10 sl=64 throughput 903 tuples/sec 908 tuples/sec 1,031 tuples/sec -0.6% -12.4%
bs=1000 sw=10 sl=64 MB/s 0.551 MB/s 0.554 MB/s 0.63 MB/s -0.5% -12.5%
bs=1000 sw=10 sl=64 p50 1,110,980 us 1,107,345 us 980,328 us +0.3% +13.3%
bs=1000 sw=10 sl=64 p95 1,155,097 us 1,136,262 us 1,027,528 us +1.7% +12.4%
bs=1000 sw=10 sl=64 p99 1,155,097 us 1,136,262 us 1,054,298 us +1.7% +9.6%
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,508.41,200,128000,393,0.240,23825.77,35435.71,35435.71
1,100,10,64,20,2516.12,2000,1280000,795,0.485,125414.36,147122.46,147122.46
2,1000,10,64,20,22144.52,20000,12800000,903,0.551,1110980.15,1155097.24,1155097.24

@aglinxinyuan aglinxinyuan 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!

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 30, 2026
Merged via the queue into apache:main with commit 75606f1 Jul 30, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Speed up LoopIntegrationSpec in the amber-integration CI job

2 participants