fix: align BroadcastPartition routing in LinkConfig with broadcast semantics#5032
Conversation
|
/request-review @aglinxinyuan |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5032 +/- ##
============================================
+ Coverage 42.77% 42.85% +0.08%
- Complexity 2193 2199 +6
============================================
Files 1045 1031 -14
Lines 39985 38156 -1829
Branches 4217 4007 -210
============================================
- Hits 17102 16352 -750
+ Misses 21824 20787 -1037
+ Partials 1059 1017 -42
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes LinkConfig.toPartitioning so that BroadcastPartition() produces the full sender×receiver channel cross product (broadcast semantics) instead of incorrectly using zip (1:1 pairing with silent truncation on length mismatch). This aligns LinkConfig routing behavior with the existing ChannelConfig.generateChannelConfigs behavior in the same package and resolves issue #4802.
Changes:
- Update
LinkConfig.toPartitioning’sBroadcastPartition()arm to generate channels via cross product (flatMap/map) rather thanzip. - Update
LinkConfigSpecbroadcast test cases to assert the cross-product routing for symmetric and asymmetric worker list sizes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| amber/src/main/scala/org/apache/texera/amber/engine/architecture/scheduling/config/LinkConfig.scala | Fix broadcast routing to emit sender×receiver channel cross product instead of zip pairing. |
| amber/src/test/scala/org/apache/texera/amber/engine/architecture/scheduling/config/LinkConfigSpec.scala | Update broadcast partition specs to match corrected broadcast semantics (including unequal list sizes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What changes were proposed in this PR?
LinkConfig.toPartitioning'sBroadcastPartition()arm was building channels withfromWorkerIds.zip(toWorkerIds), producing a 1:1 pairing rather than the full sender-by-receiver cross product that broadcast semantics imply.The sibling
ChannelConfig.generateChannelConfigsin the same package already emits a cross product for the broadcast arm, so the two helpers disagreed on routing for the same inputs. As with #4799 forOneToOnePartition,List.zipalso silently truncated the surplus side on unequal-length inputs. This PR replaces thezipwith the sameflatMap/mapcross product already used by theHashPartition,RangePartition, andUnknownPartitionarms in this file, which fixes the semantics and removes the silent truncation hazard.Any related issues, documentation, or discussions?
Closes: #4802
How was this PR tested?
The two
LinkConfigSpeccases that pinned the previous behavior (with comments explicitly inviting this realignment) were flipped to assert the cross product: 3 senders by 3 receivers now produces 9 channels, and theAsymmetric 3 by 2 case now produces 6 channels rather than silently truncating to 2.
sbt scalafmtAllwas run; CI should pass the existingLinkConfigSpec,ChannelConfigSpec, and downstream scheduling specs.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.7 in compliance with ASF