You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by so...@apache.org on 2023/04/11 11:23:08 UTC
[ozone] branch master updated: HDDS-8337. ReplicationManager: MisReplicationHandler should throw an exception if partially successful (#4536)
This is an automated email from the ASF dual-hosted git repository.
sodonnell 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 bf35dc4cff HDDS-8337. ReplicationManager: MisReplicationHandler should throw an exception if partially successful (#4536)
bf35dc4cff is described below
commit bf35dc4cffa53bbfff5ace4dfa13d3262b6cd355
Author: Doroszlai, Attila <64...@users.noreply.github.com>
AuthorDate: Tue Apr 11 13:23:01 2023 +0200
HDDS-8337. ReplicationManager: MisReplicationHandler should throw an exception if partially successful (#4536)
---
.../replication/MisReplicationHandler.java | 23 ++++---
.../pipeline/InsufficientDatanodesException.java | 7 ++
.../replication/TestECMisReplicationHandler.java | 24 ++++++-
.../replication/TestMisReplicationHandler.java | 77 ++++++++++++++--------
.../TestRatisMisReplicationHandler.java | 4 +-
5 files changed, 93 insertions(+), 42 deletions(-)
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/MisReplicationHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/MisReplicationHandler.java
index dd2dac5cf7..f84306b096 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/MisReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/MisReplicationHandler.java
@@ -28,6 +28,7 @@ import org.apache.hadoop.hdds.scm.container.ContainerInfo;
import org.apache.hadoop.hdds.scm.container.ContainerReplica;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.apache.hadoop.hdds.scm.pipeline.InsufficientDatanodesException;
import org.apache.hadoop.ozone.protocol.commands.ReplicateContainerCommand;
import org.apache.ratis.protocol.exceptions.NotLeaderException;
import org.slf4j.Logger;
@@ -195,17 +196,23 @@ public abstract class MisReplicationHandler implements
List<DatanodeDetails> excludedDns = replicasToBeReplicated.stream()
.map(ContainerReplica::getDatanodeDetails)
.collect(Collectors.toList());
+ int requiredNodes = replicasToBeReplicated.size();
List<DatanodeDetails> targetDatanodes = getTargetDatanodes(usedDns,
- excludedDns, container, replicasToBeReplicated.size());
- if (targetDatanodes.size() < replicasToBeReplicated.size()) {
+ excludedDns, container, requiredNodes);
+
+ int count = sendReplicateCommands(container, replicasToBeReplicated,
+ targetDatanodes);
+
+ int found = targetDatanodes.size();
+ if (found < requiredNodes) {
LOG.warn("Placement Policy {} found only {} nodes for Container: {}," +
- " number of required nodes: {}, usedNodes : {}",
- containerPlacement.getClass(), targetDatanodes.size(),
- container.getContainerID(), replicasToBeReplicated.size(),
- usedDns);
+ " number of required nodes: {}, usedNodes : {}",
+ containerPlacement.getClass(), found,
+ container.getContainerID(), requiredNodes,
+ usedDns);
+ throw new InsufficientDatanodesException(requiredNodes, found);
}
- return sendReplicateCommands(container, replicasToBeReplicated,
- targetDatanodes);
+ return count;
}
}
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/InsufficientDatanodesException.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/InsufficientDatanodesException.java
index a6a5a69a16..53f76320eb 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/InsufficientDatanodesException.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/InsufficientDatanodesException.java
@@ -33,4 +33,11 @@ public class InsufficientDatanodesException extends IOException {
public InsufficientDatanodesException(String message) {
super(message);
}
+
+ public InsufficientDatanodesException(int required, int available) {
+ super("Not enough datanodes" +
+ ", requested: " + required +
+ ", found: " + available
+ );
+ }
}
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECMisReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECMisReplicationHandler.java
index a660efa8b1..a0d8f32207 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECMisReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECMisReplicationHandler.java
@@ -26,6 +26,7 @@ import org.apache.hadoop.hdds.scm.PlacementPolicy;
import org.apache.hadoop.hdds.scm.container.ContainerReplica;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.apache.hadoop.hdds.scm.pipeline.InsufficientDatanodesException;
import org.apache.ratis.protocol.exceptions.NotLeaderException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -39,6 +40,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Set;
+import static java.util.Collections.singletonList;
import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_MAINTENANCE;
import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
import static org.junit.Assert.assertThrows;
@@ -157,12 +159,12 @@ public class TestECMisReplicationHandler extends TestMisReplicationHandler {
.thenReturn(true);
Mockito.when(placementPolicy.validateContainerPlacement(anyList(),
anyInt())).thenReturn(mockedContainerPlacementStatus);
- List<ContainerReplicaOp> pendingOp = Collections.singletonList(
+ List<ContainerReplicaOp> pendingOp = singletonList(
ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.ADD,
MockDatanodeDetails.randomDatanodeDetails(), 1));
testMisReplication(availableReplicas, placementPolicy,
pendingOp, 0, 1, 0);
- pendingOp = Collections.singletonList(ContainerReplicaOp
+ pendingOp = singletonList(ContainerReplicaOp
.create(ContainerReplicaOp.PendingOpType.DELETE, availableReplicas
.stream().findAny().get().getDatanodeDetails(), 1));
testMisReplication(availableReplicas, placementPolicy,
@@ -180,8 +182,24 @@ public class TestECMisReplicationHandler extends TestMisReplicationHandler {
Pair.of(IN_SERVICE, 3), Pair.of(IN_SERVICE, 4),
Pair.of(IN_SERVICE, 5));
assertThrows(CommandTargetOverloadedException.class,
+ () -> testMisReplication(availableReplicas, mockPlacementPolicy(),
+ Collections.emptyList(), 0, 1, 1, 0));
+ }
+
+ @Test
+ public void commandsForFewerThanRequiredNodes() throws IOException {
+ Set<ContainerReplica> availableReplicas = ReplicationTestUtil
+ .createReplicas(Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2),
+ Pair.of(IN_SERVICE, 3), Pair.of(IN_SERVICE, 4),
+ Pair.of(IN_SERVICE, 5));
+ PlacementPolicy placementPolicy = Mockito.mock(PlacementPolicy.class);
+ Mockito.when(placementPolicy.chooseDatanodes(
+ any(), any(), any(),
+ Mockito.anyInt(), Mockito.anyLong(), Mockito.anyLong()))
+ .thenReturn(singletonList(availableReplicas.iterator().next()));
+ assertThrows(InsufficientDatanodesException.class,
() -> testMisReplication(availableReplicas, Collections.emptyList(),
- 0, 1, 1));
+ 0, 2, 1));
}
@Override
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestMisReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestMisReplicationHandler.java
index 9350b72623..fac35fd395 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestMisReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestMisReplicationHandler.java
@@ -99,6 +99,17 @@ public abstract class TestMisReplicationHandler {
return replicationManager;
}
+ static PlacementPolicy<?> mockPlacementPolicy() {
+ PlacementPolicy<?> placementPolicy = Mockito.mock(PlacementPolicy.class);
+ ContainerPlacementStatus mockedContainerPlacementStatus =
+ Mockito.mock(ContainerPlacementStatus.class);
+ Mockito.when(mockedContainerPlacementStatus.isPolicySatisfied())
+ .thenReturn(false);
+ Mockito.when(placementPolicy.validateContainerPlacement(anyList(),
+ anyInt())).thenReturn(mockedContainerPlacementStatus);
+ return placementPolicy;
+ }
+
protected abstract MisReplicationHandler getMisreplicationHandler(
PlacementPolicy placementPolicy, OzoneConfiguration configuration,
ReplicationManager rm);
@@ -107,23 +118,28 @@ public abstract class TestMisReplicationHandler {
int maintenanceCnt, int misreplicationCount,
int expectedNumberOfNodes)
throws IOException {
- PlacementPolicy placementPolicy = Mockito.mock(PlacementPolicy.class);
- ContainerPlacementStatus mockedContainerPlacementStatus =
- Mockito.mock(ContainerPlacementStatus.class);
- Mockito.when(mockedContainerPlacementStatus.isPolicySatisfied())
- .thenReturn(false);
- Mockito.when(placementPolicy.validateContainerPlacement(anyList(),
- anyInt())).thenReturn(mockedContainerPlacementStatus);
- testMisReplication(availableReplicas, placementPolicy, pendingOp,
+ testMisReplication(availableReplicas, mockPlacementPolicy(), pendingOp,
maintenanceCnt, misreplicationCount, expectedNumberOfNodes);
}
protected void testMisReplication(Set<ContainerReplica> availableReplicas,
- PlacementPolicy mockedPlacementPolicy,
- List<ContainerReplicaOp> pendingOp,
- int maintenanceCnt, int misreplicationCount,
- int expectedNumberOfNodes)
- throws IOException {
+ PlacementPolicy mockedPlacementPolicy,
+ List<ContainerReplicaOp> pendingOp,
+ int maintenanceCnt, int misreplicationCount,
+ int expectedNumberOfNodes)
+ throws IOException {
+ testMisReplication(availableReplicas, mockedPlacementPolicy, pendingOp,
+ maintenanceCnt, misreplicationCount, expectedNumberOfNodes,
+ expectedNumberOfNodes);
+ }
+
+ protected void testMisReplication(Set<ContainerReplica> availableReplicas,
+ PlacementPolicy mockedPlacementPolicy,
+ List<ContainerReplicaOp> pendingOp,
+ int maintenanceCnt, int misreplicationCount,
+ int expectedNumberOfNodes,
+ int expectedNumberOfCommands
+ ) throws IOException {
MisReplicationHandler misReplicationHandler = getMisreplicationHandler(
mockedPlacementPolicy, conf, replicationManager);
@@ -174,22 +190,25 @@ public abstract class TestMisReplicationHandler {
Map<DatanodeDetails, Integer> copyReplicaIdxMap = copy.stream()
.collect(Collectors.toMap(ContainerReplica::getDatanodeDetails,
ContainerReplica::getReplicaIndex));
- misReplicationHandler.processAndSendCommands(availableReplicas,
- pendingOp, result, maintenanceCnt);
- Assertions.assertEquals(expectedNumberOfNodes, commandsSent.size());
- for (Pair<DatanodeDetails, SCMCommand<?>> pair : commandsSent) {
- SCMCommand<?> command = pair.getValue();
- Assertions.assertTrue(command.getType() == replicateContainerCommand);
- ReplicateContainerCommand replicateContainerCommand =
- (ReplicateContainerCommand) command;
- Assertions.assertEquals(replicateContainerCommand.getContainerID(),
- container.getContainerID());
- DatanodeDetails replicateSrcDn = pair.getKey();
- DatanodeDetails target = replicateContainerCommand.getTargetDatanode();
- Assertions.assertTrue(copyReplicaIdxMap.containsKey(replicateSrcDn));
- Assertions.assertTrue(targetNodes.contains(target));
- Assertions.assertEquals(copyReplicaIdxMap.get(replicateSrcDn),
- replicateContainerCommand.getReplicaIndex());
+ try {
+ misReplicationHandler.processAndSendCommands(availableReplicas,
+ pendingOp, result, maintenanceCnt);
+ } finally {
+ Assertions.assertEquals(expectedNumberOfCommands, commandsSent.size());
+ for (Pair<DatanodeDetails, SCMCommand<?>> pair : commandsSent) {
+ SCMCommand<?> command = pair.getValue();
+ Assertions.assertSame(replicateContainerCommand, command.getType());
+ ReplicateContainerCommand replicateContainerCommand =
+ (ReplicateContainerCommand) command;
+ Assertions.assertEquals(replicateContainerCommand.getContainerID(),
+ container.getContainerID());
+ DatanodeDetails replicateSrcDn = pair.getKey();
+ DatanodeDetails target = replicateContainerCommand.getTargetDatanode();
+ Assertions.assertTrue(copyReplicaIdxMap.containsKey(replicateSrcDn));
+ Assertions.assertTrue(targetNodes.contains(target));
+ Assertions.assertEquals(copyReplicaIdxMap.get(replicateSrcDn),
+ replicateContainerCommand.getReplicaIndex());
+ }
}
}
}
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisMisReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisMisReplicationHandler.java
index d7a857acee..7996896f07 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisMisReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisMisReplicationHandler.java
@@ -185,8 +185,8 @@ public class TestRatisMisReplicationHandler extends TestMisReplicationHandler {
.createReplicas(Pair.of(IN_SERVICE, 0), Pair.of(IN_SERVICE, 0),
Pair.of(IN_SERVICE, 0));
assertThrows(CommandTargetOverloadedException.class,
- () -> testMisReplication(availableReplicas, Collections.emptyList(),
- 0, 1, 1));
+ () -> testMisReplication(availableReplicas, mockPlacementPolicy(),
+ Collections.emptyList(), 0, 1, 1, 0));
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org