HDDS-13535. Show under/over-replication in `replicas verify --container-state` results by 0lai0 · Pull Request #9135 · apache/ozone

0lai0 · 2025-10-10T17:25:24Z

What changes were proposed in this pull request?

Enhanced container state representation to include replication health indicators. The toString() methods of ECContainerReplicaCount and RatisContainerReplicaCount now display under/over replication status with specific details without affecting the underlying container health assessment logic.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13535

How was this patch tested?

unit test

…state check]

Gargi-jais11

Thanks @0lai0 for working on this.
Your toString() changes are good additions that improve debugging, but they won't automatically appear in the CLI output without connecting them to the container-state verification flow.
The main need of this issue is that the replication related health can also be indicated in the --container-state check.
Such that in case of under/over replication, the output on running ozone debug replicas verify --container-state /volume/bucket/key can specify the case.

Gargi-jais11

You need to add logic to ContainerStateVerifier.verifyBlock().
@Tejaskriya could you also please review this?

.../src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java

sarvekshayr

Thanks for picking this up, @0lai0.

The goal of this JIRA is to introduce a flag to the ozone debug replicas verify --container-state command to indicate containers that are under or over replicated.

We can also consider including under or over replicated containers in the existing failure checks if that approach is more feasible as it avoids adding a new flag.

Tejaskriya · 2025-10-14T04:09:06Z

+1 to @sarvekshayr 's comment. If you check the other sub tasks under the main jira, there is HDDS-12595: which added a container state check. Here, as of the current implementation, only the health states of the replicas are printed.
The idea of the jira you are working on is to check for under replicated and over replicated scenarios as a part of this check. In case that proves to be difficult, we can add a different flag+check.

0lai0 · 2025-10-20T14:54:07Z

Thank you all for the detailed feedback and valuable suggestions.

I understand now that the main goal of this JIRA is to integrate the under/over-replication state check directly into the ContainerStateVerifier.verifyBlock() method, and have the result reflected in the ozone debug replicas verify --container-state command's output. My initial toString() changes were meant to aid in debugging, and I appreciate the clear direction on how to connect this to the core verification logic. Let me make that adjustment.

adoroszlai · 2025-10-21T19:00:47Z

Thanks @0lai0 for updating the patch. Please take a look at checkstyle and pmd failures:

https://github.com/0lai0/ozone/actions/runs/18685558075/job/53277131284#step:16:17

https://github.com/0lai0/ozone/actions/runs/18685558075/job/53277131255#step:16:17

0lai0 · 2025-10-22T04:57:59Z

@adoroszlai Thank you for the review. Let me fix it.

sarvekshayr

Please remove all square brackets from the output strings.

container-state-verifier.robot test is expected to fail due of this change. Please review the git diff file and apply the fix to the robot test.
container-state-verifier-test-fix.txt

0lai0 · 2025-10-22T13:41:04Z

@sarvekshayr Thank you for your suggestion!

hadoop-ozone/dist/src/main/smoketest/debug/container-state-verifier.robot

sarvekshayr

LGTM. Thank you @0lai0 for iterating on the feedback and patiently incorporating all the changes.

0lai0 · 2025-11-11T08:32:05Z

Thank you @sarvekshayr for your detailed guidance! Your detailed feedback was incredibly helpful. I really appreciate it.

sarvekshayr · 2025-11-11T08:35:39Z

@dombizita requesting your review on this.

adoroszlai

Thanks @0lai0 for working on this.

adoroszlai · 2025-11-30T08:44:45Z

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java

+
+    @Override
+    public String toString() {
+      return name();
+    }


nit: not needed, since enums provide this by default.

Suggested change

@Override

public String toString() {

return name();

}

Sure, I will remove this. Thanks

adoroszlai · 2025-11-30T09:35:10Z

...ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java

  private static class ContainerInfoToken {
    private HddsProtos.LifeCycleState state;
    private final String encodedToken;
+    private final ContainerInfo containerInfo;


ContainerInfoToken is stored in encodedTokenCache, with default cache size of 1 million. ContainerInfo is a much larger object, so the process may run out of memory. The new logic only requires the number of nodes in addition to existing items, so storing the complete ContainerInfo object is unnecessary. But I don't think even "number of required nodes" is the right information to cache (see my comment on getContainerReplicas).

Also, expected memory requirement of the cache is mentioned in CLI help. It should be updated to reflect the increased size:

ozone/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java

Line 86 in 8737837

"'--container-state'. Default is 1 million containers (which takes around 43MB). " +

adoroszlai · 2025-11-30T09:36:49Z

...ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java

+        return ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED + ": no replicas found";
+      }
+
+      int replicationFactor = containerInfo.getReplicationFactor().getNumber();


ReplicationFactor does not support EC. Please use getReplicationConfig().getRequiredNodes().

REPLICATION_CHECK_FAILED: Replication configuration of type EC does not have a replication factor property.

Thanks for the pointer. I'll change to use ReplicationConfig.

adoroszlai · 2025-11-30T09:44:13Z

...ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java

+      if (healthyReplicas < replicationFactor) {
+        return String.format("%s: %d/%d healthy replicas", 
+            ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED, healthyReplicas, replicationFactor);
+      } else if (healthyReplicas > replicationFactor) {
+        return String.format("%s: %d/%d healthy replicas", 
+            ContainerHealthResult.ReplicationStatus.OVER_REPLICATED, healthyReplicas, replicationFactor);
+      } else {
+        return ContainerHealthResult.ReplicationStatus.HEALTHY_REPLICATION.toString();
+      }


nit: avoid duplication.

Suggested change

if (healthyReplicas < replicationFactor) {

return String.format("%s: %d/%d healthy replicas",

ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED, healthyReplicas, replicationFactor);

} else if (healthyReplicas > replicationFactor) {

return String.format("%s: %d/%d healthy replicas",

ContainerHealthResult.ReplicationStatus.OVER_REPLICATED, healthyReplicas, replicationFactor);

} else {

return ContainerHealthResult.ReplicationStatus.HEALTHY_REPLICATION.toString();

}

if (healthyReplicas == replicationFactor) {

return ContainerHealthResult.ReplicationStatus.HEALTHY_REPLICATION.toString();

}

ContainerHealthResult.ReplicationStatus status = healthyReplicas < replicationFactor

? ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED

: ContainerHealthResult.ReplicationStatus.OVER_REPLICATED;

return String.format("%s: %d/%d healthy replicas", status, healthyReplicas, replicationFactor);

adoroszlai · 2025-11-30T10:33:31Z

...ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java

+      List<ContainerReplicaInfo> replicaInfos = 
+          containerOperationClient.getContainerReplicas(containerId);


verifyBlock is called for a specific datanode, but checkReplicationStatus fetches info for all replicas. So we are checking all replicas for each replica. This should be done once and stored in the cache, instead of the ContainerInfo or "number of required nodes".

adoroszlai · 2025-11-30T10:37:40Z

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java

+  public enum ReplicationStatus {
+    UNDER_REPLICATED,
+    OVER_REPLICATED,
+    HEALTHY_REPLICATION;


I'm not sure we need this new enum, it seems to be a simplified version of HealthState. Since this is just for output, I think using a subset of those states is fine.

Thanks for the pointer. I will remove ReplicationStatus.

adoroszlai · 2025-11-30T10:50:24Z

.../src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java

+
+    if (!isSufficientlyReplicated()) {
+      List<Integer> missingIndexes = unavailableIndexes(true);
+      sb.append(' ').append(ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED)
+          .append(": missing indexes ").append(missingIndexes);
+    } else if (isOverReplicated()) {
+      List<Integer> excessIndexes = overReplicatedIndexes(false);
+      sb.append(' ').append(ContainerHealthResult.ReplicationStatus.OVER_REPLICATED)
+          .append(": excess indexes ").append(excessIndexes);
+    } else {
+      sb.append(' ').append(ContainerHealthResult.ReplicationStatus.HEALTHY_REPLICATION);
+    }
+


I don't see where this output is used for the new functionality. Also, I don't think toString() should be performing such logic. So if I'm missing something and it is indeed required, please move it to a separate function, otherwise please remove it.

Thanks for the clarification. It seems I went in the wrong function. I will remove under/over/healthy logic, and also remove RatisContainerReplicaCount.toString() relpication logic.

adoroszlai · 2025-11-30T10:51:08Z

...c/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java

+
+    if (!isSufficientlyReplicated()) {
+      result += " " + ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED + ": need "
+          + additionalReplicaNeeded() + " more";
+    } else if (isOverReplicated()) {
+      result += " " + ContainerHealthResult.ReplicationStatus.OVER_REPLICATED + ": excess "
+          + getExcessRedundancy(true) + " replica(s)";
+    } else {
+      result += " " + ContainerHealthResult.ReplicationStatus.HEALTHY_REPLICATION;
+    }
+


Similarly, please delete or move.

Sure, thanks @adoroszlai for review.

0lai0 · 2025-11-30T13:56:03Z

Thank you @adoroszlai for review. I'll modify the code according to the comments above.

0lai0 · 2025-12-04T09:10:51Z

@adoroszlai , PTAL when you have a moment. Thanks.

adoroszlai

Thanks @0lai0 for updating the patch.

I would like to suggest some changes, e.g. instead of introducing a new cache, add replicationStatus to ContainerInfoToken. Please see adoroszlai@cd6f9f6.

.../src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java

…m/container/replication/ECContainerReplicaCount.java Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>

updated

...c/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java

0lai0 · 2025-12-08T15:09:07Z

Thanks @sarvekshayr for review. PTAL

...c/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java

adoroszlai · 2025-12-09T10:43:53Z

Thanks @0lai0 for the patch, @chungen0126, @Gargi-jais11, @sarvekshayr, @sodonnel, @Tejaskriya for the review.

HDDS-13535[Flag to indicate under and over replications in container-…

804a249

…state check]

Gargi-jais11 reviewed Oct 13, 2025

View reviewed changes

adoroszlai requested review from siddhantsangwan and sodonnel October 13, 2025 05:47

Merge branch 'apache:master' into HDDS-13535

2a05710

sodonnel reviewed Oct 13, 2025

View reviewed changes

.../src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java Outdated Show resolved Hide resolved

sarvekshayr reviewed Oct 13, 2025

View reviewed changes

errose28 added the tools Tools that helps with debugging label Oct 13, 2025

Merge branch 'apache:master' into HDDS-13535

a1837b2

0lai0 and others added 2 commits October 21, 2025 16:26

Merge branch 'apache:master' into HDDS-13535

b6b1055

add info to CLI

fc163c0

Merge branch 'apache:master' into HDDS-13535

7c713a5

sarvekshayr reviewed Oct 22, 2025

View reviewed changes

0lai0 and others added 2 commits October 22, 2025 21:10

Merge branch 'apache:master' into HDDS-13535

9c34e19

modify checkstyle and PMD

969f45c

0lai0 added 2 commits November 1, 2025 14:54

Merge branch 'apache:master' into HDDS-13535

bac7830

Merge branch 'apache:master' into HDDS-13535

2755662

chungen0126 requested review from Gargi-jais11, adoroszlai and sarvekshayr and removed request for Gargi-jais11 and sarvekshayr November 3, 2025 14:20

peterxcli requested review from chungen0126 and peterxcli November 3, 2025 14:23

sarvekshayr reviewed Nov 11, 2025

View reviewed changes

hadoop-ozone/dist/src/main/smoketest/debug/container-state-verifier.robot Outdated Show resolved Hide resolved

revert container-state-verifier

fe25936

sarvekshayr approved these changes Nov 11, 2025

View reviewed changes

Merge branch 'apache:master' into HDDS-13535

4376a81

adoroszlai requested a review from dombizita November 30, 2025 08:33

adoroszlai previously requested changes Nov 30, 2025

View reviewed changes

adoroszlai changed the title ~~HDDS-13535. Flag to indicate under and over replications in container-state check~~ HDDS-13535. Show under/over-replication in replicas verify --container-state results Nov 30, 2025

0lai0 and others added 2 commits December 4, 2025 15:11

Merge branch 'apache:master' into HDDS-13535

d08cd9c

modify under/over-replication verify --container-state

ce2b7ee

0lai0 requested a review from adoroszlai December 4, 2025 09:09

adoroszlai reviewed Dec 4, 2025

View reviewed changes

.../src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java Outdated Show resolved Hide resolved

0lai0 and others added 3 commits December 6, 2025 09:38

Merge branch 'apache:master' into HDDS-13535

7f662f8

Update hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/sc…

e170fb6

…m/container/replication/ECContainerReplicaCount.java Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>

modify add replicationStatus to ContainerInfoToken

125d805

adoroszlai requested review from sarvekshayr and sodonnel December 6, 2025 07:30

sarvekshayr reviewed Dec 8, 2025

View reviewed changes

...c/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java Show resolved Hide resolved

0lai0 and others added 2 commits December 8, 2025 22:31

Merge branch 'apache:master' into HDDS-13535

91d852a

revert isUnderReplicated()

60d3462

adoroszlai reviewed Dec 8, 2025

View reviewed changes

...c/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java Outdated Show resolved Hide resolved

Revert whitespace-only change

7d4a6af

sarvekshayr approved these changes Dec 9, 2025

View reviewed changes

adoroszlai merged commit 9036e1a into apache:master Dec 9, 2025
43 checks passed

		List<ContainerReplicaInfo> replicaInfos =
		containerOperationClient.getContainerReplicas(containerId);

Conversation

0lai0 commented Oct 10, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Gargi-jais11 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gargi-jais11 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sarvekshayr left a comment

Choose a reason for hiding this comment

Uh oh!

Tejaskriya commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0lai0 commented Oct 20, 2025

Uh oh!

adoroszlai commented Oct 21, 2025

Uh oh!

0lai0 commented Oct 22, 2025

Uh oh!

sarvekshayr left a comment

Choose a reason for hiding this comment

Uh oh!

0lai0 commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sarvekshayr left a comment

Choose a reason for hiding this comment

Uh oh!

0lai0 commented Nov 11, 2025

Uh oh!

sarvekshayr commented Nov 11, 2025

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0lai0 commented Nov 30, 2025

Uh oh!

0lai0 commented Dec 4, 2025

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0lai0 commented Dec 8, 2025

Uh oh!

Gargi-jais11 left a comment •

edited

Loading

Tejaskriya commented Oct 14, 2025 •

edited

Loading

0lai0 commented Oct 22, 2025 •

edited

Loading