You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sarvekshayr (via GitHub)" <gi...@apache.org> on 2026/03/21 14:55:00 UTC
[PR] HDDS-14870. Allow balancing of over replicated and quasi closed containers [ozone]
sarvekshayr opened a new pull request, #9964:
URL: https://github.com/apache/ozone/pull/9964
## What changes were proposed in this pull request?
Allow container balancer to include containers if
- Container State is CLOSED and ContainerHealthState is OVER_REPLICATED but has minimum CLOSED replicas based on replication config along with additional QUASI_CLOSED replicas
- Container State is QUASI_CLOSED and has all replicas in QUASI_CLOSED state.
This is allowed only if the new config `hdds.container.balancer.include.non.standard.containers` is set to true.
## What is the link to the Apache JIRA
[HDDS-14870](https://issues.apache.org/jira/browse/HDDS-14870)
## How was this patch tested?
Added a unit test - `TestContainerBalancerSelectionCriteria#shouldIncludeNonStandardContainers`
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2972553601
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerSelectionCriteria.java:
##########
@@ -135,4 +139,52 @@ public void shouldExcludeReplicatingContainer()
verify(replicationManager, times(1)).getContainerReplicationHealth(
eq(containerInfo), anySet());
}
+
+ @Test
+ public void shouldIncludeNonStandardContainers() throws Exception {
Review Comment:
This test only tests Case A (CLOSED, OVER_REPLICATED, 3 CLOSED + 1 QUASI_CLOSED). There is no test for Case B (QUASI_CLOSED container with all QUASI_CLOSED replicas)
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2992910314
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
Review Comment:
shouldBeExcluded still documents “container should be closed” only, but when includeNonStandardContainers is true the method uses the relaxed checks. Please update this javadoc.
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2979541037
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -199,47 +199,92 @@ public boolean shouldBeExcluded(ContainerID containerID,
* Checks whether specified container is closed. Also checks if the replica
* on the specified datanode is CLOSED. Assumes that there will only be one
* replica of a container on a particular Datanode.
+ * <p>
+ * With hdds.container.balancer.include.non.standard.containers set to true:
+ * - CLOSED containers: Allows moving QUASI_CLOSED replicas if minimum CLOSED replicas exist
+ * - QUASI_CLOSED containers: Allows moving if all replicas are QUASI_CLOSED
+ *
* @param container container to check
- * @param datanodeDetails datanode on which a replica of the container is
- * present
- * @return true if container LifeCycleState is
- * {@link HddsProtos.LifeCycleState#CLOSED} and its replica on the
- * specified datanode is CLOSED, else false
+ * @param datanodeDetails datanode on which a replica of the container is present
+ * @param replicas all replicas of the container
+ * @return true if container and replica are eligible for balancing, else false
*/
private boolean isContainerClosed(ContainerInfo container,
DatanodeDetails datanodeDetails,
Set<ContainerReplica> replicas) {
- if (!container.getState().equals(HddsProtos.LifeCycleState.CLOSED)) {
+ HddsProtos.LifeCycleState containerState = container.getState();
+ // Find the specific replica on this datanode
+ ContainerReplica targetReplica = replicas.stream()
+ .filter(r -> r.getDatanodeDetails().equals(datanodeDetails))
+ .findFirst()
+ .orElse(null);
+ if (targetReplica == null) {
return false;
}
-
- for (ContainerReplica replica : replicas) {
- if (replica.getDatanodeDetails().equals(datanodeDetails)) {
- // don't consider replica if it's not closed
- // assumption: there's only one replica of this container on this DN
- return replica.getState().equals(ContainerReplicaProto.State.CLOSED);
+ ContainerReplicaProto.State replicaState = targetReplica.getState();
+
+ // Case 1: Container is CLOSED
+ if (containerState == HddsProtos.LifeCycleState.CLOSED) {
+ if (replicaState == ContainerReplicaProto.State.CLOSED) {
+ return true;
}
+
Review Comment:
Please add a test for this as well. 3 CLOSED 1 QC empty replica
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on PR #9964:
URL: https://github.com/apache/ozone/pull/9964#issuecomment-4197617975
Thanks @sadanand48 and @sreejasahithi for the review and approval.
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2979459294
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -199,47 +199,92 @@ public boolean shouldBeExcluded(ContainerID containerID,
* Checks whether specified container is closed. Also checks if the replica
* on the specified datanode is CLOSED. Assumes that there will only be one
* replica of a container on a particular Datanode.
+ * <p>
+ * With hdds.container.balancer.include.non.standard.containers set to true:
+ * - CLOSED containers: Allows moving QUASI_CLOSED replicas if minimum CLOSED replicas exist
+ * - QUASI_CLOSED containers: Allows moving if all replicas are QUASI_CLOSED
+ *
* @param container container to check
- * @param datanodeDetails datanode on which a replica of the container is
- * present
- * @return true if container LifeCycleState is
- * {@link HddsProtos.LifeCycleState#CLOSED} and its replica on the
- * specified datanode is CLOSED, else false
+ * @param datanodeDetails datanode on which a replica of the container is present
+ * @param replicas all replicas of the container
+ * @return true if container and replica are eligible for balancing, else false
*/
private boolean isContainerClosed(ContainerInfo container,
DatanodeDetails datanodeDetails,
Set<ContainerReplica> replicas) {
- if (!container.getState().equals(HddsProtos.LifeCycleState.CLOSED)) {
+ HddsProtos.LifeCycleState containerState = container.getState();
+ // Find the specific replica on this datanode
+ ContainerReplica targetReplica = replicas.stream()
+ .filter(r -> r.getDatanodeDetails().equals(datanodeDetails))
+ .findFirst()
+ .orElse(null);
+ if (targetReplica == null) {
return false;
}
-
- for (ContainerReplica replica : replicas) {
- if (replica.getDatanodeDetails().equals(datanodeDetails)) {
- // don't consider replica if it's not closed
- // assumption: there's only one replica of this container on this DN
- return replica.getState().equals(ContainerReplicaProto.State.CLOSED);
+ ContainerReplicaProto.State replicaState = targetReplica.getState();
+
+ // Case 1: Container is CLOSED
+ if (containerState == HddsProtos.LifeCycleState.CLOSED) {
+ if (replicaState == ContainerReplicaProto.State.CLOSED) {
+ return true;
}
+
+ // With config enabled: Also allow QUASI_CLOSED replicas if we have minimum required CLOSED replicas
+ if (balancerConfiguration.getIncludeNonStandardContainers()) {
+ long numClosedReplicas = replicas.stream()
+ .filter(r -> r.getState() == ContainerReplicaProto.State.CLOSED)
+ .count();
+ int minRequiredReplicas = container.getReplicationConfig().getRequiredNodes();
+
+ if (numClosedReplicas >= minRequiredReplicas) {
+ return replicaState == ContainerReplicaProto.State.QUASI_CLOSED;
+ }
+ }
+ return false;
+ }
+
+ // Case 2: Container is QUASI_CLOSED
+ if (containerState == HddsProtos.LifeCycleState.QUASI_CLOSED) {
+ if (!balancerConfiguration.getIncludeNonStandardContainers()) {
+ return false;
+ }
+
+ // Only allow if ALL replicas are QUASI_CLOSED
+ // This ensures we don't move containers with mixed replica states
+ return replicas.stream()
+ .allMatch(r -> r.getState() == ContainerReplicaProto.State.QUASI_CLOSED);
}
-
return false;
}
/**
* This asks replication manager whether a container is under/over/mis replicated. The intention is the same as
* isContainerReplicatingOrDeleting but the check is done in a different way to be doubly sure.
+ * <p>
+ * With hdds.container.balancer.include.non.standard.containers set to true:
+ * - OVER_REPLICATED containers are allowed
+ *
* @param container container to check
* @param replicas the container's replicas
* @return false if it should not be moved, true otherwise
*/
private boolean isContainerHealthyForMove(ContainerInfo container, Set<ContainerReplica> replicas) {
ContainerHealthResult.HealthState state =
replicationManager.getContainerReplicationHealth(container, replicas).getHealthState();
- if (state != ContainerHealthResult.HealthState.HEALTHY) {
- LOG.debug("Excluding container {} with replicas {} as its health is {}.", container, replicas, state);
- return false;
+ if (state == ContainerHealthResult.HealthState.HEALTHY) {
+ return true;
Review Comment:
same here
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -199,47 +199,92 @@ public boolean shouldBeExcluded(ContainerID containerID,
* Checks whether specified container is closed. Also checks if the replica
* on the specified datanode is CLOSED. Assumes that there will only be one
* replica of a container on a particular Datanode.
+ * <p>
+ * With hdds.container.balancer.include.non.standard.containers set to true:
+ * - CLOSED containers: Allows moving QUASI_CLOSED replicas if minimum CLOSED replicas exist
+ * - QUASI_CLOSED containers: Allows moving if all replicas are QUASI_CLOSED
+ *
* @param container container to check
- * @param datanodeDetails datanode on which a replica of the container is
- * present
- * @return true if container LifeCycleState is
- * {@link HddsProtos.LifeCycleState#CLOSED} and its replica on the
- * specified datanode is CLOSED, else false
+ * @param datanodeDetails datanode on which a replica of the container is present
+ * @param replicas all replicas of the container
+ * @return true if container and replica are eligible for balancing, else false
*/
private boolean isContainerClosed(ContainerInfo container,
Review Comment:
Lets have a separate method isContainerClosedRelaxed and use that if the config is true.
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2979535095
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -199,47 +199,92 @@ public boolean shouldBeExcluded(ContainerID containerID,
* Checks whether specified container is closed. Also checks if the replica
* on the specified datanode is CLOSED. Assumes that there will only be one
* replica of a container on a particular Datanode.
+ * <p>
+ * With hdds.container.balancer.include.non.standard.containers set to true:
+ * - CLOSED containers: Allows moving QUASI_CLOSED replicas if minimum CLOSED replicas exist
+ * - QUASI_CLOSED containers: Allows moving if all replicas are QUASI_CLOSED
+ *
* @param container container to check
- * @param datanodeDetails datanode on which a replica of the container is
- * present
- * @return true if container LifeCycleState is
- * {@link HddsProtos.LifeCycleState#CLOSED} and its replica on the
- * specified datanode is CLOSED, else false
+ * @param datanodeDetails datanode on which a replica of the container is present
+ * @param replicas all replicas of the container
+ * @return true if container and replica are eligible for balancing, else false
*/
private boolean isContainerClosed(ContainerInfo container,
DatanodeDetails datanodeDetails,
Set<ContainerReplica> replicas) {
- if (!container.getState().equals(HddsProtos.LifeCycleState.CLOSED)) {
+ HddsProtos.LifeCycleState containerState = container.getState();
+ // Find the specific replica on this datanode
+ ContainerReplica targetReplica = replicas.stream()
+ .filter(r -> r.getDatanodeDetails().equals(datanodeDetails))
+ .findFirst()
+ .orElse(null);
+ if (targetReplica == null) {
return false;
}
-
- for (ContainerReplica replica : replicas) {
- if (replica.getDatanodeDetails().equals(datanodeDetails)) {
- // don't consider replica if it's not closed
- // assumption: there's only one replica of this container on this DN
- return replica.getState().equals(ContainerReplicaProto.State.CLOSED);
+ ContainerReplicaProto.State replicaState = targetReplica.getState();
+
+ // Case 1: Container is CLOSED
+ if (containerState == HddsProtos.LifeCycleState.CLOSED) {
+ if (replicaState == ContainerReplicaProto.State.CLOSED) {
+ return true;
}
+
Review Comment:
if replica state is quasi closed and replica is empty i.e bytesUsed == 0 we should exclude 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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr merged PR #9964:
URL: https://github.com/apache/ozone/pull/9964
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2974677503
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerSelectionCriteria.java:
##########
@@ -135,4 +139,52 @@ public void shouldExcludeReplicatingContainer()
verify(replicationManager, times(1)).getContainerReplicationHealth(
eq(containerInfo), anySet());
}
+
+ @Test
+ public void shouldIncludeNonStandardContainers() throws Exception {
Review Comment:
Thanks for pointing out a critical issue. I've updated `MoveManager` similar to `ContainerBalancerSelectionCriteria`.
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r2974668608
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerSelectionCriteria.java:
##########
@@ -135,4 +139,52 @@ public void shouldExcludeReplicatingContainer()
verify(replicationManager, times(1)).getContainerReplicationHealth(
eq(containerInfo), anySet());
}
+
+ @Test
+ public void shouldIncludeNonStandardContainers() throws Exception {
Review Comment:
Done!
--
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-14870. Allow balancing of over replicated and quasi closed containers [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9964:
URL: https://github.com/apache/ozone/pull/9964#discussion_r3013702257
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
Review Comment:
Done.
--
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