From 59b62408ff2d6d1b098b7c8846b7bfb6b0c67784 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 10:01:25 +0800 Subject: [PATCH 01/15] HDDS-15960. Assert no metrics sources leak after mini cluster shutdown 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) --- .../hadoop/ozone/MetricsLeakAssertion.java | 95 +++++++++++++++++++ .../hadoop/ozone/MiniOzoneClusterImpl.java | 7 ++ 2 files changed, 102 insertions(+) create mode 100644 hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java new file mode 100644 index 00000000000..250217cb815 --- /dev/null +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -0,0 +1,95 @@ +/* + * 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.ozone; + +import java.lang.reflect.Field; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; +import org.apache.hadoop.metrics2.MetricsSource; +import org.apache.hadoop.metrics2.MetricsSystem; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Asserts that no metrics sources remain registered in the + * {@link DefaultMetricsSystem} after a mini cluster is shut down. + * + *

Hadoop's {@code MetricsSystemImpl} does not remove registered sources on + * {@code stop()} or {@code shutdown()}; a metrics class that forgets to call + * {@code unregisterSource(...)} leaks its registration silently. This helper + * inspects the private {@code allSources} map via reflection and fails the + * test if anything is still registered. + */ +public final class MetricsLeakAssertion { + + private static final Logger LOG = LoggerFactory.getLogger(MetricsLeakAssertion.class); + private static final String ALL_SOURCES_FIELD = "allSources"; + + private MetricsLeakAssertion() { + } + + /** + * Throws an {@link AssertionError} if any metrics sources are still + * registered with the default metrics system. If the underlying metrics + * implementation does not expose the expected {@code allSources} field + * (e.g. a Hadoop version change), a WARN is logged and the check is + * skipped rather than failing spuriously. + */ + public static void assertNoLeaks() { + MetricsSystem ms = DefaultMetricsSystem.instance(); + Field field = findAllSourcesField(ms.getClass()); + if (field == null) { + LOG.warn("Cannot check for metrics leaks: '{}' field not found on {}. " + + "Skipping metrics leak assertion.", ALL_SOURCES_FIELD, ms.getClass().getName()); + return; + } + try { + field.setAccessible(true); + Object value = field.get(ms); + if (!(value instanceof Map)) { + LOG.warn("Cannot check for metrics leaks: '{}' is not a Map on {}. Skipping.", + ALL_SOURCES_FIELD, ms.getClass().getName()); + return; + } + @SuppressWarnings("unchecked") + Map allSources = (Map) value; + if (!allSources.isEmpty()) { + Set leaked = new TreeSet<>(allSources.keySet()); + throw new AssertionError("Found " + leaked.size() + + " metrics source(s) still registered after cluster shutdown: " + leaked); + } + } catch (IllegalAccessException e) { + LOG.warn("Cannot check for metrics leaks: unable to access '{}' on {}. Skipping.", + ALL_SOURCES_FIELD, ms.getClass().getName(), e); + } + } + + private static Field findAllSourcesField(Class clazz) { + Class current = clazz; + while (current != null) { + try { + return current.getDeclaredField(ALL_SOURCES_FIELD); + } catch (NoSuchFieldException e) { + current = current.getSuperclass(); + } + } + return null; + } +} diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java index e13719dde6e..86d2ff75488 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java @@ -392,6 +392,7 @@ public void shutdown() { ContainerCache.getInstance(conf).shutdownCache(); DefaultMetricsSystem.shutdown(); ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks(); + MetricsLeakAssertion.assertNoLeaks(); } catch (Exception e) { LOG.error("Exception while shutting down the cluster.", e); } @@ -617,6 +618,12 @@ protected void initializeConfiguration() throws IOException { // pipeline. conf.setInt(HddsConfigKeys.HDDS_SCM_SAFEMODE_MIN_DATANODE, numOfDatanodes >= 3 ? 3 : 1); + // Enable metrics percentile collection so that the quantile code paths + // are exercised by every integration test. + conf.setIfUnset(OzoneConfigKeys.OZONE_GPRC_METRICS_PERCENTILES_INTERVALS_KEY, "60,300"); + conf.setIfUnset(OzoneConfigKeys.OZONE_XCEIVER_CLIENT_METRICS_PERCENTILES_INTERVALS_SECONDS_KEY, "60,300"); + conf.setIfUnset(HddsConfigKeys.HDDS_METRICS_PERCENTILES_INTERVALS_KEY, "60,300"); + conf.setIfUnset(HddsConfigKeys.OZONE_DATANODE_IO_METRICS_PERCENTILES_INTERVALS_SECONDS_KEY, "60,300"); configureHostAndRackTopology(); } From d4e14447dedf75d24229bc1bc3a98826bbf80d41 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 10:29:37 +0800 Subject: [PATCH 02/15] HDDS-15960. Fail loudly when the allSources field is unavailable 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) --- .../hadoop/ozone/MetricsLeakAssertion.java | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java index 250217cb815..089b3e247eb 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -24,8 +24,6 @@ import org.apache.hadoop.metrics2.MetricsSource; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Asserts that no metrics sources remain registered in the @@ -39,7 +37,6 @@ */ public final class MetricsLeakAssertion { - private static final Logger LOG = LoggerFactory.getLogger(MetricsLeakAssertion.class); private static final String ALL_SOURCES_FIELD = "allSources"; private MetricsLeakAssertion() { @@ -47,37 +44,38 @@ private MetricsLeakAssertion() { /** * Throws an {@link AssertionError} if any metrics sources are still - * registered with the default metrics system. If the underlying metrics - * implementation does not expose the expected {@code allSources} field - * (e.g. a Hadoop version change), a WARN is logged and the check is - * skipped rather than failing spuriously. + * registered with the default metrics system, or if the expected + * {@code allSources} field cannot be found or read (e.g. a Hadoop version + * change restructured {@code MetricsSystemImpl}), so that a broken or + * missing check fails loudly instead of going unnoticed. */ public static void assertNoLeaks() { MetricsSystem ms = DefaultMetricsSystem.instance(); Field field = findAllSourcesField(ms.getClass()); if (field == null) { - LOG.warn("Cannot check for metrics leaks: '{}' field not found on {}. " + - "Skipping metrics leak assertion.", ALL_SOURCES_FIELD, ms.getClass().getName()); - return; + throw new AssertionError("Cannot check for metrics leaks: '" + ALL_SOURCES_FIELD + + "' field not found on " + ms.getClass().getName() + + ". The metrics system implementation may have changed."); } + final Map allSources; try { field.setAccessible(true); Object value = field.get(ms); if (!(value instanceof Map)) { - LOG.warn("Cannot check for metrics leaks: '{}' is not a Map on {}. Skipping.", - ALL_SOURCES_FIELD, ms.getClass().getName()); - return; + throw new AssertionError("Cannot check for metrics leaks: '" + ALL_SOURCES_FIELD + + "' on " + ms.getClass().getName() + " is not a Map."); } @SuppressWarnings("unchecked") - Map allSources = (Map) value; - if (!allSources.isEmpty()) { - Set leaked = new TreeSet<>(allSources.keySet()); - throw new AssertionError("Found " + leaked.size() + - " metrics source(s) still registered after cluster shutdown: " + leaked); - } + Map sources = (Map) value; + allSources = sources; } catch (IllegalAccessException e) { - LOG.warn("Cannot check for metrics leaks: unable to access '{}' on {}. Skipping.", - ALL_SOURCES_FIELD, ms.getClass().getName(), e); + throw new AssertionError("Cannot check for metrics leaks: unable to access '" + + ALL_SOURCES_FIELD + "' on " + ms.getClass().getName() + ".", e); + } + if (!allSources.isEmpty()) { + Set leaked = new TreeSet<>(allSources.keySet()); + throw new AssertionError("Found " + leaked.size() + + " metrics source(s) still registered after cluster shutdown: " + leaked); } } From 50321de0efc4cff5902b14d31dfb3f78745ac20d Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 10:34:13 +0800 Subject: [PATCH 03/15] HDDS-15960. Assert metrics leaks before DefaultMetricsSystem shutdown 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) --- .../java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java index 86d2ff75488..1ea55d44744 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java @@ -390,9 +390,12 @@ public void shutdown() { stop(); FileUtils.deleteDirectory(baseDir); ContainerCache.getInstance(conf).shutdownCache(); + // Assert before tearing down the metrics system: Hadoop does not clear + // allSources on shutdown, but checking before shutdown makes the + // assertion independent of that behavior. + MetricsLeakAssertion.assertNoLeaks(); DefaultMetricsSystem.shutdown(); ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks(); - MetricsLeakAssertion.assertNoLeaks(); } catch (Exception e) { LOG.error("Exception while shutting down the cluster.", e); } From 45fa4a8901ccd321f37eb6a012b5fc6cad19e330 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 10:51:16 +0800 Subject: [PATCH 04/15] HDDS-15960. Allowlist JVM-level singleton metrics in leak assertion 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) --- .../hadoop/ozone/MetricsLeakAssertion.java | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java index 089b3e247eb..14dadf052c8 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -18,6 +18,8 @@ package org.apache.hadoop.ozone; import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -39,6 +41,25 @@ public final class MetricsLeakAssertion { private static final String ALL_SOURCES_FIELD = "allSources"; + /** + * Names of metrics sources that are registered once per JVM (or once per + * service) and intentionally never unregistered, so they are expected to + * still be present after a mini cluster shuts down. These are matched by + * prefix to also cover the numeric suffixes the metrics system appends for + * repeated registrations (e.g. {@code JvmMetrics-1}). + * + *

Do not add per-instance service metrics here; those should be + * unregistered on service shutdown and a leftover registration is a real + * leak. Keep this list limited to JVM-level singletons. + */ + private static final List EXPECTED_LEFTOVER_SOURCES = Arrays.asList( + "JvmMetrics", // registered per service by HddsServerUtil.initializeMetrics + "JvmMetricsCpu", // registered alongside JvmMetrics + "UgiMetrics", // Hadoop security UGI metrics, JVM-level singleton + "ManagedRocksObjectMetrics", // static singleton + "ContainerCacheMetrics" // static singleton + ); + private MetricsLeakAssertion() { } @@ -72,13 +93,23 @@ public static void assertNoLeaks() { throw new AssertionError("Cannot check for metrics leaks: unable to access '" + ALL_SOURCES_FIELD + "' on " + ms.getClass().getName() + ".", e); } - if (!allSources.isEmpty()) { - Set leaked = new TreeSet<>(allSources.keySet()); + Set leaked = new TreeSet<>(allSources.keySet()); + leaked.removeIf(MetricsLeakAssertion::isExpectedLeftover); + if (!leaked.isEmpty()) { throw new AssertionError("Found " + leaked.size() + " metrics source(s) still registered after cluster shutdown: " + leaked); } } + private static boolean isExpectedLeftover(String name) { + for (String prefix : EXPECTED_LEFTOVER_SOURCES) { + if (name.equals(prefix) || name.startsWith(prefix + "-")) { + return true; + } + } + return false; + } + private static Field findAllSourcesField(Class clazz) { Class current = clazz; while (current != null) { From 21806dae2569fefff0eb27ce0d70b35903a6f0ce Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 10:58:29 +0800 Subject: [PATCH 05/15] HDDS-15960. Unregister ContainerCacheMetrics on cache shutdown 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) --- .../ozone/container/common/utils/ContainerCache.java | 1 + .../container/common/utils/ContainerCacheMetrics.java | 7 +++++++ .../java/org/apache/hadoop/ozone/MetricsLeakAssertion.java | 3 +-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java index 6fa6e1f10ec..0fc0fa44f67 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java @@ -96,6 +96,7 @@ public void shutdownCache() { } // reset the cache cache.clear(); + ContainerCacheMetrics.unregister(); } finally { lock.unlock(); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java index c4ca78ffb41..f78df3a6c31 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java @@ -59,6 +59,13 @@ public static ContainerCacheMetrics create() { return ms.register(name, "null", new ContainerCacheMetrics()); } + public static void unregister() { + MetricsSystem ms = DefaultMetricsSystem.instance(); + String name = "ContainerCacheMetrics"; + + ms.unregisterSource(name); + } + public void incNumDbGetOps() { numDbGetOps.incr(); } diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java index 14dadf052c8..d63b09461c8 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -56,8 +56,7 @@ public final class MetricsLeakAssertion { "JvmMetrics", // registered per service by HddsServerUtil.initializeMetrics "JvmMetricsCpu", // registered alongside JvmMetrics "UgiMetrics", // Hadoop security UGI metrics, JVM-level singleton - "ManagedRocksObjectMetrics", // static singleton - "ContainerCacheMetrics" // static singleton + "ManagedRocksObjectMetrics" // static singleton ); private MetricsLeakAssertion() { From 1e4959e8558a4faed0523a9c2de579f27f0af816 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 11:05:13 +0800 Subject: [PATCH 06/15] HDDS-15960. Use a SOURCE_NAME constant in ContainerCacheMetrics Generated-by: Codex (GPT-5) --- .../container/common/utils/ContainerCacheMetrics.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java index f78df3a6c31..98e63deeb9c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java @@ -28,6 +28,8 @@ */ public final class ContainerCacheMetrics { + private static final String SOURCE_NAME = "ContainerCacheMetrics"; + @Metric("Rate to measure the db open latency") private MutableRate dbOpenLatency; @@ -54,16 +56,12 @@ private ContainerCacheMetrics() { public static ContainerCacheMetrics create() { MetricsSystem ms = DefaultMetricsSystem.instance(); - String name = "ContainerCacheMetrics"; - - return ms.register(name, "null", new ContainerCacheMetrics()); + return ms.register(SOURCE_NAME, "null", new ContainerCacheMetrics()); } public static void unregister() { MetricsSystem ms = DefaultMetricsSystem.instance(); - String name = "ContainerCacheMetrics"; - - ms.unregisterSource(name); + ms.unregisterSource(SOURCE_NAME); } public void incNumDbGetOps() { From 003c814d7889bf425bb4bd1b68a07e50d8ded600 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 13:41:37 +0800 Subject: [PATCH 07/15] HDDS-15960. Expand leak-assertion allowlist to known leaks, grouped by 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) --- .../hadoop/ozone/MetricsLeakAssertion.java | 132 ++++++++++++++++-- 1 file changed, 118 insertions(+), 14 deletions(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java index d63b09461c8..334a4463757 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -42,21 +42,97 @@ public final class MetricsLeakAssertion { private static final String ALL_SOURCES_FIELD = "allSources"; /** - * Names of metrics sources that are registered once per JVM (or once per - * service) and intentionally never unregistered, so they are expected to - * still be present after a mini cluster shuts down. These are matched by - * prefix to also cover the numeric suffixes the metrics system appends for - * repeated registrations (e.g. {@code JvmMetrics-1}). + * Metrics sources that are known to still be registered after a mini + * cluster shuts down. This is a transitional list: the JVM-level + * singletons (first group) are never unregistered by design, while the rest + * are genuine per-service leaks that are being burned down in follow-up + * issues. Do not add new entries; fix the leak instead and remove the + * entry here. * - *

Do not add per-instance service metrics here; those should be - * unregistered on service shutdown and a leftover registration is a real - * leak. Keep this list limited to JVM-level singletons. + *

Matching rules: + *

*/ private static final List EXPECTED_LEFTOVER_SOURCES = Arrays.asList( - "JvmMetrics", // registered per service by HddsServerUtil.initializeMetrics - "JvmMetricsCpu", // registered alongside JvmMetrics - "UgiMetrics", // Hadoop security UGI metrics, JVM-level singleton - "ManagedRocksObjectMetrics" // static singleton + // JVM-level singletons, never unregistered by design. + "JvmMetrics*", // per service, via HddsServerUtil.initializeMetrics + "JvmMetricsCpu", + "UgiMetrics", // Hadoop security UGI metrics + "ManagedRocksObjectMetrics", + + // RPC / HTTP server metrics (name embeds the listening port). + "RpcActivityForPort*", + "RpcDetailedActivityForPort*", + "HttpServer2*", + "LocalJobRunnerMetrics*", + + // Datanode per-instance metrics. + "StorageContainerMetrics", + "ContainerDataScannerMetrics*", // name embeds the volume path + "ContainerMetadataScannerMetrics", + "On-demand container scanner metrics", + "BackgroundVolumeScannerMetrics", + "VolumeHealthMetrics-*", + "VolumeIOStats-*", // name embeds the volume path + "VolumeInfoMetrics-*", // name embeds the volume path + "CommandHandlerMetrics", + "HddsDispatcher", + "ECReconstructionMetrics", + "ReplicationSupervisorMetrics", + "ContainerReplicator/push", + "BlockDeletingService", + "GrpcMetrics", + + // SCM metrics. + "SCMNodeMetrics", + "SCMContainerManagerMetrics", + "SCMContainerMetrics", + "SCMMetrics", + "SafeModeMetrics", + "ContainerBalancerMetrics", + "NodeDecommissionMetrics", + "SCMDatanodeProtocol", + "ScmBlockLocationProtocol", + "ScmContainerLocationProtocol", + "ScmSecurityProtocol", + "EventQueue*", // per event/handler pair, registered by SCM EventQueue + + // OM metrics. + "OMMetrics", + "OMPerformanceMetrics", + "OMLockMetrics", + "OMHAMetrics", + "OMSnapshotDirectoryMetrics", + "OmSnapshotInternalMetrics", + "OmSnapshotMetrics", + "OmClientProtocol", + "DeletingServiceMetrics", + "KeyLifecycleServiceMetrics", + "BucketUtilizationMetrics", + "DelegationTokenSecretManagerMetrics", + + // OM / SCM RocksDB table cache and DB metrics (name embeds path/uuid). + "Rocksdb_*", + "SSTFilePruningMetrics-*", + "DBCheckpointMetrics", + "*TableCache", + + // Ratis / third-party infrastructure metrics. + "CSMMetricsgroup-*", // Ratis container state machine, embeds a random id + "CacheMetrics-*", // Hadoop cache metrics (XceiverClientManager, etc.) + "NettyMetrics*", + "ContainerClientMetrics1", + "ReconTaskMetrics", + "ReconTaskControllerMetrics" ); private MetricsLeakAssertion() { @@ -101,14 +177,42 @@ public static void assertNoLeaks() { } private static boolean isExpectedLeftover(String name) { - for (String prefix : EXPECTED_LEFTOVER_SOURCES) { - if (name.equals(prefix) || name.startsWith(prefix + "-")) { + for (String entry : EXPECTED_LEFTOVER_SOURCES) { + if (entry.startsWith("*")) { + // Suffix match, also tolerating a trailing numeric suffix (-N). + String suffix = entry.substring(1); + if (name.endsWith(suffix) || stripNumericSuffix(name).endsWith(suffix)) { + return true; + } + } else if (entry.endsWith("*")) { + if (name.startsWith(entry.substring(0, entry.length() - 1))) { + return true; + } + } else if (name.equals(entry) || name.startsWith(entry + "-")) { return true; } } return false; } + private static String stripNumericSuffix(String name) { + int idx = name.lastIndexOf('-'); + if (idx > 0 && idx < name.length() - 1) { + String tail = name.substring(idx + 1); + boolean numeric = true; + for (int i = 0; i < tail.length(); i++) { + if (!Character.isDigit(tail.charAt(i))) { + numeric = false; + break; + } + } + if (numeric) { + return name.substring(0, idx); + } + } + return name; + } + private static Field findAllSourcesField(Class clazz) { Class current = clazz; while (current != null) { From ef5499170d80a4a70601ed3901a78faebc2d9fc0 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 13:58:59 +0800 Subject: [PATCH 08/15] HDDS-15960. Separate never-cleaned singletons from burn-down leaks 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) --- .../common/utils/ContainerCache.java | 1 - .../common/utils/ContainerCacheMetrics.java | 5 -- .../hadoop/ozone/MetricsLeakAssertion.java | 57 ++++++++++++------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java index 0fc0fa44f67..6fa6e1f10ec 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java @@ -96,7 +96,6 @@ public void shutdownCache() { } // reset the cache cache.clear(); - ContainerCacheMetrics.unregister(); } finally { lock.unlock(); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java index 98e63deeb9c..a7917d4d35d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCacheMetrics.java @@ -59,11 +59,6 @@ public static ContainerCacheMetrics create() { return ms.register(SOURCE_NAME, "null", new ContainerCacheMetrics()); } - public static void unregister() { - MetricsSystem ms = DefaultMetricsSystem.instance(); - ms.unregisterSource(SOURCE_NAME); - } - public void incNumDbGetOps() { numDbGetOps.incr(); } diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java index 334a4463757..59cd9df74dc 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -42,33 +42,31 @@ public final class MetricsLeakAssertion { private static final String ALL_SOURCES_FIELD = "allSources"; /** - * Metrics sources that are known to still be registered after a mini - * cluster shuts down. This is a transitional list: the JVM-level - * singletons (first group) are never unregistered by design, while the rest - * are genuine per-service leaks that are being burned down in follow-up - * issues. Do not add new entries; fix the leak instead and remove the - * entry here. + * Metrics sources that are registered once per JVM (or once per service) + * and intentionally never unregistered, so they are expected to still be + * present after a mini cluster shuts down. These are JVM-level singletons + * with no per-service owner; do not add per-service metrics here. * - *

Matching rules: - *

    - *
  • An entry ending in {@code *} is a prefix match, used for sources - * whose registered name embeds a random id, an absolute path, or a port - * (e.g. {@code RpcActivityForPort15000}).
    • - *
  • An entry starting with {@code *} is a suffix match, used for sources - * whose registered name embeds a table-specific prefix (e.g. - * {@code keyTableCache-1}).
    • - *
  • Any other entry matches the source name exactly, or the name with a - * numeric suffix the metrics system appends for repeated registrations - * (e.g. {@code JvmMetrics-1}).
    • - *
+ *

For the matching rules see {@link #isExpectedLeftover(String)}. */ private static final List EXPECTED_LEFTOVER_SOURCES = Arrays.asList( - // JVM-level singletons, never unregistered by design. "JvmMetrics*", // per service, via HddsServerUtil.initializeMetrics "JvmMetricsCpu", "UgiMetrics", // Hadoop security UGI metrics "ManagedRocksObjectMetrics", + "ContainerCacheMetrics" // ContainerCache singleton, created once per JVM + ); + /** + * Metrics sources that leak: they should be unregistered when the mini + * cluster shuts down but currently are not. This is a transitional list to + * keep the integration suite green while the leaks are being fixed. Each + * entry should be removed once the corresponding source unregisters + * properly; do not add new entries, fix the leak instead. + * + *

For the matching rules see {@link #isExpectedLeftover(String)}. + */ + private static final List TODO_LEFTOVER_SOURCES = Arrays.asList( // RPC / HTTP server metrics (name embeds the listening port). "RpcActivityForPort*", "RpcDetailedActivityForPort*", @@ -176,8 +174,27 @@ public static void assertNoLeaks() { } } + /** + * Matching rules for {@link #EXPECTED_LEFTOVER_SOURCES} and + * {@link #TODO_LEFTOVER_SOURCES}: + *

    + *
  • An entry ending in {@code *} is a prefix match, used for sources + * whose registered name embeds a random id, an absolute path, or a port + * (e.g. {@code RpcActivityForPort15000}).
    • + *
  • An entry starting with {@code *} is a suffix match, used for sources + * whose registered name embeds a table-specific prefix (e.g. + * {@code keyTableCache-1}).
    • + *
  • Any other entry matches the source name exactly, or the name with a + * numeric suffix the metrics system appends for repeated registrations + * (e.g. {@code JvmMetrics-1}).
    • + *
+ */ private static boolean isExpectedLeftover(String name) { - for (String entry : EXPECTED_LEFTOVER_SOURCES) { + return matches(EXPECTED_LEFTOVER_SOURCES, name) || matches(TODO_LEFTOVER_SOURCES, name); + } + + private static boolean matches(List entries, String name) { + for (String entry : entries) { if (entry.startsWith("*")) { // Suffix match, also tolerating a trailing numeric suffix (-N). String suffix = entry.substring(1); From 9d3b58266a7877ad0b1d2efb833a8663a87b06e2 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 14:03:21 +0800 Subject: [PATCH 09/15] HDDS-15960. Run RocksDB leak assertion before the metrics leak assertion 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) --- .../java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java index 1ea55d44744..cb8383117cf 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java @@ -390,12 +390,13 @@ public void shutdown() { stop(); FileUtils.deleteDirectory(baseDir); ContainerCache.getInstance(conf).shutdownCache(); + // RocksDB object leaks are often the root cause; check them first. + ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks(); // Assert before tearing down the metrics system: Hadoop does not clear // allSources on shutdown, but checking before shutdown makes the // assertion independent of that behavior. MetricsLeakAssertion.assertNoLeaks(); DefaultMetricsSystem.shutdown(); - ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks(); } catch (Exception e) { LOG.error("Exception while shutting down the cluster.", e); } From 85ddb7667329b795bc0cd49ef5aaa1392161fa50 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 14:37:27 +0800 Subject: [PATCH 10/15] HDDS-15960. Unregister VolumeInfoMetrics on failVolume 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) --- .../container/common/volume/HddsVolume.java | 3 +++ .../common/volume/TestHddsVolume.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java index 8827960248d..f9b76f7eb01 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java @@ -205,6 +205,9 @@ public void failVolume() { if (volumeIOStats != null) { volumeIOStats.unregister(); } + if (volumeInfoMetrics != null) { + volumeInfoMetrics.unregister(); + } closeDbStore(); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java index 1dd927b6f48..04afbff4171 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java @@ -50,7 +50,9 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult; import org.apache.hadoop.metrics2.MetricsCollector; +import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.impl.MetricsCollectorImpl; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.common.Storage; import org.apache.hadoop.ozone.container.common.ContainerTestUtils; @@ -279,6 +281,28 @@ public void testShutdown() throws Exception { reportedUsage.getAvailable() + reservedSpaceInBytes); } + @Test + public void testFailVolumeUnregistersMetrics() throws Exception { + HddsVolume volume = volumeBuilder.build(); + volume.format(CLUSTER_ID); + volume.createWorkingDir(CLUSTER_ID, null); + + MetricsSystem ms = DefaultMetricsSystem.instance(); + String ioStatsName = volume.getVolumeIOStats().getMetricsSourceName(); + String infoMetricsName = + VolumeInfoMetrics.class.getSimpleName() + "-" + folder.toString(); + + // Both metrics should be registered after the volume is created. + assertNotNull(ms.getSource(ioStatsName)); + assertNotNull(ms.getSource(infoMetricsName)); + + volume.failVolume(); + + // Both metrics should be unregistered when the volume is failed. + assertNull(ms.getSource(ioStatsName)); + assertNull(ms.getSource(infoMetricsName)); + } + /** * Test conservative avail space. * |----used----| (avail) |++++++++reserved++++++++| From f4a20ddff1060361f3399e3d3d6224a2befe44f3 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 14:41:54 +0800 Subject: [PATCH 11/15] Revert "HDDS-15960. Unregister VolumeInfoMetrics on failVolume" This reverts commit 85ddb7667329b795bc0cd49ef5aaa1392161fa50. --- .../container/common/volume/HddsVolume.java | 3 --- .../common/volume/TestHddsVolume.java | 24 ------------------- 2 files changed, 27 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java index f9b76f7eb01..8827960248d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java @@ -205,9 +205,6 @@ public void failVolume() { if (volumeIOStats != null) { volumeIOStats.unregister(); } - if (volumeInfoMetrics != null) { - volumeInfoMetrics.unregister(); - } closeDbStore(); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java index 04afbff4171..1dd927b6f48 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java @@ -50,9 +50,7 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult; import org.apache.hadoop.metrics2.MetricsCollector; -import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.impl.MetricsCollectorImpl; -import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.common.Storage; import org.apache.hadoop.ozone.container.common.ContainerTestUtils; @@ -281,28 +279,6 @@ public void testShutdown() throws Exception { reportedUsage.getAvailable() + reservedSpaceInBytes); } - @Test - public void testFailVolumeUnregistersMetrics() throws Exception { - HddsVolume volume = volumeBuilder.build(); - volume.format(CLUSTER_ID); - volume.createWorkingDir(CLUSTER_ID, null); - - MetricsSystem ms = DefaultMetricsSystem.instance(); - String ioStatsName = volume.getVolumeIOStats().getMetricsSourceName(); - String infoMetricsName = - VolumeInfoMetrics.class.getSimpleName() + "-" + folder.toString(); - - // Both metrics should be registered after the volume is created. - assertNotNull(ms.getSource(ioStatsName)); - assertNotNull(ms.getSource(infoMetricsName)); - - volume.failVolume(); - - // Both metrics should be unregistered when the volume is failed. - assertNull(ms.getSource(ioStatsName)); - assertNull(ms.getSource(infoMetricsName)); - } - /** * Test conservative avail space. * |----used----| (avail) |++++++++reserved++++++++| From 61fac9f3c21ce36d8b9ab3d73421d4c294cf4efa Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 14:44:28 +0800 Subject: [PATCH 12/15] HDDS-15960. Note that VolumeInfoMetrics is intentional for failed volumes 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) --- .../java/org/apache/hadoop/ozone/MetricsLeakAssertion.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java index 59cd9df74dc..f6274f2d320 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MetricsLeakAssertion.java @@ -81,7 +81,9 @@ public final class MetricsLeakAssertion { "BackgroundVolumeScannerMetrics", "VolumeHealthMetrics-*", "VolumeIOStats-*", // name embeds the volume path - "VolumeInfoMetrics-*", // name embeds the volume path + // Name embeds the volume path. Also intentionally kept registered for + // failed volumes (HDDS-7086), so only the healthy-volume case is a leak. + "VolumeInfoMetrics-*", "CommandHandlerMetrics", "HddsDispatcher", "ECReconstructionMetrics", From 720bfd71a34efe168cd98e7d77517603c759d56d Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 15:22:07 +0800 Subject: [PATCH 13/15] HDDS-15960. Shut down failed volumes to unregister their VolumeInfoMetrics 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) --- .../container/common/volume/MutableVolumeSet.java | 11 +++++++++++ .../container/common/volume/TestVolumeSet.java | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java index a79a06b6541..e9c2f9df17a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java @@ -360,6 +360,17 @@ public void shutdown() { } volumeMap.clear(); + // Shut down failed volumes too: their VolumeInfoMetrics is registered on + // creation (HDDS-7086) and must be unregistered on shutdown. + for (StorageVolume volume : failedVolumeMap.values()) { + try { + volume.shutdown(); + } catch (Exception ex) { + LOG.error("Failed to shutdown failed volume : " + volume.getStorageDir(), ex); + } + } + failedVolumeMap.clear(); + if (volumeHealthMetrics != null) { volumeHealthMetrics.unregister(); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java index 932101dc526..0bfc88f8ac8 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java @@ -25,6 +25,8 @@ import static org.assertj.core.api.Assumptions.assumeThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; @@ -39,6 +41,8 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.metrics2.MetricsRecordBuilder; +import org.apache.hadoop.metrics2.MetricsSystem; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.container.common.utils.HddsVolumeUtil; import org.junit.jupiter.api.AfterEach; @@ -191,7 +195,16 @@ void testFailVolumes(@TempDir File readOnlyVolumePath, @TempDir File volumePath) assertEquals(readOnlyVolumePath, volSet.getFailedVolumesList().get(0) .getStorageDir()); assertNumVolumes(volSet, 1, 1); + + // The failed volume's VolumeInfoMetrics is registered on creation + // (HDDS-7086) and must be unregistered when the volume set shuts down. + MetricsSystem ms = DefaultMetricsSystem.instance(); + String failedVolumeMetrics = VolumeInfoMetrics.class.getSimpleName() + + "-" + readOnlyVolumePath.getAbsolutePath(); + assertNotNull(ms.getSource(failedVolumeMetrics)); + volSet.shutdown(); + assertNull(ms.getSource(failedVolumeMetrics)); } @Test From 82f73d3f6fb678f9a336d63ca713999df3f32401 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 4 Aug 2026 16:42:36 +0800 Subject: [PATCH 14/15] HDDS-15960. Disable metrics leak assertion for multi-cluster tests 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) --- .../org/apache/hadoop/fs/ozone/TestSafeMode.java | 4 +++- .../scm/node/TestDecommissionAndMaintenance.java | 5 ++++- .../hadoop/hdds/upgrade/TestHDDSUpgrade.java | 3 +++ .../org/apache/hadoop/ozone/MiniOzoneCluster.java | 12 ++++++++++++ .../apache/hadoop/ozone/MiniOzoneClusterImpl.java | 15 +++++++++++++-- .../hadoop/ozone/MiniOzoneHAClusterImpl.java | 1 + 6 files changed, 36 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestSafeMode.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestSafeMode.java index 1300c61b1f8..ef086782948 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestSafeMode.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestSafeMode.java @@ -59,7 +59,9 @@ class TestSafeMode { static void setup() { OzoneConfiguration conf = new OzoneConfiguration(); clusterProvider = new MiniOzoneClusterProvider( - MiniOzoneCluster.newBuilder(conf), 2); + // Clusters overlap via MiniOzoneClusterProvider, so the metrics leak + // assertion would see a concurrent cluster's sources. + MiniOzoneCluster.newBuilder(conf).setMetricsLeakAssertEnabled(false), 2); } @BeforeEach diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java index 5260c973ba0..4e0f36f316b 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java @@ -148,7 +148,10 @@ public static void init() { conf.setFromObject(replicationConf); MiniOzoneCluster.Builder builder = MiniOzoneCluster.newBuilder(conf) - .setNumDatanodes(DATANODE_COUNT); + .setNumDatanodes(DATANODE_COUNT) + // Clusters overlap via MiniOzoneClusterProvider, so the metrics leak + // assertion would see a concurrent cluster's sources. + .setMetricsLeakAssertEnabled(false); clusterProvider = new MiniOzoneClusterProvider(builder, 9); } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java index c0435088d25..1ecdb5226f6 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java @@ -161,6 +161,9 @@ public static void initClass() { builder.setNumOfStorageContainerManagers(NUM_SCMS) .setSCMConfigurator(scmConfigurator) .setNumDatanodes(NUM_DATA_NODES) + // Clusters overlap via MiniOzoneClusterProvider, so the metrics leak + // assertion would see a concurrent cluster's sources. + .setMetricsLeakAssertEnabled(false) .setDatanodeFactory(UniformDatanodesFactory.newBuilder() .setLayoutVersion(HDDSLayoutFeature.INITIAL_VERSION.layoutVersion()) .build()); diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index 8765c2aaaae..68fa05e13b2 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -262,6 +262,7 @@ abstract class Builder { protected int numOfDatanodes = 3; protected boolean startDataNodes = true; + protected boolean metricsLeakAssertEnabled = true; protected CertificateClient certClient; protected SecretKeyClient secretKeyClient; protected DatanodeFactory dnFactory = UniformDatanodesFactory.newBuilder().build(); @@ -354,6 +355,17 @@ public Builder setStartDataNodes(boolean nodes) { return this; } + /** + * Whether to assert that no metrics sources leak on cluster shutdown. + * Disable for tests that run multiple clusters concurrently (e.g. via + * MiniOzoneClusterProvider), because the metrics registry is shared + * JVM-wide and a concurrent cluster's sources would be flagged as leaks. + */ + public Builder setMetricsLeakAssertEnabled(boolean enabled) { + this.metricsLeakAssertEnabled = enabled; + return this; + } + public Builder setCertificateClient(CertificateClient client) { this.certClient = client; return this; diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java index cb8383117cf..afbf5952c71 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java @@ -113,6 +113,9 @@ public class MiniOzoneClusterImpl implements MiniOzoneCluster { private OzoneConfiguration conf; private final SCMConfigurator scmConfigurator; + // Whether to assert no metrics sources leak on shutdown. Tests running + // multiple clusters concurrently disable this via the builder. + private boolean metricsLeakAssertEnabled = true; private StorageContainerManager scm; private OzoneManager ozoneManager; private final List hddsDatanodes; @@ -165,6 +168,10 @@ protected void setConf(OzoneConfiguration newConf) { this.conf = newConf; } + void setMetricsLeakAssertEnabled(boolean enabled) { + this.metricsLeakAssertEnabled = enabled; + } + public void waitForSCMToBeReady() throws TimeoutException, InterruptedException { GenericTestUtils.waitFor(scm::checkLeader, @@ -394,8 +401,11 @@ public void shutdown() { ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks(); // Assert before tearing down the metrics system: Hadoop does not clear // allSources on shutdown, but checking before shutdown makes the - // assertion independent of that behavior. - MetricsLeakAssertion.assertNoLeaks(); + // assertion independent of that behavior. Tests running multiple + // clusters concurrently disable this because the registry is JVM-wide. + if (metricsLeakAssertEnabled) { + MetricsLeakAssertion.assertNoLeaks(); + } DefaultMetricsSystem.shutdown(); } catch (Exception e) { LOG.error("Exception while shutting down the cluster.", e); @@ -531,6 +541,7 @@ public MiniOzoneCluster build() throws IOException { MiniOzoneClusterImpl cluster = new MiniOzoneClusterImpl(conf, scmConfigurator, om, scm, hddsDatanodes, getServices()); + cluster.setMetricsLeakAssertEnabled(metricsLeakAssertEnabled); cluster.startServices(); cluster.setCAClient(certClient); diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java index 8df0f587c60..f0272737263 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java @@ -502,6 +502,7 @@ public MiniOzoneHAClusterImpl build() throws IOException { MiniOzoneHAClusterImpl cluster = new MiniOzoneHAClusterImpl(conf, scmConfigurator, omService, scmService, hddsDatanodes, path, getServices()); + cluster.setMetricsLeakAssertEnabled(metricsLeakAssertEnabled); try { cluster.startServices(); } catch (Exception e) { From b7772d888d3abcce5f44803b6364cd1ec48bada8 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Thu, 6 Aug 2026 10:32:28 +0800 Subject: [PATCH 15/15] HDDS-15960. Unregister OmSnapshotInternalMetrics on OM stop 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) --- .../src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 101cbbfadb0..53335faf56a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -2491,6 +2491,7 @@ public boolean stop() { serviceManager.stop(); DeletingServiceMetrics.unregister(); OMPerformanceMetrics.unregister(); + OmSnapshotInternalMetrics.unregister(); RatisDropwizardExports.clear(ratisMetricsMap, ratisReporterList); scmClient.close(); if (certClient != null) {