From 907e27e1b80418bac6b0208865b9da1f8f7e6262 Mon Sep 17 00:00:00 2001 From: haosenchen Date: Fri, 11 Sep 2026 11:33:50 +0800 Subject: [PATCH] HBASE-27486 Fix per-table MetricsTableLatencies/QueryMeter memory leak Backport of the registry-cleanup mechanism only. The per-table MetricsTableLatenciesImpl.histogramsByTable and MetricsTableQueryMeterImpl.metersByTable are populated on first access but never cleaned up when a table leaves the RegionServer (drop / disable / move / region close). For short-lived tables (e.g. hourly tables with a 7-day retention window) this leaks both the maps and the per-table entries under the sub=TableLatencies JMX bean without bound. * MetricsTableLatencies / MetricsTableQueryMeter: add deleteTable(...) on both interfaces and their impls. * MetricsTableLatenciesImpl: switch histogramsByTable to ConcurrentHashMap + computeIfAbsent (also closes a pre-existing TODO) and implement deleteTable(String) which removes the entry AND unregisters the 12 MutableTimeHistogram / MutableSizeHistogram objects from the underlying DynamicMetricsRegistry via removeMetric(baseName). removeMetric is used rather than removeHistogramMetrics because the histogram is stored under baseName itself and the 14 suffix metrics (_num_ops / _sum_ops / _min / _max / _mean / *_percentile) are produced dynamically at snapshot time -- removeHistogramMetrics only tries to remove baseName + 8 fixed suffix keys that never actually exist in the map, so it is a no-op for MutableHistogram entries. * MetricsTableQueryMeterImpl: implement deleteTable(TableName) that removes the entry and unregisters both read/write meters from the shared MetricRegistry. * RegionServerTableMetrics: expose a single deleteTable(TableName) that forwards to both cleanup paths. * MetricsRegionServer: expose getRegionServerTableMetrics() so the cleanup driver can reach the container. * MetricsTableWrapperAggregateImpl.TableMetricsWrapperRunnable: compute onlineTables at the top of run(), take the difference with a new lastSeenTables snapshot, and invoke RegionServerTableMetrics.deleteTable() for every table that has left the RegionServer since the previous scheduled run. The cleanup path runs before the aggregate metrics collection body so it also fires for users that only enabled per-table latency / query meters and not the aggregate table metrics. Tests ----- * TestMetricsTableLatencies#testDeleteTableRemovesAllLatencyHistograms (new, unit): verifies that all 12 histogram families of the dropped table disappear, other tables are untouched, re-adding samples lazily re-registers the histograms, and deleting an unknown table is a no-op. * TestMetricsTableWrapperCleanup (new, MediumTests, end-to-end): wires a mocked HRegionServer + real RegionServerTableMetrics + real MetricsTableLatenciesImpl and hand-drives two rounds of TableMetricsWrapperRunnable across an online-regions transition {keep_table, drop_table} -> {keep_table}. Asserts that every latency histogram and both query meters of drop_table have vanished from the metrics registry / JMX snapshot after round 2 while keep_table is preserved. Notes ----- HBASE-27681 (the upstream fix for HBASE-27486 on branch-2.6+) is not backported wholesale because it renames the JMX beans (sub=TableLatencies -> sub=TableRequests_) which is not backward-compatible for existing monitoring. This change intentionally keeps the JMX bean names and metric key layout stable and only backports the registry cleanup mechanism. Signed-off-by: haosenchen --- .../regionserver/MetricsTableLatencies.java | 10 + .../regionserver/MetricsTableQueryMeter.java | 9 + .../MetricsTableLatenciesImpl.java | 51 +++- .../MetricsTableQueryMeterImpl.java | 11 + .../regionserver/MetricsRegionServer.java | 10 + .../MetricsTableWrapperAggregateImpl.java | 35 +++ .../RegionServerTableMetrics.java | 11 + .../TestMetricsTableLatencies.java | 96 ++++++++ .../TestMetricsTableWrapperCleanup.java | 220 ++++++++++++++++++ 9 files changed, 445 insertions(+), 8 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableWrapperCleanup.java diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java index e7d447aef491..def1dd5b1d4a 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java @@ -142,4 +142,14 @@ public interface MetricsTableLatencies { */ void updateCheckAndMutate(String nameAsString, long time); + /** + * Remove all latency histograms of the given table. Should be called when the table is no + * longer online on this RegionServer (e.g. dropped, moved out, disabled) to avoid unbounded + * growth of histogramsByTable and the associated metrics registry (see HBASE-27486 / + * HBASE-27681). + * + * @param tableName The table whose latency histograms should be removed. + */ + void deleteTable(String tableName); + } diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java index 1ff86de67ec5..3f5e9a5dbe49 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeter.java @@ -54,4 +54,13 @@ public interface MetricsTableQueryMeter { * @param tableName The table the metric is for */ void updateTableWriteQueryMeter(TableName tableName); + + /** + * Remove the read/write query meters of the given table. Should be called when the table is no + * longer online on this RegionServer to avoid unbounded growth of metersByTable and the + * associated metric registry (see HBASE-27486 / HBASE-27681). + * + * @param tableName The table whose meters should be removed. + */ + void deleteTable(TableName tableName); } diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java index 1c90b33d5d4d..7478fe8b5200 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java @@ -17,7 +17,7 @@ */ package org.apache.hadoop.hbase.regionserver; -import java.util.HashMap; +import java.util.concurrent.ConcurrentHashMap; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.metrics.BaseSourceImpl; import org.apache.hadoop.metrics2.MetricHistogram; @@ -33,7 +33,8 @@ @InterfaceAudience.Private public class MetricsTableLatenciesImpl extends BaseSourceImpl implements MetricsTableLatencies { - private final HashMap histogramsByTable = new HashMap<>(); + private final ConcurrentHashMap histogramsByTable = + new ConcurrentHashMap<>(); public static class TableHistograms { final MetricHistogram getTimeHisto; @@ -124,14 +125,48 @@ public static String qualifyMetricsName(TableName tableName, String metric) { } public TableHistograms getOrCreateTableHistogram(String tableName) { - // TODO Java8's ConcurrentHashMap#computeIfAbsent would be stellar instead final TableName tn = TableName.valueOf(tableName); - TableHistograms latency = histogramsByTable.get(tn); - if (latency == null) { - latency = new TableHistograms(getMetricsRegistry(), tn); - histogramsByTable.put(tn, latency); + return histogramsByTable.computeIfAbsent(tn, + t -> new TableHistograms(getMetricsRegistry(), t)); + } + + /** + * Remove all latency histograms of the given table from both {@link #histogramsByTable} and the + * underlying {@link DynamicMetricsRegistry}. This should be called when the table is no longer + * online on this RegionServer to avoid unbounded growth (see HBASE-27486 / HBASE-27681). + * + *

Note: this only removes the histograms registered by {@link TableHistograms}. Other + * per-table metrics registered by different components (e.g. + * {@link MetricsTableQueryMeterImpl}) are cleaned up separately.

+ * + *

Implementation detail: each {@code MutableHistogram} is registered in the underlying + * {@link DynamicMetricsRegistry#metricsMap} under its base name (e.g. + * {@code Namespace_default_table_foo_metric_getTime}); the {@code _num_ops / _min / _max / + * _mean / *_percentile} suffixes are produced dynamically at snapshot time and are NOT stored + * as separate map entries. Therefore we must call {@link DynamicMetricsRegistry#removeMetric} + * on the base name to actually stop them from showing up in JMX; calling + * {@code removeHistogramMetrics} alone is a no-op for this scenario.

+ */ + @Override + public void deleteTable(String tableName) { + final TableName tn = TableName.valueOf(tableName); + TableHistograms removed = histogramsByTable.remove(tn); + if (removed == null) { + return; } - return latency; + DynamicMetricsRegistry reg = getMetricsRegistry(); + reg.removeMetric(qualifyMetricsName(tn, GET_TIME)); + reg.removeMetric(qualifyMetricsName(tn, INCREMENT_TIME)); + reg.removeMetric(qualifyMetricsName(tn, APPEND_TIME)); + reg.removeMetric(qualifyMetricsName(tn, PUT_TIME)); + reg.removeMetric(qualifyMetricsName(tn, PUT_BATCH_TIME)); + reg.removeMetric(qualifyMetricsName(tn, DELETE_TIME)); + reg.removeMetric(qualifyMetricsName(tn, DELETE_BATCH_TIME)); + reg.removeMetric(qualifyMetricsName(tn, SCAN_TIME)); + reg.removeMetric(qualifyMetricsName(tn, SCAN_SIZE)); + reg.removeMetric(qualifyMetricsName(tn, CHECK_AND_DELETE_TIME)); + reg.removeMetric(qualifyMetricsName(tn, CHECK_AND_PUT_TIME)); + reg.removeMetric(qualifyMetricsName(tn, CHECK_AND_MUTATE_TIME)); } public MetricsTableLatenciesImpl() { diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java index dc53c940166d..04a59ea8c23e 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableQueryMeterImpl.java @@ -96,4 +96,15 @@ public void updateTableWriteQueryMeter(TableName tableName, long count) { public void updateTableWriteQueryMeter(TableName tableName) { getOrCreateTableMeter(tableName).updateTableWriteQueryMeter(); } + + @Override + public void deleteTable(TableName tableName) { + TableMeters removed = metersByTable.remove(tableName); + if (removed == null) { + return; + } + // Also remove the meters from the underlying MetricRegistry so they no longer show up in JMX. + metricRegistry.remove(qualifyMetricsName(tableName, TABLE_READ_QUERY_PER_SECOND)); + metricRegistry.remove(qualifyMetricsName(tableName, TABLE_WRITE_QUERY_PER_SECOND)); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java index ba86afc4cb0a..7b7cea9171c6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java @@ -122,6 +122,16 @@ public MetricsRegionServerWrapper getRegionServerWrapper() { return regionServerWrapper; } + /** + * Returns the per-table metrics container (may be {@code null} if + * {@link #RS_ENABLE_TABLE_METRICS_KEY} is disabled). Exposed so that callers such as + * {@link MetricsTableWrapperAggregateImpl} can clean up per-table latency / query meter metrics + * when a table leaves the RegionServer. + */ + public RegionServerTableMetrics getRegionServerTableMetrics() { + return tableMetrics; + } + public void updatePutBatch(TableName tn, long t) { if (tableMetrics != null && tn != null) { tableMetrics.updatePutBatch(tn, 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 2794df9d49c9..65fda7b6c107 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 @@ -20,6 +20,7 @@ import java.io.Closeable; import java.io.IOException; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -43,6 +44,16 @@ public class MetricsTableWrapperAggregateImpl implements MetricsTableWrapperAggr private ScheduledFuture tableMetricsUpdateTask; private ConcurrentHashMap metricsTableMap = new ConcurrentHashMap<>(); + /** + * Tables that were seen online in the previous scheduled run. Used to detect tables that + * have left this RegionServer so that their per-table latency histograms and query meters + * can be cleaned up (HBASE-27486). Per-table latencies and query meters are controlled by + * their own switches (hbase.regionserver.enable.table.latencies / + * hbase.regionserver.enable.table.queryMeter); this cleanup path runs unconditionally so + * that users who enabled either of those switches still get their per-table metrics + * released when a table leaves the RegionServer. + */ + private final Set lastSeenTables = ConcurrentHashMap.newKeySet(); public MetricsTableWrapperAggregateImpl(final HRegionServer regionServer) { this.regionServer = regionServer; @@ -58,6 +69,30 @@ public class TableMetricsWrapperRunnable implements Runnable { @Override public void run() { + // Collect currently online tables first so that the per-table latency / query meter + // cleanup path can run before the aggregate metrics collection below. + Set onlineTables = new HashSet<>(); + for (Region r : regionServer.getOnlineRegionsLocalContext()) { + onlineTables.add(r.getTableDescriptor().getTableName()); + } + + // ---- Cleanup path for per-table latency histograms & query meters (HBASE-27486) ---- + // Tables that were seen last round but are no longer online must have their per-table + // metrics removed to avoid unbounded growth for short-lived tables. + RegionServerTableMetrics rsTableMetrics = regionServer.getMetrics() != null + ? regionServer.getMetrics().getRegionServerTableMetrics() + : null; + if (rsTableMetrics != null) { + Set goneTables = Sets.newHashSet(lastSeenTables); + goneTables.removeAll(onlineTables); + for (TableName gone : goneTables) { + rsTableMetrics.deleteTable(gone); + } + } + // Refresh lastSeenTables snapshot for next round. + lastSeenTables.clear(); + lastSeenTables.addAll(onlineTables); + Map localMetricsTableMap = new HashMap<>(); for (Region r : regionServer.getOnlineRegionsLocalContext()) { TableName tbl = r.getTableDescriptor().getTableName(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java index efc103b8bca7..16695ec66006 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java @@ -105,4 +105,15 @@ public void updateTableWriteQueryMeter(TableName table) { queryMeter.updateTableWriteQueryMeter(table); } } + + /** + * Clean up all per-table metrics of the given table. Should be called when the table leaves the + * RegionServer (dropped, moved, or all its regions closed) so that the underlying + * {@link MetricsTableLatencies} / {@link MetricsTableQueryMeter} do not keep growing forever + * (see HBASE-27486 / HBASE-27681). + */ + public void deleteTable(TableName table) { + latencies.deleteTable(table.getNameAsString()); + queryMeter.deleteTable(table); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java index 33a3d8bde63d..a4fc359bd90a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java @@ -115,4 +115,100 @@ public void testTableQueryMeterSwitch() { MetricsTableQueryMeterImpl.TABLE_WRITE_QUERY_PER_SECOND + "_" + "count"), 500L, latenciesImpl); } + /** + * Verifies that {@link MetricsTableLatencies#deleteTable(String)} removes every histogram + * family (across all metric suffixes) previously registered for the given table on + * {@link MetricsTableLatenciesImpl}, while leaving other tables untouched. Also verifies that + * re-writing samples for the dropped table lazily re-registers its histograms and that + * deleting an unknown table is a no-op (HBASE-27486). + */ + @Test + public void testDeleteTableRemovesAllLatencyHistograms() throws IOException { + TableName tnKeep = TableName.valueOf("keep_table"); + TableName tnDrop = TableName.valueOf("drop_table"); + MetricsTableLatencies latencies = + CompatibilitySingletonFactory.getInstance(MetricsTableLatencies.class); + assertTrue(latencies instanceof MetricsTableLatenciesImpl, + "'latencies' is actually " + latencies.getClass()); + MetricsTableLatenciesImpl latenciesImpl = (MetricsTableLatenciesImpl) latencies; + RegionServerTableMetrics tableMetrics = new RegionServerTableMetrics(false); + + // Every metric family registerd by MetricsTableLatenciesImpl.TableHistograms for a table. + String[] families = new String[] { + MetricsTableLatencies.GET_TIME, + MetricsTableLatencies.PUT_TIME, + MetricsTableLatencies.PUT_BATCH_TIME, + MetricsTableLatencies.DELETE_TIME, + MetricsTableLatencies.DELETE_BATCH_TIME, + MetricsTableLatencies.INCREMENT_TIME, + MetricsTableLatencies.APPEND_TIME, + MetricsTableLatencies.SCAN_TIME, + MetricsTableLatencies.SCAN_SIZE, + MetricsTableLatencies.CHECK_AND_DELETE_TIME, + MetricsTableLatencies.CHECK_AND_PUT_TIME, + MetricsTableLatencies.CHECK_AND_MUTATE_TIME }; + + // Populate both tables so every histogram family is registered in the underlying + // DynamicMetricsRegistry. + tableMetrics.updateGet(tnKeep, 100L); + tableMetrics.updatePut(tnKeep, 20L); + tableMetrics.updatePutBatch(tnKeep, 21L); + tableMetrics.updateDelete(tnKeep, 22L); + tableMetrics.updateDeleteBatch(tnKeep, 23L); + tableMetrics.updateIncrement(tnKeep, 24L); + tableMetrics.updateAppend(tnKeep, 25L); + tableMetrics.updateScanTime(tnKeep, 26L); + tableMetrics.updateScanSize(tnKeep, 27L); + tableMetrics.updateCheckAndDelete(tnKeep, 28L); + tableMetrics.updateCheckAndPut(tnKeep, 29L); + tableMetrics.updateCheckAndMutate(tnKeep, 30L); + + tableMetrics.updateGet(tnDrop, 200L); + tableMetrics.updatePut(tnDrop, 40L); + tableMetrics.updatePutBatch(tnDrop, 41L); + tableMetrics.updateDelete(tnDrop, 42L); + tableMetrics.updateDeleteBatch(tnDrop, 43L); + tableMetrics.updateIncrement(tnDrop, 44L); + tableMetrics.updateAppend(tnDrop, 45L); + tableMetrics.updateScanTime(tnDrop, 46L); + tableMetrics.updateScanSize(tnDrop, 47L); + tableMetrics.updateCheckAndDelete(tnDrop, 48L); + tableMetrics.updateCheckAndPut(tnDrop, 49L); + tableMetrics.updateCheckAndMutate(tnDrop, 50L); + + // Sanity: every family exists for both tables before deletion. + for (String f : families) { + assertTrue( + HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(tnKeep, f) + + "_999th_percentile", latenciesImpl), + "keep_table." + f + " should exist before deleteTable"); + assertTrue( + HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(tnDrop, f) + + "_999th_percentile", latenciesImpl), + "drop_table." + f + " should exist before deleteTable"); + } + + // Act: drop only tnDrop. + latencies.deleteTable(tnDrop.getNameAsString()); + + // Assert: all histogram families of tnDrop are gone from the registry, tnKeep is intact. + for (String f : families) { + assertFalse( + HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(tnDrop, f) + + "_999th_percentile", latenciesImpl), + "drop_table." + f + " should have been removed by deleteTable"); + assertTrue( + HELPER.checkGaugeExists(MetricsTableLatenciesImpl.qualifyMetricsName(tnKeep, f) + + "_999th_percentile", latenciesImpl), + "keep_table." + f + " must not be affected by deleteTable(drop_table)"); + } + + // Re-adding samples for the dropped table should lazily re-register its histograms. + tableMetrics.updateGet(tnDrop, 999L); + HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(tnDrop, + MetricsTableLatencies.GET_TIME) + "_999th_percentile", 999L, latenciesImpl); + + // Deleting an unknown table must be a no-op. + latencies.deleteTable(TableName.valueOf("never_seen").getNameAsString()); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableWrapperCleanup.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableWrapperCleanup.java new file mode 100644 index 000000000000..15017e772e3f --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableWrapperCleanup.java @@ -0,0 +1,220 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.regionserver; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.CompatibilityFactory; +import org.apache.hadoop.hbase.CompatibilitySingletonFactory; +import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.test.MetricsAssertHelper; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import org.apache.hbase.thirdparty.com.google.common.collect.Lists; + +/** + * End-to-end verification of the per-table metric cleanup path added in HBASE-27486. + * + *

Drives two successive rounds of + * {@link MetricsTableWrapperAggregateImpl.TableMetricsWrapperRunnable#run()} against a mocked + * {@link HRegionServer} whose online-region set transitions from + * {keep_table, drop_table} to {keep_table}, then asserts that every + * per-table latency histogram and every per-table query-meter belonging to + * drop_table has been removed from the shared metric registry, while everything + * belonging to keep_table is preserved. + */ +@Tag(SmallTests.TAG) +@Tag(RegionServerTests.TAG) +public class TestMetricsTableWrapperCleanup { + + private static final MetricsAssertHelper HELPER = + CompatibilityFactory.getInstance(MetricsAssertHelper.class); + + private static final TableName KEEP = TableName.valueOf("keep_table"); + private static final TableName DROP = TableName.valueOf("drop_table"); + + @Test + public void testMetricsAreRemovedWhenTableLeavesRegionServer() throws IOException { + // A real RegionServerTableMetrics so we exercise both the latency and query-meter cleanup + // paths through RegionServerTableMetrics.deleteTable(TableName). + RegionServerTableMetrics tableMetrics = new RegionServerTableMetrics(true); + MetricsTableLatencies latencies = + CompatibilitySingletonFactory.getInstance(MetricsTableLatencies.class); + assertTrue(latencies instanceof MetricsTableLatenciesImpl, + "expected MetricsTableLatenciesImpl, got " + latencies.getClass()); + // Both the latency histograms and the query meters live under this BaseSource, so we can + // use it as the target for HELPER.checkGaugeExists() for both metric bean families. + MetricsTableLatenciesImpl source = (MetricsTableLatenciesImpl) latencies; + + // Populate metrics for both tables via the public RegionServerTableMetrics API so every + // histogram / meter family is registered in the underlying DynamicMetricsRegistry. + updateAllLatencies(tableMetrics, KEEP); + updateAllLatencies(tableMetrics, DROP); + tableMetrics.updateTableReadQueryMeter(KEEP, 3L); + tableMetrics.updateTableWriteQueryMeter(KEEP, 4L); + tableMetrics.updateTableReadQueryMeter(DROP, 5L); + tableMetrics.updateTableWriteQueryMeter(DROP, 6L); + + // Sanity: every family exists for both tables before we run the wrapper. + assertLatencyGaugesExist(source, KEEP); + assertLatencyGaugesExist(source, DROP); + assertQueryMeterGaugesExist(source, KEEP); + assertQueryMeterGaugesExist(source, DROP); + + // Wire up a mocked HRegionServer whose online regions transition from {KEEP, DROP} on round + // 1 to {KEEP} on round 2. Long metrics-period so the internal ScheduledFuture never fires + // on its own; we drive runnable.run() by hand for determinism. + Configuration conf = HBaseConfiguration.create(); + conf.setLong(HConstants.REGIONSERVER_METRICS_PERIOD, 600 * 1000); + + HRegionServer rs = mock(HRegionServer.class); + when(rs.getConfiguration()).thenReturn(conf); + MetricsRegionServer mrs = mock(MetricsRegionServer.class); + when(mrs.getRegionServerTableMetrics()).thenReturn(tableMetrics); + when(rs.getMetrics()).thenReturn(mrs); + List roundOneRegions = Lists.newArrayList(regionOf(KEEP), regionOf(DROP)); + List roundTwoRegions = Collections.singletonList(regionOf(KEEP)); + when(rs.getOnlineRegionsLocalContext()).thenReturn(roundOneRegions).thenReturn(roundTwoRegions); + + MetricsTableWrapperAggregateImpl wrapper = new MetricsTableWrapperAggregateImpl(rs); + try { + MetricsTableWrapperAggregateImpl.TableMetricsWrapperRunnable runnable = + wrapper.new TableMetricsWrapperRunnable(); + + // Round 1: both tables online -> lastSeenTables becomes {KEEP, DROP}, no cleanup fires. + runnable.run(); + assertLatencyGaugesExist(source, KEEP); + assertLatencyGaugesExist(source, DROP); + assertQueryMeterGaugesExist(source, KEEP); + assertQueryMeterGaugesExist(source, DROP); + + // Round 2: DROP has left the RS -> lastSeenTables diff detects it and + // RegionServerTableMetrics.deleteTable(DROP) fires. + runnable.run(); + assertLatencyGaugesExist(source, KEEP); + assertQueryMeterGaugesExist(source, KEEP); + assertLatencyGaugesAbsent(source, DROP); + assertQueryMeterGaugesAbsent(source, DROP); + } finally { + wrapper.close(); + } + } + + // ------------------------------------------------------------------- helpers ---- + + private static HRegion regionOf(TableName table) { + TableDescriptor descriptor = mock(TableDescriptor.class); + when(descriptor.getTableName()).thenReturn(table); + HRegion region = mock(HRegion.class); + when(region.getTableDescriptor()).thenReturn(descriptor); + // No stores so the per-store aggregation loop in TableMetricsWrapperRunnable is a no-op; + // we only care about the cleanup path here. + when(region.getStores()).thenReturn(Collections.emptyList()); + return region; + } + + private static void updateAllLatencies(RegionServerTableMetrics tableMetrics, TableName t) { + tableMetrics.updateGet(t, 1L); + tableMetrics.updatePut(t, 1L); + tableMetrics.updatePutBatch(t, 1L); + tableMetrics.updateDelete(t, 1L); + tableMetrics.updateDeleteBatch(t, 1L); + tableMetrics.updateIncrement(t, 1L); + tableMetrics.updateAppend(t, 1L); + tableMetrics.updateScanTime(t, 1L); + tableMetrics.updateScanSize(t, 1L); + tableMetrics.updateCheckAndDelete(t, 1L); + tableMetrics.updateCheckAndPut(t, 1L); + tableMetrics.updateCheckAndMutate(t, 1L); + } + + private static final String[] LATENCY_FAMILIES = new String[] { + MetricsTableLatencies.GET_TIME, + MetricsTableLatencies.PUT_TIME, + MetricsTableLatencies.PUT_BATCH_TIME, + MetricsTableLatencies.DELETE_TIME, + MetricsTableLatencies.DELETE_BATCH_TIME, + MetricsTableLatencies.INCREMENT_TIME, + MetricsTableLatencies.APPEND_TIME, + MetricsTableLatencies.SCAN_TIME, + MetricsTableLatencies.SCAN_SIZE, + MetricsTableLatencies.CHECK_AND_DELETE_TIME, + MetricsTableLatencies.CHECK_AND_PUT_TIME, + MetricsTableLatencies.CHECK_AND_MUTATE_TIME }; + + private static void assertLatencyGaugesExist(MetricsTableLatenciesImpl source, TableName t) { + for (String f : LATENCY_FAMILIES) { + assertTrue( + HELPER.checkGaugeExists( + MetricsTableLatenciesImpl.qualifyMetricsName(t, f) + "_999th_percentile", source), + t + "." + f + " should exist"); + } + } + + private static void assertLatencyGaugesAbsent(MetricsTableLatenciesImpl source, TableName t) { + for (String f : LATENCY_FAMILIES) { + assertFalse( + HELPER.checkGaugeExists( + MetricsTableLatenciesImpl.qualifyMetricsName(t, f) + "_999th_percentile", source), + t + "." + f + " should have been removed by deleteTable"); + } + } + + private static void assertQueryMeterGaugesExist(MetricsTableLatenciesImpl source, TableName t) { + assertTrue( + HELPER.checkGaugeExists( + MetricsTableLatenciesImpl.qualifyMetricsName(t, + MetricsTableQueryMeterImpl.TABLE_READ_QUERY_PER_SECOND) + "_count", + source), + t + " read query meter should exist"); + assertTrue( + HELPER.checkGaugeExists( + MetricsTableLatenciesImpl.qualifyMetricsName(t, + MetricsTableQueryMeterImpl.TABLE_WRITE_QUERY_PER_SECOND) + "_count", + source), + t + " write query meter should exist"); + } + + private static void assertQueryMeterGaugesAbsent(MetricsTableLatenciesImpl source, TableName t) { + assertFalse( + HELPER.checkGaugeExists( + MetricsTableLatenciesImpl.qualifyMetricsName(t, + MetricsTableQueryMeterImpl.TABLE_READ_QUERY_PER_SECOND) + "_count", + source), + t + " read query meter should have been removed by deleteTable"); + assertFalse( + HELPER.checkGaugeExists( + MetricsTableLatenciesImpl.qualifyMetricsName(t, + MetricsTableQueryMeterImpl.TABLE_WRITE_QUERY_PER_SECOND) + "_count", + source), + t + " write query meter should have been removed by deleteTable"); + } +}