[GitHub] [ozone] lokeshj1703 commented on a change in pull request #2676: HDDS-5757. balancer should stop when the cluster can not be balanced any more

[GitBox <gi...@apache.org>]

lokeshj1703 commented on a change in pull request #2676:
URL: https://github.com/apache/ozone/pull/2676#discussion_r717485960



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -652,8 +670,24 @@ private double ratioToBytes(Long nodeCapacity, double utilizationRatio) {
    */
   boolean canSizeEnterTarget(DatanodeDetails target, long size) {
     if (sizeEnteringNode.containsKey(target)) {
-      return sizeEnteringNode.get(target) + size <=
-          config.getMaxSizeEnteringTarget();
+      long sizeEnteringAfterMove = sizeEnteringNode.get(target) + size;
+      SCMNodeMetric scmNM = nodeManager.getNodeStat(target);
+      Preconditions.checkNotNull(scmNM);
+      SCMNodeStat scmNodeStat = scmNM.get();
+      double capacity = scmNodeStat.getCapacity().get();
+      Preconditions.checkArgument(capacity > 0);
+
+      //size can be moved into target datanode only when the following
+      //two condition are met.
+      //1 sizeEnteringAfterMove does not succeed the configured
+      // MaxSizeEnteringTarget
+      //2 current usage of target datanode plus sizeEnteringAfterMove
+      // is smaller than or equal to upperLimit
+      double usedAfterMove =
+          capacity - scmNodeStat.getRemaining().get() + sizeEnteringAfterMove;
+

Review comment:
       Can we move this calculation in a separate Utils class or sth like that? @siddhantsangwan had added a function in DatanodeUsageInfo I think?
   I feel we can reuse the same function. We will only need a parameter for used size to add.

##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -187,7 +190,13 @@ private void balance() {
         stop();
         return;
       }
-      doIteration();
+
+      //if no new move option is generated, it means the cluster can
+      //not be balanced any more , so just stop
+      if (doIteration() == IterationResult.CAN_NOT_BALANCE_ANY_MORE) {

Review comment:
       Can we call the iteration function in a separate line for better visibility?




-- 
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