[
https://issues.apache.org/jira/browse/HBASE-27486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18114081#comment-18114081
]
haosen chen commented on HBASE-27486:
-------------------------------------
Hi, I've opened PR #8648 proposing a fix for this issue on branch-2.5. Kindly
requesting a review from anyone familiar with the RegionServer metrics stack.
Scope
The PR fixes the per-table entry leak on the sub=TableLatencies JMX bean, i.e.
the two ConcurrentHashMaps inside MetricsTableLatenciesImpl.histogramsByTable
and MetricsTableQueryMeterImpl.metersByTable. On a RegionServer that regularly
sees short-lived tables (e.g. hourly rolling tables with a 7-day retention
window) these maps -- and the 12 latency histograms + 2 query meters they
register per table into the shared DynamicMetricsRegistry -- grow without bound
because nothing ever removes them when a table leaves the RegionServer (drop /
disable / move / region close).
Approach (kept intentionally narrow for a branch-2.5 backport)
Add deleteTable(...) on MetricsTableLatencies / MetricsTableQueryMeter and
implement it on both *Impls. The implementation removes the map entry AND calls
DynamicMetricsRegistry.removeMetric(baseName) for every histogram/meter
registered under the table. removeMetric(baseName) is used rather than
removeHistogramMetrics(baseName) because the MutableHistogram is stored under
baseName itself while the 14 percentile / count suffix metrics are produced
dynamically at snapshot() time and are not stored as separate map entries --
calling removeHistogramMetrics(baseName) alone would leave the MutableHistogram
orphaned in the registry.
Wire a differential cleanup driver into
MetricsTableWrapperAggregateImpl.TableMetricsWrapperRunnable.run(): keep a
lastSeenTables snapshot, diff it against the current online-region set at the
top of every scheduled run, and call RegionServerTableMetrics.deleteTable(gone)
for every table that has left the RegionServer since the previous round. The
diff runs unconditionally so users who only enabled per-table latency /
query-meter metrics (and not the aggregate table wrapper) still get their
per-table metrics released.
No JMX bean rename, no metric key rename, no config knob added -- this is
intentionally minimal and backward-compatible for existing monitoring
dashboards.
Note on scope vs. sub=Tables
The sibling sub=Tables bean populated by MetricsTableSourceImpl had the same
class of leak, but that path has already been fixed on branch-2.5 by
HBASE-27440 (fix table HistogramMetrics leak in table metrics map, commit
784d1a96cd2), which added metricsMap.remove(baseName) to the bottom of
DynamicMetricsRegistry.removeHistogramMetrics(baseName). That already lets
MetricsTableSourceImpl.deregisterMetrics() -> removeHistogramMetrics(...) do
the right thing end-to-end. So this PR intentionally does not touch
MetricsTableSourceImpl -- the residual leak is only on the two beans populated
by MetricsTableLatenciesImpl / MetricsTableQueryMeterImpl, which is exactly
what HBASE-27486 originally reported.
Testing
Two new tests, both SmallTests, in the style of the existing
TestMetricsTableWrapperAggregate / TestMetricsTableLatencies:
TestMetricsTableLatencies#testDeleteTableRemovesAllLatencyHistograms -- unit
test: writes samples for two tables through the public RegionServerTableMetrics
API, calls deleteTable(drop), then asserts via HELPER.checkGaugeExists() that
all 12 histogram families for drop_table are gone while keep_table is
untouched, and that re-writing samples for the dropped table lazily
re-registers its histograms.
TestMetricsTableWrapperCleanup#testMetricsAreRemovedWhenTableLeavesRegionServer
-- end-to-end: builds a mocked HRegionServer whose
getOnlineRegionsLocalContext() returns \{keep, drop} on round 1 and \{keep} on
round 2, drives two hand-rolled TableMetricsWrapperRunnable.run() invocations,
and asserts every latency histogram + both query meters for drop_table
disappear after round 2 while keep_table survives.
Both tests were reverse-verified locally by stubbing out
MetricsTableLatenciesImpl.deleteTable() to a no-op: the two new tests fail
while the pre-existing tests continue to pass, so the assertions actually
distinguish a real cleanup from a no-op.
The change is 9 files, +445 / -8, DCO-signed. Happy to iterate on API naming
(deleteTable vs. removeTable etc.) or on the choice of cleanup trigger point
(TableMetricsWrapperRunnable vs. a listener on region close) if you'd prefer a
different shape.
Thanks!
> HTable MetricsTableLatencies not remove trigger memory leak
> ------------------------------------------------------------
>
> Key: HBASE-27486
> URL: https://issues.apache.org/jira/browse/HBASE-27486
> Project: HBase
> Issue Type: Bug
> Components: metrics, regionserver
> Affects Versions: 2.0.0
> Reporter: Moran
> Priority: Major
> Labels: pull-request-available
>
> MetricsTableLatenciesImpl histogramsByTable only put but not remove.Maybe we
> should remove it when table disabled.
> supplement:
> MetricsTableQueryMeterImpl metersByTable has the same problem.
> Recurrence steps:
> # create table: create 'student','data'
> # write data: put 'student','2015001','data:name','Zhangsan'
> # disable table: disable 'student'
> # drop table: drop 'student'
> # visit regionserver:port/jmx
> # search Namespace_default_table_student_metric
> Table is deleted, the latency data of the deleted table always in the /jmx
> endpoint.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
