HDDS-9235. ReplicationManager metrics not collected after restart. (a…

…pache#6280)
adoroszlai · Feb 28, 2024 · 543c9e7 · 543c9e7
1 parent 4da5a64
commit 543c9e7
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 4 deletions.
diff --git a/hadoop-hdds/server-scm/dev-support/findbugsExcludeFile.xml b/hadoop-hdds/server-scm/dev-support/findbugsExcludeFile.xml
@@ -51,4 +51,9 @@
     <Class name="org.apache.hadoop.hdds.scm.metadata.TestSCMTransactionInfoCodec"/>
     <Bug pattern="NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.hdds.scm.container.replication.ReplicationManager"/>
+    <Field name="metrics" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>
 </FindBugsFilter>
diff --git a/...cm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java b/...cm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java
@@ -294,6 +294,7 @@ public synchronized void start() {
     if (!isRunning()) {
       LOG.info("Starting Replication Monitor Thread.");
       running = true;
+      metrics = ReplicationManagerMetrics.create(this);
       if (rmConf.isLegacyEnabled()) {
         legacyReplicationManager.setMetrics(metrics);
       }

diff --git a/...main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManagerMetrics.java b/...main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManagerMetrics.java
@@ -235,10 +235,15 @@ public ReplicationManagerMetrics(ReplicationManager manager) {
   }
 
   public static ReplicationManagerMetrics create(ReplicationManager manager) {
-    return DefaultMetricsSystem.instance().register(METRICS_SOURCE_NAME,
-        "SCM Replication manager (closed container replication) related "
-            + "metrics",
-        new ReplicationManagerMetrics(manager));
+    ReplicationManagerMetrics replicationManagerMetrics = (ReplicationManagerMetrics)
+        DefaultMetricsSystem.instance().getSource(METRICS_SOURCE_NAME);
+    if (replicationManagerMetrics == null) {
+      return DefaultMetricsSystem.instance().register(METRICS_SOURCE_NAME,
+          "SCM Replication manager (closed container replication) related "
+              + "metrics",
+          new ReplicationManagerMetrics(manager));
+    }
+    return replicationManagerMetrics;
   }
 
   @Override

diff --git a/...java/org/apache/hadoop/hdds/scm/container/replication/TestContainerReplicaPendingOps.java b/...java/org/apache/hadoop/hdds/scm/container/replication/TestContainerReplicaPendingOps.java
@@ -24,6 +24,7 @@
 import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
 import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.ozone.test.TestClock;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -74,6 +75,13 @@ public void setup() {
     dn3 = MockDatanodeDetails.randomDatanodeDetails();
   }
 
+  @AfterEach
+  void cleanup() {
+    if (metrics != null) {
+      metrics.unRegister();
+    }
+  }
+
   @Test
   public void testGetPendingOpsReturnsEmptyList() {
     List<ContainerReplicaOp> ops =

diff --git a/.../java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java b/.../java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java
@@ -47,6 +47,7 @@
 import org.apache.hadoop.ozone.protocol.commands.SCMCommand;
 import org.apache.ratis.protocol.exceptions.NotLeaderException;
 import org.assertj.core.util.Lists;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -174,6 +175,13 @@ public NodeStatus getNodeStatus(DatanodeDetails dd) {
         .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3));
   }
 
+  @AfterEach
+  void cleanup() {
+    if (metrics != null) {
+      metrics.unRegister();
+    }
+  }
+
   @ParameterizedTest
   @ValueSource(strings = {"rs-6-3-1024k", "rs-10-4-1024k"})
   void defersNonCriticalPartialReconstruction(String rep) throws IOException {

diff --git a/...rc/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java b/...rc/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java
@@ -55,6 +55,7 @@
 import org.apache.hadoop.util.Lists;
 import org.apache.ozone.test.TestClock;
 import org.apache.ratis.protocol.exceptions.NotLeaderException;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -190,6 +191,13 @@ public void setup() throws IOException {
     when(scmContext.getScm()).thenReturn(scm);
   }
 
+  @AfterEach
+  void cleanup() {
+    if (replicationManager.getMetrics() != null) {
+      replicationManager.getMetrics().unRegister();
+    }
+  }
+
   private ReplicationManager createReplicationManager() throws IOException {
     return new ReplicationManager(
         configuration,