Copilot commented on code in PR #11557:
URL: https://github.com/apache/ozone/pull/11557#discussion_r4288054877
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -519,12 +521,20 @@ private int processDecommissioningIndexes(
new ArrayList<>(decomIndexes), sources, availableSourceNodes,
excludedNodes, usedNodes);
if (commandsSent > 0) {
+ metrics.incrEcReconstructionDecommissionTriggeredTotal();
return commandsSent;
}
+ metrics.incrEcDecommissionReconstructionFallbackNoOpTotal();
} catch (CommandTargetOverloadedException e) {
LOG.debug("Reconstruction throttled for decommissioning container "
+ "{}, using 1-1 replication.",
container.containerID(), e);
+
metrics.incrEcDecommissionReconstructionFallbackTargetOverloadedTotal();
+ } catch (InsufficientDatanodesException e) {
+ LOG.debug("Reconstruction could not place targets for
decommissioning "
+ + "container {}, using 1-1 replication.",
+ container.containerID(), e);
+ metrics.incrEcDecommissionReconstructionFallbackNoOpTotal();
}
Review Comment:
This newly added fallback/metrics path (target placement failure via
`InsufficientDatanodesException`) is not exercised by the updated tests in this
PR diff. Add a unit test that forces
`processDecommissioningWithReconstructEC(...)` (or its placement call path) to
throw `InsufficientDatanodesException`, then assert the expected counter
increment to prevent regressions.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManagerMetrics.java:
##########
@@ -225,6 +234,26 @@ public final class ReplicationManagerMetrics implements
MetricsSource {
+ "to the pending commands on all source datanodes")
private MutableCounterLong replicateContainerCmdsDeferredTotal;
+ @Metric("Number of times EC reconstruction was triggered for decommission "
Review Comment:
The metric description says “reconstruction was triggered”, but the
increment occurs only when a reconstruction command is actually sent (ie,
`commandsSent > 0`). To avoid misleading interpretation in dashboards, either
(a) update the description to explicitly state it counts “reconstruction
commands sent for decommission due to source node load”, or (b) increment this
counter on “attempt/trigger” and add a separate counter for “commands sent”.
--
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]
