apurtell commented on code in PR #8389:
URL: https://github.com/apache/hbase/pull/8389#discussion_r3455294878
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java:
##########
@@ -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) {
Review Comment:
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."
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java:
##########
@@ -533,6 +533,20 @@ public long getStoreFileSize() {
return aggregate.storeFileSize;
}
+ @Override
+ public long getStoreFileUncompressedSize() {
+ return aggregate.storeFileUncompressedSize;
+ }
+
+ @Override
+ public double getStoreFileCompressionRatio() {
Review Comment:
Generally for compression factor, higher is better. This is what command
line compression tools like `xz` or `zstd` or etc. typically report. And for
compression savings, higher is also better, e.g `1 − 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 labeled
`storeFileCompressionRatio` with a value e.g. of `0.33` will routinely
interpret it the opposite of what is intended.
Recommendation: either rename to `storeFileCompressedToUncompressedRatio` so
the direction is clearly established by the name of the metric, or reimplement
as `uncompressed / compressed`, or `storeFileCompressionSavingsRatio` as (`1 −
compressed/uncompressed`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
