Copilot commented on code in PR #10122:
URL: https://github.com/apache/ozone/pull/10122#discussion_r3560730861
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -421,6 +434,57 @@ public long getInflightReplicationCount() {
.getPendingOpCount(ContainerReplicaOp.PendingOpType.ADD);
}
+ /**
+ * Returns the number of active EC reconstruction commands currently in
+ * progress across the cluster.
+ */
+ public int getInflightReconstructionCount() {
+ return inflightReconstructionCount.get();
+ }
+
+ /**
+ * Returns the maximum number of inflight reconstruction commands allowed
+ * across the cluster at any given time.
+ * @return the maximum number of inflight reconstruction commands allowed
+ */
+ public int getReconstructionInFlightLimit() {
+ return rmConf.getReconstructionGlobalLimit();
+ }
+
+ /**
+ * Returns true if the number of inflight reconstruction commands has reached
+ * the global limit.
+ * @return true if the limit is reached, false otherwise
+ */
+ public boolean isReconstructionLimitReached() {
+ int limit = getReconstructionInFlightLimit();
+ return limit > 0 && getInflightReconstructionCount() >= limit;
+ }
+
+ private void decrementInflightReconstructionCount() {
+ inflightReconstructionCount.updateAndGet(count -> Math.max(0, count - 1));
+ }
+
+ private boolean tryReserveReconstructionSlot() {
+ int limit = getReconstructionInFlightLimit();
+ if (limit <= 0) {
+ return true;
+ }
Review Comment:
tryReserveReconstructionSlot() does not increment
inflightReconstructionCount when the limit is disabled (<= 0). Because the
config is reconfigurable, enabling
hdds.scm.replication.reconstruction.global.limit at runtime would start
enforcing from an artificially low count (often 0), allowing more
reconstruction commands than intended until the counter catches up.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -421,6 +434,57 @@ public long getInflightReplicationCount() {
.getPendingOpCount(ContainerReplicaOp.PendingOpType.ADD);
}
+ /**
+ * Returns the number of active EC reconstruction commands currently in
+ * progress across the cluster.
+ */
+ public int getInflightReconstructionCount() {
+ return inflightReconstructionCount.get();
+ }
+
+ /**
+ * Returns the maximum number of inflight reconstruction commands allowed
+ * across the cluster at any given time.
+ * @return the maximum number of inflight reconstruction commands allowed
+ */
+ public int getReconstructionInFlightLimit() {
+ return rmConf.getReconstructionGlobalLimit();
+ }
+
+ /**
+ * Returns true if the number of inflight reconstruction commands has reached
+ * the global limit.
+ * @return true if the limit is reached, false otherwise
+ */
+ public boolean isReconstructionLimitReached() {
+ int limit = getReconstructionInFlightLimit();
+ return limit > 0 && getInflightReconstructionCount() >= limit;
+ }
Review Comment:
The PR description says UnhealthyReplicationProcessor checks the global
reconstruction limit before dequeuing work, but there are currently no
production call sites of
isReconstructionLimitReached()/getInflightReconstructionCount() (only tests and
sendThrottledReconstructionCommand). If queue-dequeue throttling is still
intended (to avoid repeatedly dequeuing and failing to schedule when the limit
is reached), it needs to be wired into the dequeue/processing path, or the PR
description should be updated.
--
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]
