mnpoonia commented on code in PR #8389:
URL: https://github.com/apache/hbase/pull/8389#discussion_r3781451351


##########
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;

Review Comment:
   `aggregate` is a volatile snapshot as andrew mentioned and can be replaced 
between these two reads. That can calculate the ratio from different refreshes, 
including `nonzero / 0` when regions are removed.  Could we capture `aggregate` 
once, read both values from that snapshot, and guard the compressed denominator 
before dividing?
   
   ```
   RegionMetricAggregate current = aggregate;
   if (current.storeFileSize <= 0) {
     return 0.0;
   }
   return (double) current.storeFileUncompressedSize / current.storeFileSize;
   ```



-- 
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]

Reply via email to