apache · adoroszlai · Mar 30, 2025 · Mar 28, 2025
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java
@@ -258,16 +258,6 @@ void addContainer(DatanodeDetails datanodeDetails,
   void removeContainer(DatanodeDetails datanodeDetails,
       ContainerID containerId) throws NodeNotFoundException;
 
-  /**
-   * Remaps datanode to containers mapping to the new set of containers.
-   * @param datanodeDetails - DatanodeDetails
-   * @param containerIds - Set of containerIDs
-   * @throws NodeNotFoundException - if datanode is not known. For new datanode
-   *                        use addDatanodeInContainerMap call.
-   */
-  void setContainers(DatanodeDetails datanodeDetails,
-      Set<ContainerID> containerIds) throws NodeNotFoundException;
-
   /**
    * Return set of containerIDs available on a datanode.
    * @param datanodeDetails DatanodeDetails

diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
@@ -697,12 +697,13 @@ public void removeContainer(final DatanodeID datanodeID,
   }
 
   /**
-   * Update set of containers available on a datanode.
+   * Set the containers for the given datanode.
+   * This method is only used for testing.
    * @throws NodeNotFoundException - if datanode is not known.
    */
-  public void setContainers(DatanodeID datanodeID, Set<ContainerID> containerIds)
+  void setContainersForTesting(DatanodeID datanodeID, Set<ContainerID> containerIds)
       throws NodeNotFoundException {
-    nodeStateMap.setContainers(datanodeID, containerIds);
+    nodeStateMap.setContainersForTesting(datanodeID, containerIds);
   }
 
   /**

diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
@@ -1597,20 +1597,6 @@ public void removeContainer(final DatanodeDetails datanodeDetails,
     nodeStateManager.removeContainer(datanodeDetails.getID(), containerId);
   }
 
-  /**
-   * Update set of containers available on a datanode.
-   *
-   * @param datanodeDetails - DatanodeID
-   * @param containerIds    - Set of containerIDs
-   * @throws NodeNotFoundException - if datanode is not known. For new datanode
-   *                               use addDatanodeInContainerMap call.
-   */
-  @Override
-  public void setContainers(DatanodeDetails datanodeDetails,
-      Set<ContainerID> containerIds) throws NodeNotFoundException {
-    nodeStateManager.setContainers(datanodeDetails.getID(), containerIds);
-  }
-
   /**
    * Return set of containerIDs available on a datanode. This is a copy of the
    * set which resides inside NodeManager and hence can be modified without

diff --git a/...op-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java b/...op-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
@@ -304,7 +304,11 @@ public void addContainer(final DatanodeID datanodeID,
     }
   }
 
-  public void setContainers(DatanodeID id, Set<ContainerID> containers)
+  /**
+   * Set the containers for the given datanode.
+   * This method is only used for testing.
+   */
+  public void setContainersForTesting(DatanodeID id, Set<ContainerID> containers)
       throws NodeNotFoundException {
     lock.writeLock().lock();
     try {

diff --git a/...p-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java b/...p-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
@@ -50,7 +50,6 @@
 import org.apache.hadoop.hdds.scm.HddsTestUtils;
 import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric;
 import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeStat;
-import org.apache.hadoop.hdds.scm.exceptions.SCMException;
 import org.apache.hadoop.hdds.scm.net.NetConstants;
 import org.apache.hadoop.hdds.scm.net.NetworkTopology;
 import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl;
@@ -617,15 +616,10 @@ public Map<SCMCommandProto.Type, Integer> getTotalDatanodeCommandCounts(
 
   /**
    * Update set of containers available on a datanode.
-   * @param uuid - DatanodeID
-   * @param containerIds - Set of containerIDs
-   * @throws SCMException - if datanode is not known. For new datanode use
-   *                        addDatanodeInContainerMap call.
    */
-  @Override
   public void setContainers(DatanodeDetails uuid, Set<ContainerID> containerIds)
       throws NodeNotFoundException {
-    node2ContainerMap.setContainers(uuid.getID(), containerIds);
+    node2ContainerMap.setContainersForTesting(uuid.getID(), containerIds);
   }
 
   /**

diff --git a/.../server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java b/.../server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java
@@ -158,10 +158,7 @@ public int getPipelinesCount(DatanodeDetails datanodeDetails) {
     return 0;
   }
 
-  @Override
-  public void setContainers(DatanodeDetails dn,
-                            Set<ContainerID> containerIds)
-      throws NodeNotFoundException {
+  public void setContainers(DatanodeDetails dn, Set<ContainerID> containerIds) {
     containerMap.put(dn.getUuid(), containerIds);
   }
 

diff --git a/...er-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java b/...er-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java
@@ -60,7 +60,6 @@
 import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
 import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub;
 import org.apache.hadoop.hdds.scm.metadata.SCMDBDefinition;
-import org.apache.hadoop.hdds.scm.node.NodeManager;
 import org.apache.hadoop.hdds.scm.node.NodeStatus;
 import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
 import org.apache.hadoop.hdds.scm.pipeline.MockPipelineManager;
@@ -89,7 +88,7 @@
  */
 public class TestContainerReportHandler {
 
-  private NodeManager nodeManager;
+  private MockNodeManager nodeManager;
   private ContainerManager containerManager;
   private ContainerStateManager containerStateManager;
   private EventPublisher publisher;

diff --git a/...one/container/testutils/package-info.java → ...hadoop/hdds/scm/node/ScmNodeTestUtil.java b/...one/container/testutils/package-info.java → ...hadoop/hdds/scm/node/ScmNodeTestUtil.java
@@ -15,5 +15,17 @@
  * limitations under the License.
  */
 
-/** Helper classes for ozone and container tests. */
-package org.apache.hadoop.ozone.container.testutils;
+package org.apache.hadoop.hdds.scm.node;
+
+import java.util.Set;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+
+/** Utilities for testing the {@link org.apache.hadoop.hdds.scm.node} package. */
+public interface ScmNodeTestUtil {
+  static void setContainers(SCMNodeManager scm, DatanodeDetails datanode,
+      Set<ContainerID> containers) throws NodeNotFoundException {
+    scm.getNodeStateManager().setContainersForTesting(datanode.getID(), containers);
+  }
+}
diff --git a/...op-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java b/...op-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java
@@ -299,15 +299,12 @@ private void registerReplicas(ContainerManager contManager,
 
   /**
    * Update containers available on the datanode.
-   * @param datanode
-   * @param containers
-   * @throws NodeNotFoundException
    */
   private void registerContainers(DatanodeDetails datanode,
       ContainerInfo... containers)
       throws NodeNotFoundException {
-    nodeManager
-        .setContainers(datanode,
+    ScmNodeTestUtil.setContainers(nodeManager,
+        datanode,
             Arrays.stream(containers)
                 .map(ContainerInfo::containerID)
                 .collect(Collectors.toSet()));

diff --git a/...server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java b/...server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java
@@ -68,7 +68,7 @@ public class TestNodeDecommissionManager {
 
   private NodeDecommissionManager decom;
   private StorageContainerManager scm;
-  private NodeManager nodeManager;
+  private SCMNodeManager nodeManager;
   private ContainerManager containerManager;
   private OzoneConfiguration conf;
   private static int id = 1;
@@ -78,7 +78,7 @@ void setup(@TempDir File dir) throws Exception {
     conf = new OzoneConfiguration();
     conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, dir.getAbsolutePath());
     scm = HddsTestUtils.getScm(conf);
-    nodeManager = scm.getScmNodeManager();
+    nodeManager = (SCMNodeManager)scm.getScmNodeManager();
     containerManager = mock(ContainerManager.class);
     decom = new NodeDecommissionManager(conf, nodeManager, containerManager,
         SCMContext.emptyContext(), new EventQueue(), null);
@@ -87,6 +87,10 @@ void setup(@TempDir File dir) throws Exception {
             (String) invocation.getArguments()[1]));
   }
 
+  void setContainers(DatanodeDetails datanode, Set<ContainerID> containers) throws NodeNotFoundException {
+    ScmNodeTestUtil.setContainers(nodeManager, datanode, containers);
+  }
+
   private ContainerInfo createMockContainer(ReplicationConfig rep, String owner) {
     ContainerInfo.Builder builder = new ContainerInfo.Builder()
         .setReplicationConfig(rep)
@@ -426,7 +430,7 @@ public void testInsufficientNodeDecommissionThrowsExceptionForRatis() throws
     }
 
     for (DatanodeDetails dn  : nodeManager.getAllNodes().subList(0, 3)) {
-      nodeManager.setContainers(dn, idsRatis);
+      setContainers(dn, idsRatis);
     }
 
     error = decom.decommissionNodes(Arrays.asList(dns.get(1).getIpAddress(),
@@ -480,7 +484,7 @@ public void testInsufficientNodeDecommissionThrowsExceptionForEc() throws
     }
 
     for (DatanodeDetails dn  : nodeManager.getAllNodes()) {
-      nodeManager.setContainers(dn, idsEC);
+      setContainers(dn, idsEC);
     }
 
     error = decom.decommissionNodes(Arrays.asList(dns.get(1).getIpAddress()), false);
@@ -529,10 +533,10 @@ public void testInsufficientNodeDecommissionThrowsExceptionRatisAndEc() throws
         });
 
     for (DatanodeDetails dn  : nodeManager.getAllNodes().subList(0, 3)) {
-      nodeManager.setContainers(dn, idsRatis);
+      setContainers(dn, idsRatis);
     }
     for (DatanodeDetails dn  : nodeManager.getAllNodes()) {
-      nodeManager.setContainers(dn, idsEC);
+      setContainers(dn, idsEC);
     }
 
     error = decom.decommissionNodes(Arrays.asList(dns.get(1).getIpAddress()), false);
@@ -573,7 +577,7 @@ public void testInsufficientNodeDecommissionChecksNotInService() throws
     }
 
     for (DatanodeDetails dn  : nodeManager.getAllNodes().subList(0, 3)) {
-      nodeManager.setContainers(dn, idsRatis);
+      setContainers(dn, idsRatis);
     }
 
     // decommission one node successfully
@@ -608,7 +612,7 @@ public void testInsufficientNodeDecommissionChecksForNNF() throws
       idsRatis.add(container.containerID());
     }
 
-    nodeManager = mock(NodeManager.class);
+    nodeManager = mock(SCMNodeManager.class);
     decom = new NodeDecommissionManager(conf, nodeManager, containerManager,
         SCMContext.emptyContext(), new EventQueue(), null);
     when(containerManager.getContainer(any(ContainerID.class)))
@@ -669,7 +673,7 @@ public void testInsufficientNodeMaintenanceThrowsExceptionForRatis() throws
       idsRatis.add(container.containerID());
     }
     for (DatanodeDetails dn  : nodeManager.getAllNodes().subList(0, 3)) {
-      nodeManager.setContainers(dn, idsRatis);
+      setContainers(dn, idsRatis);
     }
 
     decom.setMaintenanceConfigs(2, 1); // default config
@@ -769,7 +773,7 @@ public void testInsufficientNodeMaintenanceThrowsExceptionForEc() throws
       idsEC.add(container.containerID());
     }
     for (DatanodeDetails dn  : nodeManager.getAllNodes()) {
-      nodeManager.setContainers(dn, idsEC);
+      setContainers(dn, idsEC);
     }
 
     decom.setMaintenanceConfigs(2, 1); // default config
@@ -852,10 +856,10 @@ public void testInsufficientNodeMaintenanceThrowsExceptionForRatisAndEc() throws
               (ContainerID)invocation.getArguments()[0]);
         });
     for (DatanodeDetails dn  : nodeManager.getAllNodes().subList(0, 3)) {
-      nodeManager.setContainers(dn, idsRatis);
+      setContainers(dn, idsRatis);
     }
     for (DatanodeDetails dn  : nodeManager.getAllNodes()) {
-      nodeManager.setContainers(dn, idsEC);
+      setContainers(dn, idsEC);
     }
 
     decom.setMaintenanceConfigs(2, 1); // default config
@@ -926,7 +930,7 @@ public void testInsufficientNodeMaintenanceChecksNotInService() throws
       idsRatis.add(container.containerID());
     }
     for (DatanodeDetails dn  : nodeManager.getAllNodes().subList(0, 3)) {
-      nodeManager.setContainers(dn, idsRatis);
+      setContainers(dn, idsRatis);
     }
 
     // put 2 nodes into maintenance successfully
@@ -966,7 +970,7 @@ public void testInsufficientNodeMaintenanceChecksForNNF() throws
       idsRatis.add(container.containerID());
     }
 
-    nodeManager = mock(NodeManager.class);
+    nodeManager = mock(SCMNodeManager.class);
     decom = new NodeDecommissionManager(conf, nodeManager, containerManager,
         SCMContext.emptyContext(), new EventQueue(), null);
     when(containerManager.getContainer(any(ContainerID.class)))