diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index c51f6d0429f..82e4b050963 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -411,6 +411,13 @@ protected int getRequiredRackCount(int numReplicas, int excludedRackCount) { * @return The max number of replicas per rack */ protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { + if (numberOfRacks <= 0) { + // No rack information means there is no per-rack constraint to + // enforce. Callers are expected to short-circuit before reaching + // here, but guard the divide site against transient empty-topology + // windows (HDDS-15350). + return numReplicas; + } return numReplicas / numberOfRacks + Math.min(numReplicas % numberOfRacks, 1); } @@ -436,7 +443,16 @@ public ContainerPlacementStatus validateContainerPlacement( NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); // We have a network topology so calculate if it is satisfied or not. int requiredRacks = getRequiredRackCount(replicas, 0); - if (topology == null || replicas == 1 || requiredRacks == 1) { + // The leaf nodes are all at max level, so the number of nodes at + // maxLevel - 1 is the rack count. Compute up front so we can + // short-circuit when the topology has no rack information, which + // would otherwise reach getMaxReplicasPerRack with numberOfRacks + // == 0 (HDDS-15350: transient empty-topology window during a DN + // decommission crashed the ReplicationMonitor with "/ by zero"). + final int numRacks = topology == null ? 0 + : topology.getNumOfNodes(topology.getMaxLevel() - 1); + if (topology == null || replicas == 1 || requiredRacks <= 1 + || numRacks <= 0) { if (!dns.isEmpty()) { // placement is always satisfied if there is at least one DN. return validPlacement; @@ -471,10 +487,6 @@ public ContainerPlacementStatus validateContainerPlacement( Function.identity(), Collectors.reducing(0, e -> 1, Integer::sum))) .values()); - final int maxLevel = topology.getMaxLevel(); - // The leaf nodes are all at max level, so the number of nodes at - // leafLevel - 1 is the rack count - int numRacks = topology.getNumOfNodes(maxLevel - 1); if (replicas < requiredRacks) { requiredRacks = replicas; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java index dba2d60b98c..18f8727483a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java @@ -566,6 +566,46 @@ public void testValidatePlacementWithDeadMaintenanceNode() throws NodeNotFoundEx assertTrue(placementStatus.isPolicySatisfied()); } + /** + * HDDS-15350: when the network topology transiently reports zero racks + * (observed during a DN decommission), validateContainerPlacement must + * not crash with ArithmeticException ("/ by zero") in + * getMaxReplicasPerRack. Without the fix this test throws and SCM's + * ReplicationMonitor thread dies along with it. + */ + @Test + public void testValidateContainerPlacementWithZeroRackTopology() { + List nodes = ImmutableList.of( + MockDatanodeDetails.randomDatanodeDetails(), + MockDatanodeDetails.randomDatanodeDetails(), + MockDatanodeDetails.randomDatanodeDetails()); + NodeManager mockNodeManager = mock(NodeManager.class); + when(mockNodeManager.getAllNodes()).thenAnswer(inv -> nodes); + + // Topology that reports zero racks at the rack level - the empty + // -topology window observed during DN decommission. + NetworkTopology topology = mock(NetworkTopology.class); + when(topology.getMaxLevel()).thenReturn(3); + when(topology.getNumOfNodes(anyInt())).thenReturn(0); + when(mockNodeManager.getClusterNetworkTopologyMap()).thenReturn(topology); + + // rackCnt=2 makes DummyPlacementPolicy.getRequiredRackCount return + // min(replicas, 2) > 1, so the original early-return guard does NOT + // fire and execution proceeds to the divide site. + Map rackMap = new HashMap<>(); + rackMap.put(0, 0); + rackMap.put(1, 0); + rackMap.put(2, 0); + DummyPlacementPolicy policy = new DummyPlacementPolicy( + mockNodeManager, conf, rackMap, 2); + + ContainerPlacementStatus status = + policy.validateContainerPlacement(nodes, 3); + assertTrue(status.isPolicySatisfied(), + "placement should not crash and should be considered satisfied " + + "when the topology reports no racks"); + } + private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { private Map rackMap; private List racks;