-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-30242 Add Region server level HFile Compression Metrics #8389
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -533,6 +533,20 @@ public long getStoreFileSize() { | |
| return aggregate.storeFileSize; | ||
| } | ||
|
|
||
| @Override | ||
| public long getStoreFileUncompressedSize() { | ||
| return aggregate.storeFileUncompressedSize; | ||
| } | ||
|
|
||
| @Override | ||
| public double getStoreFileCompressionRatio() { | ||
| long uncompressed = aggregate.storeFileUncompressedSize; | ||
| if (uncompressed == 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A value of "1.0" will generally look identical to an operator like somehow compression has stopped working, and will generate false positive alerts. (Claude calls this a "footgun for dashboards", lol. He means well...) Recommendation: Return 0.0 instead, and update the description to something like "Returns 0.0 when there is no data."
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. Thanks |
||
| return 0.0; | ||
| } | ||
| return (double) uncompressed / aggregate.storeFileSize; | ||
| } | ||
|
|
||
| @Override | ||
| public double getStoreFileSizeGrowthRate() { | ||
| return aggregate.storeFileSizeGrowthRate; | ||
|
|
@@ -783,6 +797,7 @@ private static final class RegionMetricAggregate { | |
| private long onHeapMemstoreSize = 0; | ||
| private long offHeapMemstoreSize = 0; | ||
| private long storeFileSize = 0; | ||
| private long storeFileUncompressedSize = 0; | ||
| private double storeFileSizeGrowthRate = 0; | ||
| private long maxStoreFileCount = 0; | ||
| private long maxStoreFileAge = 0; | ||
|
|
@@ -979,6 +994,7 @@ private StoreFileStats aggregateStores(List<HStore> stores) { | |
| onHeapMemstoreSize += store.getMemStoreSize().getHeapSize(); | ||
| offHeapMemstoreSize += store.getMemStoreSize().getOffHeapSize(); | ||
| storeFileSize += store.getStorefilesSize(); | ||
| storeFileUncompressedSize += store.getStoreSizeUncompressed(); | ||
| maxStoreFileCount = Math.max(maxStoreFileCount, store.getStorefilesCount()); | ||
|
|
||
| maxStoreFileAge = | ||
|
|
||
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.
Generally for compression factor, higher is better. This is what command line compression tools like
xzorzstdor etc. typically report. And for compression savings, higher is also better, e.g1 − compressed / uncompressed, which is easy to read as a percentage. This method provides the inverse of that so it makes the desired direction "down", which is awkward for Prometheus or Grafana dashboards. Operators reading a dashboard tile labeledstoreFileCompressionRatiowith a value e.g. of0.33will routinely interpret it the opposite of what is intended.Recommendation: either rename to
storeFileCompressedToUncompressedRatioso the direction is clearly established by the name of the metric, or reimplement asuncompressed / compressed, orstoreFileCompressionSavingsRatioas (1 − compressed/uncompressed).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.
reimplemented as uncompressed / compressed as that is more common.