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.

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

[Improvement] better default shuffle server configuration #642

Closed
2 of 3 tasks
advancedxy opened this issue Feb 22, 2023 · 4 comments · Fixed by #662
Closed
2 of 3 tasks

[Improvement] better default shuffle server configuration #642

advancedxy opened this issue Feb 22, 2023 · 4 comments · Fixed by #662

Comments

@advancedxy
Copy link
Contributor

advancedxy commented Feb 22, 2023 •
edited
Loading

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

  1. rss.server.buffer.capacity could have a default settings, JVM MaxHeapSize * some factor
  2. rss.server.read.buffer.capacity is similar with rss.server.buffer.capacity

Otherwise, once the max heap size of shuffle server is changed, users/admins have to reconfigure these two buffer settings.

  1. rss.server.disk.capacity is -1(by default), shuffle server uses the total space of one disk, which seems operation unfriendly. Shuffle server should reserve some partition of disk space and not use all the disks, such as at most 95% the capacity.

How should we improve?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@advancedxy
Copy link
Contributor Author

cc @jerqi, WDYT? would you like to refine this settings?

@jerqi
Copy link
Contributor

jerqi commented Feb 22, 2023

cc @jerqi, WDYT? would you like to refine this settings?

OK

@zuston
Copy link
Member

zuston commented Feb 25, 2023 •
edited
Loading

rss.server.disk.capacity is -1(by default), shuffle server uses the total space of one disk, which seems operation unfriendly. Shuffle server should reserve some partition of disk space and not use all the disks, such as at most 95% the capacity.

I think this config as a ratio value will be better, the current concrete disk capacity is not flexible especially for heterogeneous storage. @advancedxy

@advancedxy
Copy link
Contributor Author

rss.server.disk.capacity is -1(by default), shuffle server uses the total space of one disk, which seems operation unfriendly. Shuffle server should reserve some partition of disk space and not use all the disks, such as at most 95% the capacity.

I think this config as a ratio value will be better, the current concrete disk capacity is not flexible especially for heterogeneous storage. @advancedxy

:+1. Sound good to me. But the fixed capacity has its own use cases, I will not remove this configuration, rather add a ratio configuration then use it preferably.

@advancedxy advancedxy mentioned this issue Feb 26, 2023
Merged
advancedxy added a commit that referenced this issue Feb 27, 2023
@advancedxy
[#642]feat(server): better default options for shuffle server (#662)
9a1bc07 
### What changes were proposed in this pull request?
1. `rss.server.buffer.capacity` uses JVM heap size * ratio(0.6) by default
2. `rss.server.read.buffer.capacity` uses JVM heap size * ratio(0.2) by default
3. `rss.server.disk.capacity` uses disk space * ratio(0.9) by default

### Why are the changes needed?
Fix: #642 

### Does this PR introduce _any_ user-facing change?
Yes. Three new configurations are introduced, users can specify ratio values
for buffer, read buffer and disk capacity

### How was this patch tested?
New UTs.
advancedxy added a commit to advancedxy/incubator-uniffle that referenced this issue Mar 21, 2023
@advancedxy
[apache#642]feat(server): better default options for shuffle server (a…
6ae0d74 
…pache#662)

### What changes were proposed in this pull request?
1. `rss.server.buffer.capacity` uses JVM heap size * ratio(0.6) by default
2. `rss.server.read.buffer.capacity` uses JVM heap size * ratio(0.2) by default
3. `rss.server.disk.capacity` uses disk space * ratio(0.9) by default

### Why are the changes needed?
Fix: apache#642 

### Does this PR introduce _any_ user-facing change?
Yes. Three new configurations are introduced, users can specify ratio values
for buffer, read buffer and disk capacity

### How was this patch tested?
New UTs.
xianjingfeng pushed a commit to xianjingfeng/incubator-uniffle that referenced this issue Apr 5, 2023
@advancedxy
[apache#642]feat(server): better default options for shuffle server (a…
174a7ea 
…pache#662)

### What changes were proposed in this pull request?
1. `rss.server.buffer.capacity` uses JVM heap size * ratio(0.6) by default
2. `rss.server.read.buffer.capacity` uses JVM heap size * ratio(0.2) by default
3. `rss.server.disk.capacity` uses disk space * ratio(0.9) by default

### Why are the changes needed?
Fix: apache#642 

### Does this PR introduce _any_ user-facing change?
Yes. Three new configurations are introduced, users can specify ratio values
for buffer, read buffer and disk capacity

### How was this patch tested?
New UTs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#642]feat(server): better default options for shuffle server advancedxy/incubator-uniffle
3 participants
@advancedxy @jerqi @zuston