branch-4.2: [fix](local shuffle) address all receiver instances for a non-serial exchange #65348 - #68187
branch-4.2: [fix](local shuffle) address all receiver instances for a non-serial exchange #65348#68187924060929 wants to merge 1 commit into
Conversation
|
run buildall |
|
/review |
|
Superseded by #68199, which backports the whole local shuffle series as one PR. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Static review of exact head e08da52f3097c7c2a30c48dfc6869d1579f1ea92 found one blocking issue: the new regression test calls a nonexistent four-argument constructor, so FE test compilation cannot succeed until the branch-specific fifth argument is supplied.
The production destination-selection change was traced through seriality derivation, local-shuffle assignment, Thrift serialization, BE receiver/EOS handling, bucket shuffle, shared-hash broadcast, and multicast routing. No additional reachable correctness issue was found.
User focus: no additional focus was provided.
Validation: static review only; no builds or tests were run, per the review constraints.
Live-state note: PR #68187 is now closed and marked superseded by #68199. This review remains scoped to the exact #68187 head above; the superseding PR was not reviewed here.
|
|
||
| private DistributePlanner newPlanner() { | ||
| List<PlanFragment> noFragments = Lists.newArrayList(); | ||
| return new DistributePlanner(Mockito.mock(StatementContext.class), noFragments, false, false); |
There was a problem hiding this comment.
[P1] Pass the branch's fifth constructor argument
DistributePlanner has only the five-argument constructor (StatementContext, List<PlanFragment>, boolean, boolean, boolean) on this branch, so this new four-argument call cannot compile and none of these regression assertions can run. Please pass the missing useLoadBackendSelection value (false for this isolated test).
| return new DistributePlanner(Mockito.mock(StatementContext.class), noFragments, false, false); | |
| return new DistributePlanner(Mockito.mock(StatementContext.class), noFragments, false, false, false); |
1 participant
Cherry-picked from #65348