From 415a83f3f3d32f34f27a1dcdca14a1dfd96f7f4b Mon Sep 17 00:00:00 2001 From: David Manning Date: Thu, 1 Oct 2026 12:09:03 -0700 Subject: [PATCH 1/2] HBASE-30447 Table-level metrics can stop updating until the RegionServer restarts --- .../MetricsRegionWrapperImpl.java | 220 +++++++++--------- .../MetricsTableWrapperAggregateImpl.java | 175 +++++++------- 2 files changed, 209 insertions(+), 186 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java index dfcbf3d10312..2d60ea7d5acd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java @@ -275,117 +275,127 @@ public class HRegionMetricsWrapperRunnable implements Runnable { @Override public void run() { - long tempNumStoreFiles = 0; - int tempStoreRefCount = 0; - int tempMaxCompactedStoreFileRefCount = 0; - long tempMemstoreSize = 0; - long tempMemstoreHeapSize = 0; - long tempMemstoreOffHeapSize = 0; - long tempStoreFileSize = 0; - long tempMaxStoreFileAge = 0; - long tempMinStoreFileAge = Long.MAX_VALUE; - long tempNumReferenceFiles = 0; - long tempMaxCompactionQueueSize = 0; - long tempMaxFlushQueueSize = 0; - long avgAgeNumerator = 0; - long numHFiles = 0; - if (region.stores != null) { - for (HStore store : region.stores.values()) { - tempNumStoreFiles += store.getStorefilesCount(); - int currentStoreRefCount = store.getStoreRefCount(); - tempStoreRefCount += currentStoreRefCount; - int currentMaxCompactedStoreFileRefCount = store.getMaxCompactedStoreFileRefCount(); - tempMaxCompactedStoreFileRefCount = - Math.max(tempMaxCompactedStoreFileRefCount, currentMaxCompactedStoreFileRefCount); - final MemStoreSize memStore = store.getMemStoreSize(); - tempMemstoreSize += memStore.getDataSize(); - tempMemstoreHeapSize += memStore.getHeapSize(); - tempMemstoreOffHeapSize += memStore.getOffHeapSize(); - tempStoreFileSize += store.getStorefilesSize(); - OptionalLong storeMaxStoreFileAge = store.getMaxStoreFileAge(); - if ( - storeMaxStoreFileAge.isPresent() - && storeMaxStoreFileAge.getAsLong() > tempMaxStoreFileAge - ) { - tempMaxStoreFileAge = storeMaxStoreFileAge.getAsLong(); + // The executor only reschedules this task (scheduleWithFixedDelay) if run() returns + // normally. Any Throwable that escapes here (e.g. an NPE from a region closing mid-scan) + // is swallowed by the underlying ScheduledFutureTask with no log output anywhere, and this + // task permanently stops running. Catch everything so a failure is visible AND the next + // scheduled run still happens. + try { + long tempNumStoreFiles = 0; + int tempStoreRefCount = 0; + int tempMaxCompactedStoreFileRefCount = 0; + long tempMemstoreSize = 0; + long tempMemstoreHeapSize = 0; + long tempMemstoreOffHeapSize = 0; + long tempStoreFileSize = 0; + long tempMaxStoreFileAge = 0; + long tempMinStoreFileAge = Long.MAX_VALUE; + long tempNumReferenceFiles = 0; + long tempMaxCompactionQueueSize = 0; + long tempMaxFlushQueueSize = 0; + long avgAgeNumerator = 0; + long numHFiles = 0; + if (region.stores != null) { + for (HStore store : region.stores.values()) { + tempNumStoreFiles += store.getStorefilesCount(); + int currentStoreRefCount = store.getStoreRefCount(); + tempStoreRefCount += currentStoreRefCount; + int currentMaxCompactedStoreFileRefCount = store.getMaxCompactedStoreFileRefCount(); + tempMaxCompactedStoreFileRefCount = + Math.max(tempMaxCompactedStoreFileRefCount, currentMaxCompactedStoreFileRefCount); + final MemStoreSize memStore = store.getMemStoreSize(); + tempMemstoreSize += memStore.getDataSize(); + tempMemstoreHeapSize += memStore.getHeapSize(); + tempMemstoreOffHeapSize += memStore.getOffHeapSize(); + tempStoreFileSize += store.getStorefilesSize(); + OptionalLong storeMaxStoreFileAge = store.getMaxStoreFileAge(); + if ( + storeMaxStoreFileAge.isPresent() + && storeMaxStoreFileAge.getAsLong() > tempMaxStoreFileAge + ) { + tempMaxStoreFileAge = storeMaxStoreFileAge.getAsLong(); + } + + OptionalLong storeMinStoreFileAge = store.getMinStoreFileAge(); + if ( + storeMinStoreFileAge.isPresent() + && storeMinStoreFileAge.getAsLong() < tempMinStoreFileAge + ) { + tempMinStoreFileAge = storeMinStoreFileAge.getAsLong(); + } + + long storeHFiles = store.getNumHFiles(); + numHFiles += storeHFiles; + tempNumReferenceFiles += store.getNumReferenceFiles(); + + OptionalDouble storeAvgStoreFileAge = store.getAvgStoreFileAge(); + if (storeAvgStoreFileAge.isPresent()) { + avgAgeNumerator += (long) storeAvgStoreFileAge.getAsDouble() * storeHFiles; + } + if (mixedReadsOnStore == null) { + mixedReadsOnStore = new HashMap(); + } + Long tempVal = mixedReadsOnStore.get(store.getColumnFamilyName()); + if (tempVal == null) { + tempVal = 0L; + } else { + tempVal += store.getMixedRowReadsCount(); + } + mixedReadsOnStore.put(store.getColumnFamilyName(), tempVal); + if (readsOnlyFromMemstore == null) { + readsOnlyFromMemstore = new HashMap(); + } + tempVal = readsOnlyFromMemstore.get(store.getColumnFamilyName()); + if (tempVal == null) { + tempVal = 0L; + } else { + tempVal += store.getMemstoreOnlyRowReadsCount(); + } + readsOnlyFromMemstore.put(store.getColumnFamilyName(), tempVal); } - - OptionalLong storeMinStoreFileAge = store.getMinStoreFileAge(); - if ( - storeMinStoreFileAge.isPresent() - && storeMinStoreFileAge.getAsLong() < tempMinStoreFileAge - ) { - tempMinStoreFileAge = storeMinStoreFileAge.getAsLong(); - } - - long storeHFiles = store.getNumHFiles(); - numHFiles += storeHFiles; - tempNumReferenceFiles += store.getNumReferenceFiles(); - - OptionalDouble storeAvgStoreFileAge = store.getAvgStoreFileAge(); - if (storeAvgStoreFileAge.isPresent()) { - avgAgeNumerator += (long) storeAvgStoreFileAge.getAsDouble() * storeHFiles; - } - if (mixedReadsOnStore == null) { - mixedReadsOnStore = new HashMap(); - } - Long tempVal = mixedReadsOnStore.get(store.getColumnFamilyName()); - if (tempVal == null) { - tempVal = 0L; - } else { - tempVal += store.getMixedRowReadsCount(); - } - mixedReadsOnStore.put(store.getColumnFamilyName(), tempVal); - if (readsOnlyFromMemstore == null) { - readsOnlyFromMemstore = new HashMap(); - } - tempVal = readsOnlyFromMemstore.get(store.getColumnFamilyName()); - if (tempVal == null) { - tempVal = 0L; - } else { - tempVal += store.getMemstoreOnlyRowReadsCount(); + } + MutableLong regionCachedAmount = new MutableLong(0); + region.getBlockCache().getRegionCachedInfo() + .ifPresent(regionCacheRatio -> regionCachedAmount + .addAndGet(regionCacheRatio.getOrDefault(region.getRegionInfo().getEncodedName(), 0L))); + if (tempStoreFileSize > 0) { + LOG.debug("Region {}, had cached {} bytes from a total of {}", + region.getRegionInfo().getEncodedName(), regionCachedAmount.getValue(), + tempStoreFileSize); + currentRegionCacheRatio = regionCachedAmount.floatValue() / tempStoreFileSize; + if (DataTieringManager.getInstance() != null) { + currentRegionColdDataRatio = DataTieringManager.getInstance().getRegionColdDataSize() + .getOrDefault(region.getRegionInfo().getEncodedName(), new Pair<>(null, 0L)) + .getSecond() / (float) tempStoreFileSize; } - readsOnlyFromMemstore.put(store.getColumnFamilyName(), tempVal); } - } - MutableLong regionCachedAmount = new MutableLong(0); - region.getBlockCache().getRegionCachedInfo().ifPresent(regionCacheRatio -> regionCachedAmount - .addAndGet(regionCacheRatio.getOrDefault(region.getRegionInfo().getEncodedName(), 0L))); - if (tempStoreFileSize > 0) { - LOG.debug("Region {}, had cached {} bytes from a total of {}", - region.getRegionInfo().getEncodedName(), regionCachedAmount.getValue(), - tempStoreFileSize); - currentRegionCacheRatio = regionCachedAmount.floatValue() / tempStoreFileSize; - if (DataTieringManager.getInstance() != null) { - currentRegionColdDataRatio = DataTieringManager.getInstance().getRegionColdDataSize() - .getOrDefault(region.getRegionInfo().getEncodedName(), new Pair<>(null, 0L)).getSecond() - / (float) tempStoreFileSize; + numStoreFiles = tempNumStoreFiles; + storeRefCount = tempStoreRefCount; + maxCompactedStoreFileRefCount = tempMaxCompactedStoreFileRefCount; + memstoreSize = tempMemstoreSize; + memstoreHeapSize = tempMemstoreHeapSize; + memstoreOffHeapSize = tempMemstoreOffHeapSize; + storeFileSize = tempStoreFileSize; + maxStoreFileAge = tempMaxStoreFileAge; + if (tempMinStoreFileAge != Long.MAX_VALUE) { + minStoreFileAge = tempMinStoreFileAge; } - } - numStoreFiles = tempNumStoreFiles; - storeRefCount = tempStoreRefCount; - maxCompactedStoreFileRefCount = tempMaxCompactedStoreFileRefCount; - memstoreSize = tempMemstoreSize; - memstoreHeapSize = tempMemstoreHeapSize; - memstoreOffHeapSize = tempMemstoreOffHeapSize; - storeFileSize = tempStoreFileSize; - maxStoreFileAge = tempMaxStoreFileAge; - if (tempMinStoreFileAge != Long.MAX_VALUE) { - minStoreFileAge = tempMinStoreFileAge; - } - if (numHFiles != 0) { - avgStoreFileAge = avgAgeNumerator / numHFiles; - } + if (numHFiles != 0) { + avgStoreFileAge = avgAgeNumerator / numHFiles; + } - numReferenceFiles = tempNumReferenceFiles; - tempMaxCompactionQueueSize = getNumCompactionsQueued(); - tempMaxFlushQueueSize = getNumFlushesQueued(); - if (tempMaxCompactionQueueSize > maxCompactionQueueSize) { - maxCompactionQueueSize = tempMaxCompactionQueueSize; - } - if (tempMaxFlushQueueSize > maxFlushQueueSize) { - maxFlushQueueSize = tempMaxFlushQueueSize; + numReferenceFiles = tempNumReferenceFiles; + tempMaxCompactionQueueSize = getNumCompactionsQueued(); + tempMaxFlushQueueSize = getNumFlushesQueued(); + if (tempMaxCompactionQueueSize > maxCompactionQueueSize) { + maxCompactionQueueSize = tempMaxCompactionQueueSize; + } + if (tempMaxFlushQueueSize > maxFlushQueueSize) { + maxFlushQueueSize = tempMaxFlushQueueSize; + } + } catch (Throwable t) { + LOG.error("Failed to compute per-region metrics; will retry on next scheduled run", t); } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java index 00a4c1a48d5e..f1e48ded5314 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java @@ -31,11 +31,15 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.metrics2.MetricsExecutor; import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.collect.Sets; @InterfaceAudience.Private public class MetricsTableWrapperAggregateImpl implements MetricsTableWrapperAggregate, Closeable { + private static final Logger LOG = + LoggerFactory.getLogger(MetricsTableWrapperAggregateImpl.class); private final HRegionServer regionServer; private ScheduledExecutorService executor; private Runnable runnable; @@ -58,95 +62,104 @@ public class TableMetricsWrapperRunnable implements Runnable { @Override public void run() { - Map localMetricsTableMap = new HashMap<>(); - for (Region r : regionServer.getOnlineRegionsLocalContext()) { - TableName tbl = r.getTableDescriptor().getTableName(); - MetricsTableValues mt = localMetricsTableMap.get(tbl); - if (mt == null) { - mt = new MetricsTableValues(); - localMetricsTableMap.put(tbl, mt); - } - long memstoreReadCount = 0L; - long mixedReadCount = 0L; - String tempKey = null; - if (r.getStores() != null) { - String familyName = null; - for (Store store : r.getStores()) { - familyName = store.getColumnFamilyName(); - - mt.storeFileCount += store.getStorefilesCount(); - mt.maxStoreFileCount = Math.max(mt.maxStoreFileCount, store.getStorefilesCount()); - final MemStoreSize memstoreSize = store.getMemStoreSize(); - mt.memstoreSize += memstoreSize.getDataSize(); - mt.memstoreHeapSize += memstoreSize.getHeapSize(); - mt.memstoreOffHeapSize += memstoreSize.getOffHeapSize(); - mt.storeFileSize += store.getStorefilesSize(); - mt.referenceFileCount += store.getNumReferenceFiles(); - if (store.getMaxStoreFileAge().isPresent()) { - mt.maxStoreFileAge = - Math.max(mt.maxStoreFileAge, store.getMaxStoreFileAge().getAsLong()); - } - if (store.getMinStoreFileAge().isPresent()) { - mt.minStoreFileAge = - Math.min(mt.minStoreFileAge, store.getMinStoreFileAge().getAsLong()); - } - if (store.getAvgStoreFileAge().isPresent()) { - mt.totalStoreFileAge += - (long) (store.getAvgStoreFileAge().getAsDouble() * store.getStorefilesCount()); + // The executor only reschedules this task (scheduleWithFixedDelay) if run() returns + // normally. Any Throwable that escapes here (e.g. an NPE from a region closing mid-scan) + // is swallowed by the underlying ScheduledFutureTask with no log output anywhere, and this + // task permanently stops running. Catch everything so a failure is visible AND the next + // scheduled run still happens. + try { + Map localMetricsTableMap = new HashMap<>(); + for (Region r : regionServer.getOnlineRegionsLocalContext()) { + TableName tbl = r.getTableDescriptor().getTableName(); + MetricsTableValues mt = localMetricsTableMap.get(tbl); + if (mt == null) { + mt = new MetricsTableValues(); + localMetricsTableMap.put(tbl, mt); + } + long memstoreReadCount = 0L; + long mixedReadCount = 0L; + String tempKey = null; + if (r.getStores() != null) { + String familyName = null; + for (Store store : r.getStores()) { + familyName = store.getColumnFamilyName(); + + mt.storeFileCount += store.getStorefilesCount(); + mt.maxStoreFileCount = Math.max(mt.maxStoreFileCount, store.getStorefilesCount()); + final MemStoreSize memstoreSize = store.getMemStoreSize(); + mt.memstoreSize += memstoreSize.getDataSize(); + mt.memstoreHeapSize += memstoreSize.getHeapSize(); + mt.memstoreOffHeapSize += memstoreSize.getOffHeapSize(); + mt.storeFileSize += store.getStorefilesSize(); + mt.referenceFileCount += store.getNumReferenceFiles(); + if (store.getMaxStoreFileAge().isPresent()) { + mt.maxStoreFileAge = + Math.max(mt.maxStoreFileAge, store.getMaxStoreFileAge().getAsLong()); + } + if (store.getMinStoreFileAge().isPresent()) { + mt.minStoreFileAge = + Math.min(mt.minStoreFileAge, store.getMinStoreFileAge().getAsLong()); + } + if (store.getAvgStoreFileAge().isPresent()) { + mt.totalStoreFileAge += + (long) (store.getAvgStoreFileAge().getAsDouble() * store.getStorefilesCount()); + } + mt.storeCount += 1; + + mt.staticIndexSize += store.getTotalStaticIndexSize(); + mt.staticBloomSize += store.getTotalStaticBloomSize(); + + mt.bloomRequestsCount += store.getBloomFilterRequestsCount(); + mt.bloomNegativeResultsCount += store.getBloomFilterNegativeResultsCount(); + mt.bloomEligibleRequestsCount += store.getBloomFilterEligibleRequestsCount(); + + tempKey = tbl.getNameAsString() + HASH + familyName; + Long tempVal = mt.perStoreMemstoreOnlyReadCount.get(tempKey); + if (tempVal == null) { + tempVal = 0L; + } + memstoreReadCount = store.getMemstoreOnlyRowReadsCount() + tempVal; + tempVal = mt.perStoreMixedReadCount.get(tempKey); + if (tempVal == null) { + tempVal = 0L; + } + mixedReadCount = store.getMixedRowReadsCount() + tempVal; + // accumulate the count + mt.perStoreMemstoreOnlyReadCount.put(tempKey, memstoreReadCount); + mt.perStoreMixedReadCount.put(tempKey, mixedReadCount); + mt.perStoreFileSize.merge(tempKey, store.getStorefilesSize(), Long::sum); } - mt.storeCount += 1; - mt.staticIndexSize += store.getTotalStaticIndexSize(); - mt.staticBloomSize += store.getTotalStaticBloomSize(); + mt.regionCount += 1; - mt.bloomRequestsCount += store.getBloomFilterRequestsCount(); - mt.bloomNegativeResultsCount += store.getBloomFilterNegativeResultsCount(); - mt.bloomEligibleRequestsCount += store.getBloomFilterEligibleRequestsCount(); - - tempKey = tbl.getNameAsString() + HASH + familyName; - Long tempVal = mt.perStoreMemstoreOnlyReadCount.get(tempKey); - if (tempVal == null) { - tempVal = 0L; - } - memstoreReadCount = store.getMemstoreOnlyRowReadsCount() + tempVal; - tempVal = mt.perStoreMixedReadCount.get(tempKey); - if (tempVal == null) { - tempVal = 0L; - } - mixedReadCount = store.getMixedRowReadsCount() + tempVal; - // accumulate the count - mt.perStoreMemstoreOnlyReadCount.put(tempKey, memstoreReadCount); - mt.perStoreMixedReadCount.put(tempKey, mixedReadCount); - mt.perStoreFileSize.merge(tempKey, store.getStorefilesSize(), Long::sum); + mt.readRequestCount += r.getReadRequestsCount(); + mt.filteredReadRequestCount += r.getFilteredReadRequestsCount(); + mt.writeRequestCount += r.getWriteRequestsCount(); } - - mt.regionCount += 1; - - mt.readRequestCount += r.getReadRequestsCount(); - mt.filteredReadRequestCount += r.getFilteredReadRequestsCount(); - mt.writeRequestCount += r.getWriteRequestsCount(); } - } - for (Map.Entry entry : localMetricsTableMap.entrySet()) { - TableName tbl = entry.getKey(); - if (metricsTableMap.get(tbl) == null) { - // this will add the Wrapper to the list of TableMetrics - CompatibilitySingletonFactory.getInstance(MetricsRegionServerSourceFactory.class) - .getTableAggregate() - .getOrCreateTableSource(tbl.getNameAsString(), MetricsTableWrapperAggregateImpl.this); + for (Map.Entry entry : localMetricsTableMap.entrySet()) { + TableName tbl = entry.getKey(); + if (metricsTableMap.get(tbl) == null) { + // this will add the Wrapper to the list of TableMetrics + CompatibilitySingletonFactory.getInstance(MetricsRegionServerSourceFactory.class) + .getTableAggregate() + .getOrCreateTableSource(tbl.getNameAsString(), MetricsTableWrapperAggregateImpl.this); + } + metricsTableMap.put(entry.getKey(), entry.getValue()); } - metricsTableMap.put(entry.getKey(), entry.getValue()); - } - Set existingTableNames = Sets.newHashSet(metricsTableMap.keySet()); - existingTableNames.removeAll(localMetricsTableMap.keySet()); - MetricsTableAggregateSource agg = CompatibilitySingletonFactory - .getInstance(MetricsRegionServerSourceFactory.class).getTableAggregate(); - for (TableName table : existingTableNames) { - agg.deleteTableSource(table.getNameAsString()); - if (metricsTableMap.get(table) != null) { - metricsTableMap.remove(table); + Set existingTableNames = Sets.newHashSet(metricsTableMap.keySet()); + existingTableNames.removeAll(localMetricsTableMap.keySet()); + MetricsTableAggregateSource agg = CompatibilitySingletonFactory + .getInstance(MetricsRegionServerSourceFactory.class).getTableAggregate(); + for (TableName table : existingTableNames) { + agg.deleteTableSource(table.getNameAsString()); + if (metricsTableMap.get(table) != null) { + metricsTableMap.remove(table); + } } + } catch (Throwable t) { + LOG.error("Failed to compute per-table metrics; will retry on next scheduled run", t); } } } From ce99f6aecddb0d8e18595f48cd0a59078c74519e Mon Sep 17 00:00:00 2001 From: David Manning Date: Mon, 5 Oct 2026 12:15:52 -0700 Subject: [PATCH 2/2] HBASE-30447 Apply spotless formatting Co-Authored-By: Claude Sonnet 5.5 --- .../hbase/regionserver/MetricsTableWrapperAggregateImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java index f1e48ded5314..79fbd6529524 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableWrapperAggregateImpl.java @@ -38,8 +38,7 @@ @InterfaceAudience.Private public class MetricsTableWrapperAggregateImpl implements MetricsTableWrapperAggregate, Closeable { - private static final Logger LOG = - LoggerFactory.getLogger(MetricsTableWrapperAggregateImpl.class); + private static final Logger LOG = LoggerFactory.getLogger(MetricsTableWrapperAggregateImpl.class); private final HRegionServer regionServer; private ScheduledExecutorService executor; private Runnable runnable;