This is an automated email from the ASF dual-hosted git repository.

swamirishi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new f19789dc88 HDDS-13170. Reclaimable filter should always reclaim 
entries when buckets and volumes have already been deleted (#8551)
f19789dc88 is described below

commit f19789dc88551b97771e65627c8418d23cc1b6ad
Author: Swaminathan Balachandran <[email protected]>
AuthorDate: Wed Jun 4 22:22:07 2025 -0400

    HDDS-13170. Reclaimable filter should always reclaim entries when buckets 
and volumes have already been deleted (#8551)
---
 .../om/snapshot/filter/ReclaimableFilter.java      | 21 +++++++++++----
 .../filter/AbstractReclaimableFilterTest.java      | 27 +++++++++++++------
 .../snapshot/filter/TestReclaimableDirFilter.java  |  2 +-
 .../om/snapshot/filter/TestReclaimableFilter.java  | 30 ++++++++++++++++++++++
 .../snapshot/filter/TestReclaimableKeyFilter.java  |  2 +-
 .../filter/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 0bb53e6280..5dc78e708f 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<String, V> 
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 fc7a53422c..e8c362d9a5 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 + " doesn't exist", 
OMException.ResultCodes.VOLUME_NOT_FOUND);
+      }
+      if (!buckets.contains(bucket)) {
+        throw new OMException("Bucket " + bucket + " doesn't exist", 
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<SnapshotInfo> getLastSnapshotInfos(
       String volume, String bucket, int numberOfSnapshotsInChain, int index) {
-    List<SnapshotInfo> infos = getSnapshotInfos().get(getKey(volume, bucket));
+    List<SnapshotInfo> 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 a85da9900a..c2fcfa30b0 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<SnapshotInfo> 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 2b986f8fb3..7b50cff3f3 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 9db680c18f..5e781ddfec 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<SnapshotInfo> 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<OmSnapshot> 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 4fad10f248..59f4cf0ca0 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<SnapshotInfo> 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<OmSnapshot> prevSnap = 
Optional.ofNullable(prevSnapshotInfo)
           .map(info -> {


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to