From 3d8641a33b28793f1c4ae5ef78b0489adeee68d1 Mon Sep 17 00:00:00 2001 From: sumitagrawl Date: Wed, 19 Oct 2022 10:57:42 +0530 Subject: [PATCH 1/2] HDDS-7349 Flaky integration test have memory leak for RatisDropwizardExports --- .../hadoop/ozone/HddsDatanodeService.java | 2 +- .../server/http/RatisDropwizardExports.java | 21 +++++++++++++------ .../scm/server/StorageContainerManager.java | 11 ++++++++-- .../apache/hadoop/ozone/om/OzoneManager.java | 5 ++++- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java index d36a17855031..a3b2743d52f8 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java @@ -213,7 +213,7 @@ public void start() { serviceRuntimeInfo.setStartTime(); RatisDropwizardExports. - registerRatisMetricReporters(ratisMetricsMap); + registerRatisMetricReporters(ratisMetricsMap, isStopped); OzoneConfiguration.activate(); HddsServerUtil.initializeMetrics(conf, "HddsDatanode"); diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java index 76e65a1d726c..58379c837420 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java @@ -22,6 +22,7 @@ import io.prometheus.client.CollectorRegistry; import io.prometheus.client.dropwizard.DropwizardExports; import io.prometheus.client.dropwizard.samplebuilder.DefaultSampleBuilder; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.ratis.metrics.MetricRegistries; import org.apache.ratis.metrics.MetricsReporting; import org.apache.ratis.metrics.RatisMetricRegistry; @@ -43,14 +44,15 @@ public RatisDropwizardExports(MetricRegistry registry) { } public static void registerRatisMetricReporters( - Map ratisMetricsMap) { + Map ratisMetricsMap, + AtomicBoolean isStopped) { //All the Ratis metrics (registered from now) will be published via JMX and //via the prometheus exporter (used by the /prom servlet MetricRegistries.global() .addReporterRegistration(MetricsReporting.jmxReporter(), MetricsReporting.stopJmxReporter()); MetricRegistries.global().addReporterRegistration( - r1 -> registerDropwizard(r1, ratisMetricsMap), + r1 -> registerDropwizard(r1, ratisMetricsMap, isStopped), r2 -> deregisterDropwizard(r2, ratisMetricsMap)); } @@ -69,12 +71,19 @@ public static void clear(Map } private static void registerDropwizard(RatisMetricRegistry registry, - Map ratisMetricsMap) { + Map ratisMetricsMap, + AtomicBoolean isStopped) { + if (isStopped.get()) { + return; + } + + String name = registry.getMetricRegistryInfo().getName(); RatisDropwizardExports rde = new RatisDropwizardExports( registry.getDropWizardMetricRegistry()); - CollectorRegistry.defaultRegistry.register(rde); - String name = registry.getMetricRegistryInfo().getName(); - ratisMetricsMap.putIfAbsent(name, rde); + if (null == ratisMetricsMap.putIfAbsent(name, rde)) { + // new rde is added for the name, so need register + CollectorRegistry.defaultRegistry.register(rde); + } } private static void deregisterDropwizard(RatisMetricRegistry registry, diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index 73a5468c397b..a835c9761cda 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -28,6 +28,7 @@ import com.google.common.cache.RemovalListener; import com.google.protobuf.BlockingService; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdds.HddsConfigKeys; @@ -290,7 +291,8 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl // Used to keep track of pending replication and pending deletes for // container replicas. private ContainerReplicaPendingOps containerReplicaPendingOps; - + private final AtomicBoolean isStopped = new AtomicBoolean(false); + /** * Creates a new StorageContainerManager. Configuration will be * updated with information on the actual listening addresses used @@ -584,7 +586,8 @@ private void initializeSystemManagers(OzoneConfiguration conf, clusterMap = new NetworkTopologyImpl(conf); } // This needs to be done before initializing Ratis. - RatisDropwizardExports.registerRatisMetricReporters(ratisMetricsMap); + RatisDropwizardExports.registerRatisMetricReporters(ratisMetricsMap, + isStopped); if (configurator.getSCMHAManager() != null) { scmHAManager = configurator.getSCMHAManager(); } else { @@ -1519,6 +1522,10 @@ private void persistSCMCertificates() throws IOException { */ @Override public void stop() { + if (isStopped.getAndSet(true)) { + LOG.info("Storage Container Manager is not running."); + return; + } try { if (containerBalancer.isBalancerRunning()) { LOG.info("Stopping Container Balancer service."); 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 70f8359fe832..3c019496461d 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 @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om; +import java.util.concurrent.atomic.AtomicBoolean; import javax.management.ObjectName; import java.io.BufferedWriter; import java.io.File; @@ -438,6 +439,7 @@ private enum State { private static final int MSECS_PER_MINUTE = 60 * 1000; private final boolean isSecurityEnabled; + private final AtomicBoolean isStopped = new AtomicBoolean(false); @SuppressWarnings("methodlength") private OzoneManager(OzoneConfiguration conf, StartupOption startupOption) @@ -1967,7 +1969,7 @@ private void initializeRatisServer(boolean shouldBootstrap) if (omRatisServer == null) { // This needs to be done before initializing Ratis. RatisDropwizardExports. - registerRatisMetricReporters(ratisMetricsMap); + registerRatisMetricReporters(ratisMetricsMap, isStopped); omRatisServer = OzoneManagerRatisServer.newOMRatisServer( configuration, this, omNodeDetails, peerNodesMap, secConfig, certClient, shouldBootstrap); @@ -2037,6 +2039,7 @@ public void stop() { } try { omState = State.STOPPED; + isStopped.set(true); // Cancel the metrics timer and set to null. if (metricsTimer != null) { metricsTimer.cancel(); From f36113e1e2ba1c487788095c46d0ae1cc7a4035b Mon Sep 17 00:00:00 2001 From: sumitagrawl Date: Wed, 26 Oct 2022 17:13:57 +0530 Subject: [PATCH 2/2] HDDS-7349 Flaky integration test have memory leak for RatisDropwizardExports --- .../org/apache/hadoop/ozone/HddsDatanodeService.java | 2 +- .../hdds/server/http/RatisDropwizardExports.java | 12 ++++++------ .../hdds/scm/server/StorageContainerManager.java | 2 +- .../org/apache/hadoop/ozone/om/OzoneManager.java | 5 +---- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java index a3b2743d52f8..92dfbf237a76 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java @@ -213,7 +213,7 @@ public void start() { serviceRuntimeInfo.setStartTime(); RatisDropwizardExports. - registerRatisMetricReporters(ratisMetricsMap, isStopped); + registerRatisMetricReporters(ratisMetricsMap, () -> isStopped.get()); OzoneConfiguration.activate(); HddsServerUtil.initializeMetrics(conf, "HddsDatanode"); diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java index 58379c837420..50d840a7ad9a 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java @@ -22,7 +22,7 @@ import io.prometheus.client.CollectorRegistry; import io.prometheus.client.dropwizard.DropwizardExports; import io.prometheus.client.dropwizard.samplebuilder.DefaultSampleBuilder; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BooleanSupplier; import org.apache.ratis.metrics.MetricRegistries; import org.apache.ratis.metrics.MetricsReporting; import org.apache.ratis.metrics.RatisMetricRegistry; @@ -45,14 +45,14 @@ public RatisDropwizardExports(MetricRegistry registry) { public static void registerRatisMetricReporters( Map ratisMetricsMap, - AtomicBoolean isStopped) { + BooleanSupplier checkStopped) { //All the Ratis metrics (registered from now) will be published via JMX and //via the prometheus exporter (used by the /prom servlet MetricRegistries.global() .addReporterRegistration(MetricsReporting.jmxReporter(), MetricsReporting.stopJmxReporter()); MetricRegistries.global().addReporterRegistration( - r1 -> registerDropwizard(r1, ratisMetricsMap, isStopped), + r1 -> registerDropwizard(r1, ratisMetricsMap, checkStopped), r2 -> deregisterDropwizard(r2, ratisMetricsMap)); } @@ -72,14 +72,14 @@ public static void clear(Map private static void registerDropwizard(RatisMetricRegistry registry, Map ratisMetricsMap, - AtomicBoolean isStopped) { - if (isStopped.get()) { + BooleanSupplier checkStopped) { + if (checkStopped.getAsBoolean()) { return; } - String name = registry.getMetricRegistryInfo().getName(); RatisDropwizardExports rde = new RatisDropwizardExports( registry.getDropWizardMetricRegistry()); + String name = registry.getMetricRegistryInfo().getName(); if (null == ratisMetricsMap.putIfAbsent(name, rde)) { // new rde is added for the name, so need register CollectorRegistry.defaultRegistry.register(rde); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index a835c9761cda..47b8931264ec 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -587,7 +587,7 @@ private void initializeSystemManagers(OzoneConfiguration conf, } // This needs to be done before initializing Ratis. RatisDropwizardExports.registerRatisMetricReporters(ratisMetricsMap, - isStopped); + () -> isStopped.get()); if (configurator.getSCMHAManager() != null) { scmHAManager = configurator.getSCMHAManager(); } else { 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 3c019496461d..273eeca50399 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 @@ -17,7 +17,6 @@ package org.apache.hadoop.ozone.om; -import java.util.concurrent.atomic.AtomicBoolean; import javax.management.ObjectName; import java.io.BufferedWriter; import java.io.File; @@ -439,7 +438,6 @@ private enum State { private static final int MSECS_PER_MINUTE = 60 * 1000; private final boolean isSecurityEnabled; - private final AtomicBoolean isStopped = new AtomicBoolean(false); @SuppressWarnings("methodlength") private OzoneManager(OzoneConfiguration conf, StartupOption startupOption) @@ -1969,7 +1967,7 @@ private void initializeRatisServer(boolean shouldBootstrap) if (omRatisServer == null) { // This needs to be done before initializing Ratis. RatisDropwizardExports. - registerRatisMetricReporters(ratisMetricsMap, isStopped); + registerRatisMetricReporters(ratisMetricsMap, () -> isStopped()); omRatisServer = OzoneManagerRatisServer.newOMRatisServer( configuration, this, omNodeDetails, peerNodesMap, secConfig, certClient, shouldBootstrap); @@ -2039,7 +2037,6 @@ public void stop() { } try { omState = State.STOPPED; - isStopped.set(true); // Cancel the metrics timer and set to null. if (metricsTimer != null) { metricsTimer.cancel();