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"); + } +}