From ae218781fb59d6c97c384a4cbf6457be7d15f2b2 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Sat, 31 May 2025 12:50:23 -0400 Subject: [PATCH 01/12] HDDS-13159. Refactor KeyManagerImpl for getting deleted subdirectories and deleted subFiles Change-Id: Ic1fc709b3963cde14c2a7fb64b687322a29e642a --- .../hadoop/ozone/om/KeyManagerImpl.java | 102 +++++------------- .../ozone/om/request/file/OMFileRequest.java | 16 +++ 2 files changed, 43 insertions(+), 75 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index b399d6bb9ceb..138e640dcda4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -160,6 +160,7 @@ import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.WithParentObjectId; import org.apache.hadoop.ozone.om.request.OMClientRequest; import org.apache.hadoop.ozone.om.request.file.OMFileRequest; import org.apache.hadoop.ozone.om.request.util.OMMultipartUploadUtils; @@ -2196,101 +2197,52 @@ private void slimLocationVersion(OmKeyInfo... keyInfos) { @Override public DeleteKeysResult getPendingDeletionSubDirs(long volumeId, long bucketId, OmKeyInfo parentInfo, long remainingBufLimit) throws IOException { - String seekDirInDB = metadataManager.getOzonePathKey(volumeId, bucketId, - parentInfo.getObjectID(), ""); - long countEntries = 0; - - Table dirTable = metadataManager.getDirectoryTable(); - try (TableIterator> - iterator = dirTable.iterator(seekDirInDB)) { - return gatherSubDirsWithIterator(parentInfo, countEntries, iterator, remainingBufLimit); - } - - } - - private DeleteKeysResult gatherSubDirsWithIterator(OmKeyInfo parentInfo, - long countEntries, - TableIterator> iterator, long remainingBufLimit) - throws IOException { - List directories = new ArrayList<>(); - long consumedSize = 0; - boolean processedSubDirs = false; - - while (iterator.hasNext() && remainingBufLimit > 0) { - Table.KeyValue entry = iterator.next(); - OmDirectoryInfo dirInfo = entry.getValue(); - long objectSerializedSize = entry.getRawSize(); - if (!OMFileRequest.isImmediateChild(dirInfo.getParentObjectID(), - parentInfo.getObjectID())) { - processedSubDirs = true; - break; - } - if (!metadataManager.getDirectoryTable().isExist(entry.getKey())) { - continue; - } - if (remainingBufLimit - objectSerializedSize < 0) { - break; - } - String dirName = OMFileRequest.getAbsolutePath(parentInfo.getKeyName(), - dirInfo.getName()); - OmKeyInfo omKeyInfo = OMFileRequest.getOmKeyInfo( - parentInfo.getVolumeName(), parentInfo.getBucketName(), dirInfo, - dirName); - directories.add(omKeyInfo); - countEntries++; - remainingBufLimit -= objectSerializedSize; - consumedSize += objectSerializedSize; - } - - processedSubDirs = processedSubDirs || (!iterator.hasNext()); - - return new DeleteKeysResult(directories, consumedSize, processedSubDirs); + return gatherSubPathsWithIterator(volumeId, bucketId, parentInfo, metadataManager.getDirectoryTable(), + omDirectoryInfo -> OMFileRequest.getKeyInfoWithFullPath(parentInfo, omDirectoryInfo), remainingBufLimit); } - @Override - public DeleteKeysResult getPendingDeletionSubFiles(long volumeId, - long bucketId, OmKeyInfo parentInfo, long remainingBufLimit) - throws IOException { - List files = new ArrayList<>(); + private DeleteKeysResult gatherSubPathsWithIterator( + long volumeId, long bucketId, OmKeyInfo parentInfo, + Table table, Function deleteKeyTransformer, + long remainingBufLimit) throws IOException { + List keyInfos = new ArrayList<>(); String seekFileInDB = metadataManager.getOzonePathKey(volumeId, bucketId, parentInfo.getObjectID(), ""); long consumedSize = 0; - boolean processedSubFiles = false; - - Table fileTable = metadataManager.getFileTable(); - try (TableIterator> - iterator = fileTable.iterator(seekFileInDB)) { - + boolean processedSubPaths = false; + try (TableIterator> iterator = table.iterator(seekFileInDB)) { while (iterator.hasNext() && remainingBufLimit > 0) { - Table.KeyValue entry = iterator.next(); - OmKeyInfo fileInfo = entry.getValue(); + Table.KeyValue entry = iterator.next(); + T withParentObjectId = entry.getValue(); long objectSerializedSize = entry.getRawSize(); - if (!OMFileRequest.isImmediateChild(fileInfo.getParentObjectID(), + if (!OMFileRequest.isImmediateChild(withParentObjectId.getParentObjectID(), parentInfo.getObjectID())) { - processedSubFiles = true; + processedSubPaths = true; break; } - if (!metadataManager.getFileTable().isExist(entry.getKey())) { + if (!table.isExist(entry.getKey())) { continue; } if (remainingBufLimit - objectSerializedSize < 0) { break; } - fileInfo.setFileName(fileInfo.getKeyName()); - String fullKeyPath = OMFileRequest.getAbsolutePath( - parentInfo.getKeyName(), fileInfo.getKeyName()); - fileInfo.setKeyName(fullKeyPath); - - files.add(fileInfo); + OmKeyInfo keyInfo = deleteKeyTransformer.apply(withParentObjectId); + keyInfos.add(keyInfo); remainingBufLimit -= objectSerializedSize; consumedSize += objectSerializedSize; } - processedSubFiles = processedSubFiles || (!iterator.hasNext()); + processedSubPaths = processedSubPaths || (!iterator.hasNext()); + return new DeleteKeysResult(keyInfos, consumedSize, processedSubPaths); } + } - return new DeleteKeysResult(files, consumedSize, processedSubFiles); + @Override + public DeleteKeysResult getPendingDeletionSubFiles(long volumeId, + long bucketId, OmKeyInfo parentInfo, long remainingBufLimit) + throws IOException { + return gatherSubPathsWithIterator(volumeId, bucketId, parentInfo, metadataManager.getFileTable(), + keyInfo -> OMFileRequest.getKeyInfoWithFullPath(parentInfo, keyInfo), + remainingBufLimit); } public boolean isBucketFSOptimized(String volName, String buckName) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java index f8058bd7a897..75ec1d5b7277 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java @@ -720,6 +720,22 @@ public static OzoneFileStatus getOMKeyInfoIfExists( return null; } + public static OmKeyInfo getKeyInfoWithFullPath(OmKeyInfo parentInfo, OmDirectoryInfo directoryInfo) { + String dirName = OMFileRequest.getAbsolutePath(parentInfo.getKeyName(), + directoryInfo.getName()); + return OMFileRequest.getOmKeyInfo( + parentInfo.getVolumeName(), parentInfo.getBucketName(), directoryInfo, + dirName); + } + + public static OmKeyInfo getKeyInfoWithFullPath(OmKeyInfo parentInfo, OmKeyInfo omKeyInfo) { + omKeyInfo.setFileName(omKeyInfo.getKeyName()); + String fullKeyPath = OMFileRequest.getAbsolutePath( + parentInfo.getKeyName(), omKeyInfo.getKeyName()); + omKeyInfo.setKeyName(fullKeyPath); + return omKeyInfo; + } + /** * Prepare OmKeyInfo from OmDirectoryInfo. * From cd0157b3b782ef30e6397bc84125fcd426ea0e07 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Sat, 31 May 2025 12:56:03 -0400 Subject: [PATCH 02/12] HDDS-13159. Static Import KeyValue Change-Id: I47b24dfc3b5afa3cefbdc85ac7b3e4a9b8c94869 --- .../hadoop/ozone/om/KeyManagerImpl.java | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 138e640dcda4..da080be68cac 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -131,6 +131,7 @@ import org.apache.hadoop.hdds.utils.BackgroundService; import org.apache.hadoop.hdds.utils.db.StringCodec; import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.Table.KeyValue; import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; @@ -735,7 +736,7 @@ public ListKeysResult listKeys(String volumeName, String bucketName, @Override public PendingKeysDeletion getPendingDeletionKeys( - final CheckedFunction, Boolean, IOException> filter, final int count) + final CheckedFunction, Boolean, IOException> filter, final int count) throws IOException { return getPendingDeletionKeys(null, null, null, filter, count); } @@ -743,13 +744,13 @@ public PendingKeysDeletion getPendingDeletionKeys( @Override public PendingKeysDeletion getPendingDeletionKeys( String volume, String bucket, String startKey, - CheckedFunction, Boolean, IOException> filter, + CheckedFunction, Boolean, IOException> filter, int count) throws IOException { List keyBlocksList = Lists.newArrayList(); Map keysToModify = new HashMap<>(); // Bucket prefix would be empty if volume is empty i.e. either null or "". Optional bucketPrefix = getBucketPrefix(volume, bucket, false); - try (TableIterator> + try (TableIterator> delKeyIter = metadataManager.getDeletedTable().iterator(bucketPrefix.orElse(""))) { /* Seeking to the start key if it not null. The next key picked up would be ensured to start with the bucket @@ -761,7 +762,7 @@ public PendingKeysDeletion getPendingDeletionKeys( int currentCount = 0; while (delKeyIter.hasNext() && currentCount < count) { RepeatedOmKeyInfo notReclaimableKeyInfo = new RepeatedOmKeyInfo(); - Table.KeyValue kv = delKeyIter.next(); + KeyValue kv = delKeyIter.next(); if (kv != null) { List blockGroupList = Lists.newArrayList(); // Multiple keys with the same path can be queued in one DB entry @@ -796,12 +797,12 @@ public PendingKeysDeletion getPendingDeletionKeys( return new PendingKeysDeletion(keyBlocksList, keysToModify); } - private List> getTableEntries(String startKey, - TableIterator> tableIterator, + private List> getTableEntries(String startKey, + TableIterator> tableIterator, Function valueFunction, - CheckedFunction, Boolean, IOException> filter, + CheckedFunction, Boolean, IOException> filter, int size) throws IOException { - List> entries = new ArrayList<>(); + List> entries = new ArrayList<>(); /* Seek to the start key if it's not null. The next key in queue is ensured to start with the bucket prefix, {@link org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this. */ @@ -812,7 +813,7 @@ private List> getTableEntries(String startKey, } int currentCount = 0; while (tableIterator.hasNext() && currentCount < size) { - Table.KeyValue kv = tableIterator.next(); + KeyValue kv = tableIterator.next(); if (kv != null && filter.apply(kv)) { entries.add(Table.newKeyValue(kv.getKey(), valueFunction.apply(kv.getValue()))); currentCount++; @@ -834,11 +835,11 @@ private Optional getBucketPrefix(String volumeName, String bucketName, b } @Override - public List> getRenamesKeyEntries( + public List> getRenamesKeyEntries( String volume, String bucket, String startKey, - CheckedFunction, Boolean, IOException> filter, int size) throws IOException { + CheckedFunction, Boolean, IOException> filter, int size) throws IOException { Optional bucketPrefix = getBucketPrefix(volume, bucket, false); - try (TableIterator> + try (TableIterator> renamedKeyIter = metadataManager.getSnapshotRenamedTable().iterator(bucketPrefix.orElse(""))) { return getTableEntries(startKey, renamedKeyIter, Function.identity(), filter, size); } @@ -883,12 +884,12 @@ private CheckedFunction getPreviousSnapshotOzone } @Override - public List>> getDeletedKeyEntries( + public List>> getDeletedKeyEntries( String volume, String bucket, String startKey, - CheckedFunction, Boolean, IOException> filter, + CheckedFunction, Boolean, IOException> filter, int size) throws IOException { Optional bucketPrefix = getBucketPrefix(volume, bucket, false); - try (TableIterator> + try (TableIterator> delKeyIter = metadataManager.getDeletedTable().iterator(bucketPrefix.orElse(""))) { return getTableEntries(startKey, delKeyIter, RepeatedOmKeyInfo::cloneOmKeyInfoList, filter, size); } @@ -1538,10 +1539,10 @@ private OmKeyInfo createFakeDirIfShould(String volume, String bucket, } } - try (TableIterator> + try (TableIterator> keyTblItr = keyTable.iterator(targetKey)) { while (keyTblItr.hasNext()) { - Table.KeyValue keyValue = keyTblItr.next(); + KeyValue keyValue = keyTblItr.next(); if (keyValue != null) { String key = keyValue.getKey(); // HDDS-7871: RocksIterator#seek() may position at the key @@ -1852,7 +1853,7 @@ public List listStatus(OmKeyArgs args, boolean recursive, String keyArgs = OzoneFSUtils.addTrailingSlashIfNeeded( metadataManager.getOzoneKey(volumeName, bucketName, keyName)); - TableIterator> iterator; + TableIterator> iterator; Table keyTable; metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, bucketName); @@ -1909,12 +1910,12 @@ public List listStatus(OmKeyArgs args, boolean recursive, return fileStatusList; } - private TableIterator> + private TableIterator> getIteratorForKeyInTableCache( boolean recursive, String startKey, String volumeName, String bucketName, TreeMap cacheKeyMap, String keyArgs, Table keyTable) throws IOException { - TableIterator> iterator; + TableIterator> iterator; Iterator, CacheValue>> cacheIter = keyTable.cacheIterator(); String startCacheKey = metadataManager.getOzoneKey(volumeName, bucketName, startKey); @@ -1932,12 +1933,12 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, TreeMap cacheKeyMap, String keyArgs, Table keyTable, TableIterator> iterator) + ? extends KeyValue> iterator) throws IOException { // Then, find key in DB String seekKeyInDb = metadataManager.getOzoneKey(volumeName, bucketName, startKey); - Table.KeyValue entry = iterator.seek(seekKeyInDb); + KeyValue entry = iterator.seek(seekKeyInDb); int countEntries = 0; if (iterator.hasNext()) { if (entry.getKey().equals(keyArgs)) { @@ -2188,7 +2189,7 @@ private void slimLocationVersion(OmKeyInfo... keyInfos) { } @Override - public TableIterator> getDeletedDirEntries( + public TableIterator> getDeletedDirEntries( String volume, String bucket) throws IOException { Optional bucketPrefix = getBucketPrefix(volume, bucket, true); return metadataManager.getDeletedDirTable().iterator(bucketPrefix.orElse("")); @@ -2210,9 +2211,9 @@ private DeleteKeysResult gatherSubPathsWithIterat parentInfo.getObjectID(), ""); long consumedSize = 0; boolean processedSubPaths = false; - try (TableIterator> iterator = table.iterator(seekFileInDB)) { + try (TableIterator> iterator = table.iterator(seekFileInDB)) { while (iterator.hasNext() && remainingBufLimit > 0) { - Table.KeyValue entry = iterator.next(); + KeyValue entry = iterator.next(); T withParentObjectId = entry.getValue(); long objectSerializedSize = entry.getRawSize(); if (!OMFileRequest.isImmediateChild(withParentObjectId.getParentObjectID(), From 4c73e3a0853d1affdcb736599a44e4bff275a469 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Mon, 2 Jun 2025 23:36:22 -0400 Subject: [PATCH 03/12] HDDS-13034. Refactor Directory Deleting Service to use ReclaimableDirectoryFilter & ReclaimableKeyFilter Change-Id: Iffdda403cba914ddef2b75808cfbef1a72b9a2d3 --- .../apache/hadoop/ozone/om/OMConfigKeys.java | 8 - .../TestDirectoryDeletingServiceWithFSO.java | 8 +- ...napshotDeletingServiceIntegrationTest.java | 73 +++- .../TestSnapshotDirectoryCleaningService.java | 8 +- .../apache/hadoop/ozone/om/KeyManager.java | 22 +- .../hadoop/ozone/om/KeyManagerImpl.java | 68 ++- .../ozone/om/OMDBCheckpointServlet.java | 2 +- .../service/AbstractKeyDeletingService.java | 83 ++-- .../om/service/DirectoryDeletingService.java | 395 ++++++++++-------- .../ozone/om/service/KeyDeletingService.java | 8 +- .../om/service/TestKeyDeletingService.java | 18 +- 11 files changed, 398 insertions(+), 295 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 12a809043761..242ae03f0ccb 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -387,14 +387,6 @@ 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; - public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL = - "ozone.snapshot.directory.service.interval"; - public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL_DEFAULT - = "24h"; - public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT = - "ozone.snapshot.directory.service.timeout"; - public static final String - OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT_DEFAULT = "300s"; public static final String OZONE_THREAD_NUMBER_DIR_DELETION = "ozone.thread.number.dir.deletion"; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java index 4c6a21f1cbb5..a39aaf565ff7 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java +++ b/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; @@ -592,8 +594,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 +628,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 -> { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDeletingServiceIntegrationTest.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDeletingServiceIntegrationTest.java index 3c7b35dd23ed..73fe9b007ac6 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDeletingServiceIntegrationTest.java +++ b/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); + 10, 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 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, 100000000); + } + 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 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; } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java index 8591c6d1e88b..f854448b1679 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java @@ -49,7 +49,7 @@ 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 +76,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 +140,8 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception { cluster.getOzoneManager().getMetadataManager().getDeletedTable(); Table snapshotInfoTable = cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable(); - SnapshotDirectoryCleaningService snapshotDirectoryCleaningService = - cluster.getOzoneManager().getKeyManager().getSnapshotDirectoryService(); + DirectoryDeletingService snapshotDirectoryCleaningService = + cluster.getOzoneManager().getKeyManager().getDirDeletingService(); /* DirTable /v/b/snapDir diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java index 0af075035704..7e76885c49bd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java @@ -41,7 +41,6 @@ import org.apache.hadoop.ozone.om.service.DirectoryDeletingService; import org.apache.hadoop.ozone.om.service.KeyDeletingService; import org.apache.hadoop.ozone.om.service.SnapshotDeletingService; -import org.apache.hadoop.ozone.om.service.SnapshotDirectoryCleaningService; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.ExpiredMultipartUploadsBucket; import org.apache.ratis.util.function.CheckedFunction; @@ -274,7 +273,14 @@ OmMultipartUploadListParts listParts(String volumeName, String bucketName, void refresh(OmKeyInfo key) throws IOException; /** - * Returns an iterator for pending deleted directories. + * Returns an iterator for pending deleted directories all buckets. + */ + default TableIterator> getDeletedDirEntries() throws IOException { + return getDeletedDirEntries(null, null); + } + + /** + * Returns an iterator for pending deleted directories for volume and bucket. * @throws IOException */ TableIterator> getDeletedDirEntries( @@ -301,7 +307,8 @@ default List> getDeletedDirEntries(String volu * @throws IOException */ DeleteKeysResult getPendingDeletionSubDirs(long volumeId, long bucketId, - OmKeyInfo parentInfo, long remainingBufLimit) throws IOException; + OmKeyInfo parentInfo, CheckedFunction, Boolean, IOException> filter, + long remainingBufLimit) throws IOException; /** * Returns all sub files under the given parent directory. @@ -311,7 +318,8 @@ DeleteKeysResult getPendingDeletionSubDirs(long volumeId, long bucketId, * @throws IOException */ DeleteKeysResult getPendingDeletionSubFiles(long volumeId, - long bucketId, OmKeyInfo parentInfo, long remainingBufLimit) + long bucketId, OmKeyInfo parentInfo, + CheckedFunction, Boolean, IOException> filter, long remainingBufLimit) throws IOException; /** @@ -344,12 +352,6 @@ DeleteKeysResult getPendingDeletionSubFiles(long volumeId, */ SnapshotDeletingService getSnapshotDeletingService(); - /** - * Returns the instance of Snapshot Directory service. - * @return Background service. - */ - SnapshotDirectoryCleaningService getSnapshotDirectoryService(); - /** * Returns the instance of CompactionService. * @return BackgroundService diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index da080be68cac..578afc630a1c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -58,10 +58,6 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_CLEANUP_SERVICE_TIMEOUT_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL_DEFAULT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_THREAD_NUMBER_DIR_DELETION; @@ -171,7 +167,6 @@ import org.apache.hadoop.ozone.om.service.MultipartUploadCleanupService; import org.apache.hadoop.ozone.om.service.OpenKeyCleanupService; import org.apache.hadoop.ozone.om.service.SnapshotDeletingService; -import org.apache.hadoop.ozone.om.service.SnapshotDirectoryCleaningService; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.ExpiredMultipartUploadsBucket; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PartKeyInfo; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; @@ -214,7 +209,6 @@ public class KeyManagerImpl implements KeyManager { private BackgroundService openKeyCleanupService; private BackgroundService multipartUploadCleanupService; - private SnapshotDirectoryCleaningService snapshotDirectoryCleaningService; private DNSToSwitchMapping dnsToSwitchMapping; private CompactionService compactionService; @@ -292,7 +286,7 @@ public void start(OzoneConfiguration configuration) { dirDeletingService = new DirectoryDeletingService(dirDeleteInterval, TimeUnit.MILLISECONDS, serviceTimeout, ozoneManager, configuration, - dirDeletingServiceCorePoolSize); + dirDeletingServiceCorePoolSize, isSnapshotDeepCleaningEnabled); dirDeletingService.start(); } @@ -350,22 +344,6 @@ public void start(OzoneConfiguration configuration) { } } - if (isSnapshotDeepCleaningEnabled && snapshotDirectoryCleaningService == null && - ozoneManager.isFilesystemSnapshotEnabled()) { - long dirDeleteInterval = configuration.getTimeDuration( - OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL, - OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL_DEFAULT, - TimeUnit.MILLISECONDS); - long serviceTimeout = configuration.getTimeDuration( - OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT, - OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT_DEFAULT, - TimeUnit.MILLISECONDS); - snapshotDirectoryCleaningService = new SnapshotDirectoryCleaningService( - dirDeleteInterval, TimeUnit.MILLISECONDS, serviceTimeout, - ozoneManager, scmClient.getBlockClient()); - snapshotDirectoryCleaningService.start(); - } - if (multipartUploadCleanupService == null) { long serviceInterval = configuration.getTimeDuration( OZONE_OM_MPU_CLEANUP_SERVICE_INTERVAL, @@ -443,10 +421,6 @@ public void stop() throws IOException { multipartUploadCleanupService.shutdown(); multipartUploadCleanupService = null; } - if (snapshotDirectoryCleaningService != null) { - snapshotDirectoryCleaningService.shutdown(); - snapshotDirectoryCleaningService = null; - } if (compactionService != null) { compactionService.shutdown(); compactionService = null; @@ -955,11 +929,6 @@ public SnapshotDeletingService getSnapshotDeletingService() { return snapshotDeletingService; } - @Override - public SnapshotDirectoryCleaningService getSnapshotDirectoryService() { - return snapshotDirectoryCleaningService; - } - @Override public CompactionService getCompactionService() { return compactionService; @@ -2197,14 +2166,19 @@ private void slimLocationVersion(OmKeyInfo... keyInfos) { @Override public DeleteKeysResult getPendingDeletionSubDirs(long volumeId, long bucketId, - OmKeyInfo parentInfo, long remainingBufLimit) throws IOException { + OmKeyInfo parentInfo, CheckedFunction, Boolean, IOException> filter, + long remainingBufLimit) throws IOException { return gatherSubPathsWithIterator(volumeId, bucketId, parentInfo, metadataManager.getDirectoryTable(), - omDirectoryInfo -> OMFileRequest.getKeyInfoWithFullPath(parentInfo, omDirectoryInfo), remainingBufLimit); + kv -> Table.newKeyValue(metadataManager.getOzoneDeletePathKey(kv.getValue().getObjectID(), kv.getKey()), + OMFileRequest.getKeyInfoWithFullPath(parentInfo, kv.getValue())), + filter, remainingBufLimit); } private DeleteKeysResult gatherSubPathsWithIterator( long volumeId, long bucketId, OmKeyInfo parentInfo, - Table table, Function deleteKeyTransformer, + Table table, + CheckedFunction, KeyValue, IOException> deleteKeyTransformer, + CheckedFunction, Boolean, IOException> deleteKeyFilter, long remainingBufLimit) throws IOException { List keyInfos = new ArrayList<>(); String seekFileInDB = metadataManager.getOzonePathKey(volumeId, bucketId, @@ -2227,10 +2201,12 @@ private DeleteKeysResult gatherSubPathsWithIterat if (remainingBufLimit - objectSerializedSize < 0) { break; } - OmKeyInfo keyInfo = deleteKeyTransformer.apply(withParentObjectId); - keyInfos.add(keyInfo); - remainingBufLimit -= objectSerializedSize; - consumedSize += objectSerializedSize; + KeyValue keyInfo = deleteKeyTransformer.apply(entry); + if (deleteKeyFilter.apply(keyInfo)) { + keyInfos.add(keyInfo.getValue()); + remainingBufLimit -= objectSerializedSize; + consumedSize += objectSerializedSize; + } } processedSubPaths = processedSubPaths || (!iterator.hasNext()); return new DeleteKeysResult(keyInfos, consumedSize, processedSubPaths); @@ -2239,11 +2215,17 @@ private DeleteKeysResult gatherSubPathsWithIterat @Override public DeleteKeysResult getPendingDeletionSubFiles(long volumeId, - long bucketId, OmKeyInfo parentInfo, long remainingBufLimit) + long bucketId, OmKeyInfo parentInfo, + CheckedFunction, Boolean, IOException> filter, long remainingBufLimit) throws IOException { - return gatherSubPathsWithIterator(volumeId, bucketId, parentInfo, metadataManager.getFileTable(), - keyInfo -> OMFileRequest.getKeyInfoWithFullPath(parentInfo, keyInfo), - remainingBufLimit); + CheckedFunction, KeyValue, IOException> tranformer = kv -> { + OmKeyInfo keyInfo = OMFileRequest.getKeyInfoWithFullPath(parentInfo, kv.getValue()); + String deleteKey = metadataManager.getOzoneDeletePathKey(keyInfo.getObjectID(), + metadataManager.getOzoneKey(keyInfo.getVolumeName(), keyInfo.getBucketName(), keyInfo.getKeyName())); + return Table.newKeyValue(deleteKey, keyInfo); + }; + return gatherSubPathsWithIterator(volumeId, bucketId, parentInfo, metadataManager.getFileTable(), tranformer, + filter, remainingBufLimit); } public boolean isBucketFSOptimized(String volName, String buckName) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java index 6117a7e373bf..bcff75fd0399 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java @@ -704,9 +704,9 @@ static class Lock extends BootstrapStateHandler.Lock { locks = Stream.of( om.getKeyManager().getDeletingService(), + om.getKeyManager().getDirDeletingService(), om.getKeyManager().getSnapshotSstFilteringService(), om.getKeyManager().getSnapshotDeletingService(), - om.getKeyManager().getSnapshotDirectoryService(), om.getMetadataManager().getStore().getRocksDBCheckpointDiffer() ) .filter(Objects::nonNull) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java index 536406111a96..ee699e16c31d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java @@ -28,11 +28,13 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -58,12 +60,14 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeletedKeys; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PurgeKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PurgePathRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SnapshotMoveKeyInfos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.hadoop.util.Time; import org.apache.ratis.protocol.ClientId; +import org.apache.ratis.util.function.CheckedFunction; /** * Abstracts common code from KeyDeletingService and DirectoryDeletingService @@ -103,7 +107,7 @@ public AbstractKeyDeletingService(String serviceName, long interval, protected Pair processKeyDeletes(List keyBlocksList, Map keysToModify, List renameEntries, - String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException, InterruptedException { + String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException { long startTime = Time.monotonicNow(); Pair purgeResult = Pair.of(0, false); @@ -143,7 +147,7 @@ protected Pair processKeyDeletes(List keyBlocksLis */ private Pair submitPurgeKeysRequest(List results, Map keysToModify, List renameEntriesToBeDeleted, - String snapTableKey, UUID expectedPreviousSnapshotId) throws InterruptedException { + String snapTableKey, UUID expectedPreviousSnapshotId) { List purgeKeys = new ArrayList<>(); // Put all keys to be purged in a list @@ -224,13 +228,13 @@ private Pair submitPurgeKeysRequest(List submitPurgeKeysRequest(List requests, - String snapTableKey, - UUID expectedPreviousSnapshotId) { + protected OMResponse submitPurgePaths(List requests, + String snapTableKey, UUID expectedPreviousSnapshotId) { OzoneManagerProtocolProtos.PurgeDirectoriesRequest.Builder purgeDirRequest = OzoneManagerProtocolProtos.PurgeDirectoriesRequest.newBuilder(); @@ -267,12 +270,13 @@ protected void submitPurgePaths(List requests, .setClientId(clientId.toString()) .build(); - // Submit Purge paths request to OM - try { - submitRequest(omRequest); - } catch (ServiceException e) { + // Submit Purge paths request to OM. Acquire bootstrap lock when processing deletes for snapshots. + try (BootstrapStateHandler.Lock lock = snapTableKey != null ? getBootstrapStateLock().lock() : null) { + return submitRequest(omRequest); + } catch (ServiceException | InterruptedException e) { LOG.error("PurgePaths request failed. Will retry at next run.", e); } + return null; } private OzoneManagerProtocolProtos.PurgePathRequest wrapPurgeRequest( @@ -305,10 +309,12 @@ private OzoneManagerProtocolProtos.PurgePathRequest wrapPurgeRequest( return purgePathsRequest.build(); } - protected PurgePathRequest prepareDeleteDirRequest( - OmKeyInfo pendingDeletedDirInfo, String delDirName, + protected Optional prepareDeleteDirRequest( + OmKeyInfo pendingDeletedDirInfo, String delDirName, boolean purgeDir, List> subDirList, - KeyManager keyManager, long remainingBufLimit) throws IOException { + KeyManager keyManager, + CheckedFunction, Boolean, IOException> reclaimableFileFilter, + long remainingBufLimit) throws IOException { // step-0: Get one pending deleted directory if (LOG.isDebugEnabled()) { LOG.debug("Pending deleted dir name: {}", @@ -322,7 +328,7 @@ protected PurgePathRequest prepareDeleteDirRequest( // step-1: get all sub directories under the deletedDir DeleteKeysResult subDirDeleteResult = keyManager.getPendingDeletionSubDirs(volumeId, bucketId, - pendingDeletedDirInfo, remainingBufLimit); + pendingDeletedDirInfo, keyInfo -> true, remainingBufLimit); List subDirs = subDirDeleteResult.getKeysToDelete(); remainingBufLimit -= subDirDeleteResult.getConsumedSize(); @@ -337,9 +343,10 @@ protected PurgePathRequest prepareDeleteDirRequest( } // step-2: get all sub files under the deletedDir + // Only remove sub files if the parent directory is going to be deleted or can be reclaimed. DeleteKeysResult subFileDeleteResult = keyManager.getPendingDeletionSubFiles(volumeId, bucketId, - pendingDeletedDirInfo, remainingBufLimit); + pendingDeletedDirInfo, keyInfo -> purgeDir || reclaimableFileFilter.apply(keyInfo), remainingBufLimit); List subFiles = subFileDeleteResult.getKeysToDelete(); if (LOG.isDebugEnabled()) { @@ -350,10 +357,13 @@ protected PurgePathRequest prepareDeleteDirRequest( // step-3: If both sub-dirs and sub-files are exhausted under a parent // directory, only then delete the parent. - String purgeDeletedDir = subDirDeleteResult.isProcessedKeys() && + String purgeDeletedDir = purgeDir && subDirDeleteResult.isProcessedKeys() && subFileDeleteResult.isProcessedKeys() ? delDirName : null; - return wrapPurgeRequest(volumeId, bucketId, - purgeDeletedDir, subFiles, subDirs); + if (purgeDeletedDir == null && subFiles.isEmpty() && subDirs.isEmpty()) { + return Optional.empty(); + } + return Optional.of(wrapPurgeRequest(volumeId, bucketId, + purgeDeletedDir, subFiles, subDirs)); } @SuppressWarnings("checkstyle:ParameterNumber") @@ -363,6 +373,8 @@ public void optimizeDirDeletesAndSubmitRequest( List purgePathRequestList, String snapTableKey, long startTime, long remainingBufLimit, KeyManager keyManager, + CheckedFunction, Boolean, IOException> reclaimableDirChecker, + CheckedFunction, Boolean, IOException> reclaimableFileChecker, UUID expectedPreviousSnapshotId, long rnCnt) { // Optimization to handle delete sub-dir and keys to remove quickly @@ -372,30 +384,31 @@ public void optimizeDirDeletesAndSubmitRequest( int consumedSize = 0; while (subDirRecursiveCnt < allSubDirList.size() && remainingBufLimit > 0) { try { - Pair stringOmKeyInfoPair - = allSubDirList.get(subDirRecursiveCnt); - PurgePathRequest request = prepareDeleteDirRequest( - stringOmKeyInfoPair.getValue(), - stringOmKeyInfoPair.getKey(), allSubDirList, keyManager, - remainingBufLimit); - consumedSize += request.getSerializedSize(); + Pair stringOmKeyInfoPair = allSubDirList.get(subDirRecursiveCnt++); + Boolean subDirectoryReclaimable = reclaimableDirChecker.apply(Table.newKeyValue(stringOmKeyInfoPair.getKey(), + stringOmKeyInfoPair.getValue())); + Optional request = prepareDeleteDirRequest( + stringOmKeyInfoPair.getValue(), stringOmKeyInfoPair.getKey(), subDirectoryReclaimable, allSubDirList, + keyManager, reclaimableFileChecker, remainingBufLimit); + if (!request.isPresent()) { + continue; + } + PurgePathRequest requestVal = request.get(); + consumedSize += requestVal.getSerializedSize(); remainingBufLimit -= consumedSize; - purgePathRequestList.add(request); + purgePathRequestList.add(requestVal); // Count up the purgeDeletedDir, subDirs and subFiles - if (request.getDeletedDir() != null - && !request.getDeletedDir().isEmpty()) { + if (requestVal.hasDeletedDir() && !StringUtils.isBlank(requestVal.getDeletedDir())) { subdirDelNum++; } - subDirNum += request.getMarkDeletedSubDirsCount(); - subFileNum += request.getDeletedSubFilesCount(); - subDirRecursiveCnt++; + subDirNum += requestVal.getMarkDeletedSubDirsCount(); + subFileNum += requestVal.getDeletedSubFilesCount(); } catch (IOException e) { LOG.error("Error while running delete directories and files " + "background task. Will retry at next run for subset.", e); break; } } - if (!purgePathRequestList.isEmpty()) { submitPurgePaths(purgePathRequestList, snapTableKey, expectedPreviousSnapshotId); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java index 7451032492ea..f0a1c1fed9e7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java @@ -18,15 +18,25 @@ package org.apache.hadoop.ozone.om.service; import com.google.common.annotations.VisibleForTesting; +import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; import java.util.List; -import java.util.Objects; +import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.conf.StorageUnit; @@ -37,15 +47,20 @@ import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.Table.KeyValue; import org.apache.hadoop.hdds.utils.db.TableIterator; +import org.apache.hadoop.ozone.om.KeyManager; import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; import org.apache.hadoop.ozone.om.OmSnapshot; import org.apache.hadoop.ozone.om.OmSnapshotManager; import org.apache.hadoop.ozone.om.OzoneManager; -import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; +import org.apache.hadoop.ozone.om.SnapshotChainManager; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils; +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.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PurgePathRequest; import org.apache.hadoop.util.Time; import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; @@ -76,31 +91,33 @@ public class DirectoryDeletingService extends AbstractKeyDeletingService { // Using multi thread for DirDeletion. Multiple threads would read // from parent directory info from deleted directory table concurrently // and send deletion requests. - private final int dirDeletingCorePoolSize; private int ratisByteLimit; private final AtomicBoolean suspended; - private AtomicBoolean isRunningOnAOS; - - private final DeletedDirSupplier deletedDirSupplier; - - private AtomicInteger taskCount = new AtomicInteger(0); + private final AtomicBoolean isRunningOnAOS; + private final SnapshotChainManager snapshotChainManager; + private final boolean deepCleanSnapshots; + private final ExecutorService deletionThreadPool; + private final int numberOfParallelThreadsPerStore; public DirectoryDeletingService(long interval, TimeUnit unit, long serviceTimeout, OzoneManager ozoneManager, - OzoneConfiguration configuration, int dirDeletingServiceCorePoolSize) { + OzoneConfiguration configuration, int dirDeletingServiceCorePoolSize, boolean deepCleanSnapshots) { super(DirectoryDeletingService.class.getSimpleName(), interval, unit, dirDeletingServiceCorePoolSize, serviceTimeout, ozoneManager, null); int limit = (int) configuration.getStorageSize( OMConfigKeys.OZONE_OM_RATIS_LOG_APPENDER_QUEUE_BYTE_LIMIT, OMConfigKeys.OZONE_OM_RATIS_LOG_APPENDER_QUEUE_BYTE_LIMIT_DEFAULT, StorageUnit.BYTES); + this.numberOfParallelThreadsPerStore = dirDeletingServiceCorePoolSize; + this.deletionThreadPool = new ThreadPoolExecutor(0, numberOfParallelThreadsPerStore, interval, unit, + new LinkedBlockingDeque<>(Integer.MAX_VALUE)); + // always go to 90% of max limit for request as other header will be added this.ratisByteLimit = (int) (limit * 0.9); this.suspended = new AtomicBoolean(false); this.isRunningOnAOS = new AtomicBoolean(false); - this.dirDeletingCorePoolSize = dirDeletingServiceCorePoolSize; - deletedDirSupplier = new DeletedDirSupplier(); - taskCount.set(0); + this.snapshotChainManager = ((OmMetadataManagerImpl)ozoneManager.getMetadataManager()).getSnapshotChainManager(); + this.deepCleanSnapshots = deepCleanSnapshots; } private boolean shouldRun() { @@ -115,10 +132,6 @@ public boolean isRunningOnAOS() { return isRunningOnAOS.get(); } - public AtomicInteger getTaskCount() { - return taskCount; - } - /** * Suspend the service. */ @@ -142,20 +155,19 @@ public void setRatisByteLimit(int ratisByteLimit) { @Override public BackgroundTaskQueue getTasks() { BackgroundTaskQueue queue = new BackgroundTaskQueue(); - if (taskCount.get() > 0) { - LOG.info("{} Directory deleting task(s) already in progress.", - taskCount.get()); - return queue; - } - try { - deletedDirSupplier.reInitItr(); - } catch (IOException ex) { - LOG.error("Unable to get the iterator.", ex); - return queue; - } - taskCount.set(dirDeletingCorePoolSize); - for (int i = 0; i < dirDeletingCorePoolSize; i++) { - queue.add(new DirectoryDeletingService.DirDeletingTask(this)); + queue.add(new DirDeletingTask(this, null)); + if (deepCleanSnapshots) { + Iterator iterator = null; + try { + iterator = snapshotChainManager.iterator(true); + } catch (IOException e) { + LOG.error("Error while initializing snapshot chain iterator."); + return queue; + } + while (iterator.hasNext()) { + UUID snapshotId = iterator.next(); + queue.add(new DirDeletingTask(this, snapshotId)); + } } return queue; } @@ -163,39 +175,35 @@ public BackgroundTaskQueue getTasks() { @Override public void shutdown() { super.shutdown(); - deletedDirSupplier.closeItr(); } - private final class DeletedDirSupplier { + private final class DeletedDirSupplier implements Closeable { private TableIterator> deleteTableIterator; - private synchronized Table.KeyValue get() - throws IOException { + private DeletedDirSupplier(TableIterator> deleteTableIterator) { + this.deleteTableIterator = deleteTableIterator; + } + + private synchronized Table.KeyValue get() { if (deleteTableIterator.hasNext()) { return deleteTableIterator.next(); } return null; } - private synchronized void closeItr() { + public void close() { IOUtils.closeQuietly(deleteTableIterator); - deleteTableIterator = null; - } - - private synchronized void reInitItr() throws IOException { - closeItr(); - deleteTableIterator = - getOzoneManager().getMetadataManager().getDeletedDirTable() - .iterator(); } } private final class DirDeletingTask implements BackgroundTask { private final DirectoryDeletingService directoryDeletingService; + private final UUID snapshotId; - private DirDeletingTask(DirectoryDeletingService service) { + private DirDeletingTask(DirectoryDeletingService service, UUID snapshotId) { this.directoryDeletingService = service; + this.snapshotId = snapshotId; } @Override @@ -203,147 +211,192 @@ public int getPriority() { return 0; } - @Override - public BackgroundTaskResult call() { - try { - if (shouldRun()) { - isRunningOnAOS.set(true); - long rnCnt = getRunCount().incrementAndGet(); - if (LOG.isDebugEnabled()) { - LOG.debug("Running DirectoryDeletingService. {}", rnCnt); - } - long dirNum = 0L; - long subDirNum = 0L; - long subFileNum = 0L; - long remainingBufLimit = ratisByteLimit; - int consumedSize = 0; - List purgePathRequestList = new ArrayList<>(); - List> allSubDirList = - new ArrayList<>(); - - Table.KeyValue pendingDeletedDirInfo; - // This is to avoid race condition b/w purge request and snapshot chain updation. For AOS taking the global - // snapshotId since AOS could process multiple buckets in one iteration. - try { - UUID expectedPreviousSnapshotId = - ((OmMetadataManagerImpl) getOzoneManager().getMetadataManager()).getSnapshotChainManager() - .getLatestGlobalSnapshotId(); - - long startTime = Time.monotonicNow(); - while (remainingBufLimit > 0) { - pendingDeletedDirInfo = getPendingDeletedDirInfo(); - if (pendingDeletedDirInfo == null) { - break; - } - // Do not reclaim if the directory is still being referenced by - // the previous snapshot. - if (previousSnapshotHasDir(pendingDeletedDirInfo)) { - continue; - } + private OzoneManagerProtocolProtos.SetSnapshotPropertyRequest getSetSnapshotRequestUpdatingExclusiveSize( + Map exclusiveSizeMap, Map exclusiveReplicatedSizeMap, UUID snapshotID) { + OzoneManagerProtocolProtos.SnapshotSize snapshotSize = OzoneManagerProtocolProtos.SnapshotSize.newBuilder() + .setExclusiveSize( + exclusiveSizeMap.getOrDefault(snapshotID, 0L)) + .setExclusiveReplicatedSize( + exclusiveReplicatedSizeMap.getOrDefault( + snapshotID, 0L)) + .build(); + + return OzoneManagerProtocolProtos.SetSnapshotPropertyRequest.newBuilder() + .setSnapshotKey(snapshotChainManager.getTableKey(snapshotID)) + .setSnapshotSizeDeltaFromDirDeepCleaning(snapshotSize) + .build(); + } - PurgePathRequest request = prepareDeleteDirRequest( - pendingDeletedDirInfo.getValue(), - pendingDeletedDirInfo.getKey(), allSubDirList, - getOzoneManager().getKeyManager(), remainingBufLimit); + /** + * + * @param currentSnapshotInfo if null, deleted directories in AOS should be processed. + * @param keyManager KeyManager of the underlying store. + */ + private void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyManager keyManager, + long remainingBufLimit, long rnCnt) throws IOException, ExecutionException, InterruptedException { + String volume, bucket, snapshotTableKey; + if (currentSnapshotInfo != null) { + volume = currentSnapshotInfo.getVolumeName(); + bucket = currentSnapshotInfo.getBucketName(); + snapshotTableKey = currentSnapshotInfo.getTableKey(); + } else { + volume = null; bucket = null; snapshotTableKey = null; + } - consumedSize += request.getSerializedSize(); - remainingBufLimit -= consumedSize; - purgePathRequestList.add(request); - // Count up the purgeDeletedDir, subDirs and subFiles - if (request.getDeletedDir() != null && !request.getDeletedDir() - .isEmpty()) { - dirNum++; - } - subDirNum += request.getMarkDeletedSubDirsCount(); - subFileNum += request.getDeletedSubFilesCount(); + OmSnapshotManager omSnapshotManager = getOzoneManager().getOmSnapshotManager(); + IOzoneManagerLock lock = getOzoneManager().getMetadataManager().getLock(); + + try (DeletedDirSupplier dirSupplier = new DeletedDirSupplier(currentSnapshotInfo == null ? + keyManager.getDeletedDirEntries() : keyManager.getDeletedDirEntries(volume, bucket)); + ReclaimableDirFilter reclaimableDirFilter = new ReclaimableDirFilter(getOzoneManager(), + omSnapshotManager, snapshotChainManager, currentSnapshotInfo, keyManager, lock); + ReclaimableKeyFilter reclaimableFileFilter = new ReclaimableKeyFilter(getOzoneManager(), + omSnapshotManager, snapshotChainManager, currentSnapshotInfo, keyManager, lock)) { + // This is to avoid race condition b/w purge request and snapshot chain update. For AOS taking the global + // snapshotId since AOS could process multiple buckets in one iteration. While using path + // previous snapshotId for a snapshot since it would process only one bucket. + UUID expectedPreviousSnapshotId = currentSnapshotInfo == null ? + snapshotChainManager.getLatestGlobalSnapshotId() : + SnapshotUtils.getPreviousSnapshotId(currentSnapshotInfo, snapshotChainManager); + CompletableFuture processedAllDeletedDirs = CompletableFuture.completedFuture(true); + for (int i = 0; i < numberOfParallelThreadsPerStore; i++) { + CompletableFuture future = new CompletableFuture<>(); + deletionThreadPool.submit(() -> { + try { + boolean processedAll = processDeletedDirectories(snapshotTableKey, dirSupplier, remainingBufLimit, + reclaimableDirFilter, reclaimableFileFilter, expectedPreviousSnapshotId, rnCnt); + future.complete(processedAll); + } catch (Throwable e) { + future.complete(false); } - - optimizeDirDeletesAndSubmitRequest(dirNum, subDirNum, - subFileNum, allSubDirList, purgePathRequestList, null, - startTime, remainingBufLimit, - getOzoneManager().getKeyManager(), expectedPreviousSnapshotId, - rnCnt); - - } catch (IOException e) { - LOG.error( - "Error while running delete directories and files " + "background task. Will retry at next run.", - e); + }); + processedAllDeletedDirs = future.thenCombine(future, (a, b) -> a && b); + } + // If AOS or all directories have been processed for snapshot, update snapshot size delta and deep clean flag + if (currentSnapshotInfo == null || processedAllDeletedDirs.get()) { + List setSnapshotPropertyRequests = new ArrayList<>(); + Map exclusiveReplicatedSizeMap = reclaimableFileFilter.getExclusiveReplicatedSizeMap(); + Map exclusiveSizeMap = reclaimableFileFilter.getExclusiveSizeMap(); + List previousPathSnapshotsInChain = + Stream.of(exclusiveSizeMap.keySet(), exclusiveReplicatedSizeMap.keySet()) + .flatMap(Collection::stream).distinct().collect(Collectors.toList()); + for (UUID snapshot : previousPathSnapshotsInChain) { + setSnapshotPropertyRequests.add(getSetSnapshotRequestUpdatingExclusiveSize(exclusiveSizeMap, + exclusiveReplicatedSizeMap, snapshot)); } - isRunningOnAOS.set(false); - synchronized (directoryDeletingService) { - this.directoryDeletingService.notify(); + + // Updating directory deep clean flag of snapshot. + if (currentSnapshotInfo != null) { + setSnapshotPropertyRequests.add(OzoneManagerProtocolProtos.SetSnapshotPropertyRequest.newBuilder() + .setSnapshotKey(snapshotTableKey) + .setDeepCleanedDeletedDir(true) + .build()); } + submitSetSnapshotRequests(setSnapshotPropertyRequests); } - } finally { - taskCount.getAndDecrement(); } - // place holder by returning empty results of this call back. - return BackgroundTaskResult.EmptyTaskResult.newResult(); } - private boolean previousSnapshotHasDir( - KeyValue pendingDeletedDirInfo) throws IOException { - String key = pendingDeletedDirInfo.getKey(); - OmKeyInfo deletedDirInfo = pendingDeletedDirInfo.getValue(); - OmSnapshotManager omSnapshotManager = - getOzoneManager().getOmSnapshotManager(); - OmMetadataManagerImpl metadataManager = (OmMetadataManagerImpl) - getOzoneManager().getMetadataManager(); - SnapshotInfo previousSnapshotInfo = SnapshotUtils.getLatestSnapshotInfo(deletedDirInfo.getVolumeName(), - deletedDirInfo.getBucketName(), getOzoneManager(), metadataManager.getSnapshotChainManager()); - if (previousSnapshotInfo == null) { + private boolean processDeletedDirectories(String snapshotTableKey, + DeletedDirSupplier dirSupplier, long remainingBufLimit, ReclaimableDirFilter reclaimableDirFilter, + ReclaimableKeyFilter reclaimableFileFilter, UUID expectedPreviousSnapshotId, long runCount) { + try { + long startTime = Time.monotonicNow(); + long dirNum = 0L; + long subDirNum = 0L; + long subFileNum = 0L; + int consumedSize = 0; + List purgePathRequestList = new ArrayList<>(); + List> allSubDirList = new ArrayList<>(); + while (remainingBufLimit > 0) { + KeyValue pendingDeletedDirInfo = dirSupplier.get(); + if (pendingDeletedDirInfo == null) { + break; + } + boolean isDirReclaimable = reclaimableDirFilter.apply(pendingDeletedDirInfo); + Optional request = prepareDeleteDirRequest( + pendingDeletedDirInfo.getValue(), + pendingDeletedDirInfo.getKey(), isDirReclaimable, allSubDirList, + getOzoneManager().getKeyManager(), reclaimableFileFilter, remainingBufLimit); + if (!request.isPresent()) { + continue; + } + PurgePathRequest purgePathRequest = request.get(); + consumedSize += purgePathRequest.getSerializedSize(); + remainingBufLimit -= consumedSize; + purgePathRequestList.add(purgePathRequest); + // Count up the purgeDeletedDir, subDirs and subFiles + if (purgePathRequest.hasDeletedDir() && !StringUtils.isBlank(purgePathRequest.getDeletedDir())) { + dirNum++; + } + subDirNum += purgePathRequest.getMarkDeletedSubDirsCount(); + subFileNum += purgePathRequest.getDeletedSubFilesCount(); + } + + optimizeDirDeletesAndSubmitRequest(dirNum, subDirNum, + subFileNum, allSubDirList, purgePathRequestList, snapshotTableKey, + startTime, remainingBufLimit, getOzoneManager().getKeyManager(), + reclaimableDirFilter, reclaimableFileFilter, expectedPreviousSnapshotId, + runCount); + + return purgePathRequestList.isEmpty(); + } catch (IOException e) { + LOG.error("Error while running delete directories for store : {} and files background task. " + + "Will retry at next run. ", snapshotTableKey, e); return false; } - // previous snapshot is not active or it has not been flushed to disk then don't process the key in this - // iteration. - if (previousSnapshotInfo.getSnapshotStatus() != SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE || - !OmSnapshotManager.areSnapshotChangesFlushedToDB(getOzoneManager().getMetadataManager(), - previousSnapshotInfo)) { - return true; - } - try (UncheckedAutoCloseableSupplier rcLatestSnapshot = - omSnapshotManager.getSnapshot( - deletedDirInfo.getVolumeName(), - deletedDirInfo.getBucketName(), - previousSnapshotInfo.getName())) { + } - if (rcLatestSnapshot != null) { - String dbRenameKey = metadataManager - .getRenameKey(deletedDirInfo.getVolumeName(), - deletedDirInfo.getBucketName(), deletedDirInfo.getObjectID()); - Table prevDirTable = - rcLatestSnapshot.get().getMetadataManager().getDirectoryTable(); - Table prevDeletedDirTable = - rcLatestSnapshot.get().getMetadataManager().getDeletedDirTable(); - OmKeyInfo prevDeletedDirInfo = prevDeletedDirTable.get(key); - if (prevDeletedDirInfo != null) { - return true; + @Override + public BackgroundTaskResult call() { + // Check if this is the Leader OM. If not leader, no need to execute this + // task. + if (shouldRun()) { + final long run = getRunCount().incrementAndGet(); + if (snapshotId == null) { + LOG.debug("Running DirectoryDeletingService for active object store, {}", run); + isRunningOnAOS.set(true); + } else { + LOG.debug("Running DirectoryDeletingService for snapshot : {}, {}", snapshotId, run); + } + OmSnapshotManager omSnapshotManager = getOzoneManager().getOmSnapshotManager(); + SnapshotInfo snapInfo = null; + try { + snapInfo = snapshotId == null ? null : + SnapshotUtils.getSnapshotInfo(getOzoneManager(), snapshotChainManager, snapshotId); + if (snapInfo != null) { + if (snapInfo.isDeepCleanedDeletedDir()) { + LOG.info("Snapshot {} has already been deep cleaned directory. Skipping the snapshot in this iteration.", + snapInfo.getSnapshotId()); + return BackgroundTaskResult.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 BackgroundTaskResult.EmptyTaskResult.newResult(); + } + } + try (UncheckedAutoCloseableSupplier omSnapshot = snapInfo == null ? null : + omSnapshotManager.getActiveSnapshot(snapInfo.getVolumeName(), snapInfo.getBucketName(), + snapInfo.getName())) { + KeyManager keyManager = snapInfo == null ? getOzoneManager().getKeyManager() + : omSnapshot.get().getKeyManager(); + processDeletedDirsForStore(snapInfo, keyManager, ratisByteLimit, run); + } + } catch (IOException | ExecutionException | InterruptedException e) { + LOG.error("Error while running delete files background task for store {}. Will retry at next run.", + snapInfo, e); + } finally { + if (snapshotId == null) { + isRunningOnAOS.set(false); + synchronized (directoryDeletingService) { + this.directoryDeletingService.notify(); + } } - String prevDirTableDBKey = metadataManager.getSnapshotRenamedTable() - .get(dbRenameKey); - // In OMKeyDeleteResponseWithFSO OzonePathKey is converted to - // OzoneDeletePathKey. Changing it back to check the previous DirTable - String prevDbKey = prevDirTableDBKey == null ? - metadataManager.getOzoneDeletePathDirKey(key) : prevDirTableDBKey; - OmDirectoryInfo prevDirInfo = prevDirTable.get(prevDbKey); - //Checking if the previous snapshot in the chain hasn't changed while checking if the deleted directory is - // present in the previous snapshot. If the chain has changed, the deleted directory could have been moved - // to the newly created snapshot. - SnapshotInfo newPreviousSnapshotInfo = SnapshotUtils.getLatestSnapshotInfo(deletedDirInfo.getVolumeName(), - deletedDirInfo.getBucketName(), getOzoneManager(), metadataManager.getSnapshotChainManager()); - return (!Objects.equals(Optional.ofNullable(newPreviousSnapshotInfo).map(SnapshotInfo::getSnapshotId), - Optional.ofNullable(previousSnapshotInfo).map(SnapshotInfo::getSnapshotId))) || (prevDirInfo != null && - prevDirInfo.getObjectID() == deletedDirInfo.getObjectID()); } } - - return false; + // By design, no one cares about the results of this call back. + return BackgroundTaskResult.EmptyTaskResult.newResult(); } } - - public KeyValue getPendingDeletedDirInfo() - throws IOException { - return deletedDirSupplier.get(); - } - } 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 d89726fd35ef..60b2ab55efd7 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 @@ -204,7 +204,7 @@ private OzoneManagerProtocolProtos.SetSnapshotPropertyRequest getSetSnapshotRequ * @param keyManager KeyManager of the underlying store. */ private void processDeletedKeysForStore(SnapshotInfo currentSnapshotInfo, KeyManager keyManager, - int remainNum) throws IOException, InterruptedException { + int remainNum) throws IOException { String volume = null, bucket = null, snapshotTableKey = null; if (currentSnapshotInfo != null) { volume = currentSnapshotInfo.getVolumeName(); @@ -323,8 +323,8 @@ public BackgroundTaskResult call() { 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()); + LOG.info("Snapshot {} has already been deep cleaned. Skipping the snapshot in this iteration. " + + "Snapshot name : {}", snapInfo.getSnapshotId(), snapInfo.getName()); return EmptyTaskResult.newResult(); } if (!OmSnapshotManager.areSnapshotChangesFlushedToDB(getOzoneManager().getMetadataManager(), snapInfo)) { @@ -345,7 +345,7 @@ public BackgroundTaskResult call() { : omSnapshot.get().getKeyManager(); processDeletedKeysForStore(snapInfo, keyManager, remainNum); } - } catch (IOException | InterruptedException e) { + } catch (IOException e) { LOG.error("Error while running delete files background task for store {}. Will retry at next run.", snapInfo, e); } finally { 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 68d9306584ae..42e76377e14d 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 @@ -21,8 +21,8 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_INTERVAL; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_DIR_DELETING_SERVICE_INTERVAL; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -140,7 +140,7 @@ class TestKeyDeletingService extends OzoneTestBase { private KeyManager keyManager; private OMMetadataManager metadataManager; private KeyDeletingService keyDeletingService; - private SnapshotDirectoryCleaningService snapshotDirectoryCleaningService; + private DirectoryDeletingService directoryDeletingService; private ScmBlockLocationTestingClient scmBlockTestingClient; @BeforeAll @@ -156,7 +156,7 @@ private void createConfig(File testDir) { 100, TimeUnit.MILLISECONDS); conf.setTimeDuration(OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL, 100, TimeUnit.MILLISECONDS); - conf.setTimeDuration(OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL, + conf.setTimeDuration(OZONE_DIR_DELETING_SERVICE_INTERVAL, 100, TimeUnit.MILLISECONDS); conf.setTimeDuration(OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL, 1, TimeUnit.SECONDS); @@ -170,7 +170,7 @@ private void createSubject() throws Exception { OmTestManagers omTestManagers = new OmTestManagers(conf, scmBlockTestingClient, null); keyManager = omTestManagers.getKeyManager(); keyDeletingService = keyManager.getDeletingService(); - snapshotDirectoryCleaningService = keyManager.getSnapshotDirectoryService(); + directoryDeletingService = keyManager.getDirDeletingService(); writeClient = omTestManagers.getWriteClient(); om = omTestManagers.getOzoneManager(); metadataManager = omTestManagers.getMetadataManager(); @@ -524,6 +524,7 @@ void testSnapshotDeepClean() throws Exception { // Suspend KeyDeletingService keyDeletingService.suspend(); + directoryDeletingService.suspend(); final long initialSnapshotCount = metadataManager.countRowsInTable(snapshotInfoTable); final long initialKeyCount = metadataManager.countRowsInTable(keyTable); @@ -571,6 +572,7 @@ void testSnapshotDeepClean() throws Exception { checkSnapDeepCleanStatus(snapshotInfoTable, volumeName, false); keyDeletingService.resume(); + directoryDeletingService.resume(); try (UncheckedAutoCloseableSupplier rcOmSnapshot = om.getOmSnapshotManager().getSnapshot(volumeName, bucketName, snap3)) { @@ -640,6 +642,7 @@ void testSnapshotExclusiveSize() throws Exception { // Supspend KDS keyDeletingService.suspend(); + directoryDeletingService.suspend(); final long initialSnapshotCount = metadataManager.countRowsInTable(snapshotInfoTable); final long initialKeyCount = metadataManager.countRowsInTable(keyTable); @@ -711,10 +714,11 @@ void testSnapshotExclusiveSize() throws Exception { createAndCommitKey(testVolumeName, testBucketName, uniqueObjectName("key"), 3); long prevKdsRunCount = getRunCount(); - long prevSnapshotDirectorServiceCnt = snapshotDirectoryCleaningService.getRunCount().get(); + long prevSnapshotDirectorServiceCnt = directoryDeletingService.getRunCount().get(); + directoryDeletingService.resume(); // Let SnapshotDirectoryCleaningService to run for some iterations GenericTestUtils.waitFor( - () -> (snapshotDirectoryCleaningService.getRunCount().get() > prevSnapshotDirectorServiceCnt + 20), + () -> (directoryDeletingService.getRunCount().get() > prevSnapshotDirectorServiceCnt + 100), 100, 100000); keyDeletingService.resume(); @@ -779,7 +783,7 @@ void cleanup() { @Test @DisplayName("Should not update keys when purge request times out during key deletion") - public void testFailingModifiedKeyPurge() throws IOException, InterruptedException { + public void testFailingModifiedKeyPurge() throws IOException { try (MockedStatic mocked = mockStatic(OzoneManagerRatisUtils.class, CALLS_REAL_METHODS)) { From d14e83da7706aa9a2a45aeaee89c95f6612519c4 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 3 Jun 2025 00:13:05 -0400 Subject: [PATCH 04/12] HDDS-13034. Fix pmd Change-Id: I11acc3782aadf8393f731adcaa2a436dd9b534ae --- .../src/main/resources/ozone-default.xml | 6 +++--- .../om/service/DirectoryDeletingService.java | 21 +++++++++++++++++-- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index f2539b589591..f257e7b1c154 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -3786,7 +3786,7 @@ ozone.snapshot.directory.service.timeout 300s - OZONE, PERFORMANCE, OM + OZONE, PERFORMANCE, OM, DEPRECATED Timeout value for SnapshotDirectoryCleaningService. @@ -3795,9 +3795,9 @@ ozone.snapshot.directory.service.interval 24h - OZONE, PERFORMANCE, OM + OZONE, PERFORMANCE, OM, DEPRECATED - The time interval between successive SnapshotDirectoryCleaningService + DEPRECATED. The time interval between successive SnapshotDirectoryCleaningService thread run. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java index f0a1c1fed9e7..a31d268e016d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java @@ -177,7 +177,7 @@ public void shutdown() { super.shutdown(); } - private final class DeletedDirSupplier implements Closeable { + private static final class DeletedDirSupplier implements Closeable { private TableIterator> deleteTableIterator; @@ -192,6 +192,7 @@ private synchronized Table.KeyValue get() { return null; } + @Override public void close() { IOUtils.closeQuietly(deleteTableIterator); } @@ -273,7 +274,8 @@ private void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyMan processedAllDeletedDirs = future.thenCombine(future, (a, b) -> a && b); } // If AOS or all directories have been processed for snapshot, update snapshot size delta and deep clean flag - if (currentSnapshotInfo == null || processedAllDeletedDirs.get()) { + // if it is a snapshot. + if (processedAllDeletedDirs.get()) { List setSnapshotPropertyRequests = new ArrayList<>(); Map exclusiveReplicatedSizeMap = reclaimableFileFilter.getExclusiveReplicatedSizeMap(); Map exclusiveSizeMap = reclaimableFileFilter.getExclusiveSizeMap(); @@ -297,6 +299,21 @@ private void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyMan } } + /** + * Processes the directories marked as deleted and performs reclamation if applicable. + * This includes preparing and submitting requests to delete directories and their + * subdirectories/files while respecting buffer limits and snapshot constraints. + * + * @param snapshotTableKey the key of the snapshot table to which the operation applies + * @param dirSupplier thread safe supplier to fetch the next directory marked as deleted. + * @param remainingBufLimit the limit for the remaining buffer size available for processing + * @param reclaimableDirFilter filter to determine whether a directory is reclaimable + * @param reclaimableFileFilter filter to determine whether a file is reclaimable + * @param expectedPreviousSnapshotId UUID of the expected previous snapshot in the snapshot chain + * @param runCount the current run count of the deletion process + * @return true if no purge requests were submitted (indicating no deletions processed), + * false otherwise + */ private boolean processDeletedDirectories(String snapshotTableKey, DeletedDirSupplier dirSupplier, long remainingBufLimit, ReclaimableDirFilter reclaimableDirFilter, ReclaimableKeyFilter reclaimableFileFilter, UUID expectedPreviousSnapshotId, long runCount) { From 034585bc936a6015a5e1df5e73af942524310c8a Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 3 Jun 2025 05:33:39 -0400 Subject: [PATCH 05/12] HDDS-13170. Reclaimable filter should always reclaim entries when buckets and volumes have already been deleted Change-Id: I16dc9d8f00686320b4e98fa5691420294a7f1e2f --- .../om/snapshot/filter/ReclaimableFilter.java | 21 +++++++++---- .../filter/AbstractReclaimableFilterTest.java | 27 ++++++++++++----- .../filter/TestReclaimableDirFilter.java | 2 +- .../filter/TestReclaimableFilter.java | 30 +++++++++++++++++++ .../filter/TestReclaimableKeyFilter.java | 2 +- .../TestReclaimableRenameEntryFilter.java | 2 +- 6 files changed, 68 insertions(+), 16 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java index 0bb53e628032..5dc78e708fcb 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java @@ -33,6 +33,7 @@ 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.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; @@ -167,11 +168,21 @@ private void initializePreviousSnapshotsFromChain(String volume, String bucket) previousOmSnapshots.add(null); previousSnapshotInfos.add(null); } - - // NOTE: Getting volumeId and bucket from active OM. - // This would be wrong on volume & bucket renames support. - bucketInfo = ozoneManager.getBucketInfo(volume, bucket); + } + // NOTE: Getting volumeId and bucket from active OM. + // This would be wrong on volume & bucket renames support. + try { + bucketInfo = ozoneManager.getBucketManager().getBucketInfo(volume, bucket); volumeId = ozoneManager.getMetadataManager().getVolumeId(volume); + } catch (OMException e) { + // If Volume or bucket has been deleted then all keys should be reclaimable as no snapshots would exist. + if (OMException.ResultCodes.VOLUME_NOT_FOUND == e.getResult() || + OMException.ResultCodes.BUCKET_NOT_FOUND == e.getResult()) { + bucketInfo = null; + volumeId = null; + return; + } + throw e; } } catch (IOException e) { this.cleanup(); @@ -187,7 +198,7 @@ public synchronized Boolean apply(Table.KeyValue keyValue) throws IOE if (!validateExistingLastNSnapshotsInChain(volume, bucket) || !snapshotIdLocks.isLockAcquired()) { initializePreviousSnapshotsFromChain(volume, bucket); } - boolean isReclaimable = isReclaimable(keyValue); + boolean isReclaimable = (bucketInfo == null) || isReclaimable(keyValue); // This is to ensure the reclamation ran on the same previous snapshot and no change occurred in the chain // while processing the entry. return isReclaimable && validateExistingLastNSnapshotsInChain(volume, bucket); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java index fc7a53422c50..4f0205a0e15a 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java @@ -48,12 +48,14 @@ import org.apache.hadoop.hdds.utils.db.RDBStore; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.managed.ManagedRocksDB; +import org.apache.hadoop.ozone.om.BucketManager; import org.apache.hadoop.ozone.om.KeyManager; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OmSnapshot; 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.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; @@ -159,19 +161,28 @@ protected void teardown() throws IOException { private void mockOzoneManager(BucketLayout bucketLayout) throws IOException { OMMetadataManager metadataManager = mock(OMMetadataManager.class); + BucketManager bucketManager = mock(BucketManager.class); when(ozoneManager.getMetadataManager()).thenReturn(metadataManager); + when(ozoneManager.getBucketManager()).thenReturn(bucketManager); long volumeCount = 0; - long bucketCount = 0; for (String volume : volumes) { when(metadataManager.getVolumeId(eq(volume))).thenReturn(volumeCount); - for (String bucket : buckets) { - when(ozoneManager.getBucketInfo(eq(volume), eq(bucket))) - .thenReturn(OmBucketInfo.newBuilder().setVolumeName(volume).setBucketName(bucket) - .setObjectID(bucketCount).setBucketLayout(bucketLayout).build()); - bucketCount++; - } volumeCount++; } + + when(bucketManager.getBucketInfo(anyString(), anyString())).thenAnswer(i -> { + String volume = i.getArgument(0, String.class); + String bucket = i.getArgument(1, String.class); + if (!volumes.contains(volume)) { + throw new OMException("Volume " + volume + " already exists", OMException.ResultCodes.VOLUME_NOT_FOUND); + } + if (!buckets.contains(bucket)) { + throw new OMException("Bucket " + bucket + " already exists", OMException.ResultCodes.BUCKET_NOT_FOUND); + } + return OmBucketInfo.newBuilder().setVolumeName(volume).setBucketName(bucket) + .setObjectID((long) volumes.indexOf(volume) * buckets.size() + buckets.indexOf(bucket)) + .setBucketLayout(bucketLayout).build(); + }); } private void mockOmSnapshotManager(OzoneManager om) throws RocksDBException, IOException { @@ -232,7 +243,7 @@ private void mockOmSnapshotManager(OzoneManager om) throws RocksDBException, IOE protected List getLastSnapshotInfos( String volume, String bucket, int numberOfSnapshotsInChain, int index) { - List infos = getSnapshotInfos().get(getKey(volume, bucket)); + List infos = getSnapshotInfos().getOrDefault(getKey(volume, bucket), Collections.emptyList()); int endIndex = Math.min(index - 1, infos.size() - 1); return IntStream.range(endIndex - numberOfSnapshotsInChain + 1, endIndex + 1).mapToObj(i -> i >= 0 ? infos.get(i) : null).collect(Collectors.toList()); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableDirFilter.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableDirFilter.java index a85da9900a03..c2fcfa30b097 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableDirFilter.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableDirFilter.java @@ -72,7 +72,7 @@ private void testReclaimableDirFilter(String volume, String bucket, int index, List snapshotInfos = getLastSnapshotInfos(volume, bucket, 1, index); assertEquals(snapshotInfos.size(), 1); SnapshotInfo prevSnapshotInfo = snapshotInfos.get(0); - OmBucketInfo bucketInfo = getOzoneManager().getBucketInfo(volume, bucket); + OmBucketInfo bucketInfo = getOzoneManager().getBucketManager().getBucketInfo(volume, bucket); long volumeId = getOzoneManager().getMetadataManager().getVolumeId(volume); KeyManager keyManager = getKeyManager(); if (prevSnapshotInfo != null) { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableFilter.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableFilter.java index 2b986f8fb32a..7b50cff3f388 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableFilter.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableFilter.java @@ -130,6 +130,36 @@ public void testReclaimableFilterSnapshotChainInitialization( false); } + @ParameterizedTest + @MethodSource("testReclaimableFilterArguments") + public void testReclaimableFilterSnapshotChainInitializationWithInvalidVolume( + int numberOfPreviousSnapshotsFromChain, int actualNumberOfSnapshots) + throws IOException, RocksDBException { + SnapshotInfo currentSnapshotInfo = + setup(numberOfPreviousSnapshotsFromChain, actualNumberOfSnapshots, actualNumberOfSnapshots + 1, 4, 2); + String volume = "volume" + 6; + String bucket = getBuckets().get(1); + testSnapshotInitAndLocking(volume, bucket, numberOfPreviousSnapshotsFromChain, actualNumberOfSnapshots + 1, + currentSnapshotInfo, true, true); + testSnapshotInitAndLocking(volume, bucket, numberOfPreviousSnapshotsFromChain, actualNumberOfSnapshots + 1, + currentSnapshotInfo, false, true); + } + + @ParameterizedTest + @MethodSource("testReclaimableFilterArguments") + public void testReclaimableFilterSnapshotChainInitializationWithInvalidBucket( + int numberOfPreviousSnapshotsFromChain, int actualNumberOfSnapshots) + throws IOException, RocksDBException { + SnapshotInfo currentSnapshotInfo = + setup(numberOfPreviousSnapshotsFromChain, actualNumberOfSnapshots, actualNumberOfSnapshots + 1, 4, 2); + String volume = getVolumes().get(3); + String bucket = "bucket" + 6; + testSnapshotInitAndLocking(volume, bucket, numberOfPreviousSnapshotsFromChain, actualNumberOfSnapshots + 1, + currentSnapshotInfo, true, true); + testSnapshotInitAndLocking(volume, bucket, numberOfPreviousSnapshotsFromChain, actualNumberOfSnapshots + 1, + currentSnapshotInfo, false, true); + } + @ParameterizedTest @MethodSource("testReclaimableFilterArguments") public void testReclaimableFilterWithBucketVolumeMismatch( diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableKeyFilter.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableKeyFilter.java index 9db680c18f97..5e781ddfec17 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableKeyFilter.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableKeyFilter.java @@ -101,7 +101,7 @@ private void testReclaimableKeyFilter(String volume, String bucket, int index, List snapshotInfos = getLastSnapshotInfos(volume, bucket, 2, index); SnapshotInfo previousToPreviousSapshotInfo = snapshotInfos.get(0); SnapshotInfo prevSnapshotInfo = snapshotInfos.get(1); - OmBucketInfo bucketInfo = getOzoneManager().getBucketInfo(volume, bucket); + OmBucketInfo bucketInfo = getOzoneManager().getBucketManager().getBucketInfo(volume, bucket); long volumeId = getOzoneManager().getMetadataManager().getVolumeId(volume); UncheckedAutoCloseableSupplier prevSnap = Optional.ofNullable(prevSnapshotInfo) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableRenameEntryFilter.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableRenameEntryFilter.java index 4fad10f248f7..59f4cf0ca02e 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableRenameEntryFilter.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/TestReclaimableRenameEntryFilter.java @@ -80,7 +80,7 @@ private void testReclaimableRenameEntryFilter(String volume, String bucket, int throws IOException { List snapshotInfos = getLastSnapshotInfos(volume, bucket, 1, index); SnapshotInfo prevSnapshotInfo = snapshotInfos.get(0); - OmBucketInfo bucketInfo = getOzoneManager().getBucketInfo(volume, bucket); + OmBucketInfo bucketInfo = getOzoneManager().getBucketManager().getBucketInfo(volume, bucket); if (prevSnapshotInfo != null) { UncheckedAutoCloseableSupplier prevSnap = Optional.ofNullable(prevSnapshotInfo) .map(info -> { From 7eb2b98102aec4b9e53e7954764c0d2c467e64b6 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 3 Jun 2025 05:37:08 -0400 Subject: [PATCH 06/12] HDDS-13160. Fix tests Change-Id: Ie5fd1406bbb8af3a9ba76440dcba9b8d8db14691 --- .../TestDirectoryDeletingServiceWithFSO.java | 14 +-- .../service/AbstractKeyDeletingService.java | 1 + .../om/service/DirectoryDeletingService.java | 104 ++++++++++-------- 3 files changed, 64 insertions(+), 55 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java index a39aaf565ff7..4eceacf918d8 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java @@ -281,7 +281,7 @@ public void testDeleteWithLargeSubPathsThanBatchSize() throws Exception { 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 @@ -653,8 +653,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(); @@ -731,7 +731,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); @@ -751,14 +750,13 @@ 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. assertTableRowCount(keyTable, 5); - assertTableRowCount(dirTable, 4); + assertTableRowCount(dirTable, 0); // KeyDeletingService and DirectoryDeletingService will not // clean up because the paths are part of a snapshot. @@ -766,7 +764,7 @@ public void testDirDeletedTableCleanUpForSnapshot() throws Exception { // 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); @@ -774,7 +772,7 @@ public void testDirDeletedTableCleanUpForSnapshot() throws Exception { 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 diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java index ee699e16c31d..b7b536b2a36b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java @@ -384,6 +384,7 @@ public void optimizeDirDeletesAndSubmitRequest( int consumedSize = 0; while (subDirRecursiveCnt < allSubDirList.size() && remainingBufLimit > 0) { try { + LOG.info("Subdir deleting request: {}", subDirRecursiveCnt); Pair stringOmKeyInfoPair = allSubDirList.get(subDirRecursiveCnt++); Boolean subDirectoryReclaimable = reclaimableDirChecker.apply(Table.newKeyValue(stringOmKeyInfoPair.getKey(), stringOmKeyInfoPair.getValue())); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java index a31d268e016d..a29a4cbb8d4a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.om.service; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Maps; import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; @@ -213,15 +214,11 @@ public int getPriority() { } private OzoneManagerProtocolProtos.SetSnapshotPropertyRequest getSetSnapshotRequestUpdatingExclusiveSize( - Map exclusiveSizeMap, Map exclusiveReplicatedSizeMap, UUID snapshotID) { + long exclusiveSize, long exclusiveReplicatedSize, UUID snapshotID) { OzoneManagerProtocolProtos.SnapshotSize snapshotSize = OzoneManagerProtocolProtos.SnapshotSize.newBuilder() - .setExclusiveSize( - exclusiveSizeMap.getOrDefault(snapshotID, 0L)) - .setExclusiveReplicatedSize( - exclusiveReplicatedSizeMap.getOrDefault( - snapshotID, 0L)) + .setExclusiveSize(exclusiveSize) + .setExclusiveReplicatedSize(exclusiveReplicatedSize) .build(); - return OzoneManagerProtocolProtos.SetSnapshotPropertyRequest.newBuilder() .setSnapshotKey(snapshotChainManager.getTableKey(snapshotID)) .setSnapshotSizeDeltaFromDirDeepCleaning(snapshotSize) @@ -235,7 +232,7 @@ private OzoneManagerProtocolProtos.SetSnapshotPropertyRequest getSetSnapshotRequ */ private void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyManager keyManager, long remainingBufLimit, long rnCnt) throws IOException, ExecutionException, InterruptedException { - String volume, bucket, snapshotTableKey; + String volume, bucket; String snapshotTableKey; if (currentSnapshotInfo != null) { volume = currentSnapshotInfo.getVolumeName(); bucket = currentSnapshotInfo.getBucketName(); @@ -244,47 +241,39 @@ private void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyMan volume = null; bucket = null; snapshotTableKey = null; } - OmSnapshotManager omSnapshotManager = getOzoneManager().getOmSnapshotManager(); - IOzoneManagerLock lock = getOzoneManager().getMetadataManager().getLock(); - try (DeletedDirSupplier dirSupplier = new DeletedDirSupplier(currentSnapshotInfo == null ? - keyManager.getDeletedDirEntries() : keyManager.getDeletedDirEntries(volume, bucket)); - ReclaimableDirFilter reclaimableDirFilter = new ReclaimableDirFilter(getOzoneManager(), - omSnapshotManager, snapshotChainManager, currentSnapshotInfo, keyManager, lock); - ReclaimableKeyFilter reclaimableFileFilter = new ReclaimableKeyFilter(getOzoneManager(), - omSnapshotManager, snapshotChainManager, currentSnapshotInfo, keyManager, lock)) { + keyManager.getDeletedDirEntries() : keyManager.getDeletedDirEntries(volume, bucket))) { // This is to avoid race condition b/w purge request and snapshot chain update. For AOS taking the global // snapshotId since AOS could process multiple buckets in one iteration. While using path // previous snapshotId for a snapshot since it would process only one bucket. UUID expectedPreviousSnapshotId = currentSnapshotInfo == null ? snapshotChainManager.getLatestGlobalSnapshotId() : SnapshotUtils.getPreviousSnapshotId(currentSnapshotInfo, snapshotChainManager); + Map> exclusiveSizeMap = Maps.newConcurrentMap(); + CompletableFuture processedAllDeletedDirs = CompletableFuture.completedFuture(true); for (int i = 0; i < numberOfParallelThreadsPerStore; i++) { - CompletableFuture future = new CompletableFuture<>(); - deletionThreadPool.submit(() -> { + CompletableFuture future = CompletableFuture.supplyAsync(() -> { try { - boolean processedAll = processDeletedDirectories(snapshotTableKey, dirSupplier, remainingBufLimit, - reclaimableDirFilter, reclaimableFileFilter, expectedPreviousSnapshotId, rnCnt); - future.complete(processedAll); + return processDeletedDirectories(currentSnapshotInfo, keyManager, dirSupplier, remainingBufLimit, + expectedPreviousSnapshotId, exclusiveSizeMap, rnCnt); } catch (Throwable e) { - future.complete(false); + return false; } - }); + }, deletionThreadPool); processedAllDeletedDirs = future.thenCombine(future, (a, b) -> a && b); } // If AOS or all directories have been processed for snapshot, update snapshot size delta and deep clean flag // if it is a snapshot. if (processedAllDeletedDirs.get()) { List setSnapshotPropertyRequests = new ArrayList<>(); - Map exclusiveReplicatedSizeMap = reclaimableFileFilter.getExclusiveReplicatedSizeMap(); - Map exclusiveSizeMap = reclaimableFileFilter.getExclusiveSizeMap(); - List previousPathSnapshotsInChain = - Stream.of(exclusiveSizeMap.keySet(), exclusiveReplicatedSizeMap.keySet()) - .flatMap(Collection::stream).distinct().collect(Collectors.toList()); - for (UUID snapshot : previousPathSnapshotsInChain) { - setSnapshotPropertyRequests.add(getSetSnapshotRequestUpdatingExclusiveSize(exclusiveSizeMap, - exclusiveReplicatedSizeMap, snapshot)); + + for (Map.Entry> entry : exclusiveSizeMap.entrySet()) { + UUID snapshotID = entry.getKey(); + long exclusiveSize = entry.getValue().getLeft(); + long exclusiveReplicatedSize = entry.getValue().getRight(); + setSnapshotPropertyRequests.add(getSetSnapshotRequestUpdatingExclusiveSize( + exclusiveSize, exclusiveReplicatedSize, snapshotID)); } // Updating directory deep clean flag of snapshot. @@ -300,24 +289,30 @@ private void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyMan } /** - * Processes the directories marked as deleted and performs reclamation if applicable. - * This includes preparing and submitting requests to delete directories and their - * subdirectories/files while respecting buffer limits and snapshot constraints. + * Processes deleted directories for snapshot management, determining whether + * directories and files can be purged, and calculates exclusive size mappings + * for snapshots. * - * @param snapshotTableKey the key of the snapshot table to which the operation applies - * @param dirSupplier thread safe supplier to fetch the next directory marked as deleted. - * @param remainingBufLimit the limit for the remaining buffer size available for processing - * @param reclaimableDirFilter filter to determine whether a directory is reclaimable - * @param reclaimableFileFilter filter to determine whether a file is reclaimable - * @param expectedPreviousSnapshotId UUID of the expected previous snapshot in the snapshot chain - * @param runCount the current run count of the deletion process - * @return true if no purge requests were submitted (indicating no deletions processed), - * false otherwise + * @param currentSnapshotInfo Information about the current snapshot whose deleted directories are being processed. + * @param keyManager Key manager of the underlying storage system to handle key operations. + * @param dirSupplier Supplier for fetching pending deleted directories to be processed. + * @param remainingBufLimit Remaining buffer limit for processing directories and files. + * @param expectedPreviousSnapshotId The UUID of the previous snapshot expected in the chain. + * @param totalExclusiveSizeMap A map for storing total exclusive size and exclusive replicated size + * for each snapshot. + * @param runCount The number of times the processing task has been executed. + * @return A boolean indicating whether the processed directory list is empty. */ - private boolean processDeletedDirectories(String snapshotTableKey, - DeletedDirSupplier dirSupplier, long remainingBufLimit, ReclaimableDirFilter reclaimableDirFilter, - ReclaimableKeyFilter reclaimableFileFilter, UUID expectedPreviousSnapshotId, long runCount) { - try { + private boolean processDeletedDirectories(SnapshotInfo currentSnapshotInfo, KeyManager keyManager, + DeletedDirSupplier dirSupplier, long remainingBufLimit, UUID expectedPreviousSnapshotId, + Map> totalExclusiveSizeMap, long runCount) { + OmSnapshotManager omSnapshotManager = getOzoneManager().getOmSnapshotManager(); + IOzoneManagerLock lock = getOzoneManager().getMetadataManager().getLock(); + String snapshotTableKey = currentSnapshotInfo == null ? null : currentSnapshotInfo.getTableKey(); + try (ReclaimableDirFilter reclaimableDirFilter = new ReclaimableDirFilter(getOzoneManager(), + omSnapshotManager, snapshotChainManager, currentSnapshotInfo, keyManager, lock); + ReclaimableKeyFilter reclaimableFileFilter = new ReclaimableKeyFilter(getOzoneManager(), + omSnapshotManager, snapshotChainManager, currentSnapshotInfo, keyManager, lock)) { long startTime = Time.monotonicNow(); long dirNum = 0L; long subDirNum = 0L; @@ -355,6 +350,21 @@ private boolean processDeletedDirectories(String snapshotTableKey, startTime, remainingBufLimit, getOzoneManager().getKeyManager(), reclaimableDirFilter, reclaimableFileFilter, expectedPreviousSnapshotId, runCount); + Map exclusiveReplicatedSizeMap = reclaimableFileFilter.getExclusiveReplicatedSizeMap(); + Map exclusiveSizeMap = reclaimableFileFilter.getExclusiveSizeMap(); + List previousPathSnapshotsInChain = + Stream.of(exclusiveSizeMap.keySet(), exclusiveReplicatedSizeMap.keySet()) + .flatMap(Collection::stream).distinct().collect(Collectors.toList()); + for (UUID snapshot : previousPathSnapshotsInChain) { + totalExclusiveSizeMap.compute(snapshot, (k, v) -> { + long exclusiveSize = exclusiveSizeMap.getOrDefault(snapshot, 0L); + long exclusiveReplicatedSize = exclusiveReplicatedSizeMap.getOrDefault(snapshot, 0L); + if (v == null) { + return Pair.of(exclusiveSize, exclusiveReplicatedSize); + } + return Pair.of(v.getLeft() + exclusiveSize, v.getRight() + exclusiveReplicatedSize); + }); + } return purgePathRequestList.isEmpty(); } catch (IOException e) { From 1fe3cfcbd29654df3bf50cdb82ac90e8f5fb6339 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 3 Jun 2025 09:04:34 -0400 Subject: [PATCH 07/12] HDDS-13034. Fix find bugs Change-Id: I61ef68263ff88daa0e53dfb9d7d8eb62495d226b --- .../hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java index 4eceacf918d8..d7c12d0b81f4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java @@ -277,8 +277,6 @@ 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 GenericTestUtils.waitFor(() -> dirDeletingService.getRunCount().get() - preRunCount >= 7, 1000, 100000); From 99b61a2813b28e2f25399b4cabac7e06d309470a Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 3 Jun 2025 09:19:19 -0400 Subject: [PATCH 08/12] HDDS-13034. Fix find bugs Change-Id: I2667c6d12523f4dee7cbcf7c48c93803fe84d3d4 --- .../hadoop/ozone/om/service/AbstractKeyDeletingService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java index b7b536b2a36b..ee699e16c31d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java @@ -384,7 +384,6 @@ public void optimizeDirDeletesAndSubmitRequest( int consumedSize = 0; while (subDirRecursiveCnt < allSubDirList.size() && remainingBufLimit > 0) { try { - LOG.info("Subdir deleting request: {}", subDirRecursiveCnt); Pair stringOmKeyInfoPair = allSubDirList.get(subDirRecursiveCnt++); Boolean subDirectoryReclaimable = reclaimableDirChecker.apply(Table.newKeyValue(stringOmKeyInfoPair.getKey(), stringOmKeyInfoPair.getValue())); From 29d26193fe033dc6d03491ee0510ebaf9b646659 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 3 Jun 2025 11:56:08 -0400 Subject: [PATCH 09/12] HDDS-13034. deprecate config instead of removing Change-Id: I5ed93af3b5ae794b0cfe4671ec2a851592edcb8c --- .../org/apache/hadoop/ozone/om/OMConfigKeys.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 242ae03f0ccb..748d5f7d6c95 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -387,6 +387,18 @@ 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; + @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"; public static final String OZONE_THREAD_NUMBER_DIR_DELETION = "ozone.thread.number.dir.deletion"; From 2db371d77d50de707c176dc31cb8dcf3d83dd2e1 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Thu, 5 Jun 2025 09:01:12 -0400 Subject: [PATCH 10/12] HDDS-13034. refactor test case Change-Id: Iac3af98a7e568a135073b6704a6ad5a5fac7b427 --- .../om/snapshot/TestSnapshotDirectoryCleaningService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java index f854448b1679..f57fc37536a0 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java @@ -140,7 +140,7 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception { cluster.getOzoneManager().getMetadataManager().getDeletedTable(); Table snapshotInfoTable = cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable(); - DirectoryDeletingService snapshotDirectoryCleaningService = + DirectoryDeletingService directoryDeletingService = cluster.getOzoneManager().getKeyManager().getDirDeletingService(); /* DirTable @@ -220,8 +220,8 @@ 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); From 464e3214557fb11b9092fa53befd1af760ab4308 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Thu, 5 Jun 2025 11:00:35 -0400 Subject: [PATCH 11/12] HDDS-13034. Fix test case Change-Id: I9b7b41cf667e03d48120a4201757e445227924f7 --- .../TestSnapshotDirectoryCleaningService.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java index f57fc37536a0..73ca77454541 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java @@ -24,6 +24,8 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import com.google.common.collect.ImmutableMap; +import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -44,6 +46,8 @@ 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; @@ -142,6 +146,8 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception { cluster.getOzoneManager().getMetadataManager().getSnapshotInfoTable(); DirectoryDeletingService directoryDeletingService = cluster.getOzoneManager().getKeyManager().getDirDeletingService(); + SnapshotChainManager snapshotChainManager = ((OmMetadataManagerImpl)cluster.getOzoneManager().getMetadataManager()) + .getSnapshotChainManager(); /* DirTable /v/b/snapDir @@ -223,8 +229,6 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception { long prevRunCount = directoryDeletingService.getRunCount().get(); GenericTestUtils.waitFor(() -> directoryDeletingService.getRunCount().get() > prevRunCount + 1, 100, 10000); - - Thread.sleep(2000); Map expectedSize = new HashMap() {{ // /v/b/snapDir/appRoot0/parentDir0-2/childFile contribute // exclusive size, /v/b/snapDir/appRoot0/parentDir0-2/childFile0-4 @@ -234,11 +238,22 @@ public void testExclusiveSizeWithDirectoryDeepClean() throws Exception { put("snap2", 5L); put("snap3", 0L); }}; + try (TableIterator> iterator = snapshotInfoTable.iterator()) { while (iterator.hasNext()) { Table.KeyValue 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()); From 0ecec316a59405685bd92871d4e90163ccc68898 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Thu, 5 Jun 2025 12:33:24 -0400 Subject: [PATCH 12/12] HDDS-13034. Fix test case Change-Id: I2ff1cf3ecf3baa00a5c5646901f6c9ffdbe6e370 --- .../ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java index 73ca77454541..80a88871590b 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java @@ -24,7 +24,6 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; -import com.google.common.collect.ImmutableMap; import java.io.IOException; import java.util.HashMap; import java.util.Map;