ChenSammi commented on code in PR #9068:
URL: https://github.com/apache/ozone/pull/9068#discussion_r2389788306


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java:
##########
@@ -64,16 +68,54 @@ public ContainerData chooseContainer(OzoneContainer 
ozoneContainer,
       if (!inProgressContainerIDs.contains(
           ContainerID.valueOf(containerData.getContainerID())) &&
           (containerData.isClosed() || (test && 
containerData.isQuasiClosed()))) {
-        return containerData;
+
+        // This is a candidate container. Now, check if moving it would be 
productive.
+        if (isMoveProductive(containerData, destVolume, threshold, volumeSet)) 
{
+          return containerData;
+        }
       }
     }
 
     if (!itr.hasNext()) {
-      CACHE.get().invalidate(hddsVolume);
+      CACHE.get().invalidate(srcVolume);
     }
     return null;
   }
 
+  /**
+   * Checks if moving the given container from source to destination would
+   * result in the destination's utilization being less than or equal to the
+   * averageUtilization + threshold. This prevents "thrashing" where a move
+   * immediately makes the destination a candidate for a source volume.
+   *
+   * @param containerData The container to be moved.
+   * @param destVolume The destination volume.
+   * @return true if the move is productive, false otherwise.
+   */
+  private boolean isMoveProductive(ContainerData containerData,
+                                   HddsVolume destVolume,
+                                   Double threshold,
+                                   MutableVolumeSet volumeSet) {
+
+    long containerSize = containerData.getBytesUsed();
+
+    double idealUsage = volumeSet.getIdealUsage();
+    double maxAllowedUtilization = idealUsage + (threshold / 100.0);

Review Comment:
   @Gargi-jais11 , thanks for working on this. It overall looks good to me.  
Here are some comments, 
   a. please follow the existing function indent style, instead of current 
style, for this function, also functions in ContainerChoosingPolicy.java and  
DefaultContainerChoosingPolicy. 
   
   b. can we move the maxAllowedUtilization to chooseContainer before while 
loop?  so it will be calculated only once and can be shared. 



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