From c4d42a516e09037ab446490f482f12d94c81a0f1 Mon Sep 17 00:00:00 2001 From: F64116045 Date: Wed, 28 Oct 2026 08:25:18 +0800 Subject: [PATCH] HDDS-17511. Fix healthy datanode counts in admin checks Subtract only healthy IN_SERVICE targets from the healthy datanode count when checking decommission and maintenance requests. Keep checking containers on STALE and DEAD targets against the required node count. Co-authored-by: Codex (GPT-6) --- .../scm/node/NodeDecommissionManager.java | 16 ++--- .../scm/node/TestNodeDecommissionManager.java | 61 +++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java index 6b5db5c14f4c..b3c228376680 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java @@ -379,7 +379,7 @@ public synchronized void startDecommission(DatanodeDetails dn) } private synchronized boolean checkIfDecommissionPossible(List dns, List errors) { - int numDecom = dns.size(); + int healthyNodesToDecommission = 0; List validDns = new ArrayList<>(dns); int inServiceTotal = nodeManager.getNodeCount(NodeStatus.inServiceHealthy()); for (DatanodeDetails dn : dns) { @@ -387,12 +387,12 @@ private synchronized boolean checkIfDecommissionPossible(List d NodeStatus nodeStatus = getNodeStatus(dn); NodeOperationalState opState = nodeStatus.getOperationalState(); if (opState != NodeOperationalState.IN_SERVICE) { - numDecom--; validDns.remove(dn); LOG.warn("Cannot decommission {} because it is not IN-SERVICE", dn.getHostName()); + } else if (nodeStatus.isHealthy()) { + healthyNodesToDecommission++; } } catch (NodeNotFoundException ex) { - numDecom--; validDns.remove(dn); LOG.warn("Cannot decommission {} because it is not found in SCM", dn.getHostName()); } @@ -422,7 +422,7 @@ private synchronized boolean checkIfDecommissionPossible(List d continue; } int reqNodes = cif.getReplicationConfig().getRequiredNodes(); - if ((inServiceTotal - numDecom) < reqNodes) { + if ((inServiceTotal - healthyNodesToDecommission) < reqNodes) { final int unHealthyTotal = nodeManager.getAllNodeCount() - inServiceTotal; String errorMsg = "Insufficient nodes. Tried to decommission " + dns.size() + " nodes out of " + inServiceTotal + " IN-SERVICE HEALTHY and " + unHealthyTotal + @@ -536,7 +536,7 @@ public synchronized void startMaintenance(DatanodeDetails dn, int endInHours) } private synchronized boolean checkIfMaintenancePossible(List dns, List errors) { - int numMaintenance = dns.size(); + int healthyNodesEnteringMaintenance = 0; List validDns = new ArrayList<>(dns); int inServiceTotal = nodeManager.getNodeCount(NodeStatus.inServiceHealthy()); for (DatanodeDetails dn : dns) { @@ -544,12 +544,12 @@ private synchronized boolean checkIfMaintenancePossible(List dn NodeStatus nodeStatus = getNodeStatus(dn); NodeOperationalState opState = nodeStatus.getOperationalState(); if (opState != NodeOperationalState.IN_SERVICE) { - numMaintenance--; validDns.remove(dn); LOG.warn("{} cannot enter maintenance because it is not IN-SERVICE", dn.getHostName()); + } else if (nodeStatus.isHealthy()) { + healthyNodesEnteringMaintenance++; } } catch (NodeNotFoundException ex) { - numMaintenance--; validDns.remove(dn); LOG.warn("{} cannot enter maintenance because it is not found in SCM", dn.getHostName()); } @@ -589,7 +589,7 @@ private synchronized boolean checkIfMaintenancePossible(List dn } else { minInService = maintenanceReplicaMinimum; } - if ((inServiceTotal - numMaintenance) < minInService) { + if ((inServiceTotal - healthyNodesEnteringMaintenance) < minInService) { final int unHealthyTotal = nodeManager.getAllNodeCount() - inServiceTotal; String errorMsg = "Insufficient nodes. Tried to start maintenance for " + dns.size() + " nodes out of " + inServiceTotal + " IN-SERVICE HEALTHY and " + unHealthyTotal + diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java index e49b5b79a71f..ce70c7556c0e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java @@ -38,6 +38,7 @@ import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.RatisReplicationConfig; @@ -48,6 +49,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.DatanodeAdminError; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.container.ContainerManager; @@ -60,6 +62,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.mockito.Mockito; /** @@ -77,6 +81,7 @@ public class TestNodeDecommissionManager { void setup(@TempDir File dir) throws Exception { conf = new OzoneConfiguration(); conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, dir.getAbsolutePath()); + conf.setTimeDuration(ScmConfigKeys.OZONE_SCM_DATANODE_ADMIN_MONITOR_INTERVAL, 1, TimeUnit.HOURS); StorageContainerManager scm = HddsTestUtils.getScm(conf); nodeManager = (SCMNodeManager) scm.getScmNodeManager(); containerManager = mock(ContainerManager.class); @@ -1028,6 +1033,62 @@ public void testInsufficientNodeMaintenanceChecksForNNF() throws nodeManager.getNodeStatus(dns.get(3)).getOperationalState()); } + @ParameterizedTest + @EnumSource(value = HddsProtos.NodeState.class, names = {"STALE", "DEAD"}) + void testDecommissionCountsOnlyHealthyTargets(HddsProtos.NodeState health) + throws IOException, NodeNotFoundException { + // The container requires three replicas, but only two healthy datanodes are available. + DatanodeDetails target = prepareTargetWithContainer(health, 2); + + List errors = decom.decommissionNodes(singletonList(target.getIpAddress()), false); + assertThat(errors).singleElement().extracting(DatanodeAdminError::getHostname).isEqualTo("AllHosts"); + assertThat(nodeManager.getNodeStatus(target).getOperationalState()) + .isEqualTo(HddsProtos.NodeOperationalState.IN_SERVICE); + + nodeManager.register(MockDatanodeDetails.randomDatanodeDetails(), null, null); + + assertThat(decom.decommissionNodes(singletonList(target.getIpAddress()), false)).isEmpty(); + assertThat(nodeManager.getNodeStatus(target).getOperationalState()) + .isEqualTo(HddsProtos.NodeOperationalState.DECOMMISSIONING); + } + + @ParameterizedTest + @EnumSource(value = HddsProtos.NodeState.class, names = {"STALE", "DEAD"}) + void testMaintenanceCountsOnlyHealthyTargets(HddsProtos.NodeState health) + throws IOException, NodeNotFoundException { + int minimumHealthyNodes = decom.getMaintenanceReplicaMinimum(); + DatanodeDetails target = prepareTargetWithContainer(health, minimumHealthyNodes - 1); + + List errors = decom.startMaintenanceNodes(singletonList(target.getIpAddress()), 1, false); + assertThat(errors).singleElement().extracting(DatanodeAdminError::getHostname).isEqualTo("AllHosts"); + assertThat(nodeManager.getNodeStatus(target).getOperationalState()) + .isEqualTo(HddsProtos.NodeOperationalState.IN_SERVICE); + + nodeManager.register(MockDatanodeDetails.randomDatanodeDetails(), null, null); + + assertThat(decom.startMaintenanceNodes(singletonList(target.getIpAddress()), 1, false)).isEmpty(); + assertThat(nodeManager.getNodeStatus(target).getOperationalState()) + .isEqualTo(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE); + } + + private DatanodeDetails prepareTargetWithContainer(HddsProtos.NodeState health, int healthyNodes) + throws IOException, NodeNotFoundException { + nodeManager.getNodeStateManager().pause(); + int totalNodes = healthyNodes + 1; + for (int index = 0; index < totalNodes; index++) { + nodeManager.register(MockDatanodeDetails.randomDatanodeDetails(), null, null); + } + DatanodeDetails target = nodeManager.getAllNodes().get(0); + nodeManager.getNodeStateManager().getNode(target).setNodeStatus( + NodeStatus.valueOf(HddsProtos.NodeOperationalState.IN_SERVICE, health)); + assertThat(nodeManager.getNodeCount(NodeStatus.inServiceHealthy())).isEqualTo(healthyNodes); + ContainerID containerID = ContainerID.valueOf(1); + setContainers(target, new HashSet<>(singletonList(containerID))); + when(containerManager.getContainer(containerID)).thenReturn( + getMockContainer(RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE), containerID)); + return target; + } + private List getDatanodeDetailsList(String ipaddress, List dns) { List datanodeDetails = new ArrayList<>(); for (DatanodeDetails dn : dns) {