Disallow snapshot deletion while a v2 snapshot is in progress (opense…

…arch-project#16430) --------- Signed-off-by: Gaurav Bafna <[email protected]>
soosinha · Oct 23, 2024 · bb1359f · bb1359f
1 parent 9489a21
commit bb1359f
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 9 deletions.
diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsV2IT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsV2IT.java
@@ -529,14 +529,13 @@ public void testDeleteWhileV2CreateOngoing() throws Exception {
         awaitNumberOfSnapshotsInProgress(1);
 
         ActionFuture<AcknowledgedResponse> a = startDeleteSnapshot(repoName, "snapshot-v1");
+        expectThrows(ConcurrentSnapshotExecutionException.class, a::actionGet);
 
         unblockNode(repoName, clusterManagerName);
         CreateSnapshotResponse csr = snapshotFuture.actionGet();
         assertTrue(csr.getSnapshotInfo().getPinnedTimestamp() != 0);
-        assertTrue(a.actionGet().isAcknowledged());
         List<SnapshotInfo> snapInfo = client().admin().cluster().prepareGetSnapshots(repoName).get().getSnapshots();
-        assertEquals(1, snapInfo.size());
-        assertThat(snapInfo, contains(csr.getSnapshotInfo()));
+        assertEquals(2, snapInfo.size());
     }
 
     @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/16205")

diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java
@@ -619,10 +619,8 @@ public void onResponse(RepositoryData repositoryData) {
                             }
                             cleanOrphanTimestamp(repositoryName, repositoryData);
                             logger.info("created snapshot-v2 [{}] in repository [{}]", repositoryName, snapshotName);
+                            leaveRepoLoop(repositoryName);
                             listener.onResponse(snapshotInfo);
-                            // For snapshot-v2, we don't allow concurrent snapshots . But meanwhile non-v2 snapshot operations
-                            // can get queued . This is triggering them.
-                            runNextQueuedOperation(repositoryData, repositoryName, true);
                         }
 
                         @Override
@@ -1021,10 +1019,8 @@ public void onResponse(RepositoryData repositoryData) {
                                         return;
                                     }
                                     logger.info("snapshot-v2 clone [{}] completed successfully", snapshot);
+                                    leaveRepoLoop(repositoryName);
                                     listener.onResponse(null);
-                                    // For snapshot-v2, we don't allow concurrent snapshots . But meanwhile non-v2 snapshot operations
-                                    // can get queued . This is triggering them.
-                                    runNextQueuedOperation(repositoryData, repositoryName, true);
                                 }
 
                                 @Override
@@ -2564,6 +2560,19 @@ public void deleteSnapshots(final DeleteSnapshotRequest request, final ActionLis
             public ClusterState execute(ClusterState currentState) throws Exception {
                 final SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE, SnapshotsInProgress.EMPTY);
                 final List<SnapshotsInProgress.Entry> snapshotEntries = findInProgressSnapshots(snapshots, snapshotNames, repoName);
+                boolean isSnapshotV2 = SHALLOW_SNAPSHOT_V2.get(repository.getMetadata().settings());
+                boolean remoteStoreIndexShallowCopy = remoteStoreShallowCopyEnabled(repository);
+                List<SnapshotsInProgress.Entry> entriesForThisRepo = snapshots.entries()
+                    .stream()
+                    .filter(entry -> Objects.equals(entry.repository(), repoName))
+                    .collect(Collectors.toList());
+                if (isSnapshotV2 && remoteStoreIndexShallowCopy && entriesForThisRepo.isEmpty() == false) {
+                    throw new ConcurrentSnapshotExecutionException(
+                        repoName,
+                        String.join(",", snapshotNames),
+                        "cannot delete snapshots in v2 repo while a snapshot is in progress"
+                    );
+                }
                 final List<SnapshotId> snapshotIds = matchingSnapshotIds(
                     snapshotEntries.stream().map(e -> e.snapshot().getSnapshotId()).collect(Collectors.toList()),
                     repositoryData,