smengcl commented on code in PR #11404:
URL: https://github.com/apache/ozone/pull/11404#discussion_r4341181937
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/MisReplicationHandler.java:
##########
@@ -158,6 +160,22 @@ public int processAndSendCommands(
excludedAndUsedNodes.getUsedNodes(),
excludedAndUsedNodes.getExcludedNodes(), currentContainerSize,
container);
+ if (!targetDatanodes.isEmpty()) {
+ // When no other rack has a usable node, placement can fall back to a
rack that already has a replica.
+ // Copying there doesn't help: the container is still mis-replicated,
the extra copy gets deleted, and the
+ // next run copies it again, over and over.
+ List<DatanodeDetails> dnsAfterCopy = new ArrayList<>(usedDns);
+ replicasToBeReplicated.forEach(r ->
dnsAfterCopy.remove(r.getDatanodeDetails()));
Review Comment:
nit: when placement returns fewer targets than `replicasToBeReplicated` (EC
partial case), this removes sources that won't be copied, so the after-copy
placement looks better than it will be and a target that doesn't help can still
pass. `ECMisReplicationHandler#sendReplicateCommands` pairs replicas with
targets in iteration order, so limiting to the number of targets matches what
is actually sent:
```suggestion
replicasToBeReplicated.stream().limit(targetDatanodes.size())
.forEach(r -> dnsAfterCopy.remove(r.getDatanodeDetails()));
```
--
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]
