Copilot commented on code in PR #11450:
URL: https://github.com/apache/ozone/pull/11450#discussion_r4224648224
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -427,71 +474,88 @@ private int processDecommissioningIndexes(
throws IOException {
ContainerInfo container = replicaCount.getContainer();
Set<Integer> decomIndexes = replicaCount.decommissioningOnlyIndexes(true);
- int commandsSent = 0;
- if (!decomIndexes.isEmpty()) {
- LOG.debug("Processing decommissioning indexes {} for container {}.",
- decomIndexes, container.containerID());
- final List<DatanodeDetails> selectedDatanodes = getTargetDatanodes(
- container, decomIndexes.size(), usedNodes, excludedNodes);
+ if (decomIndexes.isEmpty()) {
+ return 0;
+ }
- ContainerPlacementStatus placementStatusWithSelectedTargets =
- validatePlacement(container, availableSourceNodes,
selectedDatanodes);
- if (!placementStatusWithSelectedTargets.isPolicySatisfied()) {
- LOG.debug("Target nodes + existing nodes for EC container {}" +
- " will not satisfy placement policy {}. Reason: {}. Selected" +
- " nodes: {}. Available source nodes: {}. Resuming recovery " +
- "regardless.",
- container.containerID(), containerPlacement.getClass().getName(),
- placementStatusWithSelectedTargets.misReplicatedReason(),
- selectedDatanodes, availableSourceNodes);
+ if
(replicationManager.getConfig().isEcDecommissionReconstructionEnabled()) {
+ for (Integer index : decomIndexes) {
+ Pair<ContainerReplica, NodeStatus> source = sources.get(index);
+ if (source != null && replicationManager.isNodeHighlyLoaded(
+ source.getLeft().getDatanodeDetails())) {
+ LOG.info("Source node {} is highly loaded, switching to " +
+ "reconstruction for decommissioning container {}",
+ source.getLeft().getDatanodeDetails(), container.containerID());
+ return processReconstruction(replicaCount,
+ new ArrayList<>(decomIndexes), sources, availableSourceNodes,
+ excludedNodes, usedNodes);
Review Comment:
This switch treats every entry in `sources` as usable, including the
decommissioning indexes that are passed as `missingIndexes`. The reconstruction
reader explicitly removes recovery indexes from its available inputs, so with
(for example) indexes 1/decommissioning, 2/in-service, and 3/in-service for
RS-3-2, `sources.size()` passes the `data == 3` check but only two inputs
remain and the command cannot reconstruct index 1; with fewer than three total
sources this method simply returns 0. The existing 1-1 copy can handle both
cases, so only return through reconstruction when at least `data` source
indexes remain after excluding `decomIndexes`; otherwise fall through to the
copy path.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -427,71 +474,88 @@ private int processDecommissioningIndexes(
throws IOException {
ContainerInfo container = replicaCount.getContainer();
Set<Integer> decomIndexes = replicaCount.decommissioningOnlyIndexes(true);
- int commandsSent = 0;
- if (!decomIndexes.isEmpty()) {
- LOG.debug("Processing decommissioning indexes {} for container {}.",
- decomIndexes, container.containerID());
- final List<DatanodeDetails> selectedDatanodes = getTargetDatanodes(
- container, decomIndexes.size(), usedNodes, excludedNodes);
+ if (decomIndexes.isEmpty()) {
+ return 0;
+ }
- ContainerPlacementStatus placementStatusWithSelectedTargets =
- validatePlacement(container, availableSourceNodes,
selectedDatanodes);
- if (!placementStatusWithSelectedTargets.isPolicySatisfied()) {
- LOG.debug("Target nodes + existing nodes for EC container {}" +
- " will not satisfy placement policy {}. Reason: {}. Selected" +
- " nodes: {}. Available source nodes: {}. Resuming recovery " +
- "regardless.",
- container.containerID(), containerPlacement.getClass().getName(),
- placementStatusWithSelectedTargets.misReplicatedReason(),
- selectedDatanodes, availableSourceNodes);
+ if
(replicationManager.getConfig().isEcDecommissionReconstructionEnabled()) {
+ for (Integer index : decomIndexes) {
+ Pair<ContainerReplica, NodeStatus> source = sources.get(index);
+ if (source != null && replicationManager.isNodeHighlyLoaded(
+ source.getLeft().getDatanodeDetails())) {
Review Comment:
The enabled branch attempts reconstruction even when the global
reconstruction limit is already reached, and a concurrent limit rejection from
`sendThrottledReconstructionCommand` propagates out instead of falling back to
the existing 1-1 copy. This defeats the throttling contract that 1-1
replication should continue when reconstruction is capped and can delay
decommission despite replication capacity being available. Skip the switch when
the cap is reached and handle a reservation rejection by continuing through the
copy path.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -455,6 +455,27 @@ public boolean isReconstructionLimitReached() {
return globalReconstructionTracker.isLimitReached();
}
+ /**
+ * Returns true if the given datanode's replication load (queued replication
+ * and reconstruction commands) exceeds the configured load factor threshold.
+ *
+ * @param datanode the datanode to check
+ * @return true if the node is highly loaded, false otherwise
+ */
+ public boolean isNodeHighlyLoaded(DatanodeDetails datanode) {
+ try {
+ int limit = getReplicationLimit(datanode);
+ if (limit <= 0) {
+ return true;
+ }
+ double loadFactor = (double) getQueuedReplicationCount(datanode) / limit;
+ return loadFactor >= rmConf.getEcDecommissionReconstructionLoadFactor();
Review Comment:
The added handler test stubs `isNodeHighlyLoaded`, so the new load
calculation itself has no coverage. Please add focused `TestReplicationManager`
cases for the threshold boundary, weighted reconstruction commands, the scaled
out-of-service limit, and the `NodeNotFoundException` path; these determine
whether the new behavior switches at all.
--
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]
