diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java index 4f4c9038f216..680e80bfd7a2 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java +++ b/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; } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmSnapshotInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmSnapshotInfo.java index eec64b90d45b..98cc035b3c07 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmSnapshotInfo.java +++ b/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(), diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java index 9dba72eb8d6c..d89726fd35ef 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java +++ b/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(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDirectoryCleaningService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDirectoryCleaningService.java index 4c80d1600f77..a14003c2245b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDirectoryCleaningService.java +++ b/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; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java index db870e8480e5..1b97e3c3dbcf 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java +++ b/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(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java index f799d8af6190..68d9306584ae 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java +++ b/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,11 +51,14 @@ 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; @@ -60,6 +66,7 @@ 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 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 snapshotInfoTable = @@ -860,7 +904,7 @@ private static void checkSnapDeepCleanStatus(Table 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); }