Skip to content
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.

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

[MINOR] improve(client): Shuffle server list for MultiReplicaClientReadHandler to improve read serve performance #2067

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maobaolong
Copy link
Member

What changes were proposed in this pull request?

Shuffle the server list for MultiReplicaClientReadHandler.

Why are the changes needed?

To improve read serve performance if enable multiply replication.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No need.

Copy link

github-actions bot commented Aug 19, 2024

Test Results

 2 792 files  ±0   2 792 suites  ±0   5h 51m 28s ⏱️ +52s
   988 tests ±0     987 ✅ ±0   1 💤 ±0  0 ❌ ±0 
12 403 runs  ±0  12 388 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit d88566c. ± Comparison against base commit 0495fbc.

♻️ This comment has been updated with latest results.

@maobaolong maobaolong closed this Aug 20, 2024
@maobaolong maobaolong reopened this Aug 20, 2024
@maobaolong maobaolong closed this Aug 20, 2024
@maobaolong maobaolong reopened this Aug 20, 2024
@maobaolong maobaolong closed this Aug 20, 2024
@maobaolong maobaolong reopened this Aug 20, 2024
@maobaolong maobaolong closed this Aug 20, 2024
@maobaolong maobaolong reopened this Aug 20, 2024
@maobaolong maobaolong closed this Aug 20, 2024
@maobaolong maobaolong reopened this Aug 20, 2024
@@ -49,7 +51,9 @@ public MultiReplicaClientReadHandler(
this.handlers = handlers;
this.blockIdBitmap = blockIdBitmap;
this.processedBlockIds = processedBlockIds;
this.shuffleServerInfos = shuffleServerInfos;
List<ShuffleServerInfo> shuffledServerList = new ArrayList<>(shuffleServerInfos);
Collections.shuffle(shuffledServerList);
Copy link
Member

Choose a reason for hiding this comment

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

In most cases, the complete data can be read from the first shuffle server. But the shuffle server with no data may be accessed first with this change.

@maobaolong
Copy link
Member Author

Discussed with @xianjingfeng , this PR changed is affected for partition skew scenario. And I need to change the shuffle target to [0, writeReplica) since the [writeReplica, replica) could have no data for normal case.

@maobaolong maobaolong marked this pull request as draft August 27, 2024 02:11
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.

2 participants