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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to