Skip to content

[#901] feat(server): respect disk capacity watermark rather than uniffle capacity - #1337

Merged
zuston merged 8 commits into
apache:masterfrom
zuston:issue-901
Dec 2, 2023
Merged

[#901] feat(server): respect disk capacity watermark rather than uniffle capacity#1337
zuston merged 8 commits into
apache:masterfrom
zuston:issue-901

Conversation

@zuston

@zuston zuston commented Nov 28, 2023

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

respect disk capacity watermark rather than uniffle capacity

Why are the changes needed?

Fix: #901

Does this PR introduce any user-facing change?

Yes.

rss.server.disk-capacity.watermark.check.enabled is introduced.

How was this patch tested?

UTs

@zuston
zuston requested a review from roryqi November 29, 2023 02:06
Comment thread server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java Outdated
@codecov-commenter

codecov-commenter commented Nov 29, 2023

Copy link
Copy Markdown

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (34db3f1) 53.16% compared to head (639fe8c) 54.19%.
Report is 2 commits behind head on master.

Files Patch % Lines
...rg/apache/uniffle/storage/common/LocalStorage.java 85.71% 2 Missing and 1 partial ⚠️
...org/apache/uniffle/server/LocalStorageChecker.java 81.81% 1 Missing and 1 partial ⚠️
...he/uniffle/server/storage/LocalStorageManager.java 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1337      +/-   ##
============================================
+ Coverage     53.16%   54.19%   +1.02%     
- Complexity     2691     2712      +21     
============================================
  Files           414      398      -16     
  Lines         23780    21531    -2249     
  Branches       2030     2040      +10     
============================================
- Hits          12642    11668     -974     
+ Misses        10359     9149    -1210     
+ Partials        779      714      -65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.highWaterMarkOfWrite(highWaterMarkOfWrite)
.localStorageMedia(storageType)
.build();
.enableDiskCapacityWatermarkCheck();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It does not look like this line is intended?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My fault. It has been removed.

@zuston
zuston requested review from awdavidson and roryqi November 30, 2023 04:18
@zuston

zuston commented Nov 30, 2023

Copy link
Copy Markdown
Member Author

PTAL @jerqi

@roryqi roryqi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks @zuston

@zuston
zuston merged commit e3361d6 into apache:master Dec 2, 2023
zuston added a commit to zuston/incubator-uniffle that referenced this pull request Jan 18, 2024
…n uniffle capacity (apache#1337)

### What changes were proposed in this pull request?

respect disk capacity watermark rather than uniffle capacity

### Why are the changes needed?

Fix: apache#901 

### Does this PR introduce _any_ user-facing change?

Yes.

 `rss.server.disk-capacity.watermark.check.enabled` is introduced. 

### How was this patch tested?

UTs
zuston pushed a commit to zuston/incubator-uniffle that referenced this pull request Jan 18, 2024
[apache#901] feat(server): respect disk capacity watermark rather than uniffle capacity (apache#1337)

See merge request !94
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.

[Bug] Incorrect local disk size limitation when colocating with Yarn nodemanager

4 participants