diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindSourceGreedy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindSourceGreedy.java index 6350c3c76194..684df784c279 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindSourceGreedy.java +++ b/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.", diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerTask.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerTask.java index 3bed3878123d..a4d7f3761202 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerTask.java +++ b/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 containerToSource = + containerBalancerTask.getContainerToSourceMap(); + assertFalse(containerToSource.isEmpty()); + boolean zeroOrNegSizeContainerMoved = false; + for (Map.Entry 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());