add metric for largest segment size#8445
add metric for largest segment size#8445Jackie-Jiang merged 4 commits intoapache:masterfrom jadami10:jadami/largest-segment-metric
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8445 +/- ##
============================================
- Coverage 70.73% 70.72% -0.02%
- Complexity 4282 4284 +2
============================================
Files 1662 1663 +1
Lines 87227 87305 +78
Branches 13205 13216 +11
============================================
+ Hits 61703 61745 +42
- Misses 21238 21279 +41
+ Partials 4286 4281 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
Can you please add the prometheus config and some tests? You may refer to #8358 which has very similar changes
| @Deprecated // Instead use TABLE_TOTAL_SIZE_ON_SERVER | ||
| OFFLINE_TABLE_ESTIMATED_SIZE("OfflineTableEstimatedSize", false), | ||
|
|
||
| LARGEST_SEGMENT_SIZE_ON_DISK("LargestSegmentSizeOnDisk", false), |
There was a problem hiding this comment.
| LARGEST_SEGMENT_SIZE_ON_DISK("LargestSegmentSizeOnDisk", false), | |
| LARGEST_SEGMENT_SIZE_ON_SERVER("LargestSegmentSizeOnServer", false), |
| tableSizeDetails._realtimeSegments._estimatedSizeInBytes / _helixResourceManager.getNumReplicas( | ||
| realtimeTableConfig)); | ||
|
|
||
| tableSizeDetails._realtimeSegments._segments.values().stream() |
There was a problem hiding this comment.
(optional) Suggest using the regular non-stream APIs for the following reasons:
- This is indeed more compact, but slightly hard to read
- It has worse performance comparing to regular APIs
There was a problem hiding this comment.
that's fair, it was just as tedious to write as it is to read. I factored out the "default" size we see when segments are missing into a variable and rewrote it with regular APIs
Added both. It tests for realtime, offline, and all segments missing. thank you for the pointer PR |
| } | ||
| return null; | ||
| }); | ||
| String path = (String) invocationOnMock.getArguments()[0]; |
There was a problem hiding this comment.
not changes here. the reformat changed the whitespace
4 participants
Description
This adds a metric for the largest segment size on disk by table
columns.psffileI tested by running the batch quickstart with a breakpoint and asserting the value at debug time equalled what I saw on disk
Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
backward-incompat, and complete the section below on Release Notes)Does this PR fix a zero-downtime upgrade introduced earlier?
backward-incompat, and complete the section below on Release Notes)Does this PR otherwise need attention when creating release notes? Things to consider:
release-notesand complete the section on Release Notes)Release Notes
This adds a metric for the largest segment size on disk in a server. You can use this to track the growth of you largest segments.
Documentation