feat(pyamber): drain shuffle flush batches by carloea2 · Pull Request #8242 · apache/texera

carloea2 · 2026-08-31T04:51:47Z

What changes were proposed in this PR?

Drain emitted shuffle batches in the shared base class. Remove the flush-clearing flag and the stale round-robin comment. The regression checks that flushing twice emits only the control message on the second flush and preserves another receiver's pending rows.

Any related issues, documentation, discussions?

Closes #8241

How was this PR tested?

Retested with Python 3.12 in a fresh detached worktree from upstream main at 6db4a642d0. All 37 partitioner tests pass. The original regression was reproduced before the implementation; the current shared-base refactor preserves it.

From amber/, after generating protobuf modules:

python -m pytest src/test/python/core/architecture/sendsemantics/test_partitioners.py -q --tb=short -p no:cacheprovider
python -m ruff check src/main/python src/test/python
python -m ruff format --check src/main/python src/test/python

Ruff lint and formatting pass for all 218 Python files.

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

Generated-by: Codex

github-actions · 2026-08-31T04:52:01Z

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/architecture/sendsemantics/hash_based_shuffle_partitioner.py`, `amber/src/main/python/core/architecture/sendsemantics/range_based_shuffle_partitioner.py`, `amber/src/test/python/core/architecture/sendsemantics/test_partitioners.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 · 2026-08-31T04:52:04Z

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

Contributors with relevant context: @Ma77Ball
You can notify them by mentioning @Ma77Ball in a comment.

codecov-commenter · 2026-08-31T04:56:03Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.87%. Comparing base (98c6337) to head (59e8132).

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #8242   +/-   ##
=========================================
  Coverage     91.87%   91.87%           
  Complexity     5049     5049           
=========================================
  Files          1265     1265           
  Lines         54915    54911    -4     
  Branches       6940     6938    -2     
=========================================
- Hits          50453    50450    -3     
  Misses         2769     2769           
+ Partials       1693     1692    -1

Flag	Coverage Δ		*Carryforward flag
access-control-service	`77.38% <ø> (ø)`		Carriedforward from 98c6337
agent-service	`99.16% <ø> (ø)`		Carriedforward from 98c6337
amber	`86.06% <ø> (ø)`		Carriedforward from 98c6337
computing-unit-managing-service	`60.48% <ø> (ø)`		Carriedforward from 98c6337
config-service	`87.37% <ø> (ø)`		Carriedforward from 98c6337
file-service	`82.00% <ø> (ø)`		Carriedforward from 98c6337
frontend	`96.50% <ø> (ø)`		Carriedforward from 98c6337
notebook-migration-service	`83.73% <ø> (ø)`		Carriedforward from 98c6337
pyamber	`98.56% <100.00%> (+0.01%)`	⬆️
workflow-compiling-service	`72.43% <ø> (ø)`		Carriedforward from 98c6337

*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 · 2026-08-31T04:58:27Z

⚠️ Benchmark changes need a look

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

Compared against main 98c6337 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	510	0.311	19,214/25,088/25,088 us	🔴 +17.3% / 🔴 +76.3%
🔴	bs=100 sw=10 sl=64	1,200	0.732	81,840/101,990/101,990 us	🔴 +14.2% / 🟢 -8.0%
⚪	bs=1000 sw=10 sl=64	1,452	0.886	680,285/775,822/775,822 us	⚪ within ±5% / 🟢 +22.9%

Baseline details

Latest main 98c6337 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	510 tuples/sec	582 tuples/sec	879.16 tuples/sec	-12.4%	-42.0%
bs=10 sw=10 sl=64	MB/s	0.311 MB/s	0.355 MB/s	0.537 MB/s	-12.4%	-42.0%
bs=10 sw=10 sl=64	p50	19,214 us	16,374 us	11,504 us	+17.3%	+67.0%
bs=10 sw=10 sl=64	p95	25,088 us	23,112 us	14,234 us	+8.6%	+76.3%
bs=10 sw=10 sl=64	p99	25,088 us	23,112 us	17,338 us	+8.6%	+44.7%
bs=100 sw=10 sl=64	throughput	1,200 tuples/sec	1,283 tuples/sec	1,147 tuples/sec	-6.5%	+4.6%
bs=100 sw=10 sl=64	MB/s	0.732 MB/s	0.783 MB/s	0.7 MB/s	-6.5%	+4.5%
bs=100 sw=10 sl=64	p50	81,840 us	77,201 us	88,987 us	+6.0%	-8.0%
bs=100 sw=10 sl=64	p95	101,990 us	89,323 us	96,473 us	+14.2%	+5.7%
bs=100 sw=10 sl=64	p99	101,990 us	89,323 us	102,791 us	+14.2%	-0.8%
bs=1000 sw=10 sl=64	throughput	1,452 tuples/sec	1,482 tuples/sec	1,182 tuples/sec	-2.0%	+22.9%
bs=1000 sw=10 sl=64	MB/s	0.886 MB/s	0.904 MB/s	0.721 MB/s	-2.0%	+22.8%
bs=1000 sw=10 sl=64	p50	680,285 us	673,288 us	872,751 us	+1.0%	-22.1%
bs=1000 sw=10 sl=64	p95	775,822 us	743,492 us	910,151 us	+4.3%	-14.8%
bs=1000 sw=10 sl=64	p99	775,822 us	743,492 us	941,479 us	+4.3%	-17.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,392.12,200,128000,510,0.311,19214.21,25088.34,25088.34
1,100,10,64,20,1666.87,2000,1280000,1200,0.732,81839.89,101990.39,101990.39
2,1000,10,64,20,13770.13,20000,12800000,1452,0.886,680285.31,775822.36,775822.36

Yicong-Huang · 2026-08-31T20:09:06Z

I am converting this PR to a draft: please follow our PR template and mark it ready for review.

carloea2 · 2026-08-31T20:18:26Z

The description now follows the current template, and this is ready for review.

carloea2 · 2026-10-15T22:28:06Z

/request-review @Yicong-Huang

Could you review this shuffle flush change? All 37 focused tests pass locally, and CI is green.

carloea2 · 2026-10-20T23:15:06Z

/request-review @sshiv012 @parshimers

Yicong-Huang

🟡 0 must-fix · 2 advisory · 1 polish: correct, minimal fix for the duplicate-on-flush bug; notes are about the now-dead flag and the tests.

Simplifications (2)

hash_based_shuffle_partitioner.py:31: every subclass now sets the flag, so its False default only preserves the bug (advisory, see inline)
test_partitioners.py:299: duplicates the assertion at :292 and pins only internal state (advisory, see inline)

Polish: 1 quick touch-up (see inline comments).

Yicong-Huang

🟢 3 resolved · 0 open · 0 new (0 new = 0 newly introduced · 0 late catches)

0 must-fix · 0 advisory · 0 polish: all three round-1 notes are fixed in the tree. The base class now always drains flushed batches, and the double-flush test pins the #8241 behavior.

carloea2 · 2026-10-25T01:57:41Z

@Yicong-Huang, your latest review confirms all three comments are addressed, and all required checks pass. Could you approve and merge if satisfied?

carloea2 · 2026-10-25T02:03:39Z

/request-review @Yicong-Huang

Yicong-Huang

🟢 3 resolved · 0 open · 0 new (0 new = 0 newly introduced · 0 late catches)

0 must-fix · 0 advisory · 0 polish: re-requested on an unchanged head, so this re-checks the round-3 review. The base class still drains every flushed batch, and the output manager builds each frame before the clear runs. Nothing new to raise; approval is left to a maintainer.

carloea2 · 2026-11-11T02:28:10Z

/request-review @mengw15

Retested against fresh main: all 37 partitioner tests pass. Main CI is green.

github-actions Bot assigned carloea2 Aug 31, 2026

github-actions Bot added fix pyamber labels Aug 31, 2026

Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 31, 2026

github-actions Bot requested a review from xuang7 August 31, 2026 04:52

Yicong-Huang marked this pull request as draft August 31, 2026 20:09

carloea2 marked this pull request as ready for review August 31, 2026 20:18

carloea2 changed the title ~~fix(pyamber): drain shuffle flush batches~~ feat(pyamber): drain shuffle flush batches Aug 31, 2026

mengw15 removed the release/v1.2 back porting to release/v1.2 label Aug 31, 2026

xuang7 removed the fix label Sep 1, 2026

xuang7 removed their request for review September 1, 2026 01:44

This was referenced Oct 7, 2026

feat(pyamber): clear flushed shuffle batches #8188

Closed

ci: guard smoke boot on backports #8531

Open

carloea2 force-pushed the fix/drain-shuffle-flush-batches branch from e1a56ae to dc829e9 Compare October 15, 2026 06:44

github-actions Bot added the fix label Oct 15, 2026

carloea2 mentioned this pull request Oct 15, 2026

feat(pyamber): reject invalid port booleans #8240

Merged

github-actions Bot requested a review from Yicong-Huang October 15, 2026 22:41

Yicong-Huang reviewed Oct 23, 2026

View reviewed changes

feat(pyamber): drain shuffle flush batches

59e8132

carloea2 force-pushed the fix/drain-shuffle-flush-batches branch from dc829e9 to 59e8132 Compare October 24, 2026 23:53

Yicong-Huang reviewed Oct 25, 2026

View reviewed changes

github-actions Bot requested a review from Yicong-Huang October 25, 2026 02:03

Yicong-Huang reviewed Oct 26, 2026

View reviewed changes

github-actions Bot requested a review from mengw15 November 11, 2026 02:31

Conversation

carloea2 commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this PR?

Any related issues, documentation, discussions?

How was this PR tested?

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

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport auto-label report

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automated Reviewer Suggestions

Uh oh!

codecov-commenter commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Benchmark changes need a look

Uh oh!

Yicong-Huang commented Aug 31, 2026

Uh oh!

carloea2 commented Aug 31, 2026

Uh oh!

carloea2 commented Oct 15, 2026

Uh oh!

carloea2 commented Oct 20, 2026

Uh oh!

Yicong-Huang left a comment

Choose a reason for hiding this comment

Simplifications (2)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yicong-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 commented Oct 25, 2026

Uh oh!

carloea2 commented Oct 25, 2026

Uh oh!

Yicong-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 commented Nov 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

carloea2 commented Aug 31, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading

codecov-commenter commented Aug 31, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading