search

apache / incubator-uniffle

Uniffle is a high performance, general purpose Remote Shuffle Service.
https://uniffle.apache.org/
Apache License 2.0
387 stars 149 forks source link

[#2219] feat(server) add new bitmap strategy #2220

Closed lwllvyb closed 3 weeks ago

lwllvyb commented 1 month ago

What changes were proposed in this pull request?

Supports one bitmap per partition. Add a new config rss.server.bitmap.new.strategyto decide whether to enable the function.

Why are the changes needed?

Fix: #2219

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Tested locally.

github-actions[bot] commented 1 month ago

Test Results

 2 926 files  ±0   2 926 suites  ±0   6h 14m 4s ⏱️ -15s  1 088 tests ±0   1 086 ✅ ±0   2 💤 ±0  0 ❌ ±0  13 630 runs  ±0  13 600 ✅ ±0  30 💤 ±0  0 ❌ ±0 

Results for commit 1e5cd988. ± Comparison against base commit b5290b21.

:recycle: This comment has been updated with latest results.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 19.14894% with 76 lines in your changes missing coverage. Please review.

Project coverage is 52.25%. Comparing base (34bf686) to head (1e5cd98). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
.../org/apache/uniffle/server/ShuffleTaskManager.java 14.60% 70 Missing and 6 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2220 +/- ## ============================================ + Coverage 51.84% 52.25% +0.41% - Complexity 2864 2897 +33 ============================================ Files 469 461 -8 Lines 23879 21883 -1996 Branches 1966 2017 +51 ============================================ - Hits 12380 11436 -944 + Misses 10726 9716 -1010 + Partials 773 731 -42 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

maobaolong commented 4 weeks ago
image

@lwllvyb FYI, this is the maximum partition num with node metrics in our cluster, it is ~410K, it cost heap size 410K * 50KiB = 2GiB if we say a bitmap related a partition avg 10000 blocks(cost 50KiB heap).

I think this is worth to choose this new policy in our production cluster.

maobaolong commented 3 weeks ago

@lwllvyb Thanks for your initial work on this improvement, as this is needed this week, I take it over and do some refactor, see https://github.com/apache/incubator-uniffle/pull/2227/

maobaolong commented 3 weeks ago

@lwllvyb Close this as this continued by #2227