diff --git a/CHANGELOG.md b/CHANGELOG.md index 220ed642e90ae..50196606d308c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added +- Prevent deletion of snapshots that are backing searchable snapshot indexes ([#5069](https://github.com/opensearch-project/OpenSearch/pull/5069)) + ### Dependencies - Bumps `bcpg-fips` from 1.0.5.1 to 1.0.7.1 - Bumps `azure-storage-blob` from 12.16.1 to 12.20.0 ([#4995](https://github.com/opensearch-project/OpenSearch/pull/4995)) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java index 17c32bb407bc3..53b70aa915a37 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java @@ -9,6 +9,7 @@ import org.junit.BeforeClass; import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; +import org.opensearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequestBuilder; import org.opensearch.action.index.IndexRequestBuilder; @@ -27,6 +28,7 @@ import org.opensearch.index.Index; import org.opensearch.index.IndexNotFoundException; import org.opensearch.monitor.fs.FsInfo; +import org.opensearch.repositories.fs.FsRepository; import java.nio.file.Files; import java.nio.file.Path; @@ -74,23 +76,30 @@ private Settings.Builder chunkedRepositorySettings() { * Ensures availability of sufficient data nodes and search capable nodes. */ public void testCreateSearchableSnapshot() throws Exception { + final String snapshotName = "test-snap"; + final String repoName = "test-repo"; + final String indexName1 = "test-idx-1"; + final String restoredIndexName1 = indexName1 + "-copy"; + final String indexName2 = "test-idx-2"; + final String restoredIndexName2 = indexName2 + "-copy"; final int numReplicasIndex1 = randomIntBetween(1, 4); final int numReplicasIndex2 = randomIntBetween(0, 2); final Client client = client(); internalCluster().ensureAtLeastNumDataNodes(Math.max(numReplicasIndex1, numReplicasIndex2) + 1); - createIndexWithDocsAndEnsureGreen(numReplicasIndex1, 100, "test-idx-1"); - createIndexWithDocsAndEnsureGreen(numReplicasIndex2, 100, "test-idx-2"); + createIndexWithDocsAndEnsureGreen(numReplicasIndex1, 100, indexName1); + createIndexWithDocsAndEnsureGreen(numReplicasIndex2, 100, indexName2); - takeSnapshot(client, "test-idx-1", "test-idx-2"); - deleteIndicesAndEnsureGreen(client, "test-idx-1", "test-idx-2"); + createRepositoryWithSettings(null, repoName); + takeSnapshot(client, snapshotName, repoName, indexName1, indexName2); + deleteIndicesAndEnsureGreen(client, indexName1, indexName2); internalCluster().ensureAtLeastNumSearchNodes(Math.max(numReplicasIndex1, numReplicasIndex2) + 1); - restoreSnapshotAndEnsureGreen(client); + restoreSnapshotAndEnsureGreen(client, snapshotName, repoName); - assertDocCount("test-idx-1-copy", 100L); - assertDocCount("test-idx-2-copy", 100L); - assertIndexDirectoryDoesNotExist("test-idx-1-copy", "test-idx-2-copy"); + assertDocCount(restoredIndexName1, 100L); + assertDocCount(restoredIndexName2, 100L); + assertIndexDirectoryDoesNotExist(restoredIndexName1, restoredIndexName2); } /** @@ -101,16 +110,19 @@ public void testCreateSearchableSnapshotWithChunks() throws Exception { final int numReplicasIndex = randomIntBetween(1, 4); final String indexName = "test-idx"; final String restoredIndexName = indexName + "-copy"; + final String repoName = "test-repo"; + final String snapshotName = "test-snap"; final Client client = client(); Settings.Builder repositorySettings = chunkedRepositorySettings(); internalCluster().ensureAtLeastNumSearchAndDataNodes(numReplicasIndex + 1); createIndexWithDocsAndEnsureGreen(numReplicasIndex, 1000, indexName); - takeSnapshot(client, repositorySettings, indexName); + createRepositoryWithSettings(repositorySettings, repoName); + takeSnapshot(client, snapshotName, repoName, indexName); deleteIndicesAndEnsureGreen(client, indexName); - restoreSnapshotAndEnsureGreen(client); + restoreSnapshotAndEnsureGreen(client, snapshotName, repoName); assertDocCount(restoredIndexName, 1000L); } @@ -124,13 +136,16 @@ public void testSearchableSnapshotAllocationForLocalAndRemoteShardsOnSameNode() final int numReplicasIndex = randomIntBetween(1, 4); final String indexName = "test-idx"; final String restoredIndexName = indexName + "-copy"; + final String repoName = "test-repo"; + final String snapshotName = "test-snap"; final Client client = client(); internalCluster().ensureAtLeastNumSearchAndDataNodes(numReplicasIndex + 1); createIndexWithDocsAndEnsureGreen(numReplicasIndex, 100, indexName); - takeSnapshot(client, indexName); + createRepositoryWithSettings(null, repoName); + takeSnapshot(client, snapshotName, repoName, indexName); - restoreSnapshotAndEnsureGreen(client); + restoreSnapshotAndEnsureGreen(client, snapshotName, repoName); assertDocCount(restoredIndexName, 100L); assertDocCount(indexName, 100L); @@ -145,16 +160,19 @@ public void testSearchableSnapshotAllocationForFailoverAndRecovery() throws Exce final int numReplicasIndex = 1; final String indexName = "test-idx"; final String restoredIndexName = indexName + "-copy"; + final String repoName = "test-repo"; + final String snapshotName = "test-snap"; final Client client = client(); internalCluster().ensureAtLeastNumDataNodes(numReplicasIndex + 1); createIndexWithDocsAndEnsureGreen(numReplicasIndex, 100, indexName); - takeSnapshot(client, indexName); + createRepositoryWithSettings(null, repoName); + takeSnapshot(client, snapshotName, repoName, indexName); deleteIndicesAndEnsureGreen(client, indexName); internalCluster().ensureAtLeastNumSearchNodes(numReplicasIndex + 1); - restoreSnapshotAndEnsureGreen(client); + restoreSnapshotAndEnsureGreen(client, snapshotName, repoName); assertDocCount(restoredIndexName, 100L); logger.info("--> stop a random search node"); @@ -183,14 +201,17 @@ public void testSearchableSnapshotAllocationForFailoverAndRecovery() throws Exce public void testSearchableSnapshotIndexIsReadOnly() throws Exception { final String indexName = "test-index"; final String restoredIndexName = indexName + "-copy"; + final String repoName = "test-repo"; + final String snapshotName = "test-snap"; final Client client = client(); createIndexWithDocsAndEnsureGreen(0, 100, indexName); - takeSnapshot(client, indexName); + createRepositoryWithSettings(null, repoName); + takeSnapshot(client, snapshotName, repoName, indexName); deleteIndicesAndEnsureGreen(client, indexName); internalCluster().ensureAtLeastNumSearchNodes(1); - restoreSnapshotAndEnsureGreen(client); + restoreSnapshotAndEnsureGreen(client, snapshotName, repoName); assertIndexingBlocked(restoredIndexName); assertIndexSettingChangeBlocked(restoredIndexName); @@ -202,6 +223,39 @@ public void testSearchableSnapshotIndexIsReadOnly() throws Exception { ); } + public void testDeleteSearchableSnapshotBackingIndexThrowsException() throws Exception { + final String indexName = "test-index"; + final Client client = client(); + final String repoName = "test-repo"; + final String snapshotName = "test-snap"; + createRepositoryWithSettings(null, repoName); + createIndexWithDocsAndEnsureGreen(0, 100, indexName); + takeSnapshot(client, snapshotName, repoName, indexName); + internalCluster().ensureAtLeastNumSearchNodes(1); + restoreSnapshotAndEnsureGreen(client, snapshotName, repoName); + assertThrows( + SnapshotInUseDeletionException.class, + () -> client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName)).actionGet() + ); + } + + public void testDeleteSearchableSnapshotBackingIndex() throws Exception { + final String indexName1 = "test-index1"; + final String indexName2 = "test-index2"; + final Client client = client(); + final String repoName = "test-repo"; + final String snapshotName1 = "test-snapshot1"; + final String snapshotName2 = "test-snap"; + createRepositoryWithSettings(null, repoName); + createIndexWithDocsAndEnsureGreen(0, 100, indexName1); + createIndexWithDocsAndEnsureGreen(0, 100, indexName2); + takeSnapshot(client, snapshotName1, repoName, indexName1); + takeSnapshot(client, snapshotName2, repoName, indexName2); + internalCluster().ensureAtLeastNumSearchNodes(1); + restoreSnapshotAndEnsureGreen(client, snapshotName2, repoName); + client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName1)).actionGet(); + } + private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDocs, String indexName) throws InterruptedException { createIndex( indexName, @@ -216,21 +270,11 @@ private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDo ensureGreen(); } - private void takeSnapshot(Client client, String... indices) { - takeSnapshot(client, null, indices); - } - - private void takeSnapshot(Client client, Settings.Builder repositorySettings, String... indices) { - logger.info("--> Create a repository"); - if (repositorySettings == null) { - createRepository("test-repo", "fs"); - } else { - createRepository("test-repo", "fs", repositorySettings); - } + private void takeSnapshot(Client client, String snapshotName, String repoName, String... indices) { logger.info("--> Take a snapshot"); final CreateSnapshotResponse createSnapshotResponse = client.admin() .cluster() - .prepareCreateSnapshot("test-repo", "test-snap") + .prepareCreateSnapshot(repoName, snapshotName) .setWaitForCompletion(true) .setIndices(indices) .get(); @@ -242,16 +286,25 @@ private void takeSnapshot(Client client, Settings.Builder repositorySettings, St ); } + private void createRepositoryWithSettings(Settings.Builder repositorySettings, String repoName) { + logger.info("--> Create a repository"); + if (repositorySettings == null) { + createRepository(repoName, FsRepository.TYPE); + } else { + createRepository(repoName, FsRepository.TYPE, repositorySettings); + } + } + private void deleteIndicesAndEnsureGreen(Client client, String... indices) { assertTrue(client.admin().indices().prepareDelete(indices).get().isAcknowledged()); ensureGreen(); } - private void restoreSnapshotAndEnsureGreen(Client client) { + private void restoreSnapshotAndEnsureGreen(Client client, String snapshotName, String repoName) { logger.info("--> restore indices as 'remote_snapshot'"); client.admin() .cluster() - .prepareRestoreSnapshot("test-repo", "test-snap") + .prepareRestoreSnapshot(repoName, snapshotName) .setRenamePattern("(.+)") .setRenameReplacement("$1-copy") .setStorageType(RestoreSnapshotRequest.StorageType.REMOTE_SNAPSHOT) diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/server/src/main/java/org/opensearch/OpenSearchException.java index 4e667d0a9f3a5..aef098403ec2b 100644 --- a/server/src/main/java/org/opensearch/OpenSearchException.java +++ b/server/src/main/java/org/opensearch/OpenSearchException.java @@ -51,6 +51,7 @@ import org.opensearch.index.shard.ShardId; import org.opensearch.rest.RestStatus; import org.opensearch.search.aggregations.MultiBucketConsumerService; +import org.opensearch.snapshots.SnapshotInUseDeletionException; import org.opensearch.transport.TcpTransport; import java.io.IOException; @@ -1611,6 +1612,12 @@ private enum OpenSearchExceptionHandle { ClusterManagerThrottlingException::new, 165, Version.V_2_4_0 + ), + SNAPSHOT_IN_USE_DELETION_EXCEPTION( + SnapshotInUseDeletionException.class, + SnapshotInUseDeletionException::new, + 166, + UNKNOWN_VERSION_ADDED ); final Class exceptionClass; diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotInUseDeletionException.java b/server/src/main/java/org/opensearch/snapshots/SnapshotInUseDeletionException.java new file mode 100644 index 0000000000000..e93bf5ab0cd91 --- /dev/null +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotInUseDeletionException.java @@ -0,0 +1,35 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.snapshots; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.rest.RestStatus; + +import java.io.IOException; + +/** + * Thrown if requested snapshot/s can't be deleted + * + * @opensearch.internal + */ +public class SnapshotInUseDeletionException extends SnapshotException { + + public SnapshotInUseDeletionException(final String repositoryName, final String snapshotName, final String msg) { + super(repositoryName, snapshotName, msg); + } + + public SnapshotInUseDeletionException(StreamInput in) throws IOException { + super(in); + } + + @Override + public RestStatus status() { + return RestStatus.CONFLICT; + } +} diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java index 073e4f7723077..3ef3523961df8 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java @@ -31,10 +31,15 @@ package org.opensearch.snapshots; +import com.carrotsearch.hppc.cursors.ObjectCursor; import org.opensearch.action.support.IndicesOptions; +import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.regex.Regex; +import org.opensearch.index.IndexModule; import org.opensearch.index.IndexNotFoundException; +import org.opensearch.index.IndexSettings; import java.util.ArrayList; import java.util.Arrays; @@ -42,6 +47,8 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.HashMap; +import java.util.Map; /** * Snapshot utilities @@ -135,4 +142,39 @@ public static List filterIndices(List availableIndices, String[] } return Collections.unmodifiableList(new ArrayList<>(result)); } + + /** + * Validates if there are any remote snapshots backing an index + * @param metadata index metadata from cluster state + * @param snapshotIds list of snapshot Ids to be verified + * @param repoName repo name for which the verification is being done + */ + public static void validateSnapshotsBackingAnyIndex( + ImmutableOpenMap metadata, + List snapshotIds, + String repoName + ) { + final Map uuidToSnapshotId = new HashMap<>(); + final Set snapshotsToBeNotDeleted = new HashSet<>(); + snapshotIds.forEach(snapshotId -> uuidToSnapshotId.put(snapshotId.getUUID(), snapshotId)); + + for (ObjectCursor cursor : metadata.values()) { + IndexMetadata indexMetadata = cursor.value; + String storeType = indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()); + if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(storeType)) { + String snapshotId = indexMetadata.getSettings().get(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey()); + if (uuidToSnapshotId.get(snapshotId) != null) { + snapshotsToBeNotDeleted.add(uuidToSnapshotId.get(snapshotId).getName()); + } + } + } + + if (!snapshotsToBeNotDeleted.isEmpty()) { + throw new SnapshotInUseDeletionException( + repoName, + snapshotsToBeNotDeleted.toString(), + "These remote snapshots are backing some indices and hence can't be deleted! No snapshots were deleted." + ); + } + } } diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java index 48b90af98022f..645775c3ec09c 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java @@ -132,6 +132,7 @@ import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableList; import static org.opensearch.cluster.SnapshotsInProgress.completed; +import static org.opensearch.snapshots.SnapshotUtils.validateSnapshotsBackingAnyIndex; /** * Service responsible for creating snapshots. This service runs all the steps executed on the cluster-manager node during snapshot creation and @@ -1769,6 +1770,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { snapshotNames, repoName ); + validateSnapshotsBackingAnyIndex(currentState.getMetadata().getIndices(), snapshotIds, repoName); deleteFromRepoTask = createDeleteStateUpdate(snapshotIds, repoName, repositoryData, Priority.NORMAL, listener); return deleteFromRepoTask.execute(currentState); } diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java index 616ad0a57bf93..559963b0e0b68 100644 --- a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java @@ -106,6 +106,7 @@ import org.opensearch.snapshots.SnapshotException; import org.opensearch.snapshots.SnapshotId; import org.opensearch.snapshots.SnapshotInProgressException; +import org.opensearch.snapshots.SnapshotInUseDeletionException; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; import org.opensearch.transport.ActionNotFoundTransportException; @@ -862,6 +863,7 @@ public void testIds() { ids.put(163, DecommissioningFailedException.class); ids.put(164, NodeDecommissionedException.class); ids.put(165, ClusterManagerThrottlingException.class); + ids.put(166, SnapshotInUseDeletionException.class); Map, Integer> reverse = new HashMap<>(); for (Map.Entry> entry : ids.entrySet()) { diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java index 489294fd53bd4..8dae5026a18bc 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java @@ -31,13 +31,22 @@ package org.opensearch.snapshots; +import org.opensearch.Version; import org.opensearch.action.support.IndicesOptions; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.common.collect.ImmutableOpenMap; +import org.opensearch.common.settings.Settings; +import org.opensearch.index.Index; +import org.opensearch.index.IndexModule; +import org.opensearch.index.IndexSettings; import org.opensearch.test.OpenSearchTestCase; import java.util.Arrays; import java.util.List; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; public class SnapshotUtilsTests extends OpenSearchTestCase { public void testIndexNameFiltering() { @@ -85,4 +94,39 @@ private void assertIndexNameFiltering(String[] indices, String[] filter, Indices List actual = SnapshotUtils.filterIndices(indicesList, filter, indicesOptions); assertThat(actual, containsInAnyOrder(expected)); } + + public void testValidateSnapshotsBackingAnyIndex() { + final String repoName = "test-repo"; + final SnapshotId snapshotId1 = new SnapshotId("testSnapshot1", "uuid1"); + final SnapshotId snapshotId2 = new SnapshotId("testSnapshot2", "uuid2"); + SnapshotUtils.validateSnapshotsBackingAnyIndex(getIndexMetadata(snapshotId1, repoName), List.of(snapshotId2), repoName); + } + + public void testValidateSnapshotsBackingAnyIndexThrowsException() { + final String repoName = "test-repo"; + final SnapshotId snapshotId1 = new SnapshotId("testSnapshot1", "uuid1"); + expectThrows( + SnapshotInUseDeletionException.class, + () -> SnapshotUtils.validateSnapshotsBackingAnyIndex(getIndexMetadata(snapshotId1, repoName), List.of(snapshotId1), repoName) + ); + } + + private static ImmutableOpenMap getIndexMetadata(SnapshotId snapshotId, String repoName) { + final String index = "test-index"; + Snapshot snapshot = new Snapshot(repoName, snapshotId); + final Metadata.Builder builder = Metadata.builder(); + builder.put(createIndexMetadata(new Index(index, "uuid"), snapshot), true); + return builder.build().getIndices(); + } + + private static IndexMetadata createIndexMetadata(final Index index, Snapshot snapshot) { + final Settings settings = Settings.builder() + .put(SETTING_VERSION_CREATED, Version.CURRENT.id) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .put(IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.getKey(), snapshot.getRepository()) + .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey(), snapshot.getSnapshotId().getUUID()) + .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), snapshot.getSnapshotId().getName()) + .build(); + return IndexMetadata.builder(index.getName()).settings(settings).numberOfShards(1).numberOfReplicas(0).build(); + } }