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