You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "rich7420 (via GitHub)" <gi...@apache.org> on 2026/09/01 14:04:03 UTC
[PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
rich7420 opened a new pull request, #11182:
URL: https://github.com/apache/ozone/pull/11182
## What changes were proposed in this pull request?
SCM previously sent container reconciliation commands to every datanode with a replica and allowed every replica to be used as a peer, regardless of the datanode's health or operational state.
This change uses `NodeManager` status when selecting reconciliation participants:
- Healthy in-service datanodes are command targets and peers.
- Healthy datanodes that are decommissioning or entering maintenance remain peers, but do not receive commands.
- Stale, dead, unknown, decommissioned, and in-maintenance datanodes are excluded from both roles.
- An eligible target still receives a command when no eligible peers are available, so it can generate missing checksum data and scan its local container.
- Reconciliation is skipped when no eligible target is available.
The existing container and replica eligibility rules are unchanged. Changes to reconciliation status reporting are out of scope.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10714
## How was this patch tested?
- `mvn -pl :hdds-server-scm test -Dtest=TestReconcileContainerEventHandler -DskipShade -DskipRecon -DskipDocs` — 24 tests run, 0 failures, 0 errors, 0 skipped.
- `./hadoop-ozone/dev-support/checks/checkstyle.sh` — 58 modules checked, 0 violations.
- Fork CI: https://github.com/rich7420/ozone/actions/runs/33510354889
- 10 x 10 flaky-test-check for `TestReconcileContainerEventHandler` — 100/100 passed: https://github.com/rich7420/ozone/actions/runs/33515332863
Generated-by: Codex (GPT-5)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #11182:
URL: https://github.com/apache/ozone/pull/11182#discussion_r3939573765
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/ReconcileContainerEventHandler.java:
##########
@@ -67,21 +73,46 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
}
try {
- // TODO HDDS-10714 restriction peer and target nodes based on node status.
- Set<DatanodeDetails> allReplicaNodes = containerManager.getContainerReplicas(containerID)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toSet());
+ Set<DatanodeDetails> targets = new HashSet<>();
+ Set<DatanodeDetails> peers = new HashSet<>();
+ for (ContainerReplica replica : containerManager.getContainerReplicas(containerID)) {
+ DatanodeDetails datanode = replica.getDatanodeDetails();
+ final NodeStatus status;
+ try {
+ status = nodeManager.getNodeStatus(datanode);
+ } catch (NodeNotFoundException ex) {
+ LOG.warn("Skipping datanode {} for reconciliation of container {} since its status is unknown.",
+ datanode, containerID);
+ continue;
+ }
+ if (!status.isHealthy()) {
+ continue;
+ }
+ // Transitioning nodes remain peers so other replicas can recover any unique data before the node leaves.
+ if (!status.isDecommissioned() && !status.isInMaintenance()) {
Review Comment:
Updated the peer selection to explicitly allow only in-service, decommissioning, and entering-maintenance nodes.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
Posted by "chihsuan (via GitHub)" <gi...@apache.org>.
chihsuan commented on code in PR #11182:
URL: https://github.com/apache/ozone/pull/11182#discussion_r3939238184
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/ReconcileContainerEventHandler.java:
##########
@@ -67,21 +73,46 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
}
try {
- // TODO HDDS-10714 restriction peer and target nodes based on node status.
- Set<DatanodeDetails> allReplicaNodes = containerManager.getContainerReplicas(containerID)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toSet());
+ Set<DatanodeDetails> targets = new HashSet<>();
+ Set<DatanodeDetails> peers = new HashSet<>();
+ for (ContainerReplica replica : containerManager.getContainerReplicas(containerID)) {
+ DatanodeDetails datanode = replica.getDatanodeDetails();
+ final NodeStatus status;
+ try {
+ status = nodeManager.getNodeStatus(datanode);
+ } catch (NodeNotFoundException ex) {
+ LOG.warn("Skipping datanode {} for reconciliation of container {} since its status is unknown.",
+ datanode, containerID);
+ continue;
+ }
+ if (!status.isHealthy()) {
+ continue;
+ }
+ // Transitioning nodes remain peers so other replicas can recover any unique data before the node leaves.
+ if (!status.isDecommissioned() && !status.isInMaintenance()) {
Review Comment:
What do you think about writing this as an allowlist (`isInService() || isDecommissioning() || isEnteringMaintenance()`) instead? The current check is correct, but a future operational state would otherwise become eligible by default.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #11182:
URL: https://github.com/apache/ozone/pull/11182#discussion_r3905059222
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/ReconcileContainerEventHandler.java:
##########
@@ -67,21 +73,46 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
}
try {
- // TODO HDDS-10714 restriction peer and target nodes based on node status.
- Set<DatanodeDetails> allReplicaNodes = containerManager.getContainerReplicas(containerID)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toSet());
+ Set<DatanodeDetails> targets = new HashSet<>();
+ Set<DatanodeDetails> peers = new HashSet<>();
+ for (ContainerReplica replica : containerManager.getContainerReplicas(containerID)) {
+ DatanodeDetails datanode = replica.getDatanodeDetails();
+ final NodeStatus status;
+ try {
+ status = nodeManager.getNodeStatus(datanode);
+ } catch (NodeNotFoundException ex) {
+ LOG.warn("Skipping datanode {} for reconciliation of container {} since its status is unknown.",
+ datanode, containerID);
+ continue;
+ }
+ if (!status.isHealthy()) {
+ continue;
+ }
+ // Transitioning nodes remain peers so other replicas can recover any unique data before the node leaves.
+ if (!status.isDecommissioned() && !status.isInMaintenance()) {
+ peers.add(datanode);
+ }
+ if (status.isInService()) {
+ targets.add(datanode);
+ }
+ }
+
+ LOG.info("Reconcile container event triggered for container {} with targets {} and peers {}",
+ containerID, targets, peers);
- LOG.info("Reconcile container event triggered for container {} with peers {}", containerID, allReplicaNodes);
+ if (targets.isEmpty()) {
+ LOG.warn("Skipping reconciliation for container {} since no eligible target datanodes are available.",
+ containerID);
+ return;
+ }
Review Comment:
Logging `targets` and `peers` at INFO will print full `DatanodeDetails` sets, which can be very noisy (and potentially large) in busy clusters and makes logs harder to operate. Prefer logging counts at INFO (eg. target/peer sizes) and, if needed, log the full sets at DEBUG/TRACE. Also consider logging after the empty-target early return so skipped reconciliations don’t emit a large INFO line.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/ReconcileContainerEventHandler.java:
##########
@@ -67,21 +73,46 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
}
try {
- // TODO HDDS-10714 restriction peer and target nodes based on node status.
- Set<DatanodeDetails> allReplicaNodes = containerManager.getContainerReplicas(containerID)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toSet());
+ Set<DatanodeDetails> targets = new HashSet<>();
+ Set<DatanodeDetails> peers = new HashSet<>();
+ for (ContainerReplica replica : containerManager.getContainerReplicas(containerID)) {
+ DatanodeDetails datanode = replica.getDatanodeDetails();
+ final NodeStatus status;
+ try {
+ status = nodeManager.getNodeStatus(datanode);
+ } catch (NodeNotFoundException ex) {
+ LOG.warn("Skipping datanode {} for reconciliation of container {} since its status is unknown.",
+ datanode, containerID);
+ continue;
+ }
+ if (!status.isHealthy()) {
+ continue;
+ }
+ // Transitioning nodes remain peers so other replicas can recover any unique data before the node leaves.
+ if (!status.isDecommissioned() && !status.isInMaintenance()) {
+ peers.add(datanode);
+ }
+ if (status.isInService()) {
+ targets.add(datanode);
+ }
+ }
Review Comment:
This loops over replicas and calls `nodeManager.getNodeStatus(datanode)` per replica. If a container can have multiple replicas reported for the same datanode (or if `getContainerReplicas` is large), this causes redundant status lookups and repeated warn logs for the same node. Consider deduplicating `DatanodeDetails` first (or caching statuses in a `Map<DatanodeDetails, NodeStatus>` within the method) so each datanode’s status is fetched at most once per reconciliation event.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #11182:
URL: https://github.com/apache/ozone/pull/11182#discussion_r3939573765
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/ReconcileContainerEventHandler.java:
##########
@@ -67,21 +73,46 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
}
try {
- // TODO HDDS-10714 restriction peer and target nodes based on node status.
- Set<DatanodeDetails> allReplicaNodes = containerManager.getContainerReplicas(containerID)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toSet());
+ Set<DatanodeDetails> targets = new HashSet<>();
+ Set<DatanodeDetails> peers = new HashSet<>();
+ for (ContainerReplica replica : containerManager.getContainerReplicas(containerID)) {
+ DatanodeDetails datanode = replica.getDatanodeDetails();
+ final NodeStatus status;
+ try {
+ status = nodeManager.getNodeStatus(datanode);
+ } catch (NodeNotFoundException ex) {
+ LOG.warn("Skipping datanode {} for reconciliation of container {} since its status is unknown.",
+ datanode, containerID);
+ continue;
+ }
+ if (!status.isHealthy()) {
+ continue;
+ }
+ // Transitioning nodes remain peers so other replicas can recover any unique data before the node leaves.
+ if (!status.isDecommissioned() && !status.isInMaintenance()) {
Review Comment:
Updated the peer selection to explicitly allow only in-service, decommissioning, and entering-maintenance nodes.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #11182:
URL: https://github.com/apache/ozone/pull/11182#issuecomment-5523684505
cc @errose28
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10714. Restrict reconciliation requests by datanode status [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #11182:
URL: https://github.com/apache/ozone/pull/11182#discussion_r3939574992
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/ReconcileContainerEventHandler.java:
##########
@@ -67,21 +73,46 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
}
try {
- // TODO HDDS-10714 restriction peer and target nodes based on node status.
- Set<DatanodeDetails> allReplicaNodes = containerManager.getContainerReplicas(containerID)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toSet());
+ Set<DatanodeDetails> targets = new HashSet<>();
+ Set<DatanodeDetails> peers = new HashSet<>();
+ for (ContainerReplica replica : containerManager.getContainerReplicas(containerID)) {
+ DatanodeDetails datanode = replica.getDatanodeDetails();
+ final NodeStatus status;
+ try {
+ status = nodeManager.getNodeStatus(datanode);
+ } catch (NodeNotFoundException ex) {
+ LOG.warn("Skipping datanode {} for reconciliation of container {} since its status is unknown.",
+ datanode, containerID);
+ continue;
+ }
+ if (!status.isHealthy()) {
+ continue;
+ }
+ // Transitioning nodes remain peers so other replicas can recover any unique data before the node leaves.
+ if (!status.isDecommissioned() && !status.isInMaintenance()) {
Review Comment:
gooood catch! thanks for the suggestion
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org