From 37056f8002b1660c82ac26f0c262d8ff3e795ce5 Mon Sep 17 00:00:00 2001 From: Gargi Jaiswal Date: Wed, 17 Dec 2025 10:19:18 +0530 Subject: [PATCH 1/2] avoid choosing 0 size container for disk balancing --- .../DefaultContainerChoosingPolicy.java | 4 +- .../TestDefaultContainerChoosingPolicy.java | 63 ++++++++++++++++--- .../scm/node/TestContainerChoosingPolicy.java | 4 ++ .../scm/node/TestVolumeChoosingPolicy.java | 2 +- 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java index c759271f881d..29920ae75d73 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java @@ -74,8 +74,8 @@ public ContainerData chooseContainer(OzoneContainer ozoneContainer, while (itr.hasNext()) { ContainerData containerData = itr.next().getContainerData(); - if (!inProgressContainerIDs.contains( - ContainerID.valueOf(containerData.getContainerID())) && + if (containerData.getBytesUsed() != 0 && + !inProgressContainerIDs.contains(ContainerID.valueOf(containerData.getContainerID())) && (containerData.isClosed() || (test && containerData.isQuasiClosed()))) { // This is a candidate container. Now, check if moving it would be productive. diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDefaultContainerChoosingPolicy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDefaultContainerChoosingPolicy.java index 325862231593..726dad5555ae 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDefaultContainerChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDefaultContainerChoosingPolicy.java @@ -35,12 +35,14 @@ import java.util.Set; import java.util.UUID; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.conf.StorageUnit; import org.apache.hadoop.hdds.fs.MockSpaceUsageCheckFactory; import org.apache.hadoop.hdds.fs.MockSpaceUsageSource; import org.apache.hadoop.hdds.fs.SpaceUsageCheckFactory; import org.apache.hadoop.hdds.fs.SpaceUsagePersistence; import org.apache.hadoop.hdds.fs.SpaceUsageSource; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.ozone.container.common.impl.ContainerData; import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion; @@ -154,14 +156,7 @@ private HddsVolume createVolume(String dir, double utilization) */ private void createContainer(long id, long size, HddsVolume vol) throws IOException { - KeyValueContainerData containerData = new KeyValueContainerData(id, - ContainerLayoutVersion.FILE_PER_BLOCK, size, - UUID.randomUUID().toString(), UUID.randomUUID().toString()); - containerData.setState(ContainerDataProto.State.CLOSED); - containerData.setVolume(vol); - containerData.getStatistics().setBlockBytesForTesting(size); - KeyValueContainer container = new KeyValueContainer(containerData, CONF); - containerSet.addContainer(container); + createContainer(id, size, vol, containerSet); } @Test @@ -200,4 +195,56 @@ public void testContainerNotChosen() { // No containers should not be chosen assertNull(chosenContainer); } + + @Test + public void testSizeZeroContainersSkipped() throws IOException { + // Create a new container set with containers that have size 0 + ContainerSet testContainerSet = newContainerSet(); + + // Create containers with size 0 (should be skipped) + createContainer(10L, 0L, sourceVolume, testContainerSet); + createContainer(11L, 0L, sourceVolume, testContainerSet); + + // Create a container with non-zero size (should be chosen) + createContainer(12L, 200L * MB, sourceVolume, testContainerSet); + + // Mock OzoneContainer to return our test container set + OzoneContainer testOzoneContainer = mock(OzoneContainer.class); + ContainerController testController = new ContainerController(testContainerSet, null); + when(testOzoneContainer.getController()).thenReturn(testController); + + // The policy should skip containers 10 and 11 (size 0) and choose container 12 + ContainerData chosenContainer = policy.chooseContainer(testOzoneContainer, + sourceVolume, destVolume1, inProgressContainerIDs, THRESHOLD, volumeSet, deltaMap); + + // Container 12 (non-zero size) should be chosen, skipping containers 10 and 11 (size 0) + assertNotNull(chosenContainer); + assertEquals(12L, chosenContainer.getContainerID()); + assertEquals(200L * MB, chosenContainer.getBytesUsed()); + } + + /** + * Create KeyValueContainers and add it to the specified containerSet. + * @param id container ID + * @param usedBytes bytes used by the container (can be 0) + * @param vol volume where container is located + * @param targetContainerSet container set to add the container to + */ + private void createContainer(long id, long usedBytes, HddsVolume vol, + ContainerSet targetContainerSet) throws IOException { + // Use maxSize as the container capacity (must be > 0) + // If usedBytes is 0, we still need a valid maxSize for container creation + long maxSize = usedBytes > 0 ? usedBytes : (long) CONF.getStorageSize( + ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE, + ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES); + KeyValueContainerData containerData = new KeyValueContainerData(id, + ContainerLayoutVersion.FILE_PER_BLOCK, maxSize, + UUID.randomUUID().toString(), UUID.randomUUID().toString()); + containerData.setState(ContainerDataProto.State.CLOSED); + containerData.setVolume(vol); + // Set the actual used bytes (can be 0) + containerData.getStatistics().setBlockBytesForTesting(usedBytes); + KeyValueContainer container = new KeyValueContainer(containerData, CONF); + targetContainerSet.addContainer(container); + } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java index efebf0352538..3b8ae3f58612 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java @@ -273,6 +273,10 @@ public void createContainers() { containerData.setState(isOpen ? ContainerDataProto.State.OPEN : ContainerDataProto.State.CLOSED); containerData.setVolume(volume); + // Set some bytes used for containers so they can be chosen for disk balancing + // Use a small non-zero value to ensure containers are not skipped + long bytesUsed = isOpen ? 0 : (i % 1000 + 1) * 1024L; // 1KB to 1MB for closed containers + containerData.getStatistics().setBlockBytesForTesting(bytesUsed); KeyValueContainer container = new KeyValueContainer(containerData, CONF); try { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java index b65cfa637c83..1bb20c1ce716 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java @@ -262,7 +262,7 @@ private void createVolumes() throws IOException { } // Initialize the volumeSet with the new volume map - volumeSet.setVolumeMap(newVolumeMap); + volumeSet.setVolumeMapForTesting(newVolumeMap); System.out.println("Created " + NUM_VOLUMES + " volumes in " + (System.currentTimeMillis() - startTime) + " ms"); } From 538ac848586e17f9734966ae3adda176c864d780 Mon Sep 17 00:00:00 2001 From: Gargi Jaiswal Date: Thu, 18 Dec 2025 14:14:06 +0530 Subject: [PATCH 2/2] check contaienr used bytes to be greater than 0 --- .../diskbalancer/policy/DefaultContainerChoosingPolicy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java index 29920ae75d73..c30ca7c9cbf8 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java @@ -74,7 +74,7 @@ public ContainerData chooseContainer(OzoneContainer ozoneContainer, while (itr.hasNext()) { ContainerData containerData = itr.next().getContainerData(); - if (containerData.getBytesUsed() != 0 && + if (containerData.getBytesUsed() > 0 && !inProgressContainerIDs.contains(ContainerID.valueOf(containerData.getContainerID())) && (containerData.isClosed() || (test && containerData.isQuasiClosed()))) {