HDDS-13136. KeyDeleting Service should not run for already deep cleaned snapshots

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

@@ -152,7 +152,7 @@ public void setCheckpointDir(String checkpointDir) {
     this.checkpointDir = checkpointDir;
   }
 
-  public boolean getDeepClean() {
+  public boolean isDeepCleaned() {
     return deepClean;
   }
 
@@ -627,7 +627,7 @@ public long getExclusiveReplicatedSize() {
     return exclusiveReplicatedSize;
   }
 
-  public boolean getDeepCleanedDeletedDir() {
+  public boolean isDeepCleanedDeletedDir() {
     return deepCleanedDeletedDir;
   }
 

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmSnapshotInfo.java

@@ -169,8 +169,8 @@ public void testSnapshotInfoProtoToSnapshotInfo() {
         snapshotInfoActual.getSnapshotStatus());
     assertEquals(snapshotInfoExpected.getDbTxSequenceNumber(),
         snapshotInfoActual.getDbTxSequenceNumber());
-    assertEquals(snapshotInfoExpected.getDeepClean(),
-        snapshotInfoActual.getDeepClean());
+    assertEquals(snapshotInfoExpected.isDeepCleaned(),
+        snapshotInfoActual.isDeepCleaned());
     assertEquals(snapshotInfoExpected.isSstFiltered(),
         snapshotInfoActual.isSstFiltered());
     assertEquals(snapshotInfoExpected.getReferencedSize(),
@@ -181,8 +181,8 @@ public void testSnapshotInfoProtoToSnapshotInfo() {
         snapshotInfoActual.getExclusiveSize());
     assertEquals(snapshotInfoExpected.getExclusiveReplicatedSize(),
         snapshotInfoActual.getExclusiveReplicatedSize());
-    assertEquals(snapshotInfoExpected.getDeepCleanedDeletedDir(),
-        snapshotInfoActual.getDeepCleanedDeletedDir());
+    assertEquals(snapshotInfoExpected.isDeepCleanedDeletedDir(),
+        snapshotInfoActual.isDeepCleanedDeletedDir());
     assertEquals(snapshotInfoExpected.getExclusiveSizeDeltaFromDirDeepCleaning(),
         snapshotInfoActual.getExclusiveSizeDeltaFromDirDeepCleaning());
     assertEquals(snapshotInfoExpected.getExclusiveReplicatedSizeDeltaFromDirDeepCleaning(),

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java

@@ -172,11 +172,12 @@ public void setKeyLimitPerTask(int keyLimitPerTask) {
    * the blocks info in its deletedBlockLog), it removes these keys from the
    * DB.
    */
-  private final class KeyDeletingTask implements BackgroundTask {
+  @VisibleForTesting
+  final class KeyDeletingTask implements BackgroundTask {
     private final KeyDeletingService deletingService;
     private final UUID snapshotId;
 
-    private KeyDeletingTask(KeyDeletingService service, UUID snapshotId) {
+    KeyDeletingTask(KeyDeletingService service, UUID snapshotId) {
       this.deletingService = service;
       this.snapshotId = snapshotId;
     }
@@ -321,12 +322,17 @@ public BackgroundTaskResult call() {
           snapInfo = snapshotId == null ? null :
               SnapshotUtils.getSnapshotInfo(getOzoneManager(), snapshotChainManager, snapshotId);
           if (snapInfo != null) {
+            if (snapInfo.isDeepCleaned()) {
+              LOG.info("Snapshot {} has already been deep cleaned. Skipping the snapshot in this iteration.",
+                  snapInfo.getSnapshotId());
+              return EmptyTaskResult.newResult();
+            }
             if (!OmSnapshotManager.areSnapshotChangesFlushedToDB(getOzoneManager().getMetadataManager(), snapInfo)) {
               LOG.info("Skipping snapshot processing since changes to snapshot {} have not been flushed to disk",
                   snapInfo);
               return EmptyTaskResult.newResult();
             }
-            if (!snapInfo.getDeepCleanedDeletedDir()) {
+            if (!snapInfo.isDeepCleanedDeletedDir()) {
               LOG.debug("Snapshot {} hasn't done deleted directory deep cleaning yet. Skipping the snapshot in this" +
                   " iteration.", snapInfo);
               return EmptyTaskResult.newResult();

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDirectoryCleaningService.java

@@ -148,7 +148,7 @@ public BackgroundTaskResult call() {
           // Expand deleted dirs only on active snapshot. Deleted Snapshots
           // will be cleaned up by SnapshotDeletingService.
           if (currSnapInfo == null || currSnapInfo.getSnapshotStatus() != SNAPSHOT_ACTIVE ||
-              currSnapInfo.getDeepCleanedDeletedDir()) {
+              currSnapInfo.isDeepCleanedDeletedDir()) {
             continue;
           }
 

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java

@@ -479,8 +479,8 @@ private void validateSnapshotOrderInSnapshotInfoTableAndSnapshotChain(
     }
     for (SnapshotInfo snapshotInfo : snapshotInfoList) {
       assertEquals(snapshotInfo.getLastTransactionInfo(), expectedTransactionInfos.get(snapshotInfo.getSnapshotId()));
-      assertEquals(snapshotInfo.getDeepClean(), expectedDeepCleanFlags.get(snapshotInfo.getSnapshotId()));
-      assertEquals(snapshotInfo.getDeepCleanedDeletedDir(), expectedDeepCleanFlags.get(snapshotInfo.getSnapshotId()));
+      assertEquals(snapshotInfo.isDeepCleaned(), expectedDeepCleanFlags.get(snapshotInfo.getSnapshotId()));
+      assertEquals(snapshotInfo.isDeepCleanedDeletedDir(), expectedDeepCleanFlags.get(snapshotInfo.getSnapshotId()));
     }
     OmMetadataManagerImpl metadataManager =
         (OmMetadataManagerImpl) getOmMetadataManager();

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java

@@ -31,11 +31,14 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableMap;
@@ -48,18 +51,22 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.hadoop.hdds.client.BlockID;
 import org.apache.hadoop.hdds.client.RatisReplicationConfig;
 import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
 import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
 import org.apache.hadoop.hdds.utils.db.DBConfigFromFile;
 import org.apache.hadoop.hdds.utils.db.Table;
 import org.apache.hadoop.hdds.utils.db.TableIterator;
@@ -74,6 +81,7 @@
 import org.apache.hadoop.ozone.om.OzoneManager;
 import org.apache.hadoop.ozone.om.PendingKeysDeletion;
 import org.apache.hadoop.ozone.om.ScmBlockLocationTestingClient;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
 import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 import org.apache.hadoop.ozone.om.helpers.KeyInfoWithVolumeContext;
 import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
@@ -583,6 +591,42 @@ void testSnapshotDeepClean() throws Exception {
       }
     }
 
+    @Test
+    @DisplayName("KeyDeletingService should skip active snapshot retrieval for deep cleaned snapshots")
+    public void testKeyDeletingServiceWithDeepCleanedSnapshots() throws Exception {
+      OzoneManager ozoneManager = Mockito.spy(om);
+      OmMetadataManagerImpl omMetadataManager = Mockito.mock(OmMetadataManagerImpl.class);
+      SnapshotChainManager snapshotChainManager = Mockito.mock(SnapshotChainManager.class);
+      OmSnapshotManager omSnapshotManager = Mockito.mock(OmSnapshotManager.class);
+      when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
+      when(ozoneManager.getOmSnapshotManager()).thenReturn(omSnapshotManager);
+      when(omMetadataManager.getSnapshotChainManager()).thenReturn(snapshotChainManager);
+      when(snapshotChainManager.getTableKey(any(UUID.class)))
+          .thenAnswer(i -> i.getArgument(0).toString());
+      Table snapshotInfoTable = Mockito.mock(Table.class);
+      when(omMetadataManager.getSnapshotInfoTable()).thenReturn(snapshotInfoTable);
+      when(snapshotInfoTable.get(any(String.class))).thenAnswer(i -> {
+        SnapshotInfo snapshotInfo = Mockito.mock(SnapshotInfo.class);
+        when(snapshotInfo.getSnapshotId()).thenReturn(UUID.fromString(i.getArgument(0)));
+        when(snapshotInfo.isDeepCleaned()).thenReturn(true);
+        return snapshotInfo;
+      });
+      List<UUID> snapshotIds = IntStream.range(0, 10).mapToObj(i -> UUID.randomUUID()).collect(Collectors.toList());
+      when(snapshotChainManager.iterator(anyBoolean())).thenAnswer(i -> snapshotIds.iterator());
+      KeyDeletingService kds = Mockito.spy(new KeyDeletingService(ozoneManager, scmBlockTestingClient, 10000,
+          100000, conf, 10, true));
+      when(kds.getTasks()).thenAnswer(i -> {
+        BackgroundTaskQueue queue = new BackgroundTaskQueue();
+        for (UUID id : snapshotIds) {
+          queue.add(kds.new KeyDeletingTask(kds, id));
+        }
+        return queue;
+      });
+      kds.runPeriodicalTaskNow();
+      clearInvocations(omSnapshotManager);
+      verify(omSnapshotManager, Mockito.never()).getActiveSnapshot(any(), any(), any());
+    }
+
     @Test
     void testSnapshotExclusiveSize() throws Exception {
       Table<String, SnapshotInfo> snapshotInfoTable =
@@ -860,7 +904,7 @@ private static void checkSnapDeepCleanStatus(Table<String, SnapshotInfo> table,
       while (iterator.hasNext()) {
         SnapshotInfo snapInfo = iterator.next().getValue();
         if (volumeName.equals(snapInfo.getVolumeName())) {
-          assertThat(snapInfo.getDeepClean())
+          assertThat(snapInfo.isDeepCleaned())
               .as(snapInfo.toAuditMap().toString())
               .isEqualTo(deepClean);
         }