HDFS-17260. Fix the logic for reconfigure slow peer enable for Nameno…

…de. (apache#6279). Contributed by huangzhaobo99. Reviewed-by: huhaiyang <[email protected]> Signed-off-by: Ayush Saxena <[email protected]>
ahmarsuhail · Dec 3, 2023 · 2323ad2 · 2323ad2
1 parent d0b460f
commit 2323ad2
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 6 deletions.
diff --git a/...oop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java b/...oop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
@@ -420,6 +420,7 @@ public void stopSlowPeerCollector() {
       LOG.error("Slow peers collection thread did not shutdown", e);
     } finally {
       slowPeerCollectorDaemon = null;
+      slowNodesUuidSet.clear();
     }
   }
 

diff --git a/.../src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SlowPeerDisabledTracker.java b/.../src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SlowPeerDisabledTracker.java
@@ -33,7 +33,6 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.server.protocol.OutlierMetrics;
-import org.apache.hadoop.util.Preconditions;
 import org.apache.hadoop.util.Timer;
 
 /**
@@ -46,11 +45,6 @@ public class SlowPeerDisabledTracker extends SlowPeerTracker {
 
   public SlowPeerDisabledTracker(Configuration conf, Timer timer) {
     super(conf, timer);
-    final boolean dataNodePeerStatsEnabledVal =
-        conf.getBoolean(DFSConfigKeys.DFS_DATANODE_PEER_STATS_ENABLED_KEY,
-            DFSConfigKeys.DFS_DATANODE_PEER_STATS_ENABLED_DEFAULT);
-    Preconditions.checkArgument(!dataNodePeerStatsEnabledVal,
-        "SlowPeerDisabledTracker should only be used for disabled slow peer stats.");
   }
 
   @Override

diff --git a/.../org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicyExcludeSlowNodes.java b/.../org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicyExcludeSlowNodes.java
@@ -24,6 +24,8 @@
 import org.apache.hadoop.hdfs.server.namenode.NameNode;
 import org.apache.hadoop.hdfs.server.protocol.OutlierMetrics;
 
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -32,7 +34,9 @@
 import java.util.Arrays;
 import java.util.Set;
 
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_DATANODE_PEER_STATS_ENABLED_KEY;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(Parameterized.class)
@@ -136,4 +140,40 @@ public void testChooseTargetExcludeSlowNodes() throws Exception {
     NameNode.LOG.info("Done working on it");
   }
 
+  @Test
+  public void testSlowPeerTrackerEnabledClearSlowNodes() throws Exception {
+    namenode.getNamesystem().writeLock();
+    try {
+      // add nodes
+      for (DatanodeDescriptor dataNode : dataNodes) {
+        dnManager.addDatanode(dataNode);
+      }
+
+      // mock slow nodes
+      SlowPeerTracker tracker = dnManager.getSlowPeerTracker();
+      Assert.assertNotNull(tracker);
+
+      OutlierMetrics outlierMetrics = new OutlierMetrics(0.0, 0.0, 0.0, 5.0);
+      tracker.addReport(dataNodes[0].getInfoAddr(), dataNodes[3].getInfoAddr(),
+          outlierMetrics);
+      tracker.addReport(dataNodes[1].getInfoAddr(), dataNodes[3].getInfoAddr(),
+          outlierMetrics);
+      tracker.addReport(dataNodes[2].getInfoAddr(), dataNodes[3].getInfoAddr(),
+          outlierMetrics);
+
+      // check slow nodes
+      assertFalse(dnManager.isSlowPeerCollectorInitialized());
+      GenericTestUtils.waitFor(
+          () -> DatanodeManager.getSlowNodesUuidSet().size() == 3, 100, 3000);
+
+      // reconfig
+      namenode.reconfigureProperty(DFS_DATANODE_PEER_STATS_ENABLED_KEY,
+          "false");
+      assertTrue(dnManager.isSlowPeerCollectorInitialized());
+      assertEquals(0, DatanodeManager.getSlowNodesUuidSet().size());
+    } finally {
+      namenode.getNamesystem().writeUnlock();
+    }
+  }
+
 }