review

Author: tlrx

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
                         )
                     );
                 }

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));