HDFS-12423. Ozone: TopN container choosing policy should ignore containers that has no pending deletion blocks. Contributed by Yiqun Lin.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/impl/TopNOrderedContainerDeletionChoosingPolicy.java

@@ -66,13 +66,20 @@ public class TopNOrderedContainerDeletionChoosingPolicy
     int currentCount = 0;
     for (ContainerStatus entry : orderedList) {
       if (currentCount < count) {
-        result.add(entry.getContainer());
-        currentCount++;
+        if (entry.getNumPendingDeletionBlocks() > 0) {
+          result.add(entry.getContainer());
+          currentCount++;
 
-        LOG.debug("Select container {} for block deletion, "
-            + "pending deletion blocks num: {}.",
-            entry.getContainer().getContainerName(),
-            entry.getNumPendingDeletionBlocks());
+          LOG.debug(
+              "Select container {} for block deletion, "
+                  + "pending deletion blocks num: {}.",
+              entry.getContainer().getContainerName(),
+              entry.getNumPendingDeletionBlocks());
+        } else {
+          LOG.debug("Stop looking for next container, there is no"
+              + " pending deletion block contained in remaining containers.");
+          break;
+        }
       } else {
         break;
       }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/background/BlockDeletingService.java

@@ -106,6 +106,9 @@ public class BlockDeletingService extends BackgroundService{
       // configured.
       containers = containerManager.chooseContainerForBlockDeletion(
           containerLimitPerInterval);
+      LOG.info("Plan to choose {} containers for block deletion, "
+          + "actually returns {} valid containers.",
+          containerLimitPerInterval, containers.size());
 
       for(ContainerData container : containers) {
         BlockDeletingTask containerTask =

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java

@@ -31,9 +31,11 @@ import org.apache.hadoop.ozone.container.common.helpers.ContainerData;
 import org.apache.hadoop.ozone.container.common.helpers.KeyData;
 import org.apache.hadoop.ozone.container.common.helpers.KeyUtils;
 import org.apache.hadoop.ozone.container.common.impl.ContainerManagerImpl;
+import org.apache.hadoop.ozone.container.common.impl.RandomContainerDeletionChoosingPolicy;
 import org.apache.hadoop.ozone.container.common.interfaces.ContainerManager;
 import org.apache.hadoop.ozone.container.common.statemachine.background.BlockDeletingService;
 import org.apache.hadoop.ozone.web.utils.OzoneUtils;
+import org.apache.hadoop.scm.ScmConfigKeys;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.test.GenericTestUtils.LogCapturer;
 import org.apache.hadoop.utils.BackgroundService;
@@ -99,6 +101,9 @@ public class TestBlockDeletingService {
 
   private ContainerManager createContainerManager(Configuration conf)
       throws Exception {
+    // use random container choosing policy for testing
+    conf.set(ScmConfigKeys.OZONE_SCM_CONTAINER_DELETION_CHOOSING_POLICY,
+        RandomContainerDeletionChoosingPolicy.class.getName());
     conf.set(OzoneConfigKeys.OZONE_LOCALSTORAGE_ROOT,
         containersDir.getAbsolutePath());
     if (containersDir.exists()) {

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDeletionChoosingPolicy.java

@@ -138,7 +138,8 @@ public class TestContainerDeletionChoosingPolicy {
     int numContainers = 10;
     Random random = new Random();
     Map<String, Integer> name2Count = new HashMap<>();
-    for (int i = 0; i < numContainers; i++) {
+    // create [numContainers + 1] containers
+    for (int i = 0; i <= numContainers; i++) {
       String containerName = OzoneUtils.getRequestID();
       ContainerData data = new ContainerData(containerName);
       containerManager.createContainer(createSingleNodePipeline(containerName),
@@ -146,6 +147,11 @@ public class TestContainerDeletionChoosingPolicy {
       Assert.assertTrue(
           containerManager.getContainerMap().containsKey(containerName));
 
+      // don't create deletion blocks in the last container.
+      if (i == numContainers) {
+        break;
+      }
+
       // create random number of deletion blocks and write to container db
       int deletionBlocks = random.nextInt(numContainers) + 1;
       // record <ContainerName, DeletionCount> value
@@ -170,7 +176,9 @@ public class TestContainerDeletionChoosingPolicy {
     Assert.assertEquals(5, result0.size());
 
     List<ContainerData> result1 = containerManager
-        .chooseContainerForBlockDeletion(numContainers);
+        .chooseContainerForBlockDeletion(numContainers + 1);
+    // the empty deletion blocks container should not be chosen
+    Assert.assertEquals(numContainers, result1.size());
 
     // verify the order of return list
     int lastCount = Integer.MAX_VALUE;