HBASE-30242 Add Region server level HFile Compression Metrics

[Umeshkumar9414]

Local testing

[Copilot]

Pull request overview

Adds RegionServer-level metrics for total uncompressed HFile bytes and an overall store-file compression ratio, surfaced through the existing MetricsRegionServer wrapper/source plumbing and validated via unit tests.

Changes:

• Extend MetricsRegionServerWrapper with getStoreFileUncompressedSize() and getStoreFileCompressionRatio().
• Compute/aggregate uncompressed store bytes in MetricsRegionServerWrapperImpl and export both gauges via MetricsRegionServerSourceImpl.
• Update RegionServer metrics unit tests and wrapper stubs to cover the new gauges.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java	Asserts the two new gauges are emitted by the metrics source.
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperStub.java	Adds stub implementations returning deterministic values for the new metrics.
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java	Aggregates uncompressed bytes from stores and computes RS-level compression ratio.
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapper.java	Extends wrapper interface with uncompressed size and compression ratio accessors.
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java	Exports the new wrapper values as gauges in the metrics record.
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java	Defines metric names and descriptions for the two new gauges.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java:547

• aggregate is volatile and replaced by the recomputation thread, but this method reads it twice. A swap between those reads can combine the uncompressed size from one snapshot with the compressed size from another, producing an incorrect ratio (including infinity when regions are removed). Capture one aggregate snapshot and use it for both operands.

    long uncompressed = aggregate.storeFileUncompressedSize;
    if (uncompressed == 0) {
      return 0.0;
    }
    return (double) uncompressed / aggregate.storeFileSize;

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerWrapperImpl.java:997

• The added source test uses a stub that returns the ratio directly, so it does not exercise this aggregation or the production ratio calculation. Extend TestMetricsRegionServerAggregate, which already validates getStoreFileSize(), to assert the aggregated uncompressed size and computed ratio (including the zero-data case); otherwise regressions in the core implementation can pass the new test.

        storeFileUncompressedSize += store.getStoreSizeUncompressed();

[apurtell]

I think the latest copilot suggestions are good

1. MetricsRegionServerWrapperImpl.java:547 aggregate is volatile and replaced by the recomputation thread, but this method reads it twice. A swap between those reads can combine the uncompressed size from one snapshot with the compressed size from another, producing an incorrect ratio (including infinity when regions are removed). Capture one aggregate snapshot and use it for both operands.

2. MetricsRegionServerWrapperImpl.java:997 The added source test uses a stub that returns the ratio directly, so it does not exercise this aggregation or the production ratio calculation. Extend TestMetricsRegionServerAggregate, which already validates getStoreFileSize(), to assert the aggregated uncompressed size and computed ratio (including the zero-data case).

[mnpoonia]

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?

RegionMetricAggregate current = aggregate;
if (current.storeFileSize <= 0) {
  return 0.0;
}
return (double) current.storeFileUncompressedSize / current.storeFileSize;

[mnpoonia]

This verifies source export from a fixed wrapper stub, but it does not exercise MetricsRegionServerWrapperImpl aggregation or ratio calculation. Could we extend TestMetricsRegionServerAggregate to assert the summed compressed and uncompressed sizes, along with the resulting uncompressed / compressed ratio, including the empty-aggregate case?

Using stores with different individual ratios would also verify that this remains a weighted aggregate rather than an average of per-store ratios.

Basically +1 to Andrew's comment.