From 3dc8029b2c6f1fac4070a6f1f0db2517eb86a739 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 28 Jun 2024 13:46:47 -0700 Subject: [PATCH 1/2] HDDS-11084. Read SstFilteringService config once and reuse it in SnapshotDeletingService --- .../om/service/SnapshotDeletingService.java | 14 ++-- .../service/TestSnapshotDeletingService.java | 81 +++++++++++++++++++ .../om/snapshot/TestOmSnapshotUtils.java | 44 ---------- 3 files changed, 88 insertions(+), 51 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java index 29b2b319532b..bb4fc076bdee 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java +++ b/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()); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java new file mode 100644 index 000000000000..d8b72d8964d0 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java @@ -0,0 +1,81 @@ +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 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)); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java index 190db469c19b..8ab652612f56 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java +++ b/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 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)); - } - } From d0c4be7172cad8c5c24d0fd88d832f78675e5dce Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 28 Jun 2024 16:24:32 -0700 Subject: [PATCH 2/2] Added licensing info --- .../service/TestSnapshotDeletingService.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java index d8b72d8964d0..42da7377ea26 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java @@ -1,5 +1,24 @@ +/* + * 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;