sodonnel commented on code in PR #4756:
URL: https://github.com/apache/ozone/pull/4756#discussion_r1202165391
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -541,4 +552,100 @@ private static byte[] int2byte(List<Integer> src) {
}
return dst;
}
+
+ /**
+ * Deletes one UNHEALTHY replica so that its host datanode becomes available
+ * to host a healthy replica. This can be helpful if reconstruction or
+ * replication is blocked because DNs that follow the placement policy are
+ * not available as targets.
+ * @param replicaCount ECContainerReplicaCount object of this container
+ * @param deletionInFlight pending deletes of this container's replicas
+ */
+ private void checkAndRemoveUnhealthyReplica(
+ ECContainerReplicaCount replicaCount,
+ List<DatanodeDetails> deletionInFlight) {
+ if (!deletionInFlight.isEmpty()) {
+ LOG.debug("There are {} pending deletes. Completing them could " +
+ "free up nodes to fix under replication. Not deleting UNHEALTHY" +
+ " replicas in this iteration.", deletionInFlight.size());
+ return;
+ }
+
+ ContainerInfo container = replicaCount.getContainer();
+ // ensure that the container is recoverable
+ if (replicaCount.isUnrecoverable()) {
+ LOG.warn("Cannot recover container {}.", container);
+ return;
+ }
+
+ Set<ContainerReplica> unhealthyReplicas =
+ replicaCount.getUnhealthyReplicas();
+ if (unhealthyReplicas.isEmpty()) {
+ LOG.debug("Container {} does not have any UNHEALTHY replicas.",
+ container.containerID());
+ return;
+ }
+
+ // remove replicas that aren't on IN_SERVICE and HEALTHY DNs
+ // the leftover replicas will be eligible for deletion
+ Iterator<ContainerReplica> iterator =
replicaCount.getReplicas().iterator();
Review Comment:
Its not clear to me why we are removing the replicas from the iterator -
this is the list which is internal to replicaCount, so we are modifying it, as
it does not return a copy or an unModifiable list.
We don't see to use the replicaCount.getReplicas() again in the rest of the
method - we just form the closed list and use it. Do we the `iterator.remove()`
calls?
--
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]
