HDDS-847. TestBlockDeletion is failing.

Contributed by Lokesh Jain.

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/OzoneTestUtils.java

@@ -23,11 +23,11 @@
 import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
 import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
 import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup;
+import org.apache.ratis.util.CheckedConsumer;
 import org.junit.Assert;
 
 import java.io.IOException;
 import java.util.List;
-import java.util.function.Consumer;
 
 /**
  * Helper class for Tests.
@@ -48,23 +48,27 @@ private OzoneTestUtils() {
    * @return true if close containers is successful.
    * @throws IOException
    */
-  public static boolean closeContainers(
+  public static void closeContainers(
       List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups,
-      StorageContainerManager scm) throws IOException {
+      StorageContainerManager scm) throws Exception {
-    return performOperationOnKeyContainers((blockID) -> {
+    performOperationOnKeyContainers((blockID) -> {
-      try {
+      if (scm.getContainerManager()
+          .getContainer(ContainerID.valueof(blockID.getContainerID()))
+          .getState() == HddsProtos.LifeCycleState.OPEN) {
         scm.getContainerManager()
             .updateContainerState(ContainerID.valueof(blockID.getContainerID()),
                 HddsProtos.LifeCycleEvent.FINALIZE);
+      }
+      if (scm.getContainerManager()
+          .getContainer(ContainerID.valueof(blockID.getContainerID()))
+          .getState() == HddsProtos.LifeCycleState.CLOSING) {
         scm.getContainerManager()
             .updateContainerState(ContainerID.valueof(blockID.getContainerID()),
                 HddsProtos.LifeCycleEvent.CLOSE);
-        Assert.assertFalse(scm.getContainerManager()
-            .getContainer(ContainerID.valueof(
-                blockID.getContainerID())).isOpen());
-      } catch (IOException e) {
-        throw new AssertionError("Failed to close the container", e);
       }
+      Assert.assertFalse(scm.getContainerManager()
+          .getContainer(ContainerID.valueof(blockID.getContainerID()))
+          .isOpen());
     }, omKeyLocationInfoGroups);
   }
 
@@ -74,28 +78,20 @@ public static boolean closeContainers(
    *
    * @param consumer Consumer which accepts BlockID as argument.
    * @param omKeyLocationInfoGroups locationInfos for a key.
-   * @return true if consumer is successful.
    * @throws IOException
    */
-  public static boolean performOperationOnKeyContainers(
+  public static void performOperationOnKeyContainers(
-      Consumer<BlockID> consumer,
+      CheckedConsumer<BlockID, Exception> consumer,
-      List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups) throws IOException {
+      List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups) throws Exception {
 
-    try {
+    for (OmKeyLocationInfoGroup omKeyLocationInfoGroup : omKeyLocationInfoGroups) {
-      for (OmKeyLocationInfoGroup omKeyLocationInfoGroup :
+      List<OmKeyLocationInfo> omKeyLocationInfos =
-          omKeyLocationInfoGroups) {
+          omKeyLocationInfoGroup.getLocationList();
-        List<OmKeyLocationInfo> omKeyLocationInfos =
+      for (OmKeyLocationInfo omKeyLocationInfo : omKeyLocationInfos) {
-            omKeyLocationInfoGroup.getLocationList();
+        BlockID blockID = omKeyLocationInfo.getBlockID();
-        for (OmKeyLocationInfo omKeyLocationInfo : omKeyLocationInfos) {
+        consumer.accept(blockID);
-          BlockID blockID = omKeyLocationInfo.getBlockID();
-          consumer.accept(blockID);
-        }
       }
-    } catch (Error e) {
-      e.printStackTrace();
-      return false;
     }
-    return true;
   }
 
 }

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java

@@ -120,8 +120,7 @@ public static void init() throws Exception {
   }
 
   @Test
-  public void testBlockDeletion()
+  public void testBlockDeletion() throws Exception {
-      throws IOException, InterruptedException, TimeoutException {
     String volumeName = UUID.randomUUID().toString();
     String bucketName = UUID.randomUUID().toString();
 
@@ -148,7 +147,7 @@ public void testBlockDeletion()
         om.lookupKey(keyArgs).getKeyLocationVersions();
 
     // verify key blocks were created in DN.
-    Assert.assertTrue(verifyBlocksCreated(omKeyLocationInfoGroupList));
+    verifyBlocksCreated(omKeyLocationInfoGroupList);
     // No containers with deleted blocks
     Assert.assertTrue(containerIdsWithDeletedBlocks.isEmpty());
     // Delete transactionIds for the containers should be 0.
@@ -158,17 +157,21 @@ public void testBlockDeletion()
     om.deleteKey(keyArgs);
     Thread.sleep(5000);
     // The blocks should not be deleted in the DN as the container is open
-    Assert.assertTrue(!verifyBlocksDeleted(omKeyLocationInfoGroupList));
+    try {
+      verifyBlocksDeleted(omKeyLocationInfoGroupList);
+      Assert.fail("Blocks should not have been deleted");
+    } catch (Throwable e) {
+      Assert.assertTrue(e.getMessage().contains("expected null, but was"));
+      Assert.assertEquals(e.getClass(), AssertionError.class);
+    }
 
     // close the containers which hold the blocks for the key
-    Assert
+    OzoneTestUtils.closeContainers(omKeyLocationInfoGroupList, scm);
-        .assertTrue(
-            OzoneTestUtils.closeContainers(omKeyLocationInfoGroupList, scm));
 
     waitForDatanodeCommandRetry();
     waitForDatanodeBlockDeletionStart();
     // The blocks should be deleted in the DN.
-    Assert.assertTrue(verifyBlocksDeleted(omKeyLocationInfoGroupList));
+    verifyBlocksDeleted(omKeyLocationInfoGroupList);
 
     // Few containers with deleted blocks
     Assert.assertTrue(!containerIdsWithDeletedBlocks.isEmpty());
@@ -288,44 +291,32 @@ private void matchContainerTransactionIds() throws IOException {
     }
   }
 
-  private boolean verifyBlocksCreated(
+  private void verifyBlocksCreated(
-      List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups)
+      List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups) throws Exception {
-      throws IOException {
     ContainerSet dnContainerSet =
         cluster.getHddsDatanodes().get(0).getDatanodeStateMachine()
             .getContainer().getContainerSet();
-    return OzoneTestUtils.performOperationOnKeyContainers((blockID) -> {
+    OzoneTestUtils.performOperationOnKeyContainers((blockID) -> {
-      try {
+      MetadataStore db = BlockUtils.getDB((KeyValueContainerData) dnContainerSet
-        MetadataStore db = BlockUtils.getDB((KeyValueContainerData)
+          .getContainer(blockID.getContainerID()).getContainerData(), conf);
-                dnContainerSet.getContainer(blockID.getContainerID())
+      Assert.assertNotNull(db.get(Longs.toByteArray(blockID.getLocalID())));
-                    .getContainerData(), conf);
-        Assert.assertNotNull(db.get(Longs.toByteArray(blockID.getLocalID())));
-      } catch (IOException e) {
-        e.printStackTrace();
-      }
     }, omKeyLocationInfoGroups);
   }
 
-  private boolean verifyBlocksDeleted(
+  private void verifyBlocksDeleted(
-      List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups)
+      List<OmKeyLocationInfoGroup> omKeyLocationInfoGroups) throws Exception {
-      throws IOException {
     ContainerSet dnContainerSet =
         cluster.getHddsDatanodes().get(0).getDatanodeStateMachine()
             .getContainer().getContainerSet();
-    return OzoneTestUtils.performOperationOnKeyContainers((blockID) -> {
+    OzoneTestUtils.performOperationOnKeyContainers((blockID) -> {
-      try {
+      MetadataStore db = BlockUtils.getDB((KeyValueContainerData) dnContainerSet
-        MetadataStore db = BlockUtils.getDB((KeyValueContainerData)
+          .getContainer(blockID.getContainerID()).getContainerData(), conf);
-            dnContainerSet.getContainer(blockID.getContainerID())
+      Assert.assertNull(db.get(Longs.toByteArray(blockID.getLocalID())));
-                .getContainerData(), conf);
+      Assert.assertNull(db.get(DFSUtil.string2Bytes(
-        Assert.assertNull(db.get(Longs.toByteArray(blockID.getLocalID())));
+          OzoneConsts.DELETING_KEY_PREFIX + blockID.getLocalID())));
-        Assert.assertNull(db.get(DFSUtil.string2Bytes(
+      Assert.assertNotNull(DFSUtil
-            OzoneConsts.DELETING_KEY_PREFIX + blockID.getLocalID())));
+          .string2Bytes(OzoneConsts.DELETED_KEY_PREFIX + blockID.getLocalID()));
-        Assert.assertNotNull(DFSUtil.string2Bytes(
+      containerIdsWithDeletedBlocks.add(blockID.getContainerID());
-            OzoneConsts.DELETED_KEY_PREFIX + blockID.getLocalID()));
-        containerIdsWithDeletedBlocks.add(blockID.getContainerID());
-      } catch (IOException e) {
-        e.printStackTrace();
-      }
     }, omKeyLocationInfoGroups);
   }
 }