Skip to content

feat(pyamber): report batch processing errors - #8298

Open
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/pyamber-report-batch-error
Open

feat(pyamber): report batch processing errors#8298
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/pyamber-report-batch-error

Conversation

@carloea2

@carloea2 carloea2 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Report per-row MainLoop failures through the worker exception path, enter exception pause, and stop consuming the affected batch.

Before: the worker logged the failure, continued later rows, and could finish with missing output.

After: the coordinator receives the error and the batch stops at the failed row.

Any related issues, documentation, discussions?

Closes #8297

How was this PR tested?

Added a regression covering the failure report, exception pause, and stopped batch consumption.

$env:PYTHONDONTWRITEBYTECODE='1'; C:\Users\carlo\texera\texera\.venv312\Scripts\python.exe -c "import sys,pytest; sys.path[:0]=[r'C:\Users\carlo\texera\texera-worktrees\investigate-bug78\amber\src\main\python',r'C:\Users\carlo\texera\texera\amber\src\main\python']; raise SystemExit(pytest.main([r'amber/src/test/python/core/runnables/test_main_loop.py::TestMainLoop::test_failing_element_reports_exception_and_stops_batch','-p','no:cacheprovider','-q']))"
C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe check amber/src/main/python/core/runnables/main_loop.py amber/src/test/python/core/runnables/test_main_loop.py
C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe format --check amber/src/main/python/core/runnables/main_loop.py amber/src/test/python/core/runnables/test_main_loop.py

The focused regression passed. The full MainLoop test file was also run: 36 tests passed and 8 pre-existing integration cases failed during executor initialization because the shared generated protocol files do not yet contain loop_start_port_uris.

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

Generated-by: Codex

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 31, 2026 17:40
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
⚠️ release/v1.3 Not labeled automatically — none of the files this PR modifies exist on this branch (amber/src/main/python/core/runnables/main_loop.py, amber/src/test/python/core/runnables/test_main_loop.py). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.3 by hand if this fix should be backported here.
release/v1.2 Already labeled — this fix is queued to backport here.

Auto-label run.

@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: @aglinxinyuan
    You can notify them by mentioning @aglinxinyuan in a comment.

@codecov-commenter

codecov-commenter commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.93%. Comparing base (50321e4) to head (2c24b33).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8298   +/-   ##
=========================================
  Coverage     93.93%   93.93%           
  Complexity     4762     4762           
=========================================
  Files          1191     1191           
  Lines         48308    48310    +2     
  Branches       5377     5377           
=========================================
+ Hits          45380    45382    +2     
  Misses         1494     1494           
  Partials       1434     1434           
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 50321e4
agent-service 99.32% <ø> (ø) Carriedforward from 50321e4
amber 90.03% <ø> (ø) Carriedforward from 50321e4
computing-unit-managing-service 73.67% <ø> (ø) Carriedforward from 50321e4
config-service 86.86% <ø> (ø) Carriedforward from 50321e4
file-service 87.91% <ø> (ø) Carriedforward from 50321e4
frontend 96.41% <ø> (ø) Carriedforward from 50321e4
notebook-migration-service 79.31% <ø> (ø) Carriedforward from 50321e4
pyamber 98.87% <100.00%> (+<0.01%) ⬆️
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 50321e4

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

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main cb0b613 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 365 0.223 26,796/34,463/34,463 us 🔴 +20.7% / 🔴 +121.2%
🔴 bs=100 sw=10 sl=64 784 0.478 126,139/164,965/164,965 us 🔴 +7.0% / 🔴 +53.8%
bs=1000 sw=10 sl=64 914 0.558 1,085,778/1,156,456/1,156,456 us ⚪ within ±5% / 🔴 +13.0%
Baseline details

Latest main cb0b613 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 365 tuples/sec 423 tuples/sec 787.98 tuples/sec -13.7% -53.7%
bs=10 sw=10 sl=64 MB/s 0.223 MB/s 0.258 MB/s 0.481 MB/s -13.6% -53.6%
bs=10 sw=10 sl=64 p50 26,796 us 22,200 us 12,593 us +20.7% +112.8%
bs=10 sw=10 sl=64 p95 34,463 us 32,327 us 15,579 us +6.6% +121.2%
bs=10 sw=10 sl=64 p99 34,463 us 32,327 us 18,786 us +6.6% +83.4%
bs=100 sw=10 sl=64 throughput 784 tuples/sec 829 tuples/sec 1,008 tuples/sec -5.4% -22.2%
bs=100 sw=10 sl=64 MB/s 0.478 MB/s 0.506 MB/s 0.615 MB/s -5.5% -22.3%
bs=100 sw=10 sl=64 p50 126,139 us 117,845 us 100,701 us +7.0% +25.3%
bs=100 sw=10 sl=64 p95 164,965 us 161,436 us 107,244 us +2.2% +53.8%
bs=100 sw=10 sl=64 p99 164,965 us 161,436 us 116,122 us +2.2% +42.1%
bs=1000 sw=10 sl=64 throughput 914 tuples/sec 926 tuples/sec 1,042 tuples/sec -1.3% -12.3%
bs=1000 sw=10 sl=64 MB/s 0.558 MB/s 0.565 MB/s 0.636 MB/s -1.2% -12.3%
bs=1000 sw=10 sl=64 p50 1,085,778 us 1,080,403 us 981,959 us +0.5% +10.6%
bs=1000 sw=10 sl=64 p95 1,156,456 us 1,142,950 us 1,023,080 us +1.2% +13.0%
bs=1000 sw=10 sl=64 p99 1,156,456 us 1,142,950 us 1,051,697 us +1.2% +10.0%
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,548.07,200,128000,365,0.223,26796.13,34462.64,34462.64
1,100,10,64,20,2551.71,2000,1280000,784,0.478,126138.83,164965.31,164965.31
2,1000,10,64,20,21887.03,20000,12800000,914,0.558,1085778.27,1156455.70,1156455.70

@carloea2 carloea2 changed the title fix(pyamber): report batch processing errors feat(pyamber): report batch processing errors Aug 31, 2026
@mengw15 mengw15 removed the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@xuang7
xuang7 removed their request for review September 1, 2026 01:37
@xuang7 xuang7 removed the fix label Sep 1, 2026
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.

Python worker can hide batch processing failures

5 participants