From 66215115d54d58bb5b67f0cb2f9bdf4b3f9974ab Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 6 Jul 2021 14:37:06 +0200 Subject: [PATCH 1/6] Introduce searchable snapshots index setting for cascade deletion of snapshots --- .../repositories/RepositoriesService.java | 2 + .../snapshots/RestoreService.java | 119 ++++++++++++- ...archableSnapshotsRepositoryIntegTests.java | 156 ++++++++++++++++++ .../SearchableSnapshots.java | 17 +- 4 files changed, 289 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 218b9dab1aae5..5415e2391cf6d 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -83,6 +83,8 @@ public class RepositoriesService extends AbstractLifecycleComponent implements C public static final String SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY = "index.store.snapshot.repository_name"; public static final String SEARCHABLE_SNAPSHOTS_REPOSITORY_UUID_SETTING_KEY = "index.store.snapshot.repository_uuid"; + public static final String SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY = "index.store.snapshot.snapshot_uuid"; + public static final String SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION = "index.store.snapshot.delete_searchable_snapshot"; private final Map typesRegistry; private final Map internalTypesRegistry; diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 0842c50ec9b50..7eacba63ab520 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -51,6 +51,7 @@ import org.elasticsearch.cluster.routing.allocation.AllocationService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.logging.DeprecationCategory; @@ -83,6 +84,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -101,6 +103,9 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; import static org.elasticsearch.common.util.set.Sets.newHashSet; +import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; +import static org.elasticsearch.repositories.RepositoriesService.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION; +import static org.elasticsearch.repositories.RepositoriesService.SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY; import static org.elasticsearch.snapshots.SnapshotUtils.filterIndices; import static org.elasticsearch.snapshots.SnapshotsService.NO_FEATURE_STATES_VALUE; @@ -1141,6 +1146,9 @@ public ClusterState execute(ClusterState currentState) { resolveSystemIndicesToDelete(currentState, featureStatesToRestore) ); + // List of searchable snapshots indices to restore + final Set searchableSnapshotsIndices = new HashSet<>(); + // Updating cluster state final Metadata.Builder mdBuilder = Metadata.builder(currentState.metadata()); final ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); @@ -1230,6 +1238,10 @@ && isSystemIndex(snapshotIndexMetadata) == false) { : new ShardRestoreStatus(localNodeId) ); } + + if ("snapshot".equals(INDEX_STORE_TYPE_SETTING.get(updatedIndexMetadata.getSettings()))) { + searchableSnapshotsIndices.add(updatedIndexMetadata.getIndex()); + } } final ClusterState.Builder builder = ClusterState.builder(currentState); @@ -1267,10 +1279,11 @@ && isSystemIndex(snapshotIndexMetadata) == false) { } updater.accept(currentState, mdBuilder); - return allocationService.reroute( - builder.metadata(mdBuilder).blocks(blocks).routingTable(rtBuilder.build()).build(), - "restored snapshot [" + snapshot + "]" - ); + final ClusterState updatedClusterState = builder.metadata(mdBuilder).blocks(blocks).routingTable(rtBuilder.build()).build(); + if (searchableSnapshotsIndices.isEmpty() == false) { + ensureSearchableSnapshotsRestorable(updatedClusterState, snapshotInfo, searchableSnapshotsIndices); + } + return allocationService.reroute(updatedClusterState, "restored snapshot [" + snapshot + "]"); } private void applyDataStreamRestores(ClusterState currentState, Metadata.Builder mdBuilder) { @@ -1488,4 +1501,102 @@ private void ensureValidIndexName(ClusterState currentState, IndexMetadata snaps createIndexService.validateDotIndex(renamedIndexName, isHidden); createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false); } + + private static void ensureSearchableSnapshotsRestorable( + final ClusterState currentState, + final SnapshotInfo snapshotInfo, + final Set indices + ) { + final Metadata metadata = currentState.metadata(); + for (Index index : indices) { + final Settings indexSettings = metadata.getIndexSafe(index).getSettings(); + assert "snapshot".equals(INDEX_STORE_TYPE_SETTING.get(indexSettings)) : "not a snapshot backed index: " + index; + + final String repositoryUuid = indexSettings.get(RepositoriesService.SEARCHABLE_SNAPSHOTS_REPOSITORY_UUID_SETTING_KEY); + final String repositoryName = indexSettings.get(RepositoriesService.SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY); + final String snapshotUuid = indexSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY); + + final boolean deleteSnapshot = indexSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false); + if (deleteSnapshot && snapshotInfo.indices().size() != 1) { + throw new SnapshotRestoreException( + repositoryName, + snapshotInfo.snapshotId().getName(), + String.format( + Locale.ROOT, + "cannot mount snapshot [%s/%s:%s] as index [%s] with the deletion of snapshot on index removal enabled " + + "[index.store.snapshot.delete_searchable_snapshot: true]; snapshot contains [%d] indices instead of 1.", + repositoryName, + repositoryUuid, + snapshotInfo.snapshotId().getName(), + index.getName(), + snapshotInfo.indices().size() + ) + ); + } + + for (IndexMetadata other : metadata) { + if (other.getIndex().equals(index)) { + continue; // do not check the searchable snapshot index against itself + } + final Settings otherSettings = other.getSettings(); + if ("snapshot".equals(INDEX_STORE_TYPE_SETTING.get(otherSettings)) == false) { + continue; // other index is not a searchable snapshot index, skip + } + final String otherSnapshotUuid = otherSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY); + if (Objects.equals(snapshotUuid, otherSnapshotUuid) == false) { + continue; // other index is backed by a different snapshot, skip + } + final String otherRepositoryUuid = otherSettings.get(RepositoriesService.SEARCHABLE_SNAPSHOTS_REPOSITORY_UUID_SETTING_KEY); + final String otherRepositoryName = otherSettings.get(RepositoriesService.SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY); + if (matchRepository(repositoryUuid, repositoryName, otherRepositoryUuid, otherRepositoryName) == false) { + continue; // other index is backed by a snapshot from a different repository, skip + } + if (otherSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false)) { + throw new SnapshotRestoreException( + repositoryName, + snapshotInfo.snapshotId().getName(), + String.format( + Locale.ROOT, + "cannot mount snapshot [%s/%s:%s] as index [%s]; another index %s uses the snapshot " + + "with the deletion of snapshot on index removal enabled " + + "[index.store.snapshot.delete_searchable_snapshot: true].", + repositoryName, + repositoryUuid, + snapshotInfo.snapshotId().getName(), + index.getName(), + other.getIndex() + ) + ); + } else if (deleteSnapshot) { + throw new SnapshotRestoreException( + repositoryName, + snapshotInfo.snapshotId().getName(), + String.format( + Locale.ROOT, + "cannot mount snapshot [%s/%s:%s] as index [%s] with the deletion of snapshot on index removal enabled " + + "[index.store.snapshot.delete_searchable_snapshot: true]; another index %s uses the snapshot.", + repositoryName, + repositoryUuid, + snapshotInfo.snapshotId().getName(), + index.getName(), + other.getIndex() + ) + ); + } + } + } + } + + private static boolean matchRepository( + String repositoryUuid, + String repositoryName, + String otherRepositoryUuid, + String otherRepositoryName + ) { + if (Strings.hasLength(repositoryUuid) && Strings.hasLength(otherRepositoryUuid)) { + return Objects.equals(repositoryUuid, otherRepositoryUuid); + } else { + return Objects.equals(repositoryName, otherRepositoryName); + } + } } diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java index e51e62b6101ea..bad3cf0aff786 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java @@ -9,9 +9,13 @@ import org.apache.lucene.search.TotalHits; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; +import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.repositories.fs.FsRepository; +import org.elasticsearch.snapshots.SnapshotRestoreException; +import org.hamcrest.Matcher; import java.util.Arrays; import java.util.List; @@ -19,10 +23,13 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING; import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; +import static org.elasticsearch.repositories.RepositoriesService.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest.Storage; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public class SearchableSnapshotsRepositoryIntegTests extends BaseFrozenSearchableSnapshotsIntegTestCase { @@ -110,4 +117,153 @@ public void testRepositoryUsedBySearchableSnapshotCanBeUpdatedButNotUnregistered assertAcked(clusterAdmin().prepareDeleteRepository(updatedRepositoryName)); } + + public void testMountIndexWithDeletionOfSnapshotFailsIfNotSingleIndexSnapshot() throws Exception { + final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT); + createRepository(repository, FsRepository.TYPE, randomRepositorySettings()); + + final int nbIndices = randomIntBetween(2, 5); + for (int i = 0; i < nbIndices; i++) { + createAndPopulateIndex( + "index-" + i, + Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).put(INDEX_SOFT_DELETES_SETTING.getKey(), true) + ); + } + + final String snapshot = "snapshot"; + createFullSnapshot(repository, snapshot); + assertAcked(client().admin().indices().prepareDelete("index-*")); + + final String index = "index-" + randomInt(nbIndices - 1); + final String mountedIndex = "mounted-" + index; + + final SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> mountSnapshot(repository, snapshot, index, mountedIndex, deleteSnapshotIndexSettings(true), randomFrom(Storage.values())) + ); + assertThat( + exception.getMessage(), + allOf( + containsString("cannot mount snapshot [" + repository + '/'), + containsString(snapshot + "] as index [" + mountedIndex + "] with the deletion of snapshot on index removal enabled"), + containsString("[index.store.snapshot.delete_searchable_snapshot: true]; "), + containsString("snapshot contains [" + nbIndices + "] indices instead of 1.") + ) + ); + } + + public void testMountIndexWithDeletionOfSnapshot() throws Exception { + final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT); + createRepository(repository, FsRepository.TYPE, randomRepositorySettings()); + + final String index = "index"; + createAndPopulateIndex(index, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true)); + + final TotalHits totalHits = internalCluster().client().prepareSearch(index).setTrackTotalHits(true).get().getHits().getTotalHits(); + + final String snapshot = "snapshot"; + createSnapshot(repository, snapshot, List.of(index)); + assertAcked(client().admin().indices().prepareDelete(index)); + + String mounted = "mounted-with-setting-enabled"; + mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(true), randomFrom(Storage.values())); + assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo("true")); + assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); + + // the snapshot is already mounted as an index with "index.store.snapshot.delete_searchable_snapshot: true", + // any attempt to mount the snapshot again should fail + final String mountedAgain = randomValueOtherThan(mounted, () -> randomAlphaOfLength(10).toLowerCase(Locale.ROOT)); + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(randomBoolean())) + ); + assertThat( + exception.getMessage(), + allOf( + containsString("cannot mount snapshot [" + repository + '/'), + containsString(':' + snapshot + "] as index [" + mountedAgain + "]; another index [" + mounted + '/'), + containsString("] uses the snapshot with the deletion of snapshot on index removal enabled "), + containsString("[index.store.snapshot.delete_searchable_snapshot: true].") + ) + ); + + assertAcked(client().admin().indices().prepareDelete(mounted)); + mounted = "mounted-with-setting-disabled"; + mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(false), randomFrom(Storage.values())); + assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo("false")); + assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); + + // the snapshot is now mounted as an index with "index.store.snapshot.delete_searchable_snapshot: false", + // any attempt to mount the snapshot again with "delete_searchable_snapshot: true" should fail + exception = expectThrows( + SnapshotRestoreException.class, + () -> mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(true)) + ); + assertThat( + exception.getMessage(), + allOf( + containsString("cannot mount snapshot [" + repository + '/'), + containsString(snapshot + "] as index [" + mountedAgain + "] with the deletion of snapshot on index removal enabled"), + containsString("[index.store.snapshot.delete_searchable_snapshot: true]; another index [" + mounted + '/'), + containsString("] uses the snapshot.") + ) + ); + + // but we can continue to mount the snapshot, as long as it does not require the cascade deletion of the snapshot + mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(false)); + assertIndexSetting(mountedAgain, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo("false")); + assertHitCount(client().prepareSearch(mountedAgain).setTrackTotalHits(true).get(), totalHits.value); + + assertAcked(client().admin().indices().prepareDelete(mountedAgain)); + assertAcked(client().admin().indices().prepareDelete(mounted)); + } + + public void testDeletionOfSnapshotSettingCannotBeUpdated() throws Exception { + final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT); + createRepository(repository, FsRepository.TYPE, randomRepositorySettings()); + + final String index = "index"; + createAndPopulateIndex(index, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true)); + + final TotalHits totalHits = internalCluster().client().prepareSearch(index).setTrackTotalHits(true).get().getHits().getTotalHits(); + + final String snapshot = "snapshot"; + createSnapshot(repository, snapshot, List.of(index)); + assertAcked(client().admin().indices().prepareDelete(index)); + + final String mounted = "mounted-" + index; + final boolean deleteSnapshot = randomBoolean(); + + mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(deleteSnapshot), randomFrom(Storage.values())); + assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo(Boolean.toString(deleteSnapshot))); + assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); + + final IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> client().admin() + .indices() + .prepareUpdateSettings(mounted) + .setSettings(deleteSnapshotIndexSettings(deleteSnapshot == false)) + .get() + ); + assertThat( + exception.getMessage(), + containsString("can not update private setting [index.store.snapshot.delete_searchable_snapshot]; ") + ); + + assertAcked(client().admin().indices().prepareDelete(mounted)); + } + + private static Settings deleteSnapshotIndexSettings(boolean value) { + return Settings.builder().put(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, value).build(); + } + + private static void assertIndexSetting(String indexName, String indexSettingName, Matcher matcher) { + final GetSettingsResponse getSettingsResponse = client().admin().indices().prepareGetSettings(indexName).get(); + assertThat( + "Unexpected value for setting [" + indexSettingName + "] of index [" + indexName + ']', + getSettingsResponse.getSetting(indexName, indexSettingName), + matcher + ); + } } diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java index 101f93b6f9b8e..4004e00faa213 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java @@ -129,6 +129,8 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME; +import static org.elasticsearch.repositories.RepositoriesService.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION; +import static org.elasticsearch.repositories.RepositoriesService.SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY; import static org.elasticsearch.xpack.core.ClientHelper.SEARCHABLE_SNAPSHOTS_ORIGIN; import static org.elasticsearch.xpack.core.searchablesnapshots.SearchableSnapshotsConstants.CACHE_FETCH_ASYNC_THREAD_POOL_NAME; import static org.elasticsearch.xpack.core.searchablesnapshots.SearchableSnapshotsConstants.CACHE_FETCH_ASYNC_THREAD_POOL_SETTING; @@ -163,7 +165,7 @@ public class SearchableSnapshots extends Plugin implements IndexStorePlugin, Eng Setting.Property.NotCopyableOnResize ); public static final Setting SNAPSHOT_SNAPSHOT_ID_SETTING = Setting.simpleString( - "index.store.snapshot.snapshot_uuid", + SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY, Setting.Property.IndexScope, Setting.Property.PrivateIndex, Setting.Property.NotCopyableOnResize @@ -234,6 +236,18 @@ public class SearchableSnapshots extends Plugin implements IndexStorePlugin, Eng Setting.Property.NotCopyableOnResize ); + /** + * Index setting used to indicate if the snapshot that is mounted as an index should be deleted when the index is deleted. This setting + * is only set for indices mounted in clusters on or after 8.0.0. Once set this setting cannot be updated. + */ + public static final Setting DELETE_SEARCHABLE_SNAPSHOT_ON_INDEX_DELETION = Setting.boolSetting( + SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, + false, + Setting.Property.IndexScope, + Setting.Property.PrivateIndex, + Setting.Property.NotCopyableOnResize + ); + /** * Prefer to allocate to the data content tier and then the hot tier. * This affects the system searchable snapshot cache index (not the searchable snapshot index itself) @@ -283,6 +297,7 @@ public List> getSettings() { SNAPSHOT_CACHE_PREWARM_ENABLED_SETTING, SNAPSHOT_CACHE_EXCLUDED_FILE_TYPES_SETTING, SNAPSHOT_UNCACHED_CHUNK_SIZE_SETTING, + DELETE_SEARCHABLE_SNAPSHOT_ON_INDEX_DELETION, SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING, SNAPSHOT_BLOB_CACHE_METADATA_FILES_MAX_LENGTH_SETTING, CacheService.SNAPSHOT_CACHE_RANGE_SIZE_SETTING, From 76480bb28a0e89c47b3218346a26b25e0faa2249 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 8 Jul 2021 15:59:58 +0200 Subject: [PATCH 2/6] review --- .../snapshots/RestoreService.java | 27 +++------- ...archableSnapshotsRepositoryIntegTests.java | 49 +++++-------------- 2 files changed, 20 insertions(+), 56 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 7eacba63ab520..77c55e49c4d62 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -1551,35 +1551,22 @@ private static void ensureSearchableSnapshotsRestorable( if (matchRepository(repositoryUuid, repositoryName, otherRepositoryUuid, otherRepositoryName) == false) { continue; // other index is backed by a snapshot from a different repository, skip } - if (otherSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false)) { + final boolean otherDeleteSnap = otherSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false); + if (deleteSnapshot != otherDeleteSnap) { throw new SnapshotRestoreException( repositoryName, snapshotInfo.snapshotId().getName(), String.format( Locale.ROOT, - "cannot mount snapshot [%s/%s:%s] as index [%s]; another index %s uses the snapshot " - + "with the deletion of snapshot on index removal enabled " - + "[index.store.snapshot.delete_searchable_snapshot: true].", + "cannot mount snapshot [%s/%s:%s] as index [%s] with [index.store.snapshot.delete_searchable_snapshot: %b]; " + + "another index %s is mounted with [index.store.snapshot.delete_searchable_snapshot: %b].", repositoryName, repositoryUuid, snapshotInfo.snapshotId().getName(), index.getName(), - other.getIndex() - ) - ); - } else if (deleteSnapshot) { - throw new SnapshotRestoreException( - repositoryName, - snapshotInfo.snapshotId().getName(), - String.format( - Locale.ROOT, - "cannot mount snapshot [%s/%s:%s] as index [%s] with the deletion of snapshot on index removal enabled " - + "[index.store.snapshot.delete_searchable_snapshot: true]; another index %s uses the snapshot.", - repositoryName, - repositoryUuid, - snapshotInfo.snapshotId().getName(), - index.getName(), - other.getIndex() + deleteSnapshot, + other.getIndex(), + otherDeleteSnap ) ); } diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java index bad3cf0aff786..7daa71f5c14cd 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java @@ -152,7 +152,7 @@ public void testMountIndexWithDeletionOfSnapshotFailsIfNotSingleIndexSnapshot() ); } - public void testMountIndexWithDeletionOfSnapshot() throws Exception { + public void testMountIndexWithDifferentDeletionOfSnapshot() throws Exception { final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT); createRepository(repository, FsRepository.TYPE, randomRepositorySettings()); @@ -165,53 +165,30 @@ public void testMountIndexWithDeletionOfSnapshot() throws Exception { createSnapshot(repository, snapshot, List.of(index)); assertAcked(client().admin().indices().prepareDelete(index)); - String mounted = "mounted-with-setting-enabled"; - mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(true), randomFrom(Storage.values())); - assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo("true")); + final boolean deleteSnapshot = randomBoolean(); + final String mounted = "mounted-with-setting-" + deleteSnapshot; + mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(deleteSnapshot), randomFrom(Storage.values())); + assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo(Boolean.toString(deleteSnapshot))); assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); - // the snapshot is already mounted as an index with "index.store.snapshot.delete_searchable_snapshot: true", - // any attempt to mount the snapshot again should fail final String mountedAgain = randomValueOtherThan(mounted, () -> randomAlphaOfLength(10).toLowerCase(Locale.ROOT)); - SnapshotRestoreException exception = expectThrows( - SnapshotRestoreException.class, - () -> mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(randomBoolean())) - ); - assertThat( - exception.getMessage(), - allOf( - containsString("cannot mount snapshot [" + repository + '/'), - containsString(':' + snapshot + "] as index [" + mountedAgain + "]; another index [" + mounted + '/'), - containsString("] uses the snapshot with the deletion of snapshot on index removal enabled "), - containsString("[index.store.snapshot.delete_searchable_snapshot: true].") - ) - ); - - assertAcked(client().admin().indices().prepareDelete(mounted)); - mounted = "mounted-with-setting-disabled"; - mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(false), randomFrom(Storage.values())); - assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo("false")); - assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); - - // the snapshot is now mounted as an index with "index.store.snapshot.delete_searchable_snapshot: false", - // any attempt to mount the snapshot again with "delete_searchable_snapshot: true" should fail - exception = expectThrows( + final SnapshotRestoreException exception = expectThrows( SnapshotRestoreException.class, - () -> mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(true)) + () -> mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(deleteSnapshot == false)) ); assertThat( exception.getMessage(), allOf( containsString("cannot mount snapshot [" + repository + '/'), - containsString(snapshot + "] as index [" + mountedAgain + "] with the deletion of snapshot on index removal enabled"), - containsString("[index.store.snapshot.delete_searchable_snapshot: true]; another index [" + mounted + '/'), - containsString("] uses the snapshot.") + containsString(':' + snapshot + "] as index [" + mountedAgain + "] with "), + containsString("[index.store.snapshot.delete_searchable_snapshot: " + (deleteSnapshot == false) + "]; another "), + containsString("index [" + mounted + '/'), + containsString("is mounted with [index.store.snapshot.delete_searchable_snapshot: " + deleteSnapshot + "].") ) ); - // but we can continue to mount the snapshot, as long as it does not require the cascade deletion of the snapshot - mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(false)); - assertIndexSetting(mountedAgain, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo("false")); + mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(deleteSnapshot)); + assertIndexSetting(mountedAgain, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo(Boolean.toString(deleteSnapshot))); assertHitCount(client().prepareSearch(mountedAgain).setTrackTotalHits(true).get(), totalHits.value); assertAcked(client().admin().indices().prepareDelete(mountedAgain)); From 80d638cfb399103b9c46a22bfecc79c064af635a Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 9 Jul 2021 11:35:35 +0200 Subject: [PATCH 3/6] also use null value setting --- ...archableSnapshotsRepositoryIntegTests.java | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java index 7daa71f5c14cd..4d9af8442f1b2 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java @@ -13,9 +13,9 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.core.Nullable; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.snapshots.SnapshotRestoreException; -import org.hamcrest.Matcher; import java.util.Arrays; import java.util.List; @@ -30,6 +30,7 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; public class SearchableSnapshotsRepositoryIntegTests extends BaseFrozenSearchableSnapshotsIntegTestCase { @@ -167,8 +168,16 @@ public void testMountIndexWithDifferentDeletionOfSnapshot() throws Exception { final boolean deleteSnapshot = randomBoolean(); final String mounted = "mounted-with-setting-" + deleteSnapshot; - mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(deleteSnapshot), randomFrom(Storage.values())); - assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo(Boolean.toString(deleteSnapshot))); + final Settings indexSettings = deleteSnapshotIndexSettingsOrNull(deleteSnapshot); + + logger.info("--> mounting index [{}] with index settings [{}]", mounted, indexSettings); + mountSnapshot(repository, snapshot, index, mounted, indexSettings, randomFrom(Storage.values())); + assertThat( + getDeleteSnapshotIndexSetting(mounted), + indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION) + ? equalTo(Boolean.toString(deleteSnapshot)) + : nullValue() + ); assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); final String mountedAgain = randomValueOtherThan(mounted, () -> randomAlphaOfLength(10).toLowerCase(Locale.ROOT)); @@ -188,7 +197,12 @@ public void testMountIndexWithDifferentDeletionOfSnapshot() throws Exception { ); mountSnapshot(repository, snapshot, index, mountedAgain, deleteSnapshotIndexSettings(deleteSnapshot)); - assertIndexSetting(mountedAgain, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo(Boolean.toString(deleteSnapshot))); + assertThat( + getDeleteSnapshotIndexSetting(mounted), + indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION) + ? equalTo(Boolean.toString(deleteSnapshot)) + : nullValue() + ); assertHitCount(client().prepareSearch(mountedAgain).setTrackTotalHits(true).get(), totalHits.value); assertAcked(client().admin().indices().prepareDelete(mountedAgain)); @@ -210,9 +224,15 @@ public void testDeletionOfSnapshotSettingCannotBeUpdated() throws Exception { final String mounted = "mounted-" + index; final boolean deleteSnapshot = randomBoolean(); + final Settings indexSettings = deleteSnapshotIndexSettingsOrNull(deleteSnapshot); - mountSnapshot(repository, snapshot, index, mounted, deleteSnapshotIndexSettings(deleteSnapshot), randomFrom(Storage.values())); - assertIndexSetting(mounted, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, equalTo(Boolean.toString(deleteSnapshot))); + mountSnapshot(repository, snapshot, index, mounted, indexSettings, randomFrom(Storage.values())); + assertThat( + getDeleteSnapshotIndexSetting(mounted), + indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION) + ? equalTo(Boolean.toString(deleteSnapshot)) + : nullValue() + ); assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); final IllegalArgumentException exception = expectThrows( @@ -235,12 +255,19 @@ private static Settings deleteSnapshotIndexSettings(boolean value) { return Settings.builder().put(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, value).build(); } - private static void assertIndexSetting(String indexName, String indexSettingName, Matcher matcher) { + private static Settings deleteSnapshotIndexSettingsOrNull(boolean value) { + if (value) { + return deleteSnapshotIndexSettings(true); + } else if (randomBoolean()) { + return deleteSnapshotIndexSettings(false); + } else { + return Settings.EMPTY; + } + } + + @Nullable + private static String getDeleteSnapshotIndexSetting(String indexName) { final GetSettingsResponse getSettingsResponse = client().admin().indices().prepareGetSettings(indexName).get(); - assertThat( - "Unexpected value for setting [" + indexSettingName + "] of index [" + indexName + ']', - getSettingsResponse.getSetting(indexName, indexSettingName), - matcher - ); + return getSettingsResponse.getSetting(indexName, SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION); } } From 6d73227aeb7d152226574cfb693fa4c5b6f0bbb1 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 9 Jul 2021 11:55:14 +0200 Subject: [PATCH 4/6] make final --- .../SearchableSnapshotsRepositoryIntegTests.java | 4 ++++ .../xpack/searchablesnapshots/SearchableSnapshots.java | 1 + 2 files changed, 5 insertions(+) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java index 4d9af8442f1b2..34841b2dd2407 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java @@ -235,6 +235,10 @@ public void testDeletionOfSnapshotSettingCannotBeUpdated() throws Exception { ); assertHitCount(client().prepareSearch(mounted).setTrackTotalHits(true).get(), totalHits.value); + if (randomBoolean()) { + assertAcked(client().admin().indices().prepareClose(mounted)); + } + final IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, () -> client().admin() diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java index 4004e00faa213..9bde34a66d0eb 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java @@ -243,6 +243,7 @@ public class SearchableSnapshots extends Plugin implements IndexStorePlugin, Eng public static final Setting DELETE_SEARCHABLE_SNAPSHOT_ON_INDEX_DELETION = Setting.boolSetting( SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false, + Setting.Property.Final, Setting.Property.IndexScope, Setting.Property.PrivateIndex, Setting.Property.NotCopyableOnResize From ff0f8340ca72d80c76bffb5aadb17aba22d87a1a Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 12 Jul 2021 11:50:18 +0200 Subject: [PATCH 5/6] feedback --- .../snapshots/RestoreService.java | 25 ++- ...archableSnapshotsRepositoryIntegTests.java | 146 ++++++++++++++++++ 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 77c55e49c4d62..bf6e5d18aac80 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -1014,11 +1014,12 @@ private static IndexMetadata updateIndexSettings( Settings changeSettings, String[] ignoreSettings ) { + final Settings settings = indexMetadata.getSettings(); Settings normalizedChangeSettings = Settings.builder() .put(changeSettings) .normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX) .build(); - if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexMetadata.getSettings()) + if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(settings) && IndexSettings.INDEX_SOFT_DELETES_SETTING.exists(changeSettings) && IndexSettings.INDEX_SOFT_DELETES_SETTING.get(changeSettings) == false) { throw new SnapshotRestoreException( @@ -1026,8 +1027,26 @@ private static IndexMetadata updateIndexSettings( "cannot disable setting [" + IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey() + "] on restore" ); } + if ("snapshot".equals(INDEX_STORE_TYPE_SETTING.get(settings))) { + final Boolean changed = changeSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, null); + if (changed != null) { + final Boolean previous = settings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, null); + if (Objects.equals(previous, changed) == false) { + throw new SnapshotRestoreException( + snapshot, + String.format( + Locale.ROOT, + "cannot change value of [%s] when restoring searchable snapshot [%s:%s] as index %s", + SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, + snapshot.getRepository(), + snapshot.getSnapshotId().getName(), + indexMetadata.getIndex() + ) + ); + } + } + } IndexMetadata.Builder builder = IndexMetadata.builder(indexMetadata); - Settings settings = indexMetadata.getSettings(); Set keyFilters = new HashSet<>(); List simpleMatchPatterns = new ArrayList<>(); for (String ignoredSetting : ignoreSettings) { @@ -1517,7 +1536,7 @@ private static void ensureSearchableSnapshotsRestorable( final String snapshotUuid = indexSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY); final boolean deleteSnapshot = indexSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false); - if (deleteSnapshot && snapshotInfo.indices().size() != 1) { + if (deleteSnapshot && snapshotInfo.indices().size() != 1 && Objects.equals(snapshotUuid, snapshotInfo.snapshotId().getUUID())) { throw new SnapshotRestoreException( repositoryName, snapshotInfo.snapshotId().getName(), diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java index 34841b2dd2407..982010d0316ff 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java @@ -18,8 +18,10 @@ import org.elasticsearch.snapshots.SnapshotRestoreException; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Set; import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING; import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; @@ -30,6 +32,7 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.nullValue; public class SearchableSnapshotsRepositoryIntegTests extends BaseFrozenSearchableSnapshotsIntegTestCase { @@ -255,6 +258,149 @@ public void testDeletionOfSnapshotSettingCannotBeUpdated() throws Exception { assertAcked(client().admin().indices().prepareDelete(mounted)); } + public void testRestoreSearchableSnapshotIndexConflicts() throws Exception { + final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT); + createRepository(repository, FsRepository.TYPE, randomRepositorySettings()); + + final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + createAndPopulateIndex(indexName, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true)); + + final String snapshotOfIndex = "snapshot-of-index"; + createSnapshot(repository, snapshotOfIndex, List.of(indexName)); + assertAcked(client().admin().indices().prepareDelete(indexName)); + + final String mountedIndex = "mounted-index"; + final boolean deleteSnapshot = randomBoolean(); + final Settings indexSettings = deleteSnapshotIndexSettingsOrNull(deleteSnapshot); + logger.info("--> mounting snapshot of index [{}] as [{}] with index settings [{}]", indexName, mountedIndex, indexSettings); + mountSnapshot(repository, snapshotOfIndex, indexName, mountedIndex, indexSettings, randomFrom(Storage.values())); + assertThat( + getDeleteSnapshotIndexSetting(mountedIndex), + indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION) + ? equalTo(Boolean.toString(deleteSnapshot)) + : nullValue() + ); + + final String snapshotOfMountedIndex = "snapshot-of-mounted-index"; + createSnapshot(repository, snapshotOfMountedIndex, List.of(mountedIndex)); + assertAcked(client().admin().indices().prepareDelete(mountedIndex)); + + final String mountedIndexAgain = "mounted-index-again"; + final boolean deleteSnapshotAgain = deleteSnapshot == false; + final Settings indexSettingsAgain = deleteSnapshotIndexSettings(deleteSnapshotAgain); + logger.info("--> mounting snapshot of index [{}] again as [{}] with index settings [{}]", indexName, mountedIndex, indexSettings); + mountSnapshot(repository, snapshotOfIndex, indexName, mountedIndexAgain, indexSettingsAgain, randomFrom(Storage.values())); + assertThat( + getDeleteSnapshotIndexSetting(mountedIndexAgain), + indexSettingsAgain.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION) + ? equalTo(Boolean.toString(deleteSnapshotAgain)) + : nullValue() + ); + + logger.info("--> restoring snapshot of searchable snapshot index [{}] should be conflicting", mountedIndex); + final SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(repository, snapshotOfMountedIndex) + .setIndices(mountedIndex) + .setWaitForCompletion(true) + .get() + ); + assertThat( + exception.getMessage(), + allOf( + containsString("cannot mount snapshot [" + repository + '/'), + containsString(':' + snapshotOfMountedIndex + "] as index [" + mountedIndex + "] with "), + containsString("[index.store.snapshot.delete_searchable_snapshot: " + deleteSnapshot + "]; another "), + containsString("index [" + mountedIndexAgain + '/'), + containsString("is mounted with [index.store.snapshot.delete_searchable_snapshot: " + deleteSnapshotAgain + "].") + ) + ); + assertAcked(client().admin().indices().prepareDelete("mounted-*")); + } + + public void testRestoreSearchableSnapshotIndexWithDifferentSettingsConflicts() throws Exception { + final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT); + createRepository(repository, FsRepository.TYPE, randomRepositorySettings()); + + final int nbIndices = randomIntBetween(1, 3); + for (int i = 0; i < nbIndices; i++) { + createAndPopulateIndex("index-" + i, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true)); + } + + final String snapshotOfIndices = "snapshot-of-indices"; + createFullSnapshot(repository, snapshotOfIndices); + + final int nbMountedIndices = randomIntBetween(1, 3); + final Set mountedIndices = new HashSet<>(nbMountedIndices); + + final boolean deleteSnapshot = nbIndices == 1 && randomBoolean(); + final Settings indexSettings = deleteSnapshotIndexSettingsOrNull(deleteSnapshot); + + for (int i = 0; i < nbMountedIndices; i++) { + final String index = "index-" + randomInt(nbIndices - 1); + final String mountedIndex = "mounted-" + i; + logger.info("--> mounting snapshot of index [{}] as [{}] with index settings [{}]", index, mountedIndex, indexSettings); + mountSnapshot(repository, snapshotOfIndices, index, mountedIndex, indexSettings, randomFrom(Storage.values())); + assertThat( + getDeleteSnapshotIndexSetting(mountedIndex), + indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION) + ? equalTo(Boolean.toString(deleteSnapshot)) + : nullValue() + ); + if (randomBoolean()) { + assertAcked(client().admin().indices().prepareClose(mountedIndex)); + } + mountedIndices.add(mountedIndex); + } + + final String snapshotOfMountedIndices = "snapshot-of-mounted-indices"; + createSnapshot(repository, snapshotOfMountedIndices, List.of("mounted-*")); + + List restorables = randomBoolean() + ? List.of("mounted-*") + : randomSubsetOf(randomIntBetween(1, nbMountedIndices), mountedIndices); + final SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(repository, snapshotOfMountedIndices) + .setIndices(restorables.toArray(String[]::new)) + .setIndexSettings(deleteSnapshotIndexSettings(deleteSnapshot == false)) + .setRenameReplacement("restored-with-different-setting-$1") + .setRenamePattern("(.+)") + .setWaitForCompletion(true) + .get() + ); + + assertThat( + exception.getMessage(), + containsString( + "cannot change value of [index.store.snapshot.delete_searchable_snapshot] when restoring searchable snapshot [" + + repository + + ':' + + snapshotOfMountedIndices + + "] as index [mounted-" + ) + ); + + final RestoreSnapshotResponse restoreResponse = client().admin() + .cluster() + .prepareRestoreSnapshot(repository, snapshotOfMountedIndices) + .setIndices(restorables.toArray(String[]::new)) + .setIndexSettings(indexSettings) + .setRenameReplacement("restored-with-same-setting-$1") + .setRenamePattern("(.+)") + .setWaitForCompletion(true) + .get(); + assertThat(restoreResponse.getRestoreInfo().totalShards(), greaterThan(0)); + assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0)); + + assertAcked(client().admin().indices().prepareDelete("mounted-*")); + assertAcked(client().admin().indices().prepareDelete("restored-with-same-setting-*")); + } + private static Settings deleteSnapshotIndexSettings(boolean value) { return Settings.builder().put(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, value).build(); } From 8f1d1d436fc1aa229092e04b6de1182ba8e57427 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 15 Jul 2021 11:28:26 +0200 Subject: [PATCH 6/6] merge master --- .../SearchableSnapshotsRepositoryIntegTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java index 982010d0316ff..1cdc6d228ac57 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java @@ -25,7 +25,7 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING; import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; -import static org.elasticsearch.repositories.RepositoriesService.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION; +import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest.Storage;