You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/07/25 07:47:19 UTC

[PR] test(amber): trim subsumed loop-body e2e cases to speed up amber-integration [texera]

aglinxinyuan opened a new pull request, #6884:
URL: https://github.com/apache/texera/pull/6884

   ### What changes were proposed in this PR?
   
   `LoopIntegrationSpec` costs **~4m16s of the ~6-minute `amber-integration` test step (~70%)** on every PR. The cost tracks worker spawns almost perfectly (~2s per Python worker boot + Flight handshake; 132 spawns across the suite), because every loop iteration respawns each worker in the re-executed regions:
   
   | Test | Time | Worker spawns (py + jvm) |
   |---|---:|---:|
   | single loop (Python-only) | 17s | 6 + 1 |
   | nested 3×3 (Python-only) | 63s | 24 + 1 |
   | single + Limit | 19s | 6 + 4 |
   | nested + Limit | 64s | 24 + 10 |
   | nested + chain (Limit→Sleep) | 72s | 24 + 19 |
   | single + chain | 21s | 6 + 7 |
   
   Two cases are **strict subsets** of the nested-chain case, which already covers nested routing, counter magnitude across JVM hops, the JVM→JVM state handoff, and both operator kinds in one workflow:
   
   - `nested + Limit` (~64s) — nested × Limit ⊂ nested × (Limit → Sleep)
   - `single + chain` (~21s) — chain ⊂ nested chain
   
   This PR removes those two, saving **~85s (~33% of the loop suite)** with no unique coverage lost. Retained matrix: Python-only single (base), Python-only nested (pins the no-JVM path), single + Limit (verbatim repro of the #6660 bug), nested + chain (superset JVM case). The subsumption reasoning is recorded in the retained test's comment so the cases don't get re-added.
   
   The deeper fix (reusing workers across loop iterations instead of respawn-per-jump) is an engine design change and out of scope here.
   
   ### Any related issues, documentation, discussions?
   
   Follow-up to #6661 (which introduced the JVM loop-body cases). Related engine context: #6660.
   
   ### How was this PR tested?
   
   Test-only change (removes two e2e cases; no source touched). `WorkflowExecutionService/Test/compile` + `scalafmtCheckAll` + `scalafixAll --check` pass locally (Java 17); the remaining suite runs in the `amber-integration` CI job on this PR.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Fable 5)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(amber): trim subsumed loop-body e2e cases to speed up amber-integration [texera]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #6884:
URL: https://github.com/apache/texera/pull/6884#issuecomment-5077585218

   <!-- texera-benchmarks-comment -->
   ## ⚠️ Benchmark changes need a look
   
   🟢 2 better · 🔴 4 worse · ⚪ 9 noise (<±5%) · 0 without baseline
   
   > Compared against main `52584a8` 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](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/30149939278)
   
   | | config | throughput | MB/s | latency | max Δ latest / 7d |
   |---|---|---:|---:|---:|---:|
   | 🔴 | bs=10 sw=10 sl=64 | 443 | 0.27 | 22,371/31,160/31,160 us | 🟢 -13.0% / 🔴 +87.8% |
   | 🔴 | bs=100 sw=10 sl=64 | 931 | 0.568 | 104,701/130,470/130,470 us | 🔴 -5.8% / 🔴 +19.6% |
   | ⚪ | bs=1000 sw=10 sl=64 | 1,112 | 0.679 | 892,382/946,962/946,962 us | ⚪ within ±5% / 🟢 -11.8% |
   
   <details><summary>Baseline details</summary>
   
   Latest main `52584a8` from same runner
   
   | config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
   |---|---|---:|---:|---:|---:|---:|
   | bs=10 sw=10 sl=64 | throughput | 443 tuples/sec | 438 tuples/sec | 754.55 tuples/sec | +1.1% | -41.3% |
   | bs=10 sw=10 sl=64 | MB/s | 0.27 MB/s | 0.268 MB/s | 0.461 MB/s | +0.7% | -41.4% |
   | bs=10 sw=10 sl=64 | p50 | 22,371 us | 21,261 us | 12,816 us | +5.2% | +74.6% |
   | bs=10 sw=10 sl=64 | p95 | 31,160 us | 35,821 us | 16,594 us | -13.0% | +87.8% |
   | bs=10 sw=10 sl=64 | p99 | 31,160 us | 35,821 us | 19,806 us | -13.0% | +57.3% |
   | bs=100 sw=10 sl=64 | throughput | 931 tuples/sec | 988 tuples/sec | 969.38 tuples/sec | -5.8% | -4.0% |
   | bs=100 sw=10 sl=64 | MB/s | 0.568 MB/s | 0.603 MB/s | 0.592 MB/s | -5.8% | -4.0% |
   | bs=100 sw=10 sl=64 | p50 | 104,701 us | 98,981 us | 103,584 us | +5.8% | +1.1% |
   | bs=100 sw=10 sl=64 | p95 | 130,470 us | 130,386 us | 109,097 us | +0.1% | +19.6% |
   | bs=100 sw=10 sl=64 | p99 | 130,470 us | 130,386 us | 117,304 us | +0.1% | +11.2% |
   | bs=1000 sw=10 sl=64 | throughput | 1,112 tuples/sec | 1,113 tuples/sec | 1,004 tuples/sec | -0.1% | +10.8% |
   | bs=1000 sw=10 sl=64 | MB/s | 0.679 MB/s | 0.679 MB/s | 0.613 MB/s | 0.0% | +10.8% |
   | bs=1000 sw=10 sl=64 | p50 | 892,382 us | 882,660 us | 1,002,357 us | +1.1% | -11.0% |
   | bs=1000 sw=10 sl=64 | p95 | 946,962 us | 980,448 us | 1,046,463 us | -3.4% | -9.5% |
   | bs=1000 sw=10 sl=64 | p99 | 946,962 us | 980,448 us | 1,073,661 us | -3.4% | -11.8% |
   
   </details>
   
   <details><summary>Raw CSV</summary>
   
   ```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,451.94,200,128000,443,0.270,22370.92,31160.13,31160.13
   1,100,10,64,20,2148.16,2000,1280000,931,0.568,104700.97,130469.73,130469.73
   2,1000,10,64,20,17983.79,20000,12800000,1112,0.679,892381.87,946962.35,946962.35
   ```
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(amber): trim subsumed loop-body e2e cases to speed up amber-integration [texera]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #6884:
URL: https://github.com/apache/texera/pull/6884#issuecomment-5077568332

   <!-- texera-reviewer-suggestion -->
   ### 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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] test(amber): trim subsumed loop-body e2e cases to speed up amber-integration [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.

aglinxinyuan merged PR #6884:
URL: https://github.com/apache/texera/pull/6884


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org