HDDS-10483. Container Balancer should only move containers with size greater than 0 bytes

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindSourceGreedy.java

@@ -152,11 +152,16 @@ public boolean canSizeLeaveSource(DatanodeDetails source, long size) {
     if (sizeLeavingNode.containsKey(source)) {
       long sizeLeavingAfterMove = sizeLeavingNode.get(source) + size;
       //size can be moved out of source datanode only when the following
-      //two condition are met.
-      //1 sizeLeavingAfterMove does not succeed the configured
+      //three conditions are met.
+      //1 size should be greater than zero bytes
+      //2 sizeLeavingAfterMove does not succeed the configured
       // MaxSizeLeavingTarget
-      //2 after subtracting sizeLeavingAfterMove, the usage is bigger
+      //3 after subtracting sizeLeavingAfterMove, the usage is bigger
       // than or equal to lowerLimit
+      if (size <= 0) {
+        LOG.debug("{} bytes container cannot leave datanode {}", size, source.getUuidString());
+        return false;
+      }
       if (sizeLeavingAfterMove > config.getMaxSizeLeavingSource()) {
         LOG.debug("{} bytes cannot leave datanode {} because 'size.leaving" +
                 ".source.max' limit is {} and {} bytes have already left.",

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerTask.java

@@ -55,6 +55,7 @@
 import org.apache.ozone.test.GenericTestUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.event.Level;
@@ -132,7 +133,7 @@ public class TestContainerBalancerTask {
    * Sets up configuration values and creates a mock cluster.
    */
   @BeforeEach
-  public void setup() throws IOException, NodeNotFoundException,
+  public void setup(TestInfo testInfo) throws IOException, NodeNotFoundException,
       TimeoutException {
     conf = new OzoneConfiguration();
     rmConf = new ReplicationManagerConfiguration();
@@ -164,7 +165,11 @@ public void setup() throws IOException, NodeNotFoundException,
     conf.setFromObject(balancerConfiguration);
     GenericTestUtils.setLogLevel(ContainerBalancerTask.LOG, Level.DEBUG);
 
-    averageUtilization = createCluster();
+    int[] sizeArray = testInfo.getTestMethod()
+            .filter(method -> method.getName().equals("balancerShouldMoveOnlyPositiveSizeContainers"))
+            .map(method -> new int[]{0, 0, 0, 0, 0, 1, 2, 3, 4, 5})
+            .orElse(null);
+    averageUtilization = createCluster(sizeArray);
     mockNodeManager = new MockNodeManager(datanodeToContainersMap);
 
     NetworkTopology clusterMap = mockNodeManager.getClusterNetworkTopologyMap();
@@ -1114,6 +1119,34 @@ public void balancerShouldExcludeECContainersWhenLegacyRmIsEnabled()
     }
   }
 
+  /**
+   * Test to check if balancer picks up only positive size
+   * containers to move from source to destination.
+   */
+  @Test
+  public void balancerShouldMoveOnlyPositiveSizeContainers()
+      throws IllegalContainerBalancerStateException, IOException,
+      InvalidContainerBalancerConfigurationException, TimeoutException {
+
+    startBalancer(balancerConfiguration);
+    /*
+     Get all containers that were selected by balancer and assert none of
+     them is a zero or negative size container.
+     */
+    Map<ContainerID, DatanodeDetails> containerToSource =
+            containerBalancerTask.getContainerToSourceMap();
+    assertFalse(containerToSource.isEmpty());
+    boolean zeroOrNegSizeContainerMoved = false;
+    for (Map.Entry<ContainerID, DatanodeDetails> entry :
+            containerToSource.entrySet()) {
+      ContainerInfo containerInfo = cidToInfoMap.get(entry.getKey());
+      if (containerInfo.getUsedBytes() <= 0) {
+        zeroOrNegSizeContainerMoved = true;
+      }
+    }
+    assertFalse(zeroOrNegSizeContainerMoved);
+  }
+
   /**
    * Determines unBalanced nodes, that is, over and under utilized nodes,
    * according to the generated utilization values for nodes and the threshold.
@@ -1169,8 +1202,8 @@ private void generateUtilizations(int count) throws IllegalArgumentException {
    * cluster have utilization values determined by generateUtilizations method.
    * @return average utilization (used space / capacity) of the cluster
    */
-  private double createCluster() {
-    generateData();
+  private double createCluster(int[] sizeArray) {
+    generateData(sizeArray);
     createReplicasForContainers();
     long clusterCapacity = 0, clusterUsedSpace = 0;
 
@@ -1204,7 +1237,7 @@ private double createCluster() {
   /**
    * Create some datanodes and containers for each node.
    */
-  private void generateData() {
+  private void generateData(int[] sizeArray) {
     this.numberOfNodes = 10;
     generateUtilizations(numberOfNodes);
     nodesInCluster = new ArrayList<>(nodeUtilizations.size());
@@ -1216,13 +1249,19 @@ private void generateData() {
           new DatanodeUsageInfo(MockDatanodeDetails.randomDatanodeDetails(),
               new SCMNodeStat());
 
-      // create containers with varying used space
       int sizeMultiple = 0;
+      if (sizeArray == null) {
+        sizeArray = new int[10];
+        for (int j = 0; j < numberOfNodes; j++) {
+          sizeArray[j] = sizeMultiple;
+          sizeMultiple %= 5;
+          sizeMultiple++;
+        }
+      }
+      // create containers with varying used space
       for (int j = 0; j < i; j++) {
-        sizeMultiple %= 5;
-        sizeMultiple++;
         ContainerInfo container =
-            createContainer((long) i * i + j, sizeMultiple);
+            createContainer((long) i * i + j, sizeArray[j]);
 
         cidToInfoMap.put(container.containerID(), container);
         containerIDSet.add(container.containerID());