diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java index 9f42c7286636..017e93ee2a68 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java @@ -400,6 +400,9 @@ private void processMaintenanceOnlyIndexes( // this many maintenance replicas need another copy int additionalMaintenanceCopiesNeeded = replicaCount.additionalMaintenanceCopiesNeeded(true); + if (additionalMaintenanceCopiesNeeded == 0) { + return; + } List targets = getTargetDatanodes(excludedNodes, container, additionalMaintenanceCopiesNeeded); excludedNodes.addAll(targets); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java index 0ba2c65dc2d0..f2cac1f2a18c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java @@ -69,6 +69,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.times; /** @@ -478,6 +479,44 @@ public void testUnderAndOverReplication() throws IOException { } } + /** + * HDDS-7683 was a case where the maintenance logic was calling the placement + * policy requesting zero nodes. This test asserts that it is never called + * with zero nodes to ensure that issue is fixed. + */ + @Test + public void testMaintenanceDoesNotRequestZeroNodes() throws IOException { + Set availableReplicas = ReplicationTestUtil + .createReplicas(Pair.of(DECOMMISSIONING, 1), Pair.of(IN_SERVICE, 2), + Pair.of(IN_MAINTENANCE, 3), Pair.of(IN_SERVICE, 4), + Pair.of(IN_SERVICE, 5)); + + Mockito.when(ecPlacementPolicy.chooseDatanodes(anyList(), Mockito.isNull(), + anyInt(), anyLong(), anyLong())) + .thenAnswer(invocationOnMock -> { + int numNodes = invocationOnMock.getArgument(2); + List targets = new ArrayList<>(); + for (int i = 0; i < numNodes; i++) { + targets.add(MockDatanodeDetails.randomDatanodeDetails()); + } + return targets; + }); + + ContainerHealthResult.UnderReplicatedHealthResult result = + Mockito.mock(ContainerHealthResult.UnderReplicatedHealthResult.class); + Mockito.when(result.getContainerInfo()).thenReturn(container); + ECUnderReplicationHandler handler = new ECUnderReplicationHandler( + ecPlacementPolicy, conf, nodeManager, replicationManager); + + Map> commands = + handler.processAndCreateCommands(availableReplicas, + Collections.emptyList(), result, 1); + Assertions.assertEquals(1, commands.size()); + Mockito.verify(ecPlacementPolicy, times(0)) + .chooseDatanodes(anyList(), Mockito.isNull(), eq(0), anyLong(), + anyLong()); + } + /** * Create 3 replicas with 1 pending ADD. This means that 1 replica needs to * be reconstructed. The target DN selected for reconstruction should not be