Skip to content

Commit ff0f834

Browse files
tlrxtlrx
committed
feedback
1 parent 6d73227 commit ff0f834

File tree

2 files changed

+168
-3
lines changed

2 files changed

+168
-3
lines changed

‎server/src/main/java/org/elasticsearch/snapshots/RestoreService.java‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,20 +1014,39 @@ private static IndexMetadata updateIndexSettings(
10141014
Settings changeSettings,
10151015
String[] ignoreSettings
10161016
) {
1017+
final Settings settings = indexMetadata.getSettings();
10171018
Settings normalizedChangeSettings = Settings.builder()
10181019
.put(changeSettings)
10191020
.normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX)
10201021
.build();
1021-
if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexMetadata.getSettings())
1022+
if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(settings)
10221023
&& IndexSettings.INDEX_SOFT_DELETES_SETTING.exists(changeSettings)
10231024
&& IndexSettings.INDEX_SOFT_DELETES_SETTING.get(changeSettings) == false) {
10241025
throw new SnapshotRestoreException(
10251026
snapshot,
10261027
"cannot disable setting [" + IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey() + "] on restore"
10271028
);
10281029
}
1030+
if ("snapshot".equals(INDEX_STORE_TYPE_SETTING.get(settings))) {
1031+
final Boolean changed = changeSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, null);
1032+
if (changed != null) {
1033+
final Boolean previous = settings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, null);
1034+
if (Objects.equals(previous, changed) == false) {
1035+
throw new SnapshotRestoreException(
1036+
snapshot,
1037+
String.format(
1038+
Locale.ROOT,
1039+
"cannot change value of [%s] when restoring searchable snapshot [%s:%s] as index %s",
1040+
SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION,
1041+
snapshot.getRepository(),
1042+
snapshot.getSnapshotId().getName(),
1043+
indexMetadata.getIndex()
1044+
)
1045+
);
1046+
}
1047+
}
1048+
}
10291049
IndexMetadata.Builder builder = IndexMetadata.builder(indexMetadata);
1030-
Settings settings = indexMetadata.getSettings();
10311050
Set<String> keyFilters = new HashSet<>();
10321051
List<String> simpleMatchPatterns = new ArrayList<>();
10331052
for (String ignoredSetting : ignoreSettings) {
@@ -1517,7 +1536,7 @@ private static void ensureSearchableSnapshotsRestorable(
15171536
final String snapshotUuid = indexSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY);
15181537

15191538
final boolean deleteSnapshot = indexSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false);
1520-
if (deleteSnapshot && snapshotInfo.indices().size() != 1) {
1539+
if (deleteSnapshot && snapshotInfo.indices().size() != 1 && Objects.equals(snapshotUuid, snapshotInfo.snapshotId().getUUID())) {
15211540
throw new SnapshotRestoreException(
15221541
repositoryName,
15231542
snapshotInfo.snapshotId().getName(),

‎x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsRepositoryIntegTests.java‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
import org.elasticsearch.snapshots.SnapshotRestoreException;
1919

2020
import java.util.Arrays;
21+
import java.util.HashSet;
2122
import java.util.List;
2223
import java.util.Locale;
24+
import java.util.Set;
2325

2426
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
2527
import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING;
@@ -30,6 +32,7 @@
3032
import static org.hamcrest.Matchers.allOf;
3133
import static org.hamcrest.Matchers.containsString;
3234
import static org.hamcrest.Matchers.equalTo;
35+
import static org.hamcrest.Matchers.greaterThan;
3336
import static org.hamcrest.Matchers.nullValue;
3437

3538
public class SearchableSnapshotsRepositoryIntegTests extends BaseFrozenSearchableSnapshotsIntegTestCase {
@@ -255,6 +258,149 @@ public void testDeletionOfSnapshotSettingCannotBeUpdated() throws Exception {
255258
assertAcked(client().admin().indices().prepareDelete(mounted));
256259
}
257260

261+
public void testRestoreSearchableSnapshotIndexConflicts() throws Exception {
262+
final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT);
263+
createRepository(repository, FsRepository.TYPE, randomRepositorySettings());
264+
265+
final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
266+
createAndPopulateIndex(indexName, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true));
267+
268+
final String snapshotOfIndex = "snapshot-of-index";
269+
createSnapshot(repository, snapshotOfIndex, List.of(indexName));
270+
assertAcked(client().admin().indices().prepareDelete(indexName));
271+
272+
final String mountedIndex = "mounted-index";
273+
final boolean deleteSnapshot = randomBoolean();
274+
final Settings indexSettings = deleteSnapshotIndexSettingsOrNull(deleteSnapshot);
275+
logger.info("--> mounting snapshot of index [{}] as [{}] with index settings [{}]", indexName, mountedIndex, indexSettings);
276+
mountSnapshot(repository, snapshotOfIndex, indexName, mountedIndex, indexSettings, randomFrom(Storage.values()));
277+
assertThat(
278+
getDeleteSnapshotIndexSetting(mountedIndex),
279+
indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION)
280+
? equalTo(Boolean.toString(deleteSnapshot))
281+
: nullValue()
282+
);
283+
284+
final String snapshotOfMountedIndex = "snapshot-of-mounted-index";
285+
createSnapshot(repository, snapshotOfMountedIndex, List.of(mountedIndex));
286+
assertAcked(client().admin().indices().prepareDelete(mountedIndex));
287+
288+
final String mountedIndexAgain = "mounted-index-again";
289+
final boolean deleteSnapshotAgain = deleteSnapshot == false;
290+
final Settings indexSettingsAgain = deleteSnapshotIndexSettings(deleteSnapshotAgain);
291+
logger.info("--> mounting snapshot of index [{}] again as [{}] with index settings [{}]", indexName, mountedIndex, indexSettings);
292+
mountSnapshot(repository, snapshotOfIndex, indexName, mountedIndexAgain, indexSettingsAgain, randomFrom(Storage.values()));
293+
assertThat(
294+
getDeleteSnapshotIndexSetting(mountedIndexAgain),
295+
indexSettingsAgain.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION)
296+
? equalTo(Boolean.toString(deleteSnapshotAgain))
297+
: nullValue()
298+
);
299+
300+
logger.info("--> restoring snapshot of searchable snapshot index [{}] should be conflicting", mountedIndex);
301+
final SnapshotRestoreException exception = expectThrows(
302+
SnapshotRestoreException.class,
303+
() -> client().admin()
304+
.cluster()
305+
.prepareRestoreSnapshot(repository, snapshotOfMountedIndex)
306+
.setIndices(mountedIndex)
307+
.setWaitForCompletion(true)
308+
.get()
309+
);
310+
assertThat(
311+
exception.getMessage(),
312+
allOf(
313+
containsString("cannot mount snapshot [" + repository + '/'),
314+
containsString(':' + snapshotOfMountedIndex + "] as index [" + mountedIndex + "] with "),
315+
containsString("[index.store.snapshot.delete_searchable_snapshot: " + deleteSnapshot + "]; another "),
316+
containsString("index [" + mountedIndexAgain + '/'),
317+
containsString("is mounted with [index.store.snapshot.delete_searchable_snapshot: " + deleteSnapshotAgain + "].")
318+
)
319+
);
320+
assertAcked(client().admin().indices().prepareDelete("mounted-*"));
321+
}
322+
323+
public void testRestoreSearchableSnapshotIndexWithDifferentSettingsConflicts() throws Exception {
324+
final String repository = "repository-" + getTestName().toLowerCase(Locale.ROOT);
325+
createRepository(repository, FsRepository.TYPE, randomRepositorySettings());
326+
327+
final int nbIndices = randomIntBetween(1, 3);
328+
for (int i = 0; i < nbIndices; i++) {
329+
createAndPopulateIndex("index-" + i, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true));
330+
}
331+
332+
final String snapshotOfIndices = "snapshot-of-indices";
333+
createFullSnapshot(repository, snapshotOfIndices);
334+
335+
final int nbMountedIndices = randomIntBetween(1, 3);
336+
final Set<String> mountedIndices = new HashSet<>(nbMountedIndices);
337+
338+
final boolean deleteSnapshot = nbIndices == 1 && randomBoolean();
339+
final Settings indexSettings = deleteSnapshotIndexSettingsOrNull(deleteSnapshot);
340+
341+
for (int i = 0; i < nbMountedIndices; i++) {
342+
final String index = "index-" + randomInt(nbIndices - 1);
343+
final String mountedIndex = "mounted-" + i;
344+
logger.info("--> mounting snapshot of index [{}] as [{}] with index settings [{}]", index, mountedIndex, indexSettings);
345+
mountSnapshot(repository, snapshotOfIndices, index, mountedIndex, indexSettings, randomFrom(Storage.values()));
346+
assertThat(
347+
getDeleteSnapshotIndexSetting(mountedIndex),
348+
indexSettings.hasValue(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION)
349+
? equalTo(Boolean.toString(deleteSnapshot))
350+
: nullValue()
351+
);
352+
if (randomBoolean()) {
353+
assertAcked(client().admin().indices().prepareClose(mountedIndex));
354+
}
355+
mountedIndices.add(mountedIndex);
356+
}
357+
358+
final String snapshotOfMountedIndices = "snapshot-of-mounted-indices";
359+
createSnapshot(repository, snapshotOfMountedIndices, List.of("mounted-*"));
360+
361+
List<String> restorables = randomBoolean()
362+
? List.of("mounted-*")
363+
: randomSubsetOf(randomIntBetween(1, nbMountedIndices), mountedIndices);
364+
final SnapshotRestoreException exception = expectThrows(
365+
SnapshotRestoreException.class,
366+
() -> client().admin()
367+
.cluster()
368+
.prepareRestoreSnapshot(repository, snapshotOfMountedIndices)
369+
.setIndices(restorables.toArray(String[]::new))
370+
.setIndexSettings(deleteSnapshotIndexSettings(deleteSnapshot == false))
371+
.setRenameReplacement("restored-with-different-setting-$1")
372+
.setRenamePattern("(.+)")
373+
.setWaitForCompletion(true)
374+
.get()
375+
);
376+
377+
assertThat(
378+
exception.getMessage(),
379+
containsString(
380+
"cannot change value of [index.store.snapshot.delete_searchable_snapshot] when restoring searchable snapshot ["
381+
+ repository
382+
+ ':'
383+
+ snapshotOfMountedIndices
384+
+ "] as index [mounted-"
385+
)
386+
);
387+
388+
final RestoreSnapshotResponse restoreResponse = client().admin()
389+
.cluster()
390+
.prepareRestoreSnapshot(repository, snapshotOfMountedIndices)
391+
.setIndices(restorables.toArray(String[]::new))
392+
.setIndexSettings(indexSettings)
393+
.setRenameReplacement("restored-with-same-setting-$1")
394+
.setRenamePattern("(.+)")
395+
.setWaitForCompletion(true)
396+
.get();
397+
assertThat(restoreResponse.getRestoreInfo().totalShards(), greaterThan(0));
398+
assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0));
399+
400+
assertAcked(client().admin().indices().prepareDelete("mounted-*"));
401+
assertAcked(client().admin().indices().prepareDelete("restored-with-same-setting-*"));
402+
}
403+
258404
private static Settings deleteSnapshotIndexSettings(boolean value) {
259405
return Settings.builder().put(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, value).build();
260406
}

0 commit comments

Comments
 (0)