Added new Setting property UnmodifiableOnRestore to prevent updatin…

…g settings on restore snapshot (#16957) (#17116) * Add index.knn setting to list of unmodifiable settings when restore snapshot Signed-off-by: AnnTian Shao <[email protected]> * Add index.knn setting to list of unmodifiable settings when restore snapshot Signed-off-by: AnnTian Shao <[email protected]> * Add new Setting property UnmodifiableOnRestore to mark setting as immutable on restore snapshot Signed-off-by: AnnTian Shao <[email protected]> * Add tests for new Setting property UnmodifiableOnRestore Signed-off-by: AnnTian Shao <[email protected]> * fixes and added more tests for new setting property UnmodifiableOnRestore Signed-off-by: AnnTian Shao <[email protected]> * fix test failures Signed-off-by: AnnTian Shao <[email protected]> * Revert "fix test failures" This reverts commit 252100c. Signed-off-by: AnnTian Shao <[email protected]> * fixes by adding back USER_UNMODIFIABLE_SETTINGS for settings without Setting implementation Signed-off-by: AnnTian Shao <[email protected]> * testing CI config for registering plugin settings Signed-off-by: AnnTian Shao <[email protected]> * Revert "testing CI config for registering plugin settings" This reverts commit 9ebab5a. Signed-off-by: AnnTian Shao <[email protected]> * Add UnmodifiableOnRestore only to unmodifiable settings that are already registered, will raise separate PR for settings not yet registered. Add validation check in Setting.java. Add UnmodifiableOnRestore settings cannot be removed on restore Signed-off-by: AnnTian Shao <[email protected]> * fixes and move tests to RestoreSnapshotIT Signed-off-by: AnnTian Shao <[email protected]> * Add back testInvalidRestoreRequestScenarios test method Signed-off-by: AnnTian Shao <[email protected]> --------- Signed-off-by: AnnTian Shao <[email protected]> Signed-off-by: Tommy Shao <[email protected]> Co-authored-by: AnnTian Shao <[email protected]> (cherry picked from commit 822f80c) Signed-off-by: Tommy Shao <[email protected]>
opensearch-project · Jan 24, 2025 · 99e0c57 · 99e0c57
1 parent ca8108e
commit 99e0c57
Show file tree

Hide file tree

Showing 11 changed files with 767 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Added a new `time` field to replace the deprecated `getTime` field in `GetStats`. ([#17009](https://github.com/opensearch-project/OpenSearch/pull/17009))
 - Improve performance of the bitmap filtering([#16936](https://github.com/opensearch-project/OpenSearch/pull/16936/))
 - Added ability to retrieve value from DocValues in a flat_object filed([#16802](https://github.com/opensearch-project/OpenSearch/pull/16802))
+- Added new Setting property UnmodifiableOnRestore to prevent updating settings on restore snapshot ([#16957](https://github.com/opensearch-project/OpenSearch/pull/16957))
+- Introduce Template query ([#16818](https://github.com/opensearch-project/OpenSearch/pull/16818))
 - Propagate the sourceIncludes and excludes fields from fetchSourceContext to FieldsVisitor. ([#17080](https://github.com/opensearch-project/OpenSearch/pull/17080))
 
 ### Dependencies

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java
@@ -72,7 +72,9 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY;
 import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS;
 import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG;
 import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA;
@@ -494,6 +496,51 @@ public void testRemoteRestoreIndexRestoredFromSnapshot() throws IOException, Exe
         assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1);
     }
 
+    public void testSuccessfulIndexRestoredFromSnapshotWithUpdatedSetting() throws IOException, ExecutionException, InterruptedException {
+        internalCluster().startClusterManagerOnlyNode();
+        internalCluster().startDataOnlyNodes(2);
+
+        String indexName1 = "testindex1";
+        String snapshotRepoName = "test-restore-snapshot-repo";
+        String snapshotName1 = "test-restore-snapshot1";
+        Path absolutePath1 = randomRepoPath().toAbsolutePath();
+        logger.info("Snapshot Path [{}]", absolutePath1);
+
+        createRepository(snapshotRepoName, "fs", getRepositorySettings(absolutePath1, true));
+
+        Settings indexSettings = getIndexSettings(1, 0).build();
+        createIndex(indexName1, indexSettings);
+
+        final int numDocsInIndex1 = randomIntBetween(20, 30);
+        indexDocuments(client(), indexName1, numDocsInIndex1);
+        flushAndRefresh(indexName1);
+        ensureGreen(indexName1);
+
+        logger.info("--> snapshot");
+        SnapshotInfo snapshotInfo1 = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>(Arrays.asList(indexName1)));
+        assertThat(snapshotInfo1.successfulShards(), greaterThan(0));
+        assertThat(snapshotInfo1.successfulShards(), equalTo(snapshotInfo1.totalShards()));
+        assertThat(snapshotInfo1.state(), equalTo(SnapshotState.SUCCESS));
+
+        assertAcked(client().admin().indices().delete(new DeleteIndexRequest(indexName1)).get());
+        assertFalse(indexExists(indexName1));
+
+        // try index restore with index.number_of_replicas setting modified. index.number_of_replicas can be modified on restore
+        Settings numberOfReplicasSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build();
+
+        RestoreSnapshotResponse restoreSnapshotResponse1 = client().admin()
+            .cluster()
+            .prepareRestoreSnapshot(snapshotRepoName, snapshotName1)
+            .setWaitForCompletion(false)
+            .setIndexSettings(numberOfReplicasSettings)
+            .setIndices(indexName1)
+            .get();
+
+        assertEquals(restoreSnapshotResponse1.status(), RestStatus.ACCEPTED);
+        ensureGreen(indexName1);
+        assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1);
+    }
+
     protected IndexShard getIndexShard(String node, String indexName) {
         final Index index = resolveIndex(indexName);
         IndicesService indicesService = internalCluster().getInstance(IndicesService.class, node);
@@ -706,7 +753,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception {
         indexDocuments(client, index, numDocsInIndex, numDocsInIndex + randomIntBetween(2, 5));
         ensureGreen(index);
 
-        // try index restore with remote store disabled
+        // try index restore with index.remote_store.enabled ignored
         SnapshotRestoreException exception = expectThrows(
             SnapshotRestoreException.class,
             () -> client().admin()
@@ -721,26 +768,37 @@ public void testInvalidRestoreRequestScenarios() throws Exception {
         );
         assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore"));
 
-        // try index restore with remote store repository modified
-        Settings remoteStoreIndexSettings = Settings.builder()
-            .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo)
-            .build();
+        // try index restore with index.remote_store.segment.repository ignored
+        exception = expectThrows(
+            SnapshotRestoreException.class,
+            () -> client().admin()
+                .cluster()
+                .prepareRestoreSnapshot(snapshotRepo, snapshotName1)
+                .setWaitForCompletion(false)
+                .setIgnoreIndexSettings(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)
+                .setIndices(index)
+                .setRenamePattern(index)
+                .setRenameReplacement(restoredIndex)
+                .get()
+        );
+        assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository] on restore"));
 
+        // try index restore with index.remote_store.translog.repository ignored
         exception = expectThrows(
             SnapshotRestoreException.class,
             () -> client().admin()
                 .cluster()
                 .prepareRestoreSnapshot(snapshotRepo, snapshotName1)
                 .setWaitForCompletion(false)
-                .setIndexSettings(remoteStoreIndexSettings)
+                .setIgnoreIndexSettings(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)
                 .setIndices(index)
                 .setRenamePattern(index)
                 .setRenameReplacement(restoredIndex)
                 .get()
         );
-        assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore"));
+        assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.translog.repository] on restore"));
 
-        // try index restore with remote store repository and translog store repository disabled
+        // try index restore with index.remote_store.segment.repository and index.remote_store.translog.repository ignored
         exception = expectThrows(
             SnapshotRestoreException.class,
             () -> client().admin()
@@ -757,6 +815,81 @@ public void testInvalidRestoreRequestScenarios() throws Exception {
                 .get()
         );
         assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository]" + " on restore"));
+
+        // try index restore with index.remote_store.enabled modified
+        Settings remoteStoreIndexSettings = Settings.builder().put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false).build();
+
+        exception = expectThrows(
+            SnapshotRestoreException.class,
+            () -> client().admin()
+                .cluster()
+                .prepareRestoreSnapshot(snapshotRepo, snapshotName1)
+                .setWaitForCompletion(false)
+                .setIndexSettings(remoteStoreIndexSettings)
+                .setIndices(index)
+                .setRenamePattern(index)
+                .setRenameReplacement(restoredIndex)
+                .get()
+        );
+        assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore"));
+
+        // try index restore with index.remote_store.segment.repository modified
+        Settings remoteStoreSegmentIndexSettings = Settings.builder()
+            .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo)
+            .build();
+
+        exception = expectThrows(
+            SnapshotRestoreException.class,
+            () -> client().admin()
+                .cluster()
+                .prepareRestoreSnapshot(snapshotRepo, snapshotName1)
+                .setWaitForCompletion(false)
+                .setIndexSettings(remoteStoreSegmentIndexSettings)
+                .setIndices(index)
+                .setRenamePattern(index)
+                .setRenameReplacement(restoredIndex)
+                .get()
+        );
+        assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore"));
+
+        // try index restore with index.remote_store.translog.repository modified
+        Settings remoteStoreTranslogIndexSettings = Settings.builder()
+            .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo)
+            .build();
+
+        exception = expectThrows(
+            SnapshotRestoreException.class,
+            () -> client().admin()
+                .cluster()
+                .prepareRestoreSnapshot(snapshotRepo, snapshotName1)
+                .setWaitForCompletion(false)
+                .setIndexSettings(remoteStoreTranslogIndexSettings)
+                .setIndices(index)
+                .setRenamePattern(index)
+                .setRenameReplacement(restoredIndex)
+                .get()
+        );
+        assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.translog.repository]" + " on restore"));
+
+        // try index restore with index.remote_store.translog.repository and index.remote_store.segment.repository modified
+        Settings multipleRemoteStoreIndexSettings = Settings.builder()
+            .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo)
+            .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo)
+            .build();
+
+        exception = expectThrows(
+            SnapshotRestoreException.class,
+            () -> client().admin()
+                .cluster()
+                .prepareRestoreSnapshot(snapshotRepo, snapshotName1)
+                .setWaitForCompletion(false)
+                .setIndexSettings(multipleRemoteStoreIndexSettings)
+                .setIndices(index)
+                .setRenamePattern(index)
+                .setRenameReplacement(restoredIndex)
+                .get()
+        );
+        assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore"));
     }
 
     public void testCreateSnapshotV2_Orphan_Timestamp_Cleanup() throws Exception {