rich7420 commented on code in PR #11054:
URL: https://github.com/apache/ozone/pull/11054#discussion_r3886664350


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -536,17 +601,40 @@ public void sendThrottledReplicationCommand(ContainerInfo 
containerInfo,
   public void sendThrottledReconstructionCommand(ContainerInfo containerInfo,
       ReconstructECContainersCommand command)
       throws CommandTargetOverloadedException, NotLeaderException {
-    List<DatanodeDetails> targets = command.getTargetDatanodes();
-    List<Pair<Integer, DatanodeDetails>> targetWithCmds =
-        getAvailableDatanodesForReplication(targets);
-    if (targetWithCmds.isEmpty()) {
-      metrics.incrECReconstructionCmdsDeferredTotal();
-      throw new CommandTargetOverloadedException("No target with capacity " +
-          "available for reconstruction of " + containerInfo.getContainerID());
+    serviceLock.lock();

Review Comment:
   `serviceLock` previously guarded only `shouldRun`/`notifyStatusChanged`. 
Holding it across the whole send now makes a leader transition block behind an 
in-flight send. Since the reserve is already a CAS and the map is concurrent, 
could the lock cover just the reserve+put and let the send run outside it?



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -1266,6 +1370,39 @@ public static class ReplicationManagerConfiguration
     )
     private int containerSampleLimit = 100;
 
+    @Config(key = 
"hdds.scm.replication.decommission.ec.reconstruction.enabled",
+        type = ConfigType.BOOLEAN,
+        defaultValue = "false",
+        reconfigurable = true,
+        tags = { SCM },
+        description = "If true, SCM will switch from 1-1 replication to " +

Review Comment:
   This key and `load.factor` below have no production reader yet (the switch 
is HDDS-15072), but the descriptions say SCM already switches to reconstruction 
on load. Could we reword them as reserved until HDDS-15072, or defer them to 
that PR? Only `reconstruction.global.limit` is exercised here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to