From 6ca27ff3e9aa7e23ab9c3d2b99055ea34119cf07 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Fri, 22 May 2026 18:24:51 -0700 Subject: [PATCH 1/3] HDDS-15350. SCM crashes with ArithmeticException when topology reports zero racks during DN decommission SCMCommonPlacementPolicy.getMaxReplicasPerRack divides by numberOfRacks without a zero check. The caller (validateContainerPlacement) reaches the divide via Math.min(requiredRacks, numRacks); when the network topology transiently reports zero racks (observed during a DN decommission) the existing requiredRacks==1 short-circuit does not catch it. The ReplicationMonitor catches the exception and calls ExitUtil.terminate(1, t) ("When we get runtime exception, we should terminate SCM."), so the SCM JVM exits and is restarted by the supervisor (CM / systemd). Fix: * Compute numRacks before the early-return guard and short-circuit when numRacks <= 0 or requiredRacks <= 1 (was: == 1). * Add a defensive guard in getMaxReplicasPerRack that returns numReplicas when numberOfRacks <= 0, mirroring HDDS-14371's pattern for an analogous div-by-zero in ContainerManagerImpl. Test: * TestSCMCommonPlacementPolicy.testValidateContainerPlacementWithZeroRackTopology reproduces the empty-topology window with a mocked NetworkTopology returning 0 from getNumOfNodes. Without the fix the test errors out with "Arithmetic / by zero". With the fix it passes and all 20 tests in TestSCMCommonPlacementPolicy remain green. Generated-by: Claude Code (Opus 4.7) --- .../hdds/scm/SCMCommonPlacementPolicy.java | 22 +++++++--- .../scm/TestSCMCommonPlacementPolicy.java | 40 +++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) 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 c51f6d0429f4..82e4b050963a 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 dba2d60b98ce..18f8727483ae 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; From 4bac3fbe9a54776252dc3f1b6cf33846681dd8a8 Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Sat, 23 May 2026 23:49:33 -0700 Subject: [PATCH 2/3] Address comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 18f8727483ae..bbf92ace9bad 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 @@ -582,8 +582,8 @@ public void testValidateContainerPlacementWithZeroRackTopology() { 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. + // Topology that reports zero racks at the rack level during the + // empty-topology window observed during DN decommission. NetworkTopology topology = mock(NetworkTopology.class); when(topology.getMaxLevel()).thenReturn(3); when(topology.getNumOfNodes(anyInt())).thenReturn(0); From ea1e8fc420f45ab8c4c72770a94007fcaae38eec Mon Sep 17 00:00:00 2001 From: Siyao Meng <50227127+smengcl@users.noreply.github.com> Date: Sat, 23 May 2026 02:05:40 -0700 Subject: [PATCH 3/3] HDDS-15350. Add empty topology warn log The previous commit's defensive guard turns the SCM crash into a silent return of validPlacement. That removes the loud signal that the topology is corrupted, so downstream mis-replication and rack-aware placement decisions then silently treat any placement as valid. Add a WARN log at the guard site with numReplicas and numberOfRacks so operators monitoring SCM logs see the degradation. No in-code rate limiting; if the topology is persistently empty and the WARN floods, configure log4j appender-side filtering. A follow-up can introduce a metrics2 counter if Prometheus / JMX scraping is wanted. Co-Authored-By: Claude Opus 4.7 --- .../apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 82e4b050963a..93940b7770ff 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 @@ -415,7 +415,12 @@ protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { // 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). + // windows (HDDS-15350). The WARN makes the silent path observable; + // configure log4j appender-side filtering if it floods. + LOG.warn("Empty rack topology in placement validation: numReplicas={} " + + "numberOfRacks={}; returning numReplicas to avoid divide-by-zero " + + "(HDDS-15350).", + numReplicas, numberOfRacks); return numReplicas; } return numReplicas / numberOfRacks