feat: bind task-owned RSS callbacks to native shuffle plans (4/n) - #5491
feat: bind task-owned RSS callbacks to native shuffle plans (4/n)#5491pingzh wants to merge 2 commits into
Conversation
ziting-openai
left a comment
There was a problem hiding this comment.
The PR currently points at the already-merged 3/n implementation instead of the advertised 4/n task-owned RSS callback integration.
8281c34 to
6658894
Compare
sunchao
left a comment
There was a problem hiding this comment.
Reviewed head 6658894632adf114fb86313c282b2ecf95c8453f against base 4897161704b7b8b7dfa909f4bf897c6508b11117 with five independent reviewer agents. Existing validation: 83 targeted Rust tests passed with default features disabled, and a standalone Arrow 18.3 ownership probe confirmed retention until explicit release. Full Spark/JNI integration tests were not rerun.
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed head 69e3cfb8239806648b79b24148607be16504e378 against base 4897161704b7b8b7dfa909f4bf897c6508b11117 with five independent agents; no new actionable findings. The earlier Arrow cleanup P2 is fixed and its Spark regression passes.
Validation: 105 targeted Rust tests and all 35 CometNativeShuffleSuite tests passed on Spark 4.1.3/JDK 17, with native default features disabled. Other Spark versions and live RSS integration were not tested. CI is still running.
Which issue does this PR close?
Part of #5352. This is the fourth foundational PR and does not close the issue.
Previous PRs:
Rationale for this change
Previous PRs introduced RSS partition writers, destination-aware shuffle plans, and native RSS execution. However, serialized RSS plans cannot execute without connecting their task-owned JVM callback to the native planner.
This PR completes that connection while preserving existing local shuffle behavior and JNI compatibility.
What changes are included in this PR?
createPlansignature.CometExecIteratorinto native execution contexts and the physical planner.How are these changes tested?
CometNativeShuffleSuite.