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

[CELEBORN-1843] Optimize roundrobin for more balanced disk slot allocation #3074

Closed
wants to merge 2 commits into from

Conversation

gaoyajun02
Copy link

What changes were proposed in this pull request?

This PR optimizes the RoundRobin algorithm to achieve a more balanced disk slot allocation across workers.
Previously, when allocating 3000 partitions using RoundRobin, the slot distribution across worker disks was [668, 666, 666], which resulted in one disk having 2 more slots than the others.
After the optimization, the slot distribution is now [667, 667, 666], ensuring a more balanced allocation.

Why are the changes needed?

The changes are necessary to improve load balancing across worker disks, reducing the risk of overloading a single disk. This ensures a more predictable and fair distribution of slots, which can lead to better performance and resource utilization.

Does this PR introduce any user-facing change?

No

How was this patch tested?

UT

@FMX FMX requested review from FMX and RexXiong January 20, 2025 12:33
@FMX
Copy link
Contributor

FMX commented Jan 21, 2025

This PR looks good to me, but it should be subject to performance tests to ensure that nothing unexpected happens. The slot distribution will affect the performance.

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

Copy link

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale label Feb 11, 2025
@RexXiong RexXiong closed this in c45197c Feb 12, 2025
@RexXiong
Copy link
Contributor

Thanks merge to main(v0.6.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants