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

[Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer #173

Closed
leixm opened this issue Aug 22, 2022 · 3 comments · Fixed by #176
Assignees

Comments

@leixm
Copy link
Contributor

leixm commented Aug 22, 2022

Should shuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer? so that we can make more full use of disk io without waiting for the buffer to reach rss.server.buffer.capacity * rss.server.memory.shuffle.highWaterMark.percentage

@leixm
Copy link
Contributor Author

leixm commented Aug 22, 2022

I think it's necessary feature, what do u think? @jerqi

@jerqi
Copy link
Contributor

jerqi commented Aug 22, 2022

I think it's necessary feature, what do u think? @jerqi

Agree, Do you want to raise a pr?

@leixm
Copy link
Contributor Author

leixm commented Aug 22, 2022

yes, I will create a PR

jerqi pushed a commit that referenced this issue Aug 22, 2022
…g to the size of single ShuffleBuffer (#176)

### What changes were proposed in this pull request?
for issue #173, Add a new flush strategy for ShuffleBufferManager, flush if the size of a single buffer reaches `rss.server.buffer.flush.threshold`, which can make more full use of disk io without waiting for the buffer to reach `(rss.server.buffer.capacity * rss.server.memory.shuffle.highWaterMark.percentage)`

### Why are the changes needed?
Improvement performance

### Does this PR introduce any user-facing change?
Yes, already add the document

### How was this patch tested?
Already added

Co-authored-by: leixianming <leixianming@didiglobal.com>
leixm pushed a commit to leixm/incubator-uniffle that referenced this issue Aug 22, 2022
leixm pushed a commit to leixm/incubator-uniffle that referenced this issue Aug 22, 2022
jerqi pushed a commit that referenced this issue Aug 22, 2022
…s.server.flush.cold.storage.threshold.size (#178)

###What changes were proposed in this pull request?
follow  #173, when single buffer flush is triggered, the buffer size should reach `rss.server.flush.cold.storage.threshold.size`,  we keep the same logic as MultiStorageManager#selectStorageManager

###Why are the changes needed?
we keep the same logic as MultiStorageManager#selectStorageManager

###Does this PR introduce any user-facing change?
No

###How was this patch tested?
Already added

Co-authored-by: leixianming <leixianming@didiglobal.com>
jerqi pushed a commit that referenced this issue Aug 23, 2022
…s.server.flush.cold.storage.threshold.size (#180)

### What changes were proposed in this pull request?
follow #173, when single buffer flush is triggered, the buffer size should reach rss.server.flush.cold.storage.threshold.size, we keep the same logic as MultiStorageManager#selectStorageManager

### Why are the changes needed?
Make sure cold storage is valid

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
Already added

Co-authored-by: leixianming <leixianming@didiglobal.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants