-
Notifications
You must be signed in to change notification settings - Fork 168
[#1119] improvement(client): Explicitly throw BUFFER_LIMIT_OF_HUGE_PARTITION
#1425
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
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1425 +/- ##
============================================
+ Coverage 53.43% 54.23% +0.80%
+ Complexity 2723 2720 -3
============================================
Files 419 402 -17
Lines 23992 21659 -2333
Branches 2046 2043 -3
============================================
- Hits 12820 11747 -1073
+ Misses 10378 9190 -1188
+ Partials 794 722 -72 ☔ View full report in Codecov by Sentry. |
zuston
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for your effort. @lifeSo
|
|
||
| // Add NoBufferForHugePartitionException check | ||
| // and ShuffleServerMetrics.TOTAL_REQUIRE_BUFFER_FAILED_FOR_HUGE_PARTITION metric should be 1 | ||
| String content = TestUtils.httpGet(SHUFFLE_SERVER_METRICS_URL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really weird. Maybe we could optimize this in the another PR.
BUFFER_LIMIT_OF_HUGE_PARTITION
|
|
||
| gaugeIsHealthy = metricsManager.addLabeledGauge(IS_HEALTHY); | ||
| gaugeAllocatedBufferSize = metricsManager.addLabeledGauge(ALLOCATED_BUFFER_SIZE); | ||
| counterExpiredPreAllocatedBufferSizeTotal = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing this?
What changes were proposed in this pull request?
Explicitly throw
BUFFER_LIMIT_OF_HUGE_PARTITIONinstead ofNO_BUFFERWhy are the changes needed?
Fix: #1119
Does this PR introduce any user-facing change?
No.
How was this patch tested?
unit test