Skip to content

Commit ee8bd11

Browse files
anntiansAnnTian Shao
authored and
etolbakov
committed
Added new Setting property UnmodifiableOnRestore to prevent updating settings on restore snapshot (opensearch-project#16957)
* 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]> Signed-off-by: Eugene Tolbakov <[email protected]>
1 parent 0553b3c commit ee8bd11

File tree

11 files changed

+766
-25
lines changed

11 files changed

+766
-25
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3535
- Added a new `time` field to replace the deprecated `getTime` field in `GetStats`. ([#17009](https://github.com/opensearch-project/OpenSearch/pull/17009))
3636
- Improve flat_object field parsing performance by reducing two passes to a single pass ([#16297](https://github.com/opensearch-project/OpenSearch/pull/16297))
3737
- Improve performance of the bitmap filtering([#16936](https://github.com/opensearch-project/OpenSearch/pull/16936/))
38+
- Added new Setting property UnmodifiableOnRestore to prevent updating settings on restore snapshot ([#16957](https://github.com/opensearch-project/OpenSearch/pull/16957))
3839
- Introduce Template query ([#16818](https://github.com/opensearch-project/OpenSearch/pull/16818))
3940
- Propagate the sourceIncludes and excludes fields from fetchSourceContext to FieldsVisitor. ([#17080](https://github.com/opensearch-project/OpenSearch/pull/17080))
4041

‎server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java

Lines changed: 141 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@
7272
import java.util.stream.Collectors;
7373
import java.util.stream.Stream;
7474

75+
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY;
7576
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
77+
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY;
7678
import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS;
7779
import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG;
7880
import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA;
@@ -494,6 +496,51 @@ public void testRemoteRestoreIndexRestoredFromSnapshot() throws IOException, Exe
494496
assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1);
495497
}
496498

499+
public void testSuccessfulIndexRestoredFromSnapshotWithUpdatedSetting() throws IOException, ExecutionException, InterruptedException {
500+
internalCluster().startClusterManagerOnlyNode();
501+
internalCluster().startDataOnlyNodes(2);
502+
503+
String indexName1 = "testindex1";
504+
String snapshotRepoName = "test-restore-snapshot-repo";
505+
String snapshotName1 = "test-restore-snapshot1";
506+
Path absolutePath1 = randomRepoPath().toAbsolutePath();
507+
logger.info("Snapshot Path [{}]", absolutePath1);
508+
509+
createRepository(snapshotRepoName, "fs", getRepositorySettings(absolutePath1, true));
510+
511+
Settings indexSettings = getIndexSettings(1, 0).build();
512+
createIndex(indexName1, indexSettings);
513+
514+
final int numDocsInIndex1 = randomIntBetween(20, 30);
515+
indexDocuments(client(), indexName1, numDocsInIndex1);
516+
flushAndRefresh(indexName1);
517+
ensureGreen(indexName1);
518+
519+
logger.info("--> snapshot");
520+
SnapshotInfo snapshotInfo1 = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>(Arrays.asList(indexName1)));
521+
assertThat(snapshotInfo1.successfulShards(), greaterThan(0));
522+
assertThat(snapshotInfo1.successfulShards(), equalTo(snapshotInfo1.totalShards()));
523+
assertThat(snapshotInfo1.state(), equalTo(SnapshotState.SUCCESS));
524+
525+
assertAcked(client().admin().indices().delete(new DeleteIndexRequest(indexName1)).get());
526+
assertFalse(indexExists(indexName1));
527+
528+
// try index restore with index.number_of_replicas setting modified. index.number_of_replicas can be modified on restore
529+
Settings numberOfReplicasSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build();
530+
531+
RestoreSnapshotResponse restoreSnapshotResponse1 = client().admin()
532+
.cluster()
533+
.prepareRestoreSnapshot(snapshotRepoName, snapshotName1)
534+
.setWaitForCompletion(false)
535+
.setIndexSettings(numberOfReplicasSettings)
536+
.setIndices(indexName1)
537+
.get();
538+
539+
assertEquals(restoreSnapshotResponse1.status(), RestStatus.ACCEPTED);
540+
ensureGreen(indexName1);
541+
assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1);
542+
}
543+
497544
protected IndexShard getIndexShard(String node, String indexName) {
498545
final Index index = resolveIndex(indexName);
499546
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, node);
@@ -706,7 +753,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception {
706753
indexDocuments(client, index, numDocsInIndex, numDocsInIndex + randomIntBetween(2, 5));
707754
ensureGreen(index);
708755

709-
// try index restore with remote store disabled
756+
// try index restore with index.remote_store.enabled ignored
710757
SnapshotRestoreException exception = expectThrows(
711758
SnapshotRestoreException.class,
712759
() -> client().admin()
@@ -721,26 +768,37 @@ public void testInvalidRestoreRequestScenarios() throws Exception {
721768
);
722769
assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore"));
723770

724-
// try index restore with remote store repository modified
725-
Settings remoteStoreIndexSettings = Settings.builder()
726-
.put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo)
727-
.build();
771+
// try index restore with index.remote_store.segment.repository ignored
772+
exception = expectThrows(
773+
SnapshotRestoreException.class,
774+
() -> client().admin()
775+
.cluster()
776+
.prepareRestoreSnapshot(snapshotRepo, snapshotName1)
777+
.setWaitForCompletion(false)
778+
.setIgnoreIndexSettings(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY)
779+
.setIndices(index)
780+
.setRenamePattern(index)
781+
.setRenameReplacement(restoredIndex)
782+
.get()
783+
);
784+
assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository] on restore"));
728785

786+
// try index restore with index.remote_store.translog.repository ignored
729787
exception = expectThrows(
730788
SnapshotRestoreException.class,
731789
() -> client().admin()
732790
.cluster()
733791
.prepareRestoreSnapshot(snapshotRepo, snapshotName1)
734792
.setWaitForCompletion(false)
735-
.setIndexSettings(remoteStoreIndexSettings)
793+
.setIgnoreIndexSettings(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY)
736794
.setIndices(index)
737795
.setRenamePattern(index)
738796
.setRenameReplacement(restoredIndex)
739797
.get()
740798
);
741-
assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore"));
799+
assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.translog.repository] on restore"));
742800

743-
// try index restore with remote store repository and translog store repository disabled
801+
// try index restore with index.remote_store.segment.repository and index.remote_store.translog.repository ignored
744802
exception = expectThrows(
745803
SnapshotRestoreException.class,
746804
() -> client().admin()
@@ -757,6 +815,81 @@ public void testInvalidRestoreRequestScenarios() throws Exception {
757815
.get()
758816
);
759817
assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository]" + " on restore"));
818+
819+
// try index restore with index.remote_store.enabled modified
820+
Settings remoteStoreIndexSettings = Settings.builder().put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false).build();
821+
822+
exception = expectThrows(
823+
SnapshotRestoreException.class,
824+
() -> client().admin()
825+
.cluster()
826+
.prepareRestoreSnapshot(snapshotRepo, snapshotName1)
827+
.setWaitForCompletion(false)
828+
.setIndexSettings(remoteStoreIndexSettings)
829+
.setIndices(index)
830+
.setRenamePattern(index)
831+
.setRenameReplacement(restoredIndex)
832+
.get()
833+
);
834+
assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore"));
835+
836+
// try index restore with index.remote_store.segment.repository modified
837+
Settings remoteStoreSegmentIndexSettings = Settings.builder()
838+
.put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo)
839+
.build();
840+
841+
exception = expectThrows(
842+
SnapshotRestoreException.class,
843+
() -> client().admin()
844+
.cluster()
845+
.prepareRestoreSnapshot(snapshotRepo, snapshotName1)
846+
.setWaitForCompletion(false)
847+
.setIndexSettings(remoteStoreSegmentIndexSettings)
848+
.setIndices(index)
849+
.setRenamePattern(index)
850+
.setRenameReplacement(restoredIndex)
851+
.get()
852+
);
853+
assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore"));
854+
855+
// try index restore with index.remote_store.translog.repository modified
856+
Settings remoteStoreTranslogIndexSettings = Settings.builder()
857+
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo)
858+
.build();
859+
860+
exception = expectThrows(
861+
SnapshotRestoreException.class,
862+
() -> client().admin()
863+
.cluster()
864+
.prepareRestoreSnapshot(snapshotRepo, snapshotName1)
865+
.setWaitForCompletion(false)
866+
.setIndexSettings(remoteStoreTranslogIndexSettings)
867+
.setIndices(index)
868+
.setRenamePattern(index)
869+
.setRenameReplacement(restoredIndex)
870+
.get()
871+
);
872+
assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.translog.repository]" + " on restore"));
873+
874+
// try index restore with index.remote_store.translog.repository and index.remote_store.segment.repository modified
875+
Settings multipleRemoteStoreIndexSettings = Settings.builder()
876+
.put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo)
877+
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo)
878+
.build();
879+
880+
exception = expectThrows(
881+
SnapshotRestoreException.class,
882+
() -> client().admin()
883+
.cluster()
884+
.prepareRestoreSnapshot(snapshotRepo, snapshotName1)
885+
.setWaitForCompletion(false)
886+
.setIndexSettings(multipleRemoteStoreIndexSettings)
887+
.setIndices(index)
888+
.setRenamePattern(index)
889+
.setRenameReplacement(restoredIndex)
890+
.get()
891+
);
892+
assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore"));
760893
}
761894

762895
public void testCreateSnapshotV2_Orphan_Timestamp_Cleanup() throws Exception {

0 commit comments

Comments
 (0)