HDDS-11084. Read SstFilteringService config once and reuse it in SnapshotDeletingService

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

@@ -100,6 +100,7 @@ public class SnapshotDeletingService extends AbstractKeyDeletingService {
   private final long snapshotDeletionPerTask;
   private final int keyLimitPerSnapshot;
   private final int ratisByteLimit;
+  private final boolean isSstFilteringServiceEnabled;
 
   public SnapshotDeletingService(long interval, long serviceTimeout,
       OzoneManager ozoneManager, ScmBlockLocationProtocol scmClient)
@@ -127,6 +128,8 @@ public SnapshotDeletingService(long interval, long serviceTimeout,
     this.keyLimitPerSnapshot = conf.getInt(
         OZONE_SNAPSHOT_KEY_DELETING_LIMIT_PER_TASK,
         OZONE_SNAPSHOT_KEY_DELETING_LIMIT_PER_TASK_DEFAULT);
+
+    this.isSstFilteringServiceEnabled = ((KeyManagerImpl) ozoneManager.getKeyManager()).isSstFilteringSvcEnabled();
   }
 
   private class SnapshotDeletingTask implements BackgroundTask {
@@ -153,12 +156,9 @@ public BackgroundTaskResult call() throws InterruptedException {
 
         while (iterator.hasNext() && snapshotLimit > 0) {
           SnapshotInfo snapInfo = iterator.next().getValue();
-          boolean isSstFilteringServiceEnabled =
-              ((KeyManagerImpl) ozoneManager.getKeyManager())
-                  .isSstFilteringSvcEnabled();
 
           // Only Iterate in deleted snapshot
-          if (shouldIgnoreSnapshot(snapInfo, isSstFilteringServiceEnabled)) {
+          if (shouldIgnoreSnapshot(snapInfo)) {
             continue;
           }
 
@@ -591,10 +591,10 @@ public void submitRequest(OMRequest omRequest) {
     }
   }
 
-  public static boolean shouldIgnoreSnapshot(SnapshotInfo snapInfo,
-      boolean isSstFilteringServiceEnabled) {
+  @VisibleForTesting
+  boolean shouldIgnoreSnapshot(SnapshotInfo snapInfo) {
     SnapshotInfo.SnapshotStatus snapshotStatus = snapInfo.getSnapshotStatus();
-    return !(snapshotStatus == SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED)
+    return snapshotStatus != SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED
         || (isSstFilteringServiceEnabled && !snapInfo.isSstFiltered());
   }
 

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

@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package org.apache.hadoop.ozone.om.service;
+
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol;
+import org.apache.hadoop.ozone.om.KeyManagerImpl;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Class to unit test SnapshotDeletingService.
+ */
+@ExtendWith(MockitoExtension.class)
+public class TestSnapshotDeletingService {
+  @Mock
+  private OzoneManager ozoneManager;
+  @Mock
+  private KeyManagerImpl keyManager;
+  @Mock
+  private OmSnapshotManager omSnapshotManager;
+  @Mock
+  private SnapshotChainManager chainManager;
+  @Mock
+  private OmMetadataManagerImpl omMetadataManager;
+  @Mock
+  private ScmBlockLocationProtocol scmClient;
+  private final OzoneConfiguration conf = new OzoneConfiguration();;
+  private final long sdsRunInterval = Duration.ofMillis(1000).toMillis();
+  private final  long sdsServiceTimeout = Duration.ofSeconds(10).toMillis();
+
+
+  private static Stream<Arguments> testCasesForIgnoreSnapshotGc() {
+    SnapshotInfo filteredSnapshot = SnapshotInfo.newBuilder().setSstFiltered(true).setName("snap1").build();
+    SnapshotInfo unFilteredSnapshot = SnapshotInfo.newBuilder().setSstFiltered(false).setName("snap1").build();
+    return Stream.of(
+        Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, true, false),
+        Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true, true),
+        Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, true, true),
+        Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true, true),
+        Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false, false),
+        Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false, false),
+        Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, false, true),
+        Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, false, true));
+  }
+
+  @ParameterizedTest
+  @MethodSource("testCasesForIgnoreSnapshotGc")
+  public void testProcessSnapshotLogicInSDS(SnapshotInfo snapshotInfo,
+                                            SnapshotInfo.SnapshotStatus status,
+                                            boolean sstFilteringServiceEnabled,
+                                            boolean expectedOutcome)
+      throws IOException {
+    Mockito.when(keyManager.isSstFilteringSvcEnabled()).thenReturn(sstFilteringServiceEnabled);
+    Mockito.when(omMetadataManager.getSnapshotChainManager()).thenReturn(chainManager);
+    Mockito.when(ozoneManager.getKeyManager()).thenReturn(keyManager);
+    Mockito.when(ozoneManager.getOmSnapshotManager()).thenReturn(omSnapshotManager);
+    Mockito.when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
+    Mockito.when(ozoneManager.getConfiguration()).thenReturn(conf);
+
+    SnapshotDeletingService snapshotDeletingService =
+        new SnapshotDeletingService(sdsRunInterval, sdsServiceTimeout, ozoneManager, scmClient);
+
+    snapshotInfo.setSnapshotStatus(status);
+    assertEquals(expectedOutcome, snapshotDeletingService.shouldIgnoreSnapshot(snapshotInfo));
+  }
+}

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

@@ -18,20 +18,14 @@
 
 package org.apache.hadoop.ozone.om.snapshot;
 
-import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
-import org.apache.hadoop.ozone.om.service.SnapshotDeletingService;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
 
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Set;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils.getINode;
@@ -82,42 +76,4 @@ public void testLinkFiles(@TempDir File tempDir) throws Exception {
 
     assertEquals(tree1Files, tree2Files);
   }
-
-
-  private static Stream<Arguments> testCasesForIgnoreSnapshotGc() {
-    SnapshotInfo filteredSnapshot =
-        SnapshotInfo.newBuilder().setSstFiltered(true).setName("snap1").build();
-    SnapshotInfo unFilteredSnapshot =
-        SnapshotInfo.newBuilder().setSstFiltered(false).setName("snap1")
-            .build();
-    // {IsSnapshotFiltered,isSnapshotDeleted,IsSstServiceEnabled = ShouldIgnore}
-    return Stream.of(Arguments.of(filteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, true, false),
-        Arguments.of(filteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true, true),
-        Arguments.of(unFilteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, true, true),
-        Arguments.of(unFilteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true, true),
-        Arguments.of(filteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false, false),
-        Arguments.of(unFilteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false, false),
-        Arguments.of(unFilteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, false, true),
-        Arguments.of(filteredSnapshot,
-            SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, false, true));
-  }
-
-  @ParameterizedTest
-  @MethodSource("testCasesForIgnoreSnapshotGc")
-  public void testProcessSnapshotLogicInSDS(SnapshotInfo snapshotInfo,
-      SnapshotInfo.SnapshotStatus status, boolean isSstFilteringSvcEnabled,
-      boolean expectedOutcome) {
-    snapshotInfo.setSnapshotStatus(status);
-    assertEquals(expectedOutcome,
-        SnapshotDeletingService.shouldIgnoreSnapshot(snapshotInfo,
-            isSstFilteringSvcEnabled));
-  }
-
 }