HDDS-13034. Refactor DirectoryDeletingService to use ReclaimableDirFilter and ReclaimableKeyFilter

hadoop-hdds/common/src/main/resources/ozone-default.xml

@@ -3786,18 +3786,18 @@
   <property>
     <name>ozone.snapshot.directory.service.timeout</name>
     <value>300s</value>
-    <tag>OZONE, PERFORMANCE, OM</tag>
+    <tag>OZONE, PERFORMANCE, OM, DEPRECATED</tag>
     <description>
-      Timeout value for SnapshotDirectoryCleaningService.
+      DEPRECATED. Timeout value for SnapshotDirectoryCleaningService.
     </description>
   </property>
 
   <property>
     <name>ozone.snapshot.directory.service.interval</name>
     <value>24h</value>
-    <tag>OZONE, PERFORMANCE, OM</tag>
+    <tag>OZONE, PERFORMANCE, OM, DEPRECATED</tag>
     <description>
-      The time interval between successive SnapshotDirectoryCleaningService
+      DEPRECATED. The time interval between successive SnapshotDirectoryCleaningService
       thread run.
     </description>
   </property>

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java

@@ -387,12 +387,20 @@ public final class OMConfigKeys {
    */
   public static final String OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED = "ozone.snapshot.deep.cleaning.enabled";
   public static final boolean OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT = false;
+  /**
+   * DirectoryDeepCleaning snapshots have been moved from SnapshotDirectoryCleaningService to DirectoryDeletingService.
+   * Configs related to SnapshotDirectoryCleaningService are deprecated as this won't be used anywhere.
+   */
+  @Deprecated
   public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL =
       "ozone.snapshot.directory.service.interval";
+  @Deprecated
   public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL_DEFAULT
       = "24h";
+  @Deprecated
   public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT =
       "ozone.snapshot.directory.service.timeout";
+  @Deprecated
   public static final String
       OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT_DEFAULT = "300s";
 

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

@@ -79,6 +79,8 @@
 import org.apache.hadoop.ozone.om.request.file.OMFileRequest;
 import org.apache.hadoop.ozone.om.service.DirectoryDeletingService;
 import org.apache.hadoop.ozone.om.service.KeyDeletingService;
+import org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableDirFilter;
+import org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableKeyFilter;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
 import org.apache.ozone.test.GenericTestUtils;
 import org.junit.jupiter.api.AfterAll;
@@ -275,11 +277,9 @@ public void testDeleteWithLargeSubPathsThanBatchSize() throws Exception {
     assertEquals(18, metrics.getNumSubDirsMovedToDeletedDirTable());
     assertEquals(18, metrics.getNumSubDirsSentForPurge());
 
-
-    long elapsedRunCount = dirDeletingService.getRunCount().get() - preRunCount;
     assertThat(dirDeletingService.getRunCount().get()).isGreaterThan(1);
     // Ensure dir deleting speed, here provide a backup value for safe CI
-    assertThat(elapsedRunCount).isGreaterThanOrEqualTo(7);
+    GenericTestUtils.waitFor(() -> dirDeletingService.getRunCount().get() - preRunCount >= 7, 1000, 100000);
   }
 
   @Test
@@ -592,8 +592,7 @@ public void testAOSKeyDeletingWithSnapshotCreateParallelExecution()
     OmSnapshotManager omSnapshotManager = Mockito.spy(ozoneManager.getOmSnapshotManager());
     when(ozoneManager.getOmSnapshotManager()).thenAnswer(i -> omSnapshotManager);
     DirectoryDeletingService service = Mockito.spy(new DirectoryDeletingService(1000, TimeUnit.MILLISECONDS, 1000,
-        ozoneManager,
-        cluster.getConf(), 1));
+        ozoneManager, cluster.getConf(), 1, false));
     service.shutdown();
     final int initialSnapshotCount =
         (int) cluster.getOzoneManager().getMetadataManager().countRowsInTable(snapshotInfoTable);
@@ -627,7 +626,8 @@ public void testAOSKeyDeletingWithSnapshotCreateParallelExecution()
       }
       return null;
     }).when(service).optimizeDirDeletesAndSubmitRequest(anyLong(), anyLong(),
-        anyLong(), anyList(), anyList(), eq(null), anyLong(), anyLong(), Mockito.any(), any(),
+        anyLong(), anyList(), anyList(), eq(null), anyLong(), anyLong(), any(),
+        any(ReclaimableDirFilter.class), any(ReclaimableKeyFilter.class), any(),
         anyLong());
 
     Mockito.doAnswer(i -> {
@@ -651,8 +651,8 @@ public void testAOSKeyDeletingWithSnapshotCreateParallelExecution()
         }
       }, 1000, 10000);
       return i.callRealMethod();
-    }).when(omSnapshotManager).getSnapshot(ArgumentMatchers.eq(testVolumeName), ArgumentMatchers.eq(testBucketName),
-        ArgumentMatchers.eq(snap1));
+    }).when(omSnapshotManager).getActiveSnapshot(ArgumentMatchers.eq(testVolumeName),
+        ArgumentMatchers.eq(testBucketName), ArgumentMatchers.eq(snap1));
     assertTableRowCount(snapshotInfoTable, initialSnapshotCount + 1);
     service.runPeriodicalTaskNow();
     service.runPeriodicalTaskNow();
@@ -729,7 +729,6 @@ public void testDirDeletedTableCleanUpForSnapshot() throws Exception {
     DirectoryDeletingService dirDeletingService =
         (DirectoryDeletingService) cluster.getOzoneManager().getKeyManager()
             .getDirDeletingService();
-
     // After delete. 5 more files left out under the root dir
     assertTableRowCount(keyTable, 5);
     assertTableRowCount(dirTable, 5);
@@ -749,30 +748,29 @@ public void testDirDeletedTableCleanUpForSnapshot() throws Exception {
     assertSubPathsCount(dirDeletingService::getMovedFilesCount, 0);
     assertSubPathsCount(dirDeletingService::getMovedDirsCount, 0);
     assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 0);
-
     // Case-2) Delete dir
     fs.delete(root, true);
 
     // After delete. 5 sub files are still in keyTable.
-    // 4 dirs in dirTable.
+    // 0 dirs in dirTable.
     assertTableRowCount(keyTable, 5);
-    assertTableRowCount(dirTable, 4);
+    assertTableRowCount(dirTable, 0);
 
     // KeyDeletingService and DirectoryDeletingService will not
     // clean up because the paths are part of a snapshot.
-    // As a result on 1 deleted dir and 3 deleted files will
+    // As a result on 5 deleted dir and 3 deleted files will
     // remain in dirTable and keyTable respectively.
     long prevDDSRunCount = dirDeletingService.getRunCount().get();
     long prevKDSRunCount = keyDeletingService.getRunCount().get();
-    assertTableRowCount(deletedDirTable, 1);
+    assertTableRowCount(deletedDirTable, 5);
     assertTableRowCount(deletedKeyTable, 3);
     GenericTestUtils.waitFor(() -> dirDeletingService.getRunCount().get() >
         prevDDSRunCount, 100, 10000);
     GenericTestUtils.waitFor(() -> keyDeletingService.getRunCount().get() >
         prevKDSRunCount, 100, 10000);
 
     assertSubPathsCount(dirDeletingService::getMovedFilesCount, 0);
-    assertSubPathsCount(dirDeletingService::getMovedDirsCount, 0);
+    assertSubPathsCount(dirDeletingService::getMovedDirsCount, 4);
     assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 0);
 
     // Manual cleanup deletedDirTable for next tests

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDeletingServiceIntegrationTest.java

@@ -127,8 +127,7 @@ public void setup() throws Exception {
         500, TimeUnit.MILLISECONDS);
     conf.setBoolean(OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED, true);
     conf.setTimeDuration(OZONE_SNAPSHOT_DELETING_SERVICE_TIMEOUT,
-        10000, TimeUnit.MILLISECONDS);
-    conf.setInt(OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL, 500);
+        500, TimeUnit.MILLISECONDS);
     conf.setInt(OMConfigKeys.OZONE_DIR_DELETING_SERVICE_INTERVAL, 500);
     conf.setTimeDuration(OZONE_BLOCK_DELETING_SERVICE_INTERVAL, 500,
         TimeUnit.MILLISECONDS);
@@ -251,14 +250,18 @@ public void testSnapshotWithFSO() throws Exception {
         om.getMetadataManager().getDeletedDirTable();
     Table<String, String> renamedTable =
         om.getMetadataManager().getSnapshotRenamedTable();
-
     BucketArgs bucketArgs = new BucketArgs.Builder()
         .setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED)
         .build();
-
     OzoneBucket bucket2 = TestDataUtil.createBucket(
         client, VOLUME_NAME, bucketArgs, BUCKET_NAME_FSO);
 
+    assertTableRowCount(snapshotInfoTable, 0);
+    assertTableRowCount(deletedDirTable, 0);
+    assertTableRowCount(deletedTable, 0);
+
+    om.getKeyManager().getDirDeletingService().suspend();
+    om.getKeyManager().getDeletingService().suspend();
     // Create 10 keys
     for (int i = 1; i <= 10; i++) {
       TestDataUtil.createKey(bucket2, "key" + i, CONTENT.array());
@@ -382,8 +385,35 @@ public void testSnapshotWithFSO() throws Exception {
     SnapshotInfo deletedSnap = om.getMetadataManager()
         .getSnapshotInfoTable().get("/vol1/bucketfso/snap2");
 
+    om.getKeyManager().getDirDeletingService().resume();
+    om.getKeyManager().getDeletingService().resume();
+    for (int i = 1; i <= 3; i++) {
+      String snapshotName = "snap" + i;
+      GenericTestUtils.waitFor(() -> {
+        try {
+          SnapshotInfo snap = om.getMetadataManager().getSnapshotInfo(VOLUME_NAME, BUCKET_NAME_FSO, snapshotName);
+          LOG.info("SnapshotInfo for {} is {}", snapshotName, snap.getSnapshotId());
+          return snap.isDeepCleaned() && snap.isDeepCleanedDeletedDir();
+        } catch (IOException e) {
+          throw new RuntimeException(e);
+        }
+      }, 2000, 100000);
+    }
+    om.getKeyManager().getDirDeletingService().suspend();
+    om.getKeyManager().getDeletingService().suspend();
+
+    OmSnapshot snap2 = om.getOmSnapshotManager()
+        .getSnapshot(VOLUME_NAME, BUCKET_NAME_FSO, "snap2").get();
+    //Child directories should have moved to deleted Directory table to deleted directory table of snap2
+    assertTableRowCount(dirTable, 0);
+    assertTableRowCount(keyTable, 11);
+    assertTableRowCount(snap2.getMetadataManager().getDeletedDirTable(), 12);
+    assertTableRowCount(snap2.getMetadataManager().getDeletedTable(), 11);
+
     client.getObjectStore().deleteSnapshot(VOLUME_NAME, BUCKET_NAME_FSO,
         "snap2");
+
+
     assertTableRowCount(snapshotInfoTable, 2);
 
     // Delete 2 overwritten keys
@@ -407,7 +437,28 @@ public void testSnapshotWithFSO() throws Exception {
         snap3.getMetadataManager().getDeletedTable();
 
     assertTableRowCount(snapRenamedTable, 4);
-    assertTableRowCount(snapDeletedDirTable, 3);
+    assertTableRowCount(snapDeletedDirTable, 12);
+    // All the keys deleted before snapshot2 is moved to snap3
+    assertTableRowCount(snapDeletedTable, 18);
+
+    om.getKeyManager().getDirDeletingService().resume();
+    om.getKeyManager().getDeletingService().resume();
+    for (int snapshotIndex : new int[] {1, 3}) {
+      String snapshotName = "snap" + snapshotIndex;
+      GenericTestUtils.waitFor(() -> {
+        try {
+          SnapshotInfo snap = om.getMetadataManager().getSnapshotInfo(VOLUME_NAME, BUCKET_NAME_FSO, snapshotName);
+          return snap.isDeepCleaned() && snap.isDeepCleanedDeletedDir();
+        } catch (IOException e) {
+          throw new RuntimeException(e);
+        }
+      }, 2000, 100000);
+    }
+    om.getKeyManager().getDirDeletingService().suspend();
+    om.getKeyManager().getDeletingService().suspend();
+
+    assertTableRowCount(snapRenamedTable, 4);
+    assertTableRowCount(snapDeletedDirTable, 12);
     // All the keys deleted before snapshot2 is moved to snap3
     assertTableRowCount(snapDeletedTable, 15);
 
@@ -418,11 +469,13 @@ public void testSnapshotWithFSO() throws Exception {
     // Delete Snapshot3 and check entries moved to active DB
     client.getObjectStore().deleteSnapshot(VOLUME_NAME, BUCKET_NAME_FSO,
         "snap3");
-
+    om.getKeyManager().getDirDeletingService().resume();
+    om.getKeyManager().getDeletingService().resume();
     // Check entries moved to active DB
     assertTableRowCount(snapshotInfoTable, 1);
     assertTableRowCount(renamedTable, 4);
-    assertTableRowCount(deletedDirTable, 3);
+    assertTableRowCount(deletedDirTable, 12);
+    assertTableRowCount(deletedTable, 15);
 
     UncheckedAutoCloseableSupplier<OmSnapshot> rcSnap1 =
         om.getOmSnapshotManager().getSnapshot(
@@ -469,10 +522,12 @@ private DirectoryDeletingService getMockedDirectoryDeletingService(AtomicBoolean
       throws InterruptedException, TimeoutException, IOException {
     OzoneManager ozoneManager = Mockito.spy(om);
     om.getKeyManager().getDirDeletingService().shutdown();
+    KeyManager keyManager = Mockito.spy(om.getKeyManager());
+    when(ozoneManager.getKeyManager()).thenReturn(keyManager);
     GenericTestUtils.waitFor(() -> om.getKeyManager().getDirDeletingService().getThreadCount() == 0, 1000,
         100000);
     DirectoryDeletingService directoryDeletingService = Mockito.spy(new DirectoryDeletingService(10000,
-        TimeUnit.MILLISECONDS, 100000, ozoneManager, cluster.getConf(), 1));
+        TimeUnit.MILLISECONDS, 100000, ozoneManager, cluster.getConf(), 1, false));
     directoryDeletingService.shutdown();
     GenericTestUtils.waitFor(() -> directoryDeletingService.getThreadCount() == 0, 1000,
         100000);
@@ -481,7 +536,7 @@ private DirectoryDeletingService getMockedDirectoryDeletingService(AtomicBoolean
       GenericTestUtils.waitFor(dirDeletionWaitStarted::get, 1000, 100000);
       dirDeletionStarted.set(true);
       return i.callRealMethod();
-    }).when(directoryDeletingService).getPendingDeletedDirInfo();
+    }).when(keyManager).getDeletedDirEntries();
     return directoryDeletingService;
   }
 

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java

@@ -24,6 +24,7 @@
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -44,12 +45,14 @@
 import org.apache.hadoop.ozone.client.OzoneBucket;
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
 import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
-import org.apache.hadoop.ozone.om.service.SnapshotDirectoryCleaningService;
+import org.apache.hadoop.ozone.om.service.DirectoryDeletingService;
 import org.apache.ozone.test.GenericTestUtils;
 import org.apache.ozone.test.tag.Flaky;
 import org.junit.jupiter.api.AfterAll;
@@ -76,7 +79,7 @@ public class TestSnapshotDirectoryCleaningService {
   @BeforeAll
   public static void init() throws Exception {
     OzoneConfiguration conf = new OzoneConfiguration();
-    conf.setInt(OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL, 2500);
+    conf.setInt(OMConfigKeys.OZONE_DIR_DELETING_SERVICE_INTERVAL, 2500);
     conf.setBoolean(OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED, true);
     conf.setTimeDuration(OZONE_BLOCK_DELETING_SERVICE_INTERVAL, 2500,
         TimeUnit.MILLISECONDS);
@@ -140,8 +143,10 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception {
         cluster.getOzoneManager().getMetadataManager().getDeletedTable();
     Table<String, SnapshotInfo> snapshotInfoTable =
         cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable();
-    SnapshotDirectoryCleaningService snapshotDirectoryCleaningService =
-        cluster.getOzoneManager().getKeyManager().getSnapshotDirectoryService();
+    DirectoryDeletingService directoryDeletingService =
+        cluster.getOzoneManager().getKeyManager().getDirDeletingService();
+    SnapshotChainManager snapshotChainManager = ((OmMetadataManagerImpl)cluster.getOzoneManager().getMetadataManager())
+        .getSnapshotChainManager();
 
     /*    DirTable
     /v/b/snapDir
@@ -220,11 +225,9 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception {
     fs.delete(root, true);
     assertTableRowCount(deletedKeyTable, 10);
     client.getObjectStore().createSnapshot(volumeName, bucketName, "snap3");
-    long prevRunCount = snapshotDirectoryCleaningService.getRunCount().get();
-    GenericTestUtils.waitFor(() -> snapshotDirectoryCleaningService.getRunCount().get()
+    long prevRunCount = directoryDeletingService.getRunCount().get();
+    GenericTestUtils.waitFor(() -> directoryDeletingService.getRunCount().get()
         > prevRunCount + 1, 100, 10000);
-
-    Thread.sleep(2000);
     Map<String, Long> expectedSize = new HashMap<String, Long>() {{
       // /v/b/snapDir/appRoot0/parentDir0-2/childFile contribute
       // exclusive size, /v/b/snapDir/appRoot0/parentDir0-2/childFile0-4
@@ -234,11 +237,22 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception {
         put("snap2", 5L);
         put("snap3", 0L);
       }};
+
     try (TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
         iterator = snapshotInfoTable.iterator()) {
       while (iterator.hasNext()) {
         Table.KeyValue<String, SnapshotInfo> snapshotEntry = iterator.next();
         String snapshotName = snapshotEntry.getValue().getName();
+
+        GenericTestUtils.waitFor(() -> {
+          try {
+            SnapshotInfo nextSnapshot = SnapshotUtils.getNextSnapshot(cluster.getOzoneManager(), snapshotChainManager,
+                snapshotEntry.getValue());
+            return nextSnapshot == null || (nextSnapshot.isDeepCleanedDeletedDir() && nextSnapshot.isDeepCleaned());
+          } catch (IOException e) {
+            throw new RuntimeException(e);
+          }
+        }, 1000, 10000);
         SnapshotInfo snapshotInfo = snapshotInfoTable.get(snapshotEntry.getKey());
         assertEquals(expectedSize.get(snapshotName),
             snapshotInfo.getExclusiveSize() + snapshotInfo.getExclusiveSizeDeltaFromDirDeepCleaning());