Skip to content

HDDS-7349. Flaky integration test have memory leak for RatisDropwizardExports #3858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adoroszlai merged 2 commits into from
Oct 26, 2022
Merged

HDDS-7349. Flaky integration test have memory leak for RatisDropwizardExports #3858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d8641a
HDDS-7349 Flaky integration test have memory leak for RatisDropwizard…
sumitagrawl Oct 19, 2022
f36113e
HDDS-7349 Flaky integration test have memory leak for RatisDropwizard…
sumitagrawl Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public void start() {
serviceRuntimeInfo.setStartTime();

RatisDropwizardExports.
registerRatisMetricReporters(ratisMetricsMap);
registerRatisMetricReporters(ratisMetricsMap, () -> isStopped.get());

OzoneConfiguration.activate();
HddsServerUtil.initializeMetrics(conf, "HddsDatanode");
Expand Down
19 changes: 14 additions & 5 deletions ...ds/framework/src/main/java/org/apache/hadoop/hdds/server/http/RatisDropwizardExports.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.function.BooleanSupplier;
import org.apache.ratis.metrics.MetricRegistries;
import org.apache.ratis.metrics.MetricsReporting;
import org.apache.ratis.metrics.RatisMetricRegistry;
Expand All @@ -43,14 +44,15 @@ public RatisDropwizardExports(MetricRegistry registry) {
}

public static void registerRatisMetricReporters(
Map<String, RatisDropwizardExports> ratisMetricsMap) {
Map<String, RatisDropwizardExports> ratisMetricsMap,
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),
r1 -> registerDropwizard(r1, ratisMetricsMap, checkStopped),
r2 -> deregisterDropwizard(r2, ratisMetricsMap));
}

Expand All @@ -69,12 +71,19 @@ public static void clear(Map<String, RatisDropwizardExports>
}

private static void registerDropwizard(RatisMetricRegistry registry,
Map<String, RatisDropwizardExports> ratisMetricsMap) {
Map<String, RatisDropwizardExports> ratisMetricsMap,
BooleanSupplier checkStopped) {
if (checkStopped.getAsBoolean()) {
return;
}

RatisDropwizardExports rde = new RatisDropwizardExports(
registry.getDropWizardMetricRegistry());
CollectorRegistry.defaultRegistry.register(rde);
String name = registry.getMetricRegistryInfo().getName();

@szetszwo szetszwo Oct 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep name declared after rde, i.e.

     RatisDropwizardExports rde = new RatisDropwizardExports(
         registry.getDropWizardMetricRegistry());
     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,
Expand Down
11 changes: 9 additions & 2 deletions ...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.get());
if (configurator.getSCMHAManager() != null) {
scmHAManager = configurator.getSCMHAManager();
} else {
Expand Down Expand Up @@ -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.");
Expand Down
2 changes: 1 addition & 1 deletion hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1967,7 +1967,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);
Expand Down