Skip to content

Commit

Permalink
Prevent deletion of snapshot backing index (#5069)
Browse files Browse the repository at this point in the history 
Prevent deletion of snapshots that are backing searchable snapshot indexes

Signed-off-by: Vishal Sarda <vsarda@amazon.com>
  • Loading branch information
@Vishalks
Vishalks committed Nov 23, 2022
1 parent fd495a6 commit 08ac17f
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 30 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
113 changes: 83 additions & 30 deletions server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}
Expand All @@ -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);
Expand All @@ -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");
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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();
Expand All @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/opensearch/OpenSearchException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<? extends OpenSearchException> exceptionClass;
Expand Down
35 changes: 35 additions & 0 deletions server/src/main/java/org/opensearch/snapshots/SnapshotInUseDeletionException.java
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
42 changes: 42 additions & 0 deletions server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,24 @@

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;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.HashMap;
import java.util.Map;

/**
* Snapshot utilities
Expand Down Expand Up @@ -135,4 +142,39 @@ public static List<String> filterIndices(List<String> 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<String, IndexMetadata> metadata,
List<SnapshotId> snapshotIds,
String repoName
) {
final Map<String, SnapshotId> uuidToSnapshotId = new HashMap<>();
final Set<String> snapshotsToBeNotDeleted = new HashSet<>();
snapshotIds.forEach(snapshotId -> uuidToSnapshotId.put(snapshotId.getUUID(), snapshotId));

for (ObjectCursor<IndexMetadata> 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."
);
}
}
}
2 changes: 2 additions & 0 deletions server/src/main/java/org/opensearch/snapshots/SnapshotsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions server/src/test/java/org/opensearch/ExceptionSerializationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Class<? extends OpenSearchException>, Integer> reverse = new HashMap<>();
for (Map.Entry<Integer, Class<? extends OpenSearchException>> entry : ids.entrySet()) {
Expand Down
Loading

0 comments on commit 08ac17f

Please sign in to comment.