Skip to content

Commit

Permalink
Adding unit and integration tests, moving validation function to utils
Browse files Browse the repository at this point in the history 
Signed-off-by: Vishal Sarda <vsarda@amazon.com>
  • Loading branch information
@Vishalks
Vishalks committed Nov 19, 2022
1 parent e09a8b9 commit fb6d33d
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 67 deletions.
120 changes: 94 additions & 26 deletions server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
import org.hamcrest.MatcherAssert;
import org.junit.BeforeClass;
import org.opensearch.action.StepListener;
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;
import org.opensearch.action.support.master.AcknowledgedResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.block.ClusterBlockException;
Expand All @@ -27,6 +30,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 +78,29 @@ 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");
takeSnapshot(client, snapshotName, repoName, false, 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 +111,18 @@ 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);
takeSnapshot(client, repositorySettings, snapshotName, repoName, false, indexName);

deleteIndicesAndEnsureGreen(client, indexName);
restoreSnapshotAndEnsureGreen(client);
restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);

assertDocCount(restoredIndexName, 1000L);
}
Expand All @@ -124,13 +136,15 @@ 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);
takeSnapshot(client, snapshotName, repoName, false, indexName);

restoreSnapshotAndEnsureGreen(client);
restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);

assertDocCount(restoredIndexName, 100L);
assertDocCount(indexName, 100L);
Expand All @@ -145,16 +159,18 @@ 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);
takeSnapshot(client, snapshotName, repoName, false, 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 +199,16 @@ 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);
takeSnapshot(client, snapshotName, repoName, false, indexName);
deleteIndicesAndEnsureGreen(client, indexName);

internalCluster().ensureAtLeastNumSearchNodes(1);
restoreSnapshotAndEnsureGreen(client);
restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);

assertIndexingBlocked(restoredIndexName);
assertIndexSettingChangeBlocked(restoredIndexName);
Expand All @@ -202,6 +220,47 @@ public void testSearchableSnapshotIndexIsReadOnly() throws Exception {
);
}

public void testDeleteSearchableSnapshotBackingIndexThrowsException() throws Exception {
disableRepoConsistencyCheck("reason");
final String indexName = "test-index";
final Client client = client();
final String repoName = "test-repo";
final String snapshotName = "test-snap";
final StepListener<AcknowledgedResponse> deleteSnapshotStepListener = new StepListener<>();
createRepository(repoName, FsRepository.TYPE);
createIndexWithDocsAndEnsureGreen(0, 100, indexName);
takeSnapshot(client, snapshotName, repoName, false, indexName);
internalCluster().ensureAtLeastNumSearchNodes(1);
restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
assertThrows(
SnapshotDeletionException.class,
() -> client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName)).actionGet()
);
}

public void testDeleteSearchableSnapshotBackingIndex() throws Exception {
disableRepoConsistencyCheck("reason");
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";
final StepListener<AcknowledgedResponse> deleteSnapshotStepListener = new StepListener<>();
createRepository(repoName, FsRepository.TYPE);
createIndexWithDocsAndEnsureGreen(0, 100, indexName1);
createIndexWithDocsAndEnsureGreen(0, 100, indexName2);
takeSnapshot(client, snapshotName1, repoName, false, indexName1);
takeSnapshot(client, snapshotName2, repoName, true, indexName2);
internalCluster().ensureAtLeastNumSearchNodes(1);
restoreSnapshotAndEnsureGreen(client, snapshotName2, repoName);
try {
client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName1)).actionGet();
} catch (Exception e) {
fail("Should not have thrown any exception");
}
}

private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDocs, String indexName) throws InterruptedException {
createIndex(
indexName,
Expand All @@ -216,21 +275,30 @@ private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDo
ensureGreen();
}

private void takeSnapshot(Client client, String... indices) {
takeSnapshot(client, null, indices);
private void takeSnapshot(Client client, String snapshotName, String repoName, Boolean doesRepoExist, String... indices) {
takeSnapshot(client, null, snapshotName, repoName, doesRepoExist, indices);
}

private void takeSnapshot(Client client, Settings.Builder repositorySettings, String... indices) {
private void takeSnapshot(
Client client,
Settings.Builder repositorySettings,
String snapshotName,
String repoName,
Boolean doesRepoExist,
String... indices
) {
logger.info("--> Create a repository");
if (repositorySettings == null) {
createRepository("test-repo", "fs");
} else {
createRepository("test-repo", "fs", repositorySettings);
if (!doesRepoExist) {
if (repositorySettings == null) {
createRepository(repoName, FsRepository.TYPE);
} else {
createRepository(repoName, FsRepository.TYPE, repositorySettings);
}
}
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 @@ -247,11 +315,11 @@ private void deleteIndicesAndEnsureGreen(Client client, String... indices) {
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
40 changes: 40 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 @@ -32,16 +32,24 @@
package org.opensearch.snapshots;

import org.opensearch.action.support.IndicesOptions;
import org.opensearch.cluster.ClusterState;
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;
import java.util.Iterator;

/**
* Snapshot utilities
Expand Down Expand Up @@ -135,4 +143,36 @@ 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 currentState current 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(ClusterState currentState, List<SnapshotId> snapshotIds, String repoName) {
final Map<String, SnapshotId> uuidToSnapshotId = new HashMap<>();
final Set<String> snapshotsToBeNotDeleted = new HashSet<>();
ImmutableOpenMap<String, IndexMetadata> indicesMap = currentState.getMetadata().getIndices();
snapshotIds.forEach(snapshotId -> uuidToSnapshotId.put(snapshotId.getUUID(), snapshotId));

for (Iterator<IndexMetadata> it = indicesMap.valuesIt(); it.hasNext();) {
IndexMetadata indexMetadata = it.next();
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 SnapshotDeletionException(
repoName,
snapshotsToBeNotDeleted.toString(),
"These remote snapshots are backing some indices and hence can't be deleted! No snapshots were deleted."
);
}
}
}
44 changes: 3 additions & 41 deletions server/src/main/java/org/opensearch/snapshots/SnapshotsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.index.Index;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.shard.ShardId;
import org.opensearch.repositories.IndexId;
import org.opensearch.repositories.RepositoriesService;
Expand Down Expand Up @@ -134,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 @@ -1771,48 +1770,11 @@ public ClusterState execute(ClusterState currentState) throws Exception {
snapshotNames,
repoName
);
final List<SnapshotId> snapshotIdNotBackingIndex = filterSnapshotsBackingAnyIndex(currentState, snapshotIds);
deleteFromRepoTask = createDeleteStateUpdate(
snapshotIdNotBackingIndex,
repoName,
repositoryData,
Priority.NORMAL,
listener
);
validateSnapshotsBackingAnyIndex(currentState, snapshotIds, repoName);
deleteFromRepoTask = createDeleteStateUpdate(snapshotIds, repoName, repositoryData, Priority.NORMAL, listener);
return deleteFromRepoTask.execute(currentState);
}

private List<SnapshotId> filterSnapshotsBackingAnyIndex(ClusterState currentState, List<SnapshotId> snapshotIds) {
final Set<SnapshotId> snapshotsToBeDeleted = new HashSet<>();
final Set<String> snapshotsToBeNotDeleted = new HashSet<>();
ImmutableOpenMap<String, IndexMetadata> indicesMap = currentState.getMetadata().getIndices();
for (SnapshotId snapshotId : snapshotIds) {
Boolean indexBackedBySnapshotFound = false;
for (Iterator<IndexMetadata> it = indicesMap.valuesIt(); it.hasNext();) {
IndexMetadata indexMetadata = it.next();
String storeType = indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey());
if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(storeType)
&& indexMetadata.getSettings().get(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey()).equals(snapshotId.getUUID())) {
indexBackedBySnapshotFound = true;
break;
}
}
if (indexBackedBySnapshotFound == false) {
snapshotsToBeDeleted.add(snapshotId);
} else {
snapshotsToBeNotDeleted.add(snapshotId.getName());
}
}
if (snapshotIds.size() != snapshotsToBeDeleted.size()) {
throw new SnapshotDeletionException(
repoName,
snapshotsToBeNotDeleted.toString(),
"These remote snapshots are backing some indices and hence can't be deleted! No snapshots were deleted."
);
}
return List.copyOf(snapshotsToBeDeleted);
}

@Override
public ClusterManagerTaskThrottler.ThrottlingKey getClusterManagerThrottlingKey() {
return deleteSnapshotTaskKey;
Expand Down
Loading

0 comments on commit fb6d33d

Please sign in to comment.