You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "0lai0 (via GitHub)" <gi...@apache.org> on 2025/10/10 17:25:27 UTC
[PR] HDDS-13535. Flag to indicate under and over replications in container-state check [ozone]
0lai0 opened a new pull request, #9135:
URL: https://github.com/apache/ozone/pull/9135
## 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](https://issues.apache.org/jira/browse/HDDS-13535)
## How was this patch tested?
unit test
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2588208543
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -573,6 +573,7 @@ public String toString() {
.append(", ReplicationConfig: ").append(repConfig)
.append(", RemainingMaintenanceRedundancy: ")
.append(remainingMaintenanceRedundancy);
+
Review Comment:
nit: please avoid whitespace-only change in this file
```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
Re: [PR] HDDS-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3510311643
@0lai0 fix the pmd failures - https://github.com/0lai0/ozone/actions/runs/19225116858/job/54950524462
Also, please update the robot test.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3510076480
Thanks @chungen0126 suggestion . I define an enum ReplicationStatus in ContainerHealthResult.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2598858930
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java:
##########
@@ -539,12 +539,6 @@ public boolean isSufficientlyReplicated(boolean includePendingAdd) {
return delta <= 0;
}
- /**
- * @return true if the container is under replicated, false otherwise
- */
- public boolean isUnderReplicated() {
Review Comment:
Sure, thanks for point this out.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3400035019
+ 1 to @sarvekshayr 's comment. If you check the other sub tasks under the main jira, there is [HDDS-12595](https://issues.apache.org/jira/browse/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.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2510765266
##########
hadoop-ozone/dist/src/main/smoketest/debug/container-state-verifier.robot:
##########
@@ -14,7 +14,10 @@
# limitations under the License.
*** Settings ***
-Documentation Test container state on a UNHEALTHY, DELETED and INVALID container
+Documentation Test container state on a UNHEALTHY, DELETED and INVALID container.
+ ... The output message format includes replication status information
+ ... (UNDER_REPLICATED, OVER_REPLICATED, or HEALTHY_REPLICATION) in addition
+ ... to the container state information.
Review Comment:
This is not needed.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3611013008
@adoroszlai , PTAL when you have a moment. Thanks.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3515234154
Thank you @sarvekshayr for your feedback and guidance on this.
Based on your comments, I've updated the logic to correctly handle the container-level replication status. The key change is that replication status is now evaluated at the container level, so when a container is UNDER_REPLICATED or OVER_REPLICATED, this information is reflected in all replica outputs. Is this the right way to fix it? Thank you so much.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2601629681
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java:
##########
@@ -546,6 +546,7 @@ public boolean isUnderReplicated() {
return !isSufficientlyReplicated();
}
+
Review Comment:
Thanks @adoroszlai for fix.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3483733123
> > 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](https://github.com/user-attachments/files/23048677/container-state-verifier-test-fix.txt)
>
> Failed acceptance (tools) test CI: https://github.com/0lai0/ozone/actions/runs/18718092924/job/53384096124
>
> @0lai0 Please fix the `container-state-verifier.robot` test. I've already shared the fix [here](https://github.com/user-attachments/files/23048677/container-state-verifier-test-fix.txt).
+1 to @sarvekshayr comment.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2588115724
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java:
##########
@@ -402,4 +402,21 @@ public enum HealthState {
OVER_REPLICATED,
MIS_REPLICATED
}
+
+ /**
+ * Enum representing replication status for containers.
+ * Used to indicate whether a container is under-replicated, over-replicated,
+ * or has healthy replication. This is specifically used for toString() output
+ * and display purposes.
+ */
+ public enum ReplicationStatus {
+ UNDER_REPLICATED,
+ OVER_REPLICATED,
+ HEALTHY_REPLICATION;
Review Comment:
Thanks for the pointer. I will remove ReplicationStatus.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #9135:
URL: https://github.com/apache/ozone/pull/9135
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3432426159
@sarvekshayr Thank you for you 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
Re: [PR] HDDS-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3511182809
Sure, I'll push a fix right away. Thanks.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2512933706
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -101,11 +104,15 @@ public BlockVerificationResult verifyBlock(DatanodeDetails datanode, OmKeyLocati
}
replicaCheckMsg.append(", Container state in SCM is ").append(containerInfoToken.getContainerState());
- if (pass) {
- return BlockVerificationResult.pass();
- } else {
- return BlockVerificationResult.failCheck(replicaCheckMsg.toString());
+ String replicationStatus = checkReplicationStatus(keyLocation.getContainerID());
+ replicaCheckMsg.append(", ").append(replicationStatus);
+ if (replicationStatus.contains("REPLICATION_CHECK_FAILED")) {
+ pass = false;
}
+
+ // Always return failCheck to include replication status information in the output
+ // This allows users to see replication health status even when container state is good
+ return BlockVerificationResult.failCheck(replicaCheckMsg.toString());
Review Comment:
Expected output -
```
"keys" : [ {
"volumeName" : "cli-debug-volume27858",
"bucketName" : "cli-debug-bucket",
"name" : "testfile",
"blocks" : [ {
"containerID" : 1,
"blockID" : 115816896921600101,
"replicas" : [ {
"datanode" : {
"uuid" : "2f283b6f-b5e6-4af1-b4c3-348f157db1ae",
"hostname" : "ozonesecure-ha-datanode1-1.ozonesecure-ha_ozone_net"
},
"checks" : [ {
"type" : "containerState",
"completed" : true,
"pass" : false,
"failures" : [ {
"message" : "Replica state is UNHEALTHY, Container state in SCM is CLOSED, UNDER_REPLICATED: 2/3 healthy replicas"
} ]
} ],
"replicaIndex" : 0
}, {
"datanode" : {
"uuid" : "a05d5ec6-901f-4e68-8dc4-eb087b3b7310",
"hostname" : "ozonesecure-ha-datanode3-1.ozonesecure-ha_ozone_net"
},
"checks" : [ {
"type" : "containerState",
"completed" : true,
"pass" : false,
"failures" : [ {
"message" : "Replica state is CLOSED, Container state in SCM is CLOSED, UNDER_REPLICATED: 2/3 healthy replicas"
} ]
} ],
"replicaIndex" : 0
}, {
"datanode" : {
"uuid" : "d3b0810d-7630-4fe5-adea-047b815e3f24",
"hostname" : "ozonesecure-ha-datanode5-1.ozonesecure-ha_ozone_net"
},
"checks" : [ {
"type" : "containerState",
"completed" : true,
"pass" : false,
"failures" : [ {
"message" : "Replica state is CLOSED, Container state in SCM is CLOSED, UNDER_REPLICATED: 2/3 healthy replicas"
} ]
} ],
"replicaIndex" : 0
} ]
} ],
"pass" : false
} ],
"pass" : false
}
```
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2513118182
##########
hadoop-ozone/dist/src/main/smoketest/debug/container-state-verifier.robot:
##########
@@ -43,7 +43,7 @@ Verify Container State with Rule
${output} = Execute replicas verify container state debug tool
${json} = Parse replicas verify JSON output ${output}
- Check to Verify Replicas ${json} ${CHECK_TYPE} ${DATANODE} Replica state is ${expected_state}
+ Check to Verify Replicas ${json} ${CHECK_TYPE} ${FAULT_INJ_DATANODE} Replica state is ${expected_state}
Review Comment:
Sure.I'll push a fix right away.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3481574796
> 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](https://github.com/user-attachments/files/23048677/container-state-verifier-test-fix.txt)
Failed accpetance (tools) test CI: https://github.com/0lai0/ozone/actions/runs/18718092924/job/53384096124
@0lai0 Please fix the `container-state-verifier.robot` test. I've already shared the fix [here](https://github.com/user-attachments/files/23048677/container-state-verifier-test-fix.txt).
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3484389519
@sarvekshayr Apologies, it looks like my previous commit didn't include the intended fix for container-state-verifier.robot. I'm correcting this now and will push an update shortly.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2599506419
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java:
##########
@@ -546,6 +546,7 @@ public boolean isUnderReplicated() {
return !isSufficientlyReplicated();
}
+
Review Comment:
```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
Re: [PR] HDDS-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3512498544
Thank you @sarvekshayr . I really appreciate the thorough response. I will review it and figure out how to implement these modifications.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2426051019
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -546,6 +546,10 @@ public boolean isSufficientlyReplicated() {
return isSufficientlyReplicated(false);
}
+ public boolean isUnderReplicated() {
Review Comment:
Do we really need this extra method that is just the inverse of `isSufficientlyReplicated()`? I guess it doesn't do any harm and the Ratis Count object seems to already have it.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2426051019
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -546,6 +546,10 @@ public boolean isSufficientlyReplicated() {
return isSufficientlyReplicated(false);
}
+ public boolean isUnderReplicated() {
Review Comment:
Do we really need this extra method that is just the inverse of `isSufficientlyReplicated()`?
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2512924895
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -101,11 +104,15 @@ public BlockVerificationResult verifyBlock(DatanodeDetails datanode, OmKeyLocati
}
replicaCheckMsg.append(", Container state in SCM is ").append(containerInfoToken.getContainerState());
- if (pass) {
- return BlockVerificationResult.pass();
- } else {
- return BlockVerificationResult.failCheck(replicaCheckMsg.toString());
+ String replicationStatus = checkReplicationStatus(keyLocation.getContainerID());
+ replicaCheckMsg.append(", ").append(replicationStatus);
+ if (replicationStatus.contains("REPLICATION_CHECK_FAILED")) {
+ pass = false;
}
+
+ // Always return failCheck to include replication status information in the output
+ // This allows users to see replication health status even when container state is good
+ return BlockVerificationResult.failCheck(replicaCheckMsg.toString());
Review Comment:
The replication status check evaluates container-level health by counting healthy replicas across all datanodes. Therefore, when a container is `UNDER_REPLICATED` or `OVER_REPLICATED`, this information should be reflected in all replica outputs, not just the unhealthy ones.
Revert to the previous implementation which correctly checked:
```
if (replicationStatus.contains("UNDER_REPLICATED") ||
replicationStatus.contains("OVER_REPLICATED") ||
replicationStatus.contains("REPLICATION_CHECK_FAILED")) {
pass = false;
}
```
or
```
if (replicationStatus.contains("HEALTHY_REPLICATION")) {
pass = true;
}
```
With this change, the robot test fix shared will work as expected.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3627383098
Thanks @sarvekshayr for review. PTAL
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3428914603
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
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3515260542
> The key change is that replication status is now evaluated at the container level, so when a container is UNDER_REPLICATED or OVER_REPLICATED, this information is reflected in all replica outputs. Is this the right way to fix it?
Yes, similar to how a bad container state is reflected across all replica outputs, we should also ensure that a bad replication status is displayed consistently in all replica outputs.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3515541348
Thank you @sarvekshayr for your detailed guidance! Your detailed feedback was incredibly helpful. I really appreciate it.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3515553130
@dombizita requesting your review on this.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3422434579
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.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3592570218
Thank you @adoroszlai for review. I'll modify the code according to the comments above.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2588085379
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java:
##########
@@ -402,4 +402,21 @@ public enum HealthState {
OVER_REPLICATED,
MIS_REPLICATED
}
+
+ /**
+ * Enum representing replication status for containers.
+ * Used to indicate whether a container is under-replicated, over-replicated,
+ * or has healthy replication. This is specifically used for toString() output
+ * and display purposes.
+ */
+ public enum ReplicationStatus {
+ UNDER_REPLICATED,
+ OVER_REPLICATED,
+ HEALTHY_REPLICATION;
+
+ @Override
+ public String toString() {
+ return name();
+ }
Review Comment:
Sure, I will remove this. Thanks
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2588116537
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -573,6 +573,19 @@ public String toString() {
.append(", ReplicationConfig: ").append(repConfig)
.append(", RemainingMaintenanceRedundancy: ")
.append(remainingMaintenanceRedundancy);
+
+ 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);
+ }
+
Review Comment:
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.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java:
##########
@@ -257,6 +257,17 @@ public String toString() {
if (considerUnhealthy) {
result += " +considerUnhealthy";
}
+
+ 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;
+ }
+
Review Comment:
Sure, thanks @adoroszlai for review.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2597089389
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java:
##########
@@ -539,12 +539,6 @@ public boolean isSufficientlyReplicated(boolean includePendingAdd) {
return delta <= 0;
}
- /**
- * @return true if the container is under replicated, false otherwise
- */
- public boolean isUnderReplicated() {
Review Comment:
This method removal doesn’t seem related to the changes in this PR.
Could we revert this?
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2445250676
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -546,6 +546,10 @@ public boolean isSufficientlyReplicated() {
return isSufficientlyReplicated(false);
}
+ public boolean isUnderReplicated() {
Review Comment:
I agree with this point. Using the inverse of isSufficientlyReplicated() is cleaner. I will remove this redundant method in my next commit. Thank you.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3430502840
@adoroszlai Thank you for the review. Let me fix it.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3500620187
Thanks @0lai0 for the patch. To improve readability and type-safety, could we define an enum for these replication states (e.g., ReplicationStatus)? This would be much cleaner than using separate constants.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9135:
URL: https://github.com/apache/ozone/pull/9135#issuecomment-3631545687
Thanks @0lai0 for the patch, @chungen0126, @Gargi-jais11, @sarvekshayr, @sodonnel, @Tejaskriya for the review.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2513110245
##########
hadoop-ozone/dist/src/main/smoketest/debug/container-state-verifier.robot:
##########
@@ -43,7 +43,7 @@ Verify Container State with Rule
${output} = Execute replicas verify container state debug tool
${json} = Parse replicas verify JSON output ${output}
- Check to Verify Replicas ${json} ${CHECK_TYPE} ${DATANODE} Replica state is ${expected_state}
+ Check to Verify Replicas ${json} ${CHECK_TYPE} ${FAULT_INJ_DATANODE} Replica state is ${expected_state}
Review Comment:
Revert this change.
--
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-13535. Show under/over-replication in `replicas verify --container-state` results [ozone]
Posted by "0lai0 (via GitHub)" <gi...@apache.org>.
0lai0 commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2588114786
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -155,18 +166,57 @@ private ContainerInfoToken getContainerInfoToken(long containerId)
// Cache miss - fetch and store
ContainerInfo info = containerOperationClient.getContainer(containerId);
String encodeToken = containerOperationClient.getEncodedContainerToken(containerId);
- cachedData = new ContainerInfoToken(info.getState(), encodeToken);
+ cachedData = new ContainerInfoToken(info.getState(), encodeToken, info);
encodedTokenCache.put(containerId, cachedData);
return cachedData;
}
+ private String checkReplicationStatus(long containerId) {
+ try {
+ ContainerInfoToken token = getContainerInfoToken(containerId);
+ ContainerInfo containerInfo = token.getContainerInfo();
+
+ // Get container replicas from SCM
+ List<ContainerReplicaInfo> replicaInfos =
+ containerOperationClient.getContainerReplicas(containerId);
+
+ if (replicaInfos.isEmpty()) {
+ return ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED + ": no replicas found";
+ }
+
+ int replicationFactor = containerInfo.getReplicationFactor().getNumber();
Review Comment:
Thanks for the pointer. I'll change to use ReplicationConfig.
--
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-13535. Flag to indicate under and over replications in container-state check [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9135:
URL: https://github.com/apache/ozone/pull/9135#discussion_r2573541401
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -155,18 +166,57 @@ private ContainerInfoToken getContainerInfoToken(long containerId)
// Cache miss - fetch and store
ContainerInfo info = containerOperationClient.getContainer(containerId);
String encodeToken = containerOperationClient.getEncodedContainerToken(containerId);
- cachedData = new ContainerInfoToken(info.getState(), encodeToken);
+ cachedData = new ContainerInfoToken(info.getState(), encodeToken, info);
encodedTokenCache.put(containerId, cachedData);
return cachedData;
}
+ private String checkReplicationStatus(long containerId) {
+ try {
+ ContainerInfoToken token = getContainerInfoToken(containerId);
+ ContainerInfo containerInfo = token.getContainerInfo();
+
+ // Get container replicas from SCM
+ List<ContainerReplicaInfo> replicaInfos =
+ containerOperationClient.getContainerReplicas(containerId);
+
+ if (replicaInfos.isEmpty()) {
+ return ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED + ": no replicas found";
+ }
+
+ int replicationFactor = containerInfo.getReplicationFactor().getNumber();
+ int healthyReplicas = 0;
+
+ for (ContainerReplicaInfo replicaInfo : replicaInfos) {
+ if (!replicaInfo.getState().equals("UNHEALTHY")) {
+ healthyReplicas++;
+ }
+ }
+
+ 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();
+ }
+
+ } catch (Exception e) {
+ return "REPLICATION_CHECK_FAILED: " + e.getMessage();
+ }
+ }
+
private static class ContainerInfoToken {
private HddsProtos.LifeCycleState state;
private final String encodedToken;
+ private final ContainerInfo containerInfo;
Review Comment:
`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:
https://github.com/apache/ozone/blob/8737837d5c8493bf39ba36e0e01ad70aab531339/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java#L86
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java:
##########
@@ -402,4 +402,21 @@ public enum HealthState {
OVER_REPLICATED,
MIS_REPLICATED
}
+
+ /**
+ * Enum representing replication status for containers.
+ * Used to indicate whether a container is under-replicated, over-replicated,
+ * or has healthy replication. This is specifically used for toString() output
+ * and display purposes.
+ */
+ public enum ReplicationStatus {
+ UNDER_REPLICATED,
+ OVER_REPLICATED,
+ HEALTHY_REPLICATION;
Review Comment:
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.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -155,18 +166,57 @@ private ContainerInfoToken getContainerInfoToken(long containerId)
// Cache miss - fetch and store
ContainerInfo info = containerOperationClient.getContainer(containerId);
String encodeToken = containerOperationClient.getEncodedContainerToken(containerId);
- cachedData = new ContainerInfoToken(info.getState(), encodeToken);
+ cachedData = new ContainerInfoToken(info.getState(), encodeToken, info);
encodedTokenCache.put(containerId, cachedData);
return cachedData;
}
+ private String checkReplicationStatus(long containerId) {
+ try {
+ ContainerInfoToken token = getContainerInfoToken(containerId);
+ ContainerInfo containerInfo = token.getContainerInfo();
+
+ // Get container replicas from SCM
+ List<ContainerReplicaInfo> replicaInfos =
+ containerOperationClient.getContainerReplicas(containerId);
+
+ if (replicaInfos.isEmpty()) {
+ return ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED + ": no replicas found";
+ }
+
+ int replicationFactor = containerInfo.getReplicationFactor().getNumber();
Review Comment:
`ReplicationFactor` does not support EC. Please use `getReplicationConfig().getRequiredNodes()`.
```
REPLICATION_CHECK_FAILED: Replication configuration of type EC does not have a replication factor property.
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisContainerReplicaCount.java:
##########
@@ -257,6 +257,17 @@ public String toString() {
if (considerUnhealthy) {
result += " +considerUnhealthy";
}
+
+ 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;
+ }
+
Review Comment:
Similarly, please delete or move.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -155,18 +166,57 @@ private ContainerInfoToken getContainerInfoToken(long containerId)
// Cache miss - fetch and store
ContainerInfo info = containerOperationClient.getContainer(containerId);
String encodeToken = containerOperationClient.getEncodedContainerToken(containerId);
- cachedData = new ContainerInfoToken(info.getState(), encodeToken);
+ cachedData = new ContainerInfoToken(info.getState(), encodeToken, info);
encodedTokenCache.put(containerId, cachedData);
return cachedData;
}
+ private String checkReplicationStatus(long containerId) {
+ try {
+ ContainerInfoToken token = getContainerInfoToken(containerId);
+ ContainerInfo containerInfo = token.getContainerInfo();
+
+ // Get container replicas from SCM
+ List<ContainerReplicaInfo> replicaInfos =
+ containerOperationClient.getContainerReplicas(containerId);
Review Comment:
`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".
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java:
##########
@@ -402,4 +402,21 @@ public enum HealthState {
OVER_REPLICATED,
MIS_REPLICATED
}
+
+ /**
+ * Enum representing replication status for containers.
+ * Used to indicate whether a container is under-replicated, over-replicated,
+ * or has healthy replication. This is specifically used for toString() output
+ * and display purposes.
+ */
+ public enum ReplicationStatus {
+ UNDER_REPLICATED,
+ OVER_REPLICATED,
+ HEALTHY_REPLICATION;
+
+ @Override
+ public String toString() {
+ return name();
+ }
Review Comment:
nit: not needed, since enums provide this by default.
```suggestion
```
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ContainerStateVerifier.java:
##########
@@ -155,18 +166,57 @@ private ContainerInfoToken getContainerInfoToken(long containerId)
// Cache miss - fetch and store
ContainerInfo info = containerOperationClient.getContainer(containerId);
String encodeToken = containerOperationClient.getEncodedContainerToken(containerId);
- cachedData = new ContainerInfoToken(info.getState(), encodeToken);
+ cachedData = new ContainerInfoToken(info.getState(), encodeToken, info);
encodedTokenCache.put(containerId, cachedData);
return cachedData;
}
+ private String checkReplicationStatus(long containerId) {
+ try {
+ ContainerInfoToken token = getContainerInfoToken(containerId);
+ ContainerInfo containerInfo = token.getContainerInfo();
+
+ // Get container replicas from SCM
+ List<ContainerReplicaInfo> replicaInfos =
+ containerOperationClient.getContainerReplicas(containerId);
+
+ if (replicaInfos.isEmpty()) {
+ return ContainerHealthResult.ReplicationStatus.UNDER_REPLICATED + ": no replicas found";
+ }
+
+ int replicationFactor = containerInfo.getReplicationFactor().getNumber();
+ int healthyReplicas = 0;
+
+ for (ContainerReplicaInfo replicaInfo : replicaInfos) {
+ if (!replicaInfo.getState().equals("UNHEALTHY")) {
+ healthyReplicas++;
+ }
+ }
+
+ 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();
+ }
Review Comment:
nit: avoid duplication.
```suggestion
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);
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -573,6 +573,19 @@ public String toString() {
.append(", ReplicationConfig: ").append(repConfig)
.append(", RemainingMaintenanceRedundancy: ")
.append(remainingMaintenanceRedundancy);
+
+ 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);
+ }
+
Review Comment:
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.
--
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