Skip to content

Conversation

@xianjingfeng
Copy link
Member

What changes were proposed in this pull request?

Use ack val to check the block send result.

Why are the changes needed?

For better performance.
Fix: #2674

Does this PR introduce any user-facing change?

No.

How was this patch tested?

CI

@xianjingfeng xianjingfeng requested review from jerqi and zuston December 24, 2025 03:20
@github-actions
Copy link

github-actions bot commented Dec 24, 2025

Test Results

 3 175 files  ±0   3 175 suites  ±0   6h 50m 33s ⏱️ - 4m 56s
 1 238 tests ±0   1 237 ✅ ±0   1 💤 ±0  0 ❌ ±0 
15 689 runs  ±0  15 674 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 1ce1e26. ± Comparison against base commit 7867d59.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerqi
Copy link
Contributor

jerqi commented Dec 24, 2025

Should Spark2 client be consistent with Spark3 client?

@xianjingfeng
Copy link
Member Author

xianjingfeng commented Dec 24, 2025

Should Spark2 client be consistent with Spark3 client?

Done.Merge the changes from #1179

Copy link
Member

@zuston zuston left a comment

Choose a reason for hiding this comment

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

+1


long currentAckValue = 0;
for (Long blockId : blockIds) {
currentAckValue ^= blockId;
Copy link
Member

Choose a reason for hiding this comment

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

lightweight and effective!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Optimize org.apache.spark.shuffle.writer.RssShuffleWriter#checkBlockSendResult

3 participants