HDDS-6646. Intermittent failure in TestOzoneFileSystem#testRenameToTrashEnabled

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java

@@ -84,6 +84,7 @@
 import java.util.Iterator;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.TimeoutException;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_CHECKPOINT_INTERVAL_KEY;
@@ -228,7 +229,7 @@ public static void teardown() {
   public void cleanup() {
     try {
       deleteRootDir();
-    } catch (IOException | InterruptedException ex) {
+    } catch (IOException | InterruptedException | TimeoutException ex) {
       LOG.error("Failed to cleanup files.", ex);
       fail("Failed to cleanup files.");
     }
@@ -795,18 +796,24 @@ public void testListStatusOnKeyNameContainDelimiter() throws Exception {
    *
    * @throws IOException DB failure
    */
-  protected void deleteRootDir() throws IOException, InterruptedException {
+  protected void deleteRootDir()
+      throws IOException, InterruptedException, TimeoutException {
     FileStatus[] fileStatuses = fs.listStatus(ROOT);
 
     if (fileStatuses == null) {
       return;
     }
     deleteRootRecursively(fileStatuses);
-    fileStatuses = fs.listStatus(ROOT);
-    if (fileStatuses != null) {
-      Assert.assertEquals(
-          "Delete root failed!", 0, fileStatuses.length);
-    }
+    GenericTestUtils.waitFor(() -> {
+      FileStatus[] fileStatus = new FileStatus[0];
+      try {
+        fileStatus = fs.listStatus(ROOT);
+        return fileStatus != null && fileStatus.length == 0;
+      } catch (IOException e) {
+        Assert.assertFalse(fileStatus.length == 0);
+        return false;
+      }
+    }, 100, 500);
   }
 
   private static void deleteRootRecursively(FileStatus[] fileStatuses)
@@ -1618,7 +1625,6 @@ public void testGetTrashRoots() throws IOException {
    * since fs.rename(src,dst,options) is enabled.
    */
   @Test
-  @Flaky("HDDS-6646")
   public void testRenameToTrashEnabled() throws Exception {
     // Create a file
     String testKeyName = "testKey1";
@@ -1627,19 +1633,19 @@ public void testRenameToTrashEnabled() throws Exception {
       stream.write(1);
     }
 
-    // Call moveToTrash. We can't call protected fs.rename() directly
-    trash.moveToTrash(path);
-
     // Construct paths
     String username = UserGroupInformation.getCurrentUser().getShortUserName();
     Path userTrash = new Path(TRASH_ROOT, username);
-    Path userTrashCurrent = new Path(userTrash, "Current");
-    Path trashPath = new Path(userTrashCurrent, testKeyName);
 
-    // Trash Current directory should still have been created.
-    Assert.assertTrue(o3fs.exists(userTrashCurrent));
-    // Check under trash, the key should be present
-    Assert.assertTrue(o3fs.exists(trashPath));
+    Assert.assertFalse(o3fs.exists(userTrash));
+    // Call moveToTrash. We can't call protected fs.rename() directly
+    trash.moveToTrash(path);
+    // We can safely assert only trash directory here.
+    // Asserting Current or checkpoint directory is not feasible here in this
+    // test due to independent TrashEmptier thread running in cluster and
+    // possible flakyness is hard to avoid unless we test this test case
+    // in separate mini cluster with closer accuracy of TrashEmptier.
+    Assert.assertTrue(o3fs.exists(userTrash));
   }
 
   /**
@@ -1657,15 +1663,21 @@ public void testTrash() throws Exception {
         isAssignableFrom(TrashPolicyOzone.class));
     assertEquals(TRASH_INTERVAL, trash.getConf().
         getFloat(OMConfigKeys.OZONE_FS_TRASH_INTERVAL_KEY, 0), 0);
-    // Call moveToTrash. We can't call protected fs.rename() directly
-    trash.moveToTrash(path);
 
     // Construct paths
     String username = UserGroupInformation.getCurrentUser().getShortUserName();
     Path userTrash = new Path(TRASH_ROOT, username);
     Path userTrashCurrent = new Path(userTrash, "Current");
     Path trashPath = new Path(userTrashCurrent, testKeyName);
 
+    // Call moveToTrash. We can't call protected fs.rename() directly
+    trash.moveToTrash(path);
+    // Added this assertion here and will be tested as part of testTrash
+    // test case which needs to be tested with separate mini cluster having
+    // emptier thread started with close match of timings of relevant
+    // assertion statements and corresponding trash and checkpoint interval.
+    Assert.assertTrue(o3fs.exists(userTrashCurrent));
+
     // Wait until the TrashEmptier purges the key
     GenericTestUtils.waitFor(() -> {
       try {
@@ -1675,7 +1687,7 @@ public void testTrash() throws Exception {
         Assert.fail("Delete from Trash Failed");
         return false;
       }
-    }, 1000, 120000);
+    }, 100, 120000);
 
     // userTrash path will contain the checkpoint folder
     FileStatus[] statusList = fs.listStatus(userTrash);