Skip to content

[CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read #3118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wangshengjie123 wants to merge 9 commits into from
Closed

[CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read #3118

wangshengjie123 wants to merge 9 commits into from

Conversation

@wangshengjie123
Copy link
Contributor

@wangshengjie123 wangshengjie123 commented Feb 25, 2025

What changes were proposed in this pull request?

Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read

Why are the changes needed?

In CELEBORN-1319, we have already implemented the skew partition read optimization based on chunk offsets, but we don't support skew partition shuffle retry, so we need support the stage rerun.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Cluster test

@wangshengjie123
Copy link
Contributor Author

wangshengjie123 commented Feb 25, 2025

Cluster test is doing

@wangshengjie123 wangshengjie123 changed the title [WIP]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read [WIP][CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read Feb 25, 2025
@wangshengjie123 wangshengjie123 changed the title [WIP][CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read [CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read Feb 25, 2025
@wangshengjie123 wangshengjie123 changed the title [CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read [WIP][CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read Feb 25, 2025
@Z1Wu Z1Wu mentioned this pull request Feb 25, 2025
Closed
@RexXiong
Copy link
Contributor

RexXiong commented Mar 5, 2025

Thank you for your efforts, @wangshengjie123 , is there any update?

@wangshengjie123 wangshengjie123 changed the title [WIP][CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read [CELEBORN-1856]Support stage-rerun when read partition by chunkOffsets when enable optimize skew partition read Mar 6, 2025
@wangshengjie123
Copy link
Contributor Author

wangshengjie123 commented Mar 6, 2025 •
edited
Loading

Thank you for your efforts, @wangshengjie123 , is there any update?

Updated, fix some data consistency issues.
Testing passed in out cluster.

I will rebase the code later

PTAL, thanks @RexXiong

RexXiong
RexXiong reviewed Mar 6, 2025
View reviewed changes
Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to check (!stage.isIndeterminate && task.stageAttemptId < stage.latestInfo.attemptNumber()) when the task succeeds, just as we would for an indeterminate stage. Skew Partition read task should ignore partition complete event when task.stageAttemptId < stage.latestInfo.attemptNumber()

@turboFei turboFei requested a review from RexXiong March 12, 2025 00:33
@turboFei
Copy link
Member

turboFei commented Mar 20, 2025

ping @RexXiong

@Z1Wu
Copy link
Contributor

Z1Wu commented Mar 20, 2025

I also do some simple tests in a mini-cluster, and tests work as expected. Looking forward to this feature being merged into master.

  • skew optimization occurs
    image

  • delete shuffle data manually to trigger fetch fail
    image

  • rerun all task in shuffle map stages
    image

  • ** rerun all tasks in failed stage ** ( work as expecteed)
    image

  • not sort happens during skew-handling
    image

RexXiong
RexXiong approved these changes Mar 20, 2025
View reviewed changes
Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except a nit

RexXiong
RexXiong approved these changes Mar 21, 2025
View reviewed changes
Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, also thanks @Z1Wu

@turboFei
Copy link
Member

turboFei commented Mar 24, 2025

I think there is no more comments, can we merge it? @RexXiong

@RexXiong RexXiong closed this in 4bacd1f Mar 24, 2025
@RexXiong
Copy link
Contributor

RexXiong commented Mar 24, 2025

I think there is no more comments, can we merge it? @RexXiong

Sure, thanks for your reminder, merge to main(v0.6.0)

@turboFei turboFei mentioned this pull request Apr 11, 2025
Closed
turboFei added a commit that referenced this pull request Apr 11, 2025
@turboFei
[CELEBORN-1856][FOLLOWUP] Check isCelebornSkewedShuffle before `re…
f92f9b8 
…gisterCelebornSkewedShuffle` for stage rollback

### What changes were proposed in this pull request?
Followup for #3118

Add a condition check(isCelebornShuffleIndeterminate) before `registerCelebornSkewedShuffle` for stage rollback.
### Why are the changes needed?

Fix the logical.
### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?
Minor change.

Closes #3209 from turboFei/spark_celeborn_patch.

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Wang, Fei <[email protected]>
@SteNicholas SteNicholas mentioned this pull request Jul 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@turboFei turboFei turboFei left review comments

@RexXiong RexXiong RexXiong approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wangshengjie123 @RexXiong @turboFei @Z1Wu