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..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,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"); }