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 2022/11/23 16:07:40 UTC

[ozone] branch master updated: HDDS-7532. EC: ReplicationManager - remove calls to ECHealthCheck from under and over replication processing (#3995)

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 1370de18b8 HDDS-7532. EC: ReplicationManager - remove calls to ECHealthCheck from under and over replication processing (#3995)
1370de18b8 is described below

commit 1370de18b85c3c585cfdf5ea475ae6c28024a9a9
Author: Stephen O'Donnell <st...@gmail.com>
AuthorDate: Wed Nov 23 16:07:33 2022 +0000

    HDDS-7532. EC: ReplicationManager - remove calls to ECHealthCheck from under and over replication processing (#3995)
---
 .../replication/ECOverReplicationHandler.java      |  7 +--
 .../replication/ECUnderReplicationHandler.java     | 51 +++++++---------------
 .../container/replication/ReplicationManager.java  |  6 +--
 .../replication/TestECOverReplicationHandler.java  |  7 +--
 .../replication/TestECUnderReplicationHandler.java | 11 ++---
 5 files changed, 26 insertions(+), 56 deletions(-)

diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECOverReplicationHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECOverReplicationHandler.java
index 404bd188a6..85295c9749 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECOverReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECOverReplicationHandler.java
@@ -22,7 +22,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolPro
 import org.apache.hadoop.hdds.scm.PlacementPolicy;
 import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import org.apache.hadoop.hdds.scm.container.ContainerReplica;
-import org.apache.hadoop.hdds.scm.container.replication.health.ECReplicationCheckHandler;
 import org.apache.hadoop.hdds.scm.node.NodeManager;
 import org.apache.hadoop.ozone.protocol.commands.DeleteContainerCommand;
 import org.apache.hadoop.ozone.protocol.commands.SCMCommand;
@@ -48,13 +47,11 @@ public class ECOverReplicationHandler extends AbstractOverReplicationHandler {
   public static final Logger LOG =
       LoggerFactory.getLogger(ECOverReplicationHandler.class);
 
-  private final ECReplicationCheckHandler ecReplicationCheck;
   private final NodeManager nodeManager;
 
-  public ECOverReplicationHandler(ECReplicationCheckHandler ecReplicationCheck,
-      PlacementPolicy placementPolicy, NodeManager nodeManager) {
+  public ECOverReplicationHandler(PlacementPolicy placementPolicy,
+      NodeManager nodeManager) {
     super(placementPolicy);
-    this.ecReplicationCheck = ecReplicationCheck;
     this.nodeManager = nodeManager;
 
   }
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 6dd792c188..3402f9927b 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
@@ -30,7 +30,6 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys;
 import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import org.apache.hadoop.hdds.scm.container.ContainerReplica;
-import org.apache.hadoop.hdds.scm.container.replication.health.ECReplicationCheckHandler;
 import org.apache.hadoop.hdds.scm.exceptions.SCMException;
 import org.apache.hadoop.hdds.scm.node.NodeManager;
 import org.apache.hadoop.hdds.scm.node.NodeStatus;
@@ -60,7 +59,6 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
 
   public static final Logger LOG =
       LoggerFactory.getLogger(ECUnderReplicationHandler.class);
-  private final ECReplicationCheckHandler ecReplicationCheck;
   private final PlacementPolicy containerPlacement;
   private final long currentContainerSize;
   private final NodeManager nodeManager;
@@ -72,10 +70,9 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
     }
   }
 
-  public ECUnderReplicationHandler(ECReplicationCheckHandler ecReplicationCheck,
-      final PlacementPolicy containerPlacement, final ConfigurationSource conf,
-      NodeManager nodeManager, ReplicationManager replicationManager) {
-    this.ecReplicationCheck = ecReplicationCheck;
+  public ECUnderReplicationHandler(final PlacementPolicy containerPlacement,
+      final ConfigurationSource conf, NodeManager nodeManager,
+      ReplicationManager replicationManager) {
     this.containerPlacement = containerPlacement;
     this.currentContainerSize = (long) conf
         .getStorageSize(ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
@@ -123,41 +120,19 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
       final ContainerHealthResult result,
       final int remainingMaintenanceRedundancy) throws IOException {
     ContainerInfo container = result.getContainerInfo();
+    LOG.debug("Handling under-replicated EC container: {}", container);
+
     final ECContainerReplicaCount replicaCount =
         new ECContainerReplicaCount(container, replicas, pendingOps,
             remainingMaintenanceRedundancy);
-
-    // TODO - do we really need to recheck the health via ecReplicationCheck?
-    //        Is it not enough to assume it was under-replicated if it got here
-    //        and then use the replica count object to validate it is under-rep?
-    ContainerCheckRequest request = new ContainerCheckRequest.Builder()
-        .setContainerInfo(container)
-        .setContainerReplicas(replicas)
-        .setPendingOps(pendingOps)
-        .setMaintenanceRedundancy(remainingMaintenanceRedundancy)
-        .build();
-    ContainerHealthResult currentUnderRepRes = ecReplicationCheck
-        .checkHealth(request);
-    LOG.debug("Handling under-replicated EC container: {}", container);
-    if (currentUnderRepRes
-        .getHealthState() != ContainerHealthResult.HealthState
-        .UNDER_REPLICATED) {
+    if (replicaCount.isSufficientlyReplicated()) {
       LOG.info("The container {} state changed and it's not in under"
-              + " replication any more. Current state is: {}",
-          container.getContainerID(), currentUnderRepRes);
+              + " replication any more.", container.getContainerID());
       return emptyMap();
     }
-    // don't place reconstructed replicas on exclude nodes, since they already
-    // have replicas
-    List<DatanodeDetails> excludedNodes = replicas.stream()
-        .map(ContainerReplica::getDatanodeDetails)
-        .collect(Collectors.toList());
-
-    ContainerHealthResult.UnderReplicatedHealthResult containerHealthResult =
-        ((ContainerHealthResult.UnderReplicatedHealthResult)
-            currentUnderRepRes);
-    if (containerHealthResult.isReplicatedOkAfterPending()) {
-      LOG.info("The container {} with replicas {} is sufficiently replicated",
+    if (replicaCount.isSufficientlyReplicated(true)) {
+      LOG.info("The container {} with replicas {} will be sufficiently " +
+          "replicated after pending replicas are created",
           container.getContainerID(), replicaCount.getReplicas());
       return emptyMap();
     }
@@ -166,6 +141,12 @@ public class ECUnderReplicationHandler implements UnhealthyReplicationHandler {
           " are: {}.", container.containerID(), replicaCount.getReplicas());
       return emptyMap();
     }
+
+    // don't place reconstructed replicas on exclude nodes, since they already
+    // have replicas
+    List<DatanodeDetails> excludedNodes = replicas.stream()
+        .map(ContainerReplica::getDatanodeDetails)
+        .collect(Collectors.toList());
     final ContainerID id = container.containerID();
     final Map<DatanodeDetails, SCMCommand<?>> commands = new HashMap<>();
     try {
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java
index c710144d8f..2c206224f1 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java
@@ -217,11 +217,9 @@ public class ReplicationManager implements SCMService {
     this.ratisMaintenanceMinReplicas = rmConf.getMaintenanceReplicaMinimum();
 
     ecUnderReplicationHandler = new ECUnderReplicationHandler(
-        ecReplicationCheckHandler, ecContainerPlacement, conf, nodeManager,
-        this);
+        ecContainerPlacement, conf, nodeManager, this);
     ecOverReplicationHandler =
-        new ECOverReplicationHandler(ecReplicationCheckHandler,
-            ecContainerPlacement, nodeManager);
+        new ECOverReplicationHandler(ecContainerPlacement, nodeManager);
     underReplicatedProcessor =
         new UnderReplicatedProcessor(this,
             rmConf.getUnderReplicatedInterval());
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECOverReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECOverReplicationHandler.java
index 498488697c..71eb164ec0 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECOverReplicationHandler.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECOverReplicationHandler.java
@@ -29,7 +29,6 @@ import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import org.apache.hadoop.hdds.scm.container.ContainerReplica;
 import org.apache.hadoop.hdds.scm.container.MockNodeManager;
 import org.apache.hadoop.hdds.scm.container.placement.algorithms.ContainerPlacementStatusDefault;
-import org.apache.hadoop.hdds.scm.container.replication.health.ECReplicationCheckHandler;
 import org.apache.hadoop.hdds.scm.net.NodeSchema;
 import org.apache.hadoop.hdds.scm.net.NodeSchemaManager;
 import org.apache.hadoop.hdds.scm.node.NodeManager;
@@ -65,7 +64,6 @@ public class TestECOverReplicationHandler {
   private NodeManager nodeManager;
   private OzoneConfiguration conf;
   private PlacementPolicy policy;
-  private ECReplicationCheckHandler replicationCheck;
   private PlacementPolicy placementPolicy;
 
   @BeforeEach
@@ -90,7 +88,6 @@ public class TestECOverReplicationHandler {
     Mockito.when(placementPolicy.validateContainerPlacement(
         anyList(), anyInt()))
         .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3));
-    replicationCheck = new ECReplicationCheckHandler(placementPolicy);
   }
 
   @Test
@@ -152,7 +149,7 @@ public class TestECOverReplicationHandler {
             container, 1, false, false, false);
 
     ECOverReplicationHandler ecORH =
-        new ECOverReplicationHandler(replicationCheck, policy, nodeManager);
+        new ECOverReplicationHandler(policy, nodeManager);
 
     Map<DatanodeDetails, SCMCommand<?>> commands = ecORH
         .processAndCreateCommands(availableReplicas, ImmutableList.of(),
@@ -168,7 +165,7 @@ public class TestECOverReplicationHandler {
       Set<ContainerReplica> availableReplicas,
       Map<Integer, Integer> index2excessNum) {
     ECOverReplicationHandler ecORH =
-        new ECOverReplicationHandler(replicationCheck, policy, nodeManager);
+        new ECOverReplicationHandler(policy, nodeManager);
     ContainerHealthResult.OverReplicatedHealthResult result =
         Mockito.mock(ContainerHealthResult.OverReplicatedHealthResult.class);
     Mockito.when(result.getContainerInfo()).thenReturn(container);
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 fa30206320..9aa5cec7e4 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
@@ -31,7 +31,6 @@ import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import org.apache.hadoop.hdds.scm.container.ContainerReplica;
 import org.apache.hadoop.hdds.scm.container.MockNodeManager;
 import org.apache.hadoop.hdds.scm.container.placement.algorithms.ContainerPlacementStatusDefault;
-import org.apache.hadoop.hdds.scm.container.replication.health.ECReplicationCheckHandler;
 import org.apache.hadoop.hdds.scm.exceptions.SCMException;
 import org.apache.hadoop.hdds.scm.net.NodeSchema;
 import org.apache.hadoop.hdds.scm.net.NodeSchemaManager;
@@ -83,7 +82,6 @@ public class TestECUnderReplicationHandler {
   private PlacementPolicy policy;
   private static final int DATA = 3;
   private static final int PARITY = 2;
-  private ECReplicationCheckHandler replicationCheck;
   private PlacementPolicy ecPlacementPolicy;
 
   @BeforeEach
@@ -109,7 +107,6 @@ public class TestECUnderReplicationHandler {
     Mockito.when(ecPlacementPolicy.validateContainerPlacement(
         anyList(), anyInt()))
         .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3));
-    replicationCheck = new ECReplicationCheckHandler(ecPlacementPolicy);
   }
 
   @Test
@@ -279,7 +276,7 @@ public class TestECUnderReplicationHandler {
     replicasToAdd.add(maintReplica);
 
     ECUnderReplicationHandler ecURH =
-        new ECUnderReplicationHandler(replicationCheck,
+        new ECUnderReplicationHandler(
             noNodesPolicy, conf, nodeManager, replicationManager);
     ContainerHealthResult.UnderReplicatedHealthResult underRep =
         new ContainerHealthResult.UnderReplicatedHealthResult(container,
@@ -347,7 +344,7 @@ public class TestECUnderReplicationHandler {
     replicasToAdd.add(maintReplica);
 
     ECUnderReplicationHandler ecURH =
-        new ECUnderReplicationHandler(replicationCheck,
+        new ECUnderReplicationHandler(
             sameNodePolicy, conf, nodeManager, replicationManager);
     ContainerHealthResult.UnderReplicatedHealthResult underRep =
         new ContainerHealthResult.UnderReplicatedHealthResult(container,
@@ -397,7 +394,7 @@ public class TestECUnderReplicationHandler {
             Pair.of(IN_SERVICE, 4), Pair.of(DECOMMISSIONING, 5));
 
     ECUnderReplicationHandler ecURH =
-        new ECUnderReplicationHandler(replicationCheck,
+        new ECUnderReplicationHandler(
             sameNodePolicy, conf, nodeManager, replicationManager);
 
     ContainerHealthResult.UnderReplicatedHealthResult underRep =
@@ -486,7 +483,7 @@ public class TestECUnderReplicationHandler {
       int decomIndexes, int maintenanceIndexes,
       PlacementPolicy placementPolicy) throws IOException {
     ECUnderReplicationHandler ecURH =
-        new ECUnderReplicationHandler(replicationCheck,
+        new ECUnderReplicationHandler(
             placementPolicy, conf, nodeManager, replicationManager);
     ContainerHealthResult.UnderReplicatedHealthResult result =
         Mockito.mock(ContainerHealthResult.UnderReplicatedHealthResult.class);


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