sodonnel commented on code in PR #4061:
URL: https://github.com/apache/ozone/pull/4061#discussion_r1045990695


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -480,8 +483,21 @@ public Set<ContainerReplica> 
replicasToCopyToFixMisreplication(
       requiredNumberOfPlacementGroups -= 1;
       if (numberOfReplicasToBeCopied > 0) {
         List<ContainerReplica> replicasToBeCopied = replicaList.stream()
+                .filter(replicas::get)
                 .limit(numberOfReplicasToBeCopied)
                 .collect(Collectors.toList());
+        if (numberOfReplicasToBeCopied > replicasToBeCopied.size()) {
+          Node rack = this.getPlacementGroup(
+                  replicaList.stream().findAny()
+                  .map(ContainerReplica::getDatanodeDetails)
+                  .orElse(null));
+          throw new IOException(String.format(

Review Comment:
   I am not sure if we should totally fail here. We might be able to improve 
things somewhat, eg if we ideally had 3 replicas needing a copy, 2 on the first 
rack and 1 on the second.
   
   If, when we process the first rack, we can only find 1 or 0 to copy instead 
of 2, and then if we were to process the second rack we might have been able to 
find 1 to copy there. So we could have improved the situation by 1 or 2 out of 
3.
   
   It would probably be better to log a warning here and continue rather than 
throwing an exeception.



-- 
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: issues-unsubscr...@ozone.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org

Reply via email to