[ozone] branch master updated: HDDS-8416. ReplicationManager: RatisUnderReplicationHandler should filter sources by highest BCSID (#4599)

[ad...@apache.org]

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 99bab79cef HDDS-8416. ReplicationManager: RatisUnderReplicationHandler should filter sources by highest BCSID (#4599)
99bab79cef is described below

commit 99bab79cefed1c522035af06d025aeeafa11c54a
Author: Stephen O'Donnell <st...@gmail.com>
AuthorDate: Fri Apr 21 14:26:02 2023 +0100

    HDDS-8416. ReplicationManager: RatisUnderReplicationHandler should filter sources by highest BCSID (#4599)
---
 .../replication/RatisUnderReplicationHandler.java  | 30 ++++++++++++++++++----
 .../TestRatisUnderReplicationHandler.java          | 20 +++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisUnderReplicationHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisUnderReplicationHandler.java
index cac0be0ee3..75bd8c4c41 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisUnderReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisUnderReplicationHandler.java
@@ -36,9 +36,11 @@ import org.slf4j.LoggerFactory;
 import java.io.IOException;
 import java.util.HashSet;
 import java.util.List;
+import java.util.OptionalLong;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * This class handles Ratis containers that are under replicated. It should
@@ -203,10 +205,10 @@ public class RatisUnderReplicationHandler
 
     /*
      * Return healthy datanodes which have a replica that satisfies the
-     * predicate and is not pending replica deletion. Sorted in descending
-     * order of sequence id.
+     * predicate and is not pending replica deletion
      */
-    return replicaCount.getReplicas().stream()
+    List<ContainerReplica> availableSources = replicaCount.getReplicas()
+        .stream()
         .filter(predicate)
         .filter(r -> {
           try {
@@ -217,8 +219,26 @@ public class RatisUnderReplicationHandler
           }
         })
         .filter(r -> !pendingDeletion.contains(r.getDatanodeDetails()))
-        .sorted((r1, r2) -> r2.getSequenceId().compareTo(r1.getSequenceId()))
-        .map(ContainerReplica::getDatanodeDetails)
+        .collect(Collectors.toList());
+
+    // We should replicate only the max available sequence ID, as replicas with
+    // earlier sequence IDs may be stale copies.
+    // First we get the max sequence ID, if there is at least one replica with
+    // a non-null sequence.
+    OptionalLong maxSequenceId = availableSources.stream()
+        .filter(r -> r.getSequenceId() != null)
+        .mapToLong(ContainerReplica::getSequenceId)
+        .max();
+
+    // Filter out all but the max sequence ID, or keep all if there is no
+    // max.
+    Stream<ContainerReplica> replicaStream = availableSources.stream();
+    if (maxSequenceId.isPresent()) {
+      replicaStream = replicaStream
+          .filter(r -> r.getSequenceId() != null)
+          .filter(r -> r.getSequenceId() == maxSequenceId.getAsLong());
+    }
+    return replicaStream.map(ContainerReplica::getDatanodeDetails)
         .collect(Collectors.toList());
   }
 
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisUnderReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisUnderReplicationHandler.java
index 299e0e16e0..0cf3a8a8b5 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisUnderReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisUnderReplicationHandler.java
@@ -55,6 +55,7 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalSt
 import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
 import static org.apache.hadoop.hdds.scm.container.replication.ReplicationTestUtil.createContainerReplica;
 import static org.apache.hadoop.hdds.scm.container.replication.ReplicationTestUtil.createReplicas;
+import static org.mockito.ArgumentMatchers.anyInt;
 
 /**
  * Tests for {@link RatisUnderReplicationHandler}.
@@ -262,6 +263,25 @@ public class TestRatisUnderReplicationHandler {
     Assert.assertEquals(closedReplica.getDatanodeDetails(), command.getKey());
   }
 
+  @Test
+  public void testOnlyHighestBcsidShouldBeASource() throws IOException {
+    Set<ContainerReplica> replicas = new HashSet<>();
+    replicas.add(createContainerReplica(container.containerID(), 0,
+        IN_SERVICE, State.CLOSED, 1));
+    ContainerReplica valid = createContainerReplica(
+        container.containerID(), 0, IN_SERVICE, State.CLOSED, 2);
+    replicas.add(valid);
+
+    testProcessing(replicas, Collections.emptyList(),
+        getUnderReplicatedHealthResult(), 2, 1);
+
+    // Ensure that the replica with SEQ=2 is the only source sent
+    Mockito.verify(replicationManager).sendThrottledReplicationCommand(
+        Mockito.any(ContainerInfo.class),
+        Mockito.eq(Collections.singletonList(valid.getDatanodeDetails())),
+        Mockito.any(DatanodeDetails.class), anyInt());
+  }
+
   /**
    * Tests whether the specified expectNumCommands number of commands are
    * created by the handler.


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org