You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ad...@apache.org on 2023/03/19 06:30:41 UTC
[ozone] branch master updated: HDDS-8171. Replicate commands could be sent to dead maintenance nodes if the same index is being decommissioned (#4412)
This is an automated email from the ASF dual-hosted git repository.
adoroszlai 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 9619759632 HDDS-8171. Replicate commands could be sent to dead maintenance nodes if the same index is being decommissioned (#4412)
9619759632 is described below
commit 96197596329eeaec1a2f871784df8984759bf209
Author: Stephen O'Donnell <st...@gmail.com>
AuthorDate: Sun Mar 19 06:30:34 2023 +0000
HDDS-8171. Replicate commands could be sent to dead maintenance nodes if the same index is being decommissioned (#4412)
---
.../replication/ECUnderReplicationHandler.java | 77 ++++++++++++++--------
.../replication/TestECUnderReplicationHandler.java | 42 ++++++++++++
2 files changed, 90 insertions(+), 29 deletions(-)
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 bf1aa2d3f8..7cddc1e5ce 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
@@ -165,9 +165,9 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
try {
commandsSent += processMissingIndexes(replicaCount, sources,
availableSourceNodes, excludedNodes);
- commandsSent += processDecommissioningIndexes(replicaCount, replicas,
+ commandsSent += processDecommissioningIndexes(replicaCount, sources,
availableSourceNodes, excludedNodes);
- commandsSent += processMaintenanceOnlyIndexes(replicaCount, replicas,
+ commandsSent += processMaintenanceOnlyIndexes(replicaCount, sources,
excludedNodes);
// TODO - we should be able to catch SCMException here and check the
// result code but the RackAware topology never sets the code.
@@ -337,7 +337,7 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
*/
private int processDecommissioningIndexes(
ECContainerReplicaCount replicaCount,
- Set<ContainerReplica> replicas,
+ Map<Integer, Pair<ContainerReplica, NodeStatus>> sources,
List<DatanodeDetails> availableSourceNodes,
List<DatanodeDetails> excludedNodes) throws IOException {
ContainerInfo container = replicaCount.getContainer();
@@ -350,18 +350,25 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
excludedNodes.addAll(selectedDatanodes);
Iterator<DatanodeDetails> iterator = selectedDatanodes.iterator();
// In this case we need to do one to one copy.
- for (ContainerReplica replica : replicas) {
- if (decomIndexes.contains(replica.getReplicaIndex())) {
- if (!iterator.hasNext()) {
- LOG.warn("Couldn't find enough targets. Available source"
- + " nodes: {}, the target nodes: {}, excluded nodes: {}"
- + " and the decommission indexes: {}",
- replicas, selectedDatanodes, excludedNodes, decomIndexes);
- break;
- }
- createReplicateCommand(container, iterator, replica);
- commandsSent++;
+ for (Integer decomIndex : decomIndexes) {
+ Pair<ContainerReplica, NodeStatus> source = sources.get(decomIndex);
+ if (source == null) {
+ LOG.warn("Cannot find source replica for decommissioning index " +
+ "{} in container {}", decomIndex, container.containerID());
+ continue;
}
+ ContainerReplica sourceReplica = source.getLeft();
+ if (!iterator.hasNext()) {
+ LOG.warn("Couldn't find enough targets. Available source"
+ + " nodes: {}, the target nodes: {}, excluded nodes: {}"
+ + " and the decommission indexes: {}",
+ sources.values().stream()
+ .map(Pair::getLeft).collect(Collectors.toSet()),
+ selectedDatanodes, excludedNodes, decomIndexes);
+ break;
+ }
+ createReplicateCommand(container, iterator, sourceReplica);
+ commandsSent++;
}
}
}
@@ -372,13 +379,15 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
* Processes replicas that are in maintenance nodes and should need
* additional copies.
* @param replicaCount
- * @param replicas set of container replicas
+ * @param sources Map of Replica Index to a pair of ContainerReplica and
+ * NodeStatus. This is the list of available replicas.
* @param excludedNodes nodes that should not be targets for new copies
* @@return number of commands sent
* @throws IOException
*/
private int processMaintenanceOnlyIndexes(
- ECContainerReplicaCount replicaCount, Set<ContainerReplica> replicas,
+ ECContainerReplicaCount replicaCount,
+ Map<Integer, Pair<ContainerReplica, NodeStatus>> sources,
List<DatanodeDetails> excludedNodes) throws IOException {
Set<Integer> maintIndexes = replicaCount.maintenanceOnlyIndexes(true);
if (maintIndexes.isEmpty()) {
@@ -399,20 +408,30 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
Iterator<DatanodeDetails> iterator = targets.iterator();
int commandsSent = 0;
// copy replica from source maintenance DN to a target DN
- for (ContainerReplica replica : replicas) {
- if (maintIndexes.contains(replica.getReplicaIndex()) &&
- additionalMaintenanceCopiesNeeded > 0) {
- if (!iterator.hasNext()) {
- LOG.warn("Couldn't find enough targets. Available source"
- + " nodes: {}, target nodes: {}, excluded nodes: {} and"
- + " maintenance indexes: {}",
- replicas, targets, excludedNodes, maintIndexes);
- break;
- }
- createReplicateCommand(container, iterator, replica);
- commandsSent++;
- additionalMaintenanceCopiesNeeded -= 1;
+
+ for (Integer maintIndex : maintIndexes) {
+ if (additionalMaintenanceCopiesNeeded <= 0) {
+ break;
+ }
+ Pair<ContainerReplica, NodeStatus> source = sources.get(maintIndex);
+ if (source == null) {
+ LOG.warn("Cannot find source replica for maintenance index " +
+ "{} in container {}", maintIndex, container.containerID());
+ continue;
+ }
+ ContainerReplica sourceReplica = source.getLeft();
+ if (!iterator.hasNext()) {
+ LOG.warn("Couldn't find enough targets. Available source"
+ + " nodes: {}, target nodes: {}, excluded nodes: {} and"
+ + " maintenance indexes: {}",
+ sources.values().stream()
+ .map(Pair::getLeft).collect(Collectors.toSet()),
+ targets, excludedNodes, maintIndexes);
+ break;
}
+ createReplicateCommand(container, iterator, sourceReplica);
+ commandsSent++;
+ additionalMaintenanceCopiesNeeded -= 1;
}
return commandsSent;
}
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 c0a25c517e..0239e9edbb 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
@@ -54,6 +54,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -184,6 +185,47 @@ public class TestECUnderReplicationHandler {
Assertions.assertEquals(1, cmd.getReplicaIndex());
}
+
+ // Test used to reproduce the issue reported in HDDS-8171
+ @Test
+ public void testUnderReplicationWithDecomIndexAndMaintOnSameIndex()
+ throws IOException {
+ Set<ContainerReplica> availableReplicas = new LinkedHashSet<>();
+ ContainerReplica deadMaintenance =
+ ReplicationTestUtil.createContainerReplica(container.containerID(),
+ 1, IN_MAINTENANCE, CLOSED);
+ availableReplicas.add(deadMaintenance);
+
+ nodeManager = new MockNodeManager(true, 10) {
+ @Override
+ public NodeStatus getNodeStatus(DatanodeDetails dd) {
+ if (dd.equals(deadMaintenance.getDatanodeDetails())) {
+ return new NodeStatus(dd.getPersistedOpState(),
+ HddsProtos.NodeState.DEAD);
+ }
+ return new NodeStatus(
+ dd.getPersistedOpState(), HddsProtos.NodeState.HEALTHY, 0);
+ }
+ };
+
+ availableReplicas.addAll(ReplicationTestUtil
+ .createReplicas(Pair.of(DECOMMISSIONING, 1),
+ Pair.of(IN_MAINTENANCE, 2),
+ Pair.of(IN_SERVICE, 3), Pair.of(IN_SERVICE, 4),
+ Pair.of(IN_SERVICE, 5)));
+
+ Set<Pair<DatanodeDetails, SCMCommand<?>>> cmds =
+ testUnderReplicationWithMissingIndexes(
+ Lists.emptyList(), availableReplicas, 1, 2, policy);
+ Assertions.assertEquals(2, cmds.size());
+ // Check the replicate command has index 1 set
+ for (Pair<DatanodeDetails, SCMCommand<?>> c : cmds) {
+ // Ensure neither of the commands are for the dead maintenance node
+ Assertions.assertNotEquals(deadMaintenance.getDatanodeDetails(),
+ c.getKey());
+ }
+ }
+
@Test
public void testUnderReplicationWithDecomNodesOverloaded()
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org