[#2469] feat(spark): Create a unique ShuffleId for Uniffle for stage retry - #2471
Conversation
7d59915 to
8a3d5a9
Compare
zuston
left a comment
zuston
left a comment
There was a problem hiding this comment.
Overall lgtm. Is this feature validated in your production cases?
|
And is this feature compatible with the older server version? I see some rpc params are ignored. |
zuston
changed the title
I have conducted integration tests and some tasks have been launched. |
I'm rather confused about this. Because when registering the Shuffle Server, the attributes related to Stage should not be involved. Therefore, the information of Stage should not appear during the registration stage. So I deleted them. |
yl09099
changed the title
| * @param shuffleId | ||
| * @return | ||
| */ | ||
| public int createUniffleShuffleId(int shuffleId, String appShuffleIdentifier) { |
There was a problem hiding this comment.
What's the
appShuffleIdentifier
I add some annotations. It is the combination of shuffle_stageId_stageAttemptNumber.
There was a problem hiding this comment.
If so, I think it's not a good practise to use a simple string to declare this meaning. How about creating a independent class to involve this meaning?
There was a problem hiding this comment.
If so, I think it's not a good practise to use a simple string to declare this meaning. How about creating a independent class to involve this meaning?
This unique scalar only serves an identification function and involves no business logic calculation. Isn't it a bit heavy to create a separate class.
zuston
left a comment
zuston
left a comment
There was a problem hiding this comment.
LGTM. I will merge this if no rejection.
Please go forward for next PRs.
|
Merged @yl09099 . Thanks |
|
Will this feature affect the exchange reuse? |
I missed this part compatibility. Could you help check this? @yl09099 |
This point was not taken into consideration. Let me confirm it later. |
3 participants
What changes were proposed in this pull request?
Use the auto-incrementing shuffleId to replace the shuffleId of Spark. When Stage retry occurs, use the auto-incrementing shuffleId of Uniffle to avoid clearing the existing Shuffle data of the Shuffle Server.
Why are the changes needed?
Design doc:
https://docs.google.com/document/d/1TPxg4CUNklg26-JIcKCeIDwZb9lWKj3mIu-qjMcj0wU/edit?usp=sharing
Fix: #2470
Fix: #2469
Does this PR introduce any user-facing change?
No.
How was this patch tested?
UT.