[Snapshot Interop] Fix issue when wrong shard blob files are picked u…

…p for shallow snapshot deletion. (opensearch-project#12701) Signed-off-by: Harish Bhakuni <[email protected]>
ruai0511 · Mar 19, 2024 · 2e23c87 · 2e23c87
1 parent b7ec0ea
commit 2e23c87
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 4 deletions.
diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java
@@ -304,7 +304,8 @@ public void testRemoteStoreCleanupForDeletedIndex() throws Exception {
             .getSetting(remoteStoreEnabledIndexName, IndexMetadata.SETTING_INDEX_UUID);
 
         logger.info("--> create two remote index shallow snapshots");
-        List<String> shallowCopySnapshots = createNSnapshots(snapshotRepoName, 2);
+        SnapshotInfo snapshotInfo1 = createFullSnapshot(snapshotRepoName, "snap1");
+        SnapshotInfo snapshotInfo2 = createFullSnapshot(snapshotRepoName, "snap2");
 
         String[] lockFiles = getLockFilesInRemoteStore(remoteStoreEnabledIndexName, REMOTE_REPO_NAME);
         assert (lockFiles.length == 2) : "lock files are " + Arrays.toString(lockFiles);
@@ -315,17 +316,18 @@ public void testRemoteStoreCleanupForDeletedIndex() throws Exception {
         logger.info("--> delete snapshot 1");
         AcknowledgedResponse deleteSnapshotResponse = clusterManagerClient.admin()
             .cluster()
-            .prepareDeleteSnapshot(snapshotRepoName, shallowCopySnapshots.get(0))
+            .prepareDeleteSnapshot(snapshotRepoName, snapshotInfo1.snapshotId().getName())
             .get();
         assertAcked(deleteSnapshotResponse);
 
         lockFiles = getLockFilesInRemoteStore(remoteStoreEnabledIndexName, REMOTE_REPO_NAME, indexUUID);
         assert (lockFiles.length == 1) : "lock files are " + Arrays.toString(lockFiles);
+        assertTrue(lockFiles[0].contains(snapshotInfo2.snapshotId().getUUID()));
 
         logger.info("--> delete snapshot 2");
         deleteSnapshotResponse = clusterManagerClient.admin()
             .cluster()
-            .prepareDeleteSnapshot(snapshotRepoName, shallowCopySnapshots.get(1))
+            .prepareDeleteSnapshot(snapshotRepoName, snapshotInfo2.snapshotId().getName())
             .get();
         assertAcked(deleteSnapshotResponse);
 

diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java
@@ -3388,7 +3388,8 @@ private static List<String> unusedBlobs(
                             blob.substring(SNAPSHOT_PREFIX.length(), blob.length() - ".dat".length())
                         ) == false)
                     || (remoteStoreLockManagerFactory != null
-                        && extractShallowSnapshotUUID(blob).map(survivingSnapshotUUIDs::contains).orElse(false))
+                        && extractShallowSnapshotUUID(blob).map(snapshotUUID -> !survivingSnapshotUUIDs.contains(snapshotUUID))
+                            .orElse(false))
                     || (blob.startsWith(UPLOADED_DATA_BLOB_PREFIX) && updatedSnapshots.findNameFile(canonicalName(blob)) == null)
                     || FsBlobContainer.isTempBlobName(blob)
             )