This is an automated email from the ASF dual-hosted git repository.
siddhant pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 204f8e18c5 HDDS-7683. EC: ReplicationManager - UnderRep maintenance
handler should not request nodes if none needed (#4109)
204f8e18c5 is described below
commit 204f8e18c5e12770bac9907aeba1e6160c5f1eb6
Author: Stephen O'Donnell <[email protected]>
AuthorDate: Tue Dec 20 10:18:25 2022 +0000
HDDS-7683. EC: ReplicationManager - UnderRep maintenance handler should not
request nodes if none needed (#4109)
---
.../replication/ECUnderReplicationHandler.java | 3 ++
.../replication/TestECUnderReplicationHandler.java | 39 ++++++++++++++++++++++
2 files changed, 42 insertions(+)
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 9f42c72866..017e93ee2a 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 @@ public class ECUnderReplicationHandler implements
UnhealthyReplicationHandler {
// this many maintenance replicas need another copy
int additionalMaintenanceCopiesNeeded =
replicaCount.additionalMaintenanceCopiesNeeded(true);
+ if (additionalMaintenanceCopiesNeeded == 0) {
+ return;
+ }
List<DatanodeDetails> 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 0ba2c65dc2..f2cac1f2a1 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.junit.Assert.assertThrows;
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 class TestECUnderReplicationHandler {
}
}
+ /**
+ * 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<ContainerReplica> 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<DatanodeDetails> 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<DatanodeDetails, SCMCommand<?>> 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
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
