smengcl commented on code in PR #10122:
URL: https://github.com/apache/ozone/pull/10122#discussion_r3534243773
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -159,6 +160,18 @@ public class ReplicationManager implements SCMService,
ContainerReplicaPendingOp
private final Map<DatanodeDetails, Integer> excludedNodes =
new ConcurrentHashMap<>();
+ /**
+ * Track the number of active EC reconstruction commands across the cluster.
+ */
+ private final AtomicInteger inflightReconstructionCount = new
AtomicInteger(0);
+
+ /**
+ * Mapping from reconstruction command ID to the number of pending fragments
+ * for that command. Used to know when the whole command is finished.
+ */
+ private final Map<Long, Integer>
reconstructionCommandIdToPendingFragmentCount =
+ new ConcurrentHashMap<>();
Review Comment:
I believe both new counters needs to be cleared under `notifyStatusChanged`
as well?:
```diff
// Therefore we should clear the table so RM starts from a clean
// state.
containerReplicaPendingOps.clear();
+ // clear() discards pending ops without firing opCompleted, so
also
+ // reset the reconstruction tracking that opCompleted maintains.
+ reconstructionCommandIdToPendingFragmentCount.clear();
+ inflightReconstructionCount.set(0);
serviceStatus = ServiceStatus.RUNNING;
}
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/UnhealthyReplicationProcessor.java:
##########
@@ -105,6 +112,12 @@ public void processAll(ReplicationQueue queue) {
.getMetrics().incrPendingReplicationLimitReachedTotal();
break;
}
+ if (reconstructionLimitReached(replicationManager)) {
+ LOG.info("The maximum number of pending reconstruction commands ({}) "
+
+ "are scheduled. Ending the iteration.",
+ replicationManager.getReconstructionInFlightLimit());
+ break;
+ }
Review Comment:
Should this skip/requeue only **reconstruction** work rather than halt all
under-replication recovery? `break` here ends the whole loop
So hitting the reconstruction limit will stop processing the entire
under-replicated queue, including containers that just need a normal replica
copied, which have nothing to do with reconstruction bandwidth.
--
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]
