HDDS-15960. Hadoop metrics leak detection by ivandika3 · Pull Request #10941 · apache/ozone

ivandika3 · 2026-08-04T06:10:05Z

What changes were proposed in this pull request?

Add guardrails to detect Hadoop metrics leak similar to ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks.

One idea is enable all the metrics related configuration in all the integration tests (e.g. percentiles configuration) and after the MiniOzoneCluster are shut down, check the metric registry to ensure that there are no more metrics that are still registered. We will fail the test so that we can fail loudly.

The main class is MetricsLeakAssertion, we have two list of leaky Metric sources:

EXPECTED_LEFTOVER_SOURCES: These are expected sources or the ones from Hadoop project (not handled in Ozone), we leave it be.
TODO_LEFTOVER_SOURCES: These should be reviewed and fixed in the subsequent patches. This can be moved to EXPECTED_LEFTOVER_SOURCES if necessary with valid justification.

(Generated-by: Kimi K3)

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15960

How was this patch tested?

CI. (Clean CI: https://github.com/ivandika3/ozone/actions/runs/38711698118)

Add MetricsLeakAssertion which inspects the DefaultMetricsSystem's private allSources map via reflection and fails the test if any metrics source is still registered after MiniOzoneCluster shutdown. The check verifies the field exists and logs a WARN (skipping) if the underlying Hadoop metrics implementation changes, rather than failing spuriously. Also enable the metrics percentile-interval configs by default in the mini cluster builder so quantile code paths are exercised by every integration test. Generated-by: Codex (GPT-5)

Throw AssertionError instead of logging a WARN and skipping when the metrics system's allSources field cannot be found, is not a Map, or cannot be read, so a broken leak check fails the test loudly rather than being silently ignored. Generated-by: Codex (GPT-5)

Checking allSources before DefaultMetricsSystem.shutdown() makes the assertion independent of whether Hadoop clears allSources on shutdown (it currently does not), and asserts the invariant while the metrics system is still fully populated. Well-behaved sources unregister themselves during cluster stop(), so anything still registered at this point is a genuine leak. Generated-by: Codex (GPT-5)

Add an EXPECTED_LEFTOVER_SOURCES allowlist for metrics that are registered once per JVM or per service and intentionally never unregistered (JvmMetrics/JvmMetricsCpu from HddsServerUtil, UgiMetrics, ManagedRocksObjectMetrics, ContainerCacheMetrics). Entries are matched by prefix to cover the numeric suffixes the metrics system appends for repeated registrations. This keeps the assertion focused on genuine per-instance service metrics leaks; verified to reduce the leftover count from 50 to 41 on TestMiniOzoneCluster. Generated-by: Codex (GPT-5)

ContainerCacheMetrics has a real per-cluster lifecycle: it is registered in ContainerCache.getInstance() and ContainerCache.shutdownCache() is its natural teardown. Add an unregister() and call it from shutdownCache(), and drop ContainerCacheMetrics from the leak-assertion allowlist since it no longer leaks. The remaining allowlisted Ozone sources (JvmMetrics, JvmMetricsCpu, ManagedRocksObjectMetrics) are JVM-scoped singletons with no per-service owner, so they are correctly left on the allowlist alongside Hadoop's UgiMetrics. Generated-by: Codex (GPT-5)

Generated-by: Codex (GPT-5)

…y subsystem Running the assertion across the full integration suite surfaced a broad set of pre-existing metrics leaks (JVM singletons, per-service sources, and Ratis/Hadoop infrastructure metrics). Allowlisting all of them so the suite stays green while the assertion guards against new leaks; each group is commented and is meant to be burned down in follow-up issues. Matching now supports a leading '*' (suffix match) in addition to a trailing '*' (prefix match), to cover names that embed a table-specific prefix (keyTableCache-1), a random id (CSMMetricsgroup-...), an absolute path (VolumeIOStats-/...), or a port (RpcActivityForPort15000). Verified against all 4423 distinct leftover source names from the failed CI run: zero remain unmatched. TestMiniOzoneCluster (5 tests) and TestOzoneIntegrationNonHA (318 tests) pass. Generated-by: Codex (GPT-5)

Split the leftover-sources list into EXPECTED_LEFTOVER_SOURCES (JVM-level singletons that are intentionally never unregistered) and TODO_LEFTOVER_SOURCES (genuine per-service leaks to be fixed and removed). This makes the intent of each entry explicit and keeps the burn-down list self-documenting. Also correct the ContainerCacheMetrics handling: it is registered once per JVM by the ContainerCache singleton (whose reference is never reset), so it cannot be unregistered per-cluster. Running the full TestMiniOzoneCluster showed that a single leftover ContainerCacheMetrics appears when an earlier test in the shared JVM creates the singleton. Move it back to EXPECTED_LEFTOVER_SOURCES and drop the unregister() call, keeping only the SOURCE_NAME constant cleanup. Verified: all 4424 distinct leftover names from CI are covered; TestMiniOzoneCluster (5 tests) passes. Generated-by: Codex (GPT-5)

ManagedRocksObjectMetrics.assertNoLeaks() reports RocksDB objects that were GC'd without being closed, which is often the root cause of a failing shutdown. Run it before MetricsLeakAssertion.assertNoLeaks() so the more actionable failure surfaces first. Both assertions still run before DefaultMetricsSystem.shutdown(). Generated-by: Codex (GPT-5)

HddsVolume.failVolume() unregistered VolumeIOStats but not VolumeInfoMetrics, while shutdown() unregisters both. Make failVolume() symmetric so a failed volume does not leak its VolumeInfoMetrics source. Add TestHddsVolume.testFailVolumeUnregistersMetrics which verifies both sources are removed; the test fails without the fix. Generated-by: Codex (GPT-5)

This reverts commit 85ddb76.

…umes VolumeInfoMetrics is deliberately kept registered on a failed volume (HDDS-7086) so its FAILED state is visible via JMX / the DataNode UI, so a leftover VolumeInfoMetrics is only a leak for healthy volumes that were not shut down. Document this so follow-up work does not try to unregister it from failVolume(). Generated-by: Codex (GPT-5)

…trics MutableVolumeSet.shutdown() only shut down healthy volumes in volumeMap and never touched failedVolumeMap. A failed volume registers a VolumeInfoMetrics source when it is created (HDDS-7086 keeps it registered so the FAILED state is visible while the datanode runs), but it was never unregistered because shutdown() skipped failed volumes, leaking the source for the life of the JVM. Shut down and clear failedVolumeMap in shutdown() as well. HddsVolume and StorageVolume shutdown() are null-safe for failed volumes (volumeIOStats / volumeUsage are null), so this only unregisters the info metrics and is safe. Extend TestVolumeSet.testFailVolumes to assert the failed volume's VolumeInfoMetrics is registered before shutdown and removed after; it fails without the fix. Generated-by: Codex (GPT-5)

The metrics leak assertion reflects over the JVM-wide DefaultMetricsSystem registry, so it cannot attribute a source to a specific cluster. Tests that run clusters concurrently via MiniOzoneClusterProvider (which builds a reserve cluster in the background while another is still running) would otherwise see a concurrent cluster's sources flagged as leaks. Add a setMetricsLeakAssertEnabled(boolean) flag to MiniOzoneCluster.Builder (default true); when false, MiniOzoneClusterImpl.shutdown() skips the assertion. Disable it in the three tests that use MiniOzoneClusterProvider: TestSafeMode, TestHDDSUpgrade, and TestDecommissionAndMaintenance. Verified: TestSafeMode, TestDecommissionAndMaintenance (9 tests), and TestMiniOzoneCluster (5 tests) pass; 0 checkstyle violations. Generated-by: Codex (GPT-5)

OmSnapshotInternalMetrics is created in the OzoneManager constructor and has an unregister() method, but OzoneManager.stop() never called it, so the source stayed registered after a single-OM mini cluster shut down. Add the unregister call alongside the existing DeletingServiceMetrics and OMPerformanceMetrics unregisters. Verified: a single-OM cluster no longer leaks OmSnapshotInternalMetrics (passes with the source removed from the leak-assertion allowlist). Note: in OM HA (multiple OMs in one JVM) the metrics system registers OmSnapshotInternalMetrics-1, -2, ... for the additional OMs, and those suffixed registrations still leak because unregister() only removes the base name. That shared per-OM registration problem affects many OM metrics uniformly and is tracked separately. Generated-by: Codex (GPT-5)

… mini-cluster mode Apply the pattern from HDDS-16119 (DatanodeStorageMetrics): in mini-cluster mode many OMs share one metrics system, which uniquifies a constant source name on registration (OmSnapshotInternalMetrics-1, -2, ...). Unregistering by the constant base name then leaks every source past the first. Register and unregister under a per-OM name (OmSnapshotInternalMetrics-<omNodeId>) in mini-cluster mode so the two stay symmetric; production keeps the plain name for stable JMX and Prometheus metric names. create() now takes the OM node id and stores the resolved sourceName in the instance; unregister() becomes an instance method that unregisters that name. OzoneManager passes getOMNodeId() and calls the instance unregister(). This fixes the HA leak too, so remove OmSnapshotInternalMetrics from the leak-assertion allowlist. Verified: TestOmSnapshotDisabled (3-OM HA) passes with the source removed from the allowlist; single-OM and snapshot tests pass; 0 checkstyle violations. Generated-by: Codex (GPT-5)

…etrics in mini-cluster mode Apply the HDDS-16119 pattern to SCMMetrics and SCMContainerMetrics: in mini-cluster mode many SCMs share one metrics system, which uniquifies a constant source name on registration (-1, -2, ...). Unregistering by the constant base name then leaks every source past the first. Register and unregister under a per-SCM name (suffixed with the scmId) in mini-cluster mode so the two stay symmetric; production keeps the plain name for stable JMX and Prometheus metric names. create() takes the scmId and stores the resolved sourceName in the instance; unRegister() unregisters that name. This fixes the HA leak, so remove both from the leak-assertion allowlist. Applying the per-SCM name also surfaced a pre-existing leak in the SCM constructor failure path: initMetrics() registered SCMMetrics before the storage-state check that throws SCM_NOT_INITIALIZED, so a failed SCM construction (e.g. testSCMInitializationFailure) leaked the source with no instance left to unregister it. Move the storage-state check before initMetrics()/initPerfMetrics() so a failed construction registers nothing. Verified: TestStorageContainerManagerHA (3 SCMs) and TestStorageContainerManager (9 tests) pass with both sources removed from the allowlist; TestSCMStateMachine passes; 0 checkstyle violations. Generated-by: Codex (GPT-5)

Generated-by: Codex (GPT-5)

…r change Making the SCMContainerMetrics constructor private and two-arg broke TestSCMContainerMetrics, which instantiates the source directly to test getMetrics() against a mocked SCMMXBean. Make the constructor package-private so the same-package test can use it, and update the test to the two-arg form. Generated-by: Codex (GPT-5)

PMD FieldDeclarationsShouldBeAtStartOfClass flagged the sourceName field declared after a method. Move it up with the other field declarations. Generated-by: Codex (GPT-5)

github-actions · 2026-09-09T00:07:30Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

github-actions · 2026-10-15T00:07:02Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

yandrey321 · 2026-10-15T02:30:00Z

@ivandika3 can we resurrect this PR?

ivandika3 · 2026-10-15T02:35:24Z

@yandrey321 Thanks for checking this. Yes, my current plan is first to setup the Hadoop metric leak detection framework (which should already be done in this patch) and then integrate some of the metrics to this framework.

There are some metrics that are not included yet in TODO_LEFTOVER_SOURCES, this can be addressed in follow up tasks so that the size of the patch remains small.

What do you think?

yandrey321 · 2026-10-15T02:38:15Z

@yandrey321 Thanks for checking this. Yes, my current plan is first to setup the Hadoop metric leak detection framework (which should already be done in this patch) and then integrate some of the metrics to this framework.

There are some metrics that are not included yet in TODO_LEFTOVER_SOURCES, this can be addressed in follow up tasks so that the size of the patch remains small.

What do you think?

Lets start with foundation done in this PR and file follow up JIRA(-s) to core remaining metric sources.

# Conflicts: # hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMStateMachine.java # hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

ivandika3 · 2026-10-16T02:14:22Z

@yandrey321 Ok this is ready to be reviewed. PTAL.

smengcl

Thanks @ivandika3 for the patch.

smengcl

Thanks @ivandika3 for the updates. +1

# Conflicts: # hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java # hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

ivandika3 added the test label Aug 4, 2026

ivandika3 self-assigned this Aug 4, 2026

ivandika3 force-pushed the HDDS-15960 branch from 80c9072 to 82f73d3 Compare August 4, 2026 10:12

ivandika3 added 16 commits August 18, 2026 16:21

HDDS-15960. Use a SOURCE_NAME constant in ContainerCacheMetrics

3fcbd32

Generated-by: Codex (GPT-5)

Revert "HDDS-15960. Unregister VolumeInfoMetrics on failVolume"

3405432

This reverts commit 85ddb76.

ivandika3 force-pushed the HDDS-15960 branch from b7772d8 to 0a7fcba Compare August 18, 2026 08:38

ivandika3 added 4 commits August 18, 2026 17:28

HDDS-15960. Remove verbose comments from per-instance metrics create()

32508ae

Generated-by: Codex (GPT-5)

HDDS-15960. Declare SCMMetrics sourceName field at top of class

eea2e0c

PMD FieldDeclarationsShouldBeAtStartOfClass flagged the sourceName field declared after a method. Move it up with the other field declarations. Generated-by: Codex (GPT-5)

github-actions Bot added the stale label Sep 9, 2026

ivandika3 removed the stale label Sep 16, 2026

github-actions Bot added scm om datanode labels Sep 23, 2026

github-actions Bot added the stale label Oct 15, 2026

ivandika3 removed the stale label Oct 15, 2026

ivandika3 added 5 commits October 15, 2026 10:53

Empty commit

baf3255

HDDS-15960. Fix mini-cluster metrics cleanup failures

72a4f9c

HDDS-15960. Fix metrics leak detection and restart lifecycle

fe29577

HDDS-15960. Fix PMD warning for reflective metrics test field

ed4b000

ivandika3 marked this pull request as ready for review October 16, 2026 02:14

ivandika3 requested a review from jojochuang October 16, 2026 02:15

ivandika3 added 3 commits October 20, 2026 14:32

HDDS-15960. Standardize metrics source naming

6c2fffc

Fix checkstyle

ac9350a

HDDS-15960. Isolate DB updates authorization test without Recon

25a2c8a

ivandika3 requested a review from smengcl October 21, 2026 03:52

smengcl reviewed Oct 21, 2026

View reviewed changes

ivandika3 added 2 commits October 21, 2026 14:17

HDDS-15960. Address metrics leak detection review feedback

06c2e22

Merge branch 'master' into HDDS-15960

92520df

smengcl approved these changes Oct 21, 2026

View reviewed changes

ivandika3 added 2 commits October 21, 2026 15:38

Merge branch 'master' into HDDS-15960

9d894d0

# Conflicts: # hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java # hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

HDDS-15960. Fix S3 Gateway HTTP metrics cleanup

34dd982

Navigation Menu

Repository navigation

HDDS-15960. Hadoop metrics leak detection - #10941

HDDS-15960. Hadoop metrics leak detection#10941
ivandika3 wants to merge 32 commits into
apache:masterfrom
ivandika3:HDDS-15960

ivandika3 commented Aug 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

github-actions Bot commented Oct 15, 2026

Uh oh!

yandrey321 commented Oct 15, 2026

Uh oh!

ivandika3 commented Oct 15, 2026

Uh oh!

yandrey321 commented Oct 15, 2026

Uh oh!

ivandika3 commented Oct 16, 2026

Uh oh!

smengcl left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smengcl left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ivandika3 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

github-actions Bot commented Oct 15, 2026

Uh oh!

yandrey321 commented Oct 15, 2026

Uh oh!

ivandika3 commented Oct 15, 2026

Uh oh!

yandrey321 commented Oct 15, 2026

Uh oh!

ivandika3 commented Oct 16, 2026

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ivandika3 commented Aug 4, 2026 •

edited

Loading