From 457898aaadd7d7296dd7c04ba37496b2194a8d39 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Mon, 6 Jan 2025 10:02:55 -0800 Subject: [PATCH 1/9] Add index.knn setting to list of unmodifiable settings when restore snapshot Signed-off-by: AnnTian Shao --- .../remotestore/RemoteRestoreSnapshotIT.java | 17 +++++++++++++++++ .../cluster/metadata/IndexMetadata.java | 2 ++ .../opensearch/snapshots/RestoreService.java | 4 +++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 70e283791fc3e..a029789eecf7a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -740,6 +740,23 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + // try index restore with index.knn setting modified + Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_KNN_ENABLED, false).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(indexKnnSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.knn]" + " on restore")); + // try index restore with remote store repository and translog store repository disabled exception = expectThrows( SnapshotRestoreException.class, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index f70282986ad4e..e47cfa0a07d0f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -271,6 +271,8 @@ static Setting buildNumberOfShardsSetting() { Property.IndexScope ); + public static final String SETTING_KNN_ENABLED = "index.knn"; + public static final Setting INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = Setting.intSetting( "index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING, diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 4b5bd951f80a0..8634fb1fea816 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -120,6 +120,7 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_HISTORY_UUID; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_KNN_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; @@ -171,7 +172,8 @@ public class RestoreService implements ClusterStateApplier { SETTING_HISTORY_UUID, SETTING_REMOTE_STORE_ENABLED, SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY + SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, + SETTING_KNN_ENABLED ) ); From 5f709152305c68f35cbbde38fa9c382c76ae25c3 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Mon, 6 Jan 2025 10:02:55 -0800 Subject: [PATCH 2/9] Add index.knn setting to list of unmodifiable settings when restore snapshot Signed-off-by: AnnTian Shao --- CHANGELOG.md | 1 + .../remotestore/RemoteRestoreSnapshotIT.java | 17 +++++++++++++++++ .../cluster/metadata/IndexMetadata.java | 2 ++ .../opensearch/snapshots/RestoreService.java | 4 +++- 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7c7eb7c5e8b..a119549470091 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Tiered Caching] Fix bug in cache stats API ([#16560](https://github.com/opensearch-project/OpenSearch/pull/16560)) - Bound the size of cache in deprecation logger ([16702](https://github.com/opensearch-project/OpenSearch/issues/16702)) - Ensure consistency of system flag on IndexMetadata after diff is applied ([#16644](https://github.com/opensearch-project/OpenSearch/pull/16644)) +- Fixing the bug to prevent updating the index.knn setting during restore snapshot(#)[] ### Security diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 70e283791fc3e..a029789eecf7a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -740,6 +740,23 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + // try index restore with index.knn setting modified + Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_KNN_ENABLED, false).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(indexKnnSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.knn]" + " on restore")); + // try index restore with remote store repository and translog store repository disabled exception = expectThrows( SnapshotRestoreException.class, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index f70282986ad4e..e47cfa0a07d0f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -271,6 +271,8 @@ static Setting buildNumberOfShardsSetting() { Property.IndexScope ); + public static final String SETTING_KNN_ENABLED = "index.knn"; + public static final Setting INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = Setting.intSetting( "index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING, diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 4b5bd951f80a0..8634fb1fea816 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -120,6 +120,7 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_HISTORY_UUID; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_KNN_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; @@ -171,7 +172,8 @@ public class RestoreService implements ClusterStateApplier { SETTING_HISTORY_UUID, SETTING_REMOTE_STORE_ENABLED, SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY + SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, + SETTING_KNN_ENABLED ) ); From f9c06e8a271673831760bf90fd8a0c7efc294191 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Wed, 15 Jan 2025 15:59:27 -0800 Subject: [PATCH 3/9] Add new Setting property UnmodifiableOnRestore to mark setting as immutable on restore snapshot Signed-off-by: AnnTian Shao --- CHANGELOG.md | 2 +- .../remotestore/RemoteRestoreSnapshotIT.java | 6 +++--- .../cluster/metadata/IndexMetadata.java | 12 +++++++++--- .../common/settings/AbstractScopedSettings.java | 8 ++++++++ .../org/opensearch/common/settings/Setting.java | 11 ++++++++++- .../org/opensearch/indices/IndicesService.java | 4 ++++ .../opensearch/snapshots/RestoreService.java | 17 ++++++++++++----- 7 files changed, 47 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e89caa1d4a08..67dc9b6596500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Introduce framework for auxiliary transports and an experimental gRPC transport plugin ([#16534](https://github.com/opensearch-project/OpenSearch/pull/16534)) - Changes to support IP field in star tree indexing([#16641](https://github.com/opensearch-project/OpenSearch/pull/16641/)) - Support object fields in star-tree index([#16728](https://github.com/opensearch-project/OpenSearch/pull/16728/)) +- Added new Setting property UnmodifiableOnRestore to prevent updating settings on restore snapshot ([#16957](https://github.com/opensearch-project/OpenSearch/pull/16957)) ### Dependencies - Bump `com.google.cloud:google-cloud-core-http` from 2.23.0 to 2.47.0 ([#16504](https://github.com/opensearch-project/OpenSearch/pull/16504)) @@ -88,7 +89,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix _list/shards API failing when closed indices are present ([#16606](https://github.com/opensearch-project/OpenSearch/pull/16606)) - Fix remote shards balance ([#15335](https://github.com/opensearch-project/OpenSearch/pull/15335)) - Always use `constant_score` query for `match_only_text` field ([#16964](https://github.com/opensearch-project/OpenSearch/pull/16964)) -- Fixing the bug to prevent updating the index.knn setting during restore snapshot ([#16957](https://github.com/opensearch-project/OpenSearch/pull/16957)) ### Security diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index a029789eecf7a..f866859c34823 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -740,8 +740,8 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); - // try index restore with index.knn setting modified - Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_KNN_ENABLED, false).build(); + // try index restore with index.number_of_shards setting modified + Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, false).build(); exception = expectThrows( SnapshotRestoreException.class, @@ -755,7 +755,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .setRenameReplacement(restoredIndex) .get() ); - assertTrue(exception.getMessage().contains("cannot modify setting [index.knn]" + " on restore")); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); // try index restore with remote store repository and translog store repository disabled exception = expectThrows( diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index e47cfa0a07d0f..aefb369194548 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -230,7 +230,15 @@ static Setting buildNumberOfShardsSetting() { + "]" ); } - return Setting.intSetting(SETTING_NUMBER_OF_SHARDS, defaultNumShards, 1, maxNumShards, Property.IndexScope, Property.Final); + return Setting.intSetting( + SETTING_NUMBER_OF_SHARDS, + defaultNumShards, + 1, + maxNumShards, + Property.IndexScope, + Property.Final, + Property.UnmodifiableOnRestore + ); } public static final String INDEX_SETTING_PREFIX = "index."; @@ -271,8 +279,6 @@ static Setting buildNumberOfShardsSetting() { Property.IndexScope ); - public static final String SETTING_KNN_ENABLED = "index.knn"; - public static final Setting INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = Setting.intSetting( "index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING, diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index 7655135b06d6c..8c10623e48fe4 100644 --- a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java @@ -759,6 +759,14 @@ public boolean isFinalSetting(String key) { return setting != null && setting.isFinal(); } + /** + * Returns true if the setting for the given key is unmodifiableOnRestore. Otherwise false. + */ + public boolean isUnmodifiableOnRestoreSetting(String key) { + final Setting setting = get(key); + return setting != null && setting.isUnmodifiableOnRestore(); + } + /** * Returns a settings object that contains all settings that are not * already set in the given source. The diff contains either the default value for each diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index 081029c1c106c..f5f49e5c5c6ad 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -171,7 +171,12 @@ public enum Property { /** * Extension scope */ - ExtensionScope + ExtensionScope, + + /** + * Mark this setting as immutable on restore snapshot + */ + UnmodifiableOnRestore } private final Key key; @@ -348,6 +353,10 @@ public final boolean isFinal() { return properties.contains(Property.Final); } + public final boolean isUnmodifiableOnRestore() { + return properties.contains(Property.UnmodifiableOnRestore); + } + public final boolean isInternalIndex() { return properties.contains(Property.InternalIndex); } diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index b9bad5527e3f4..c0472fb9ebc70 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -1182,6 +1182,10 @@ public IndicesQueryCache getIndicesQueryCache() { return indicesQueryCache; } + public IndexScopedSettings getIndexScopedSettings() { + return indexScopedSettings; + } + /** * Accumulate stats from the passed Object * diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 8634fb1fea816..11985886be397 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -77,6 +77,7 @@ import org.opensearch.common.lucene.Lucene; import org.opensearch.common.regex.Regex; import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.ArrayUtils; @@ -120,16 +121,15 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_HISTORY_UUID; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_KNN_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; 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.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_UPGRADED; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; import static org.opensearch.common.util.IndexUtils.filterIndices; import static org.opensearch.common.util.set.Sets.newHashSet; @@ -165,15 +165,13 @@ public class RestoreService implements ClusterStateApplier { private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( newHashSet( - SETTING_NUMBER_OF_SHARDS, SETTING_VERSION_CREATED, SETTING_INDEX_UUID, SETTING_CREATION_DATE, SETTING_HISTORY_UUID, SETTING_REMOTE_STORE_ENABLED, SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, - SETTING_KNN_ENABLED + SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY ) ); @@ -184,6 +182,7 @@ public class RestoreService implements ClusterStateApplier { static { Set unremovable = new HashSet<>(USER_UNMODIFIABLE_SETTINGS.size() + 4); unremovable.addAll(USER_UNMODIFIABLE_SETTINGS); + unremovable.add(SETTING_NUMBER_OF_SHARDS); unremovable.add(SETTING_NUMBER_OF_REPLICAS); unremovable.add(SETTING_AUTO_EXPAND_REPLICAS); unremovable.add(SETTING_VERSION_UPGRADED); @@ -204,6 +203,8 @@ public class RestoreService implements ClusterStateApplier { private final ClusterSettings clusterSettings; + private final IndexScopedSettings indexScopedSettings; + private final IndicesService indicesService; private final Supplier clusterInfoSupplier; @@ -236,6 +237,7 @@ public RestoreService( this.clusterSettings = clusterService.getClusterSettings(); this.shardLimitValidator = shardLimitValidator; this.indicesService = indicesService; + this.indexScopedSettings = indicesService.getIndexScopedSettings(); this.clusterInfoSupplier = clusterInfoSupplier; this.dataToFileCacheSizeRatioSupplier = dataToFileCacheSizeRatioSupplier; @@ -874,6 +876,11 @@ private IndexMetadata updateIndexSettings( .put(normalizedChangeSettings.filter(k -> { if (USER_UNMODIFIABLE_SETTINGS.contains(k)) { throw new SnapshotRestoreException(snapshot, "cannot modify setting [" + k + "] on restore"); + } else if (indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { + throw new SnapshotRestoreException( + snapshot, + "cannot modify UnmodifiableOnRestore setting [" + k + "] on restore" + ); } else { return true; } From 2afd94e1b40a6c251120e8a99583b67be27eea1b Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Thu, 16 Jan 2025 11:41:34 -0800 Subject: [PATCH 4/9] Add tests for new Setting property UnmodifiableOnRestore Signed-off-by: AnnTian Shao --- .../RestoreShallowSnapshotV2IT.java | 17 +++++++++++++ .../common/settings/ScopedSettingsTests.java | 24 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java index ecb97e79b348e..9ae99acb15f4e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java @@ -807,6 +807,23 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + // try index restore with index.number_of_shards setting modified + Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, false).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(indexKnnSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + // try index restore with remote store repository and translog store repository disabled exception = expectThrows( SnapshotRestoreException.class, diff --git a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java index 7780481c9deff..cb5b259954453 100644 --- a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java @@ -789,6 +789,30 @@ public void testIsFinal() { assertTrue(settings.isFinalSetting("foo.group.key")); } + public void testIsUnmodifiableOnRestore() { + ClusterSettings settings = new ClusterSettings( + Settings.EMPTY, + new HashSet<>( + Arrays.asList( + Setting.intSetting("foo.int", 1, Property.UnmodifiableOnRestore, Property.NodeScope), + Setting.groupSetting("foo.group.", Property.UnmodifiableOnRestore, Property.NodeScope), + Setting.groupSetting("foo.list.", Property.UnmodifiableOnRestore, Property.NodeScope), + Setting.intSetting("foo.int.baz", 1, Property.NodeScope) + ) + ) + ); + + assertFalse(settings.isUnmodifiableOnRestoreSetting("foo.int.baz")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.int")); + + assertFalse(settings.isUnmodifiableOnRestoreSetting("foo.list")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.list.0.key")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.list.key")); + + assertFalse(settings.isUnmodifiableOnRestoreSetting("foo.group")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.group.key")); + } + public void testDiff() throws IOException { Setting fooBarBaz = Setting.intSetting("foo.bar.baz", 1, Property.NodeScope); Setting fooBar = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope); From a24b673ce3b3c7a231e10be56acf8d39041dd425 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Fri, 17 Jan 2025 13:03:33 -0800 Subject: [PATCH 5/9] fixes and added more tests for new setting property UnmodifiableOnRestore Signed-off-by: AnnTian Shao --- .../remotestore/RemoteRestoreSnapshotIT.java | 104 ++++++++++++++++-- .../RestoreShallowSnapshotV2IT.java | 104 ++++++++++++++++-- .../cluster/metadata/IndexMetadata.java | 36 +++++- .../common/settings/IndexScopedSettings.java | 3 + .../opensearch/snapshots/RestoreService.java | 25 ++--- 5 files changed, 233 insertions(+), 39 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index f866859c34823..8ab4be1ca9bde 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -8,6 +8,7 @@ package org.opensearch.remotestore; +import org.opensearch.Version; import org.opensearch.action.DocWriteResponse; import org.opensearch.action.LatchedActionListener; import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreRequest; @@ -494,6 +495,51 @@ public void testRemoteRestoreIndexRestoredFromSnapshot() throws IOException, Exe assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); } + public void testIndexRestoredFromSnapshotWithUpdateSetting() 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); @@ -721,10 +767,8 @@ 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.number_of_shards setting modified + Settings numberOfShardsSettingsDiff = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3).build(); exception = expectThrows( SnapshotRestoreException.class, @@ -732,16 +776,16 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .cluster() .prepareRestoreSnapshot(snapshotRepo, snapshotName1) .setWaitForCompletion(false) - .setIndexSettings(remoteStoreIndexSettings) + .setIndexSettings(numberOfShardsSettingsDiff) .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 modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); - // try index restore with index.number_of_shards setting modified - Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, false).build(); + // try index restore with index.number_of_shards setting same + Settings numberOfShardsSettingsSame = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).build(); exception = expectThrows( SnapshotRestoreException.class, @@ -749,7 +793,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .cluster() .prepareRestoreSnapshot(snapshotRepo, snapshotName1) .setWaitForCompletion(false) - .setIndexSettings(indexKnnSettings) + .setIndexSettings(numberOfShardsSettingsSame) .setIndices(index) .setRenamePattern(index) .setRenameReplacement(restoredIndex) @@ -757,6 +801,48 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + // try index restore with mix of modifiable and unmodifiable settings on restore + // index.version.created is unmodifiable, index.number_of_replicas is modifiable + Settings mixedSettings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(mixedSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.version.created]" + " on restore")); + + // try index restore with multiple UnmodifiableOnRestore settings on restore + Settings unmodifiableSettings = Settings.builder() + .put(IndexMetadata.SETTING_CREATION_DATE, -1L) + .put(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + .put(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(unmodifiableSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.creation_date]" + " on restore")); + // try index restore with remote store repository and translog store repository disabled exception = expectThrows( SnapshotRestoreException.class, diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java index 9ae99acb15f4e..f474a7ec0301b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java @@ -8,6 +8,7 @@ package org.opensearch.remotestore; +import org.opensearch.Version; import org.opensearch.action.DocWriteResponse; import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreRequest; import org.opensearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; @@ -561,6 +562,51 @@ public void testRemoteRestoreIndexRestoredFromSnapshot() throws IOException, Exe assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); } + public void testIndexRestoredFromSnapshotWithUpdateSetting() 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); + } + private IndexShard getIndexShard(String node, String indexName) { final Index index = resolveIndex(indexName); IndicesService indicesService = internalCluster().getInstance(IndicesService.class, node); @@ -788,10 +834,8 @@ 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.number_of_shards setting modified + Settings numberOfShardsSettingsDiff = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3).build(); exception = expectThrows( SnapshotRestoreException.class, @@ -799,16 +843,16 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .cluster() .prepareRestoreSnapshot(snapshotRepo, snapshotName1) .setWaitForCompletion(false) - .setIndexSettings(remoteStoreIndexSettings) + .setIndexSettings(numberOfShardsSettingsDiff) .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 modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); - // try index restore with index.number_of_shards setting modified - Settings indexKnnSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, false).build(); + // try index restore with index.number_of_shards setting same + Settings numberOfShardsSettingsSame = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).build(); exception = expectThrows( SnapshotRestoreException.class, @@ -816,7 +860,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .cluster() .prepareRestoreSnapshot(snapshotRepo, snapshotName1) .setWaitForCompletion(false) - .setIndexSettings(indexKnnSettings) + .setIndexSettings(numberOfShardsSettingsSame) .setIndices(index) .setRenamePattern(index) .setRenameReplacement(restoredIndex) @@ -824,6 +868,48 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + // try index restore with mix of modifiable and unmodifiable settings on restore + // index.version.created is unmodifiable, index.number_of_replicas is modifiable + Settings mixedSettings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(mixedSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.version.created]" + " on restore")); + + // try index restore with multiple UnmodifiableOnRestore settings on restore + Settings unmodifiableSettings = Settings.builder() + .put(IndexMetadata.SETTING_CREATION_DATE, -1L) + .put(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + .put(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(unmodifiableSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.creation_date]" + " on restore")); + // try index restore with remote store repository and translog store repository disabled exception = expectThrows( SnapshotRestoreException.class, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index aefb369194548..f41e81177d06d 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -390,7 +390,8 @@ public Iterator> settings() { }, Property.IndexScope, Property.PrivateIndex, - Property.Dynamic + Property.Dynamic, + Property.UnmodifiableOnRestore ); /** @@ -424,7 +425,8 @@ public Iterator> settings() { }, Property.IndexScope, Property.PrivateIndex, - Property.Dynamic + Property.Dynamic, + Property.UnmodifiableOnRestore ); private static void validateRemoteStoreSettingEnabled(final Map, Object> settings, Setting setting) { @@ -475,7 +477,8 @@ public Iterator> settings() { }, Property.IndexScope, Property.PrivateIndex, - Property.Dynamic + Property.Dynamic, + Property.UnmodifiableOnRestore ); public static final String SETTING_AUTO_EXPAND_REPLICAS = "index.auto_expand_replicas"; @@ -567,13 +570,21 @@ public static APIBlock readFrom(StreamInput input) throws IOException { SETTING_VERSION_CREATED, Version.V_EMPTY, Property.IndexScope, - Property.PrivateIndex + Property.PrivateIndex, + Property.UnmodifiableOnRestore ); public static final String SETTING_VERSION_CREATED_STRING = "index.version.created_string"; public static final String SETTING_VERSION_UPGRADED = "index.version.upgraded"; public static final String SETTING_VERSION_UPGRADED_STRING = "index.version.upgraded_string"; public static final String SETTING_CREATION_DATE = "index.creation_date"; + + public static final Setting SETTING_INDEX_CREATION_DATE = Setting.longSetting( + SETTING_CREATION_DATE, + -1L, + Property.IndexScope, + Property.UnmodifiableOnRestore + ); /** * The user provided name for an index. This is the plain string provided by the user when the index was created. * It might still contain date math expressions etc. (added in 5.0) @@ -597,8 +608,25 @@ public static APIBlock readFrom(StreamInput input) throws IOException { Function.identity(), Property.IndexScope ); + public static final String INDEX_UUID_NA_VALUE = Strings.UNKNOWN_UUID_VALUE; + public static final Setting INDEX_UUID_SETTING = Setting.simpleString( + SETTING_INDEX_UUID, + INDEX_UUID_NA_VALUE, + Property.IndexScope, + Property.PrivateIndex, + Property.UnmodifiableOnRestore + ); + + public static final Setting SETTING_INDEX_HISTORY_UUID = Setting.simpleString( + SETTING_HISTORY_UUID, + INDEX_UUID_NA_VALUE, + Property.IndexScope, + Property.PrivateIndex, + Property.UnmodifiableOnRestore + ); + public static final String INDEX_ROUTING_REQUIRE_GROUP_PREFIX = "index.routing.allocation.require"; public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include"; public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude"; diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 8d56a942c5d6e..8f59c928d758b 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -91,6 +91,9 @@ public final class IndexScopedSettings extends AbstractScopedSettings { MergeSchedulerConfig.MAX_MERGE_COUNT_SETTING, MergeSchedulerConfig.MAX_THREAD_COUNT_SETTING, IndexMetadata.SETTING_INDEX_VERSION_CREATED, + IndexMetadata.INDEX_UUID_SETTING, + IndexMetadata.SETTING_INDEX_CREATION_DATE, + IndexMetadata.SETTING_INDEX_HISTORY_UUID, IndexMetadata.INDEX_ROUTING_EXCLUDE_GROUP_SETTING, IndexMetadata.INDEX_ROUTING_INCLUDE_GROUP_SETTING, IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_SETTING, diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index ef5b770c3dd4d..c27821274599d 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -163,7 +163,8 @@ public class RestoreService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RestoreService.class); - private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( + // It's OK to change some settings, but we shouldn't allow simply removing them + private static final Set USER_UNREMOVABLE_SETTINGS = unmodifiableSet( newHashSet( SETTING_VERSION_CREATED, SETTING_INDEX_UUID, @@ -171,24 +172,16 @@ public class RestoreService implements ClusterStateApplier { SETTING_HISTORY_UUID, SETTING_REMOTE_STORE_ENABLED, SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY + SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, + SETTING_NUMBER_OF_SHARDS, + SETTING_NUMBER_OF_REPLICAS, + SETTING_AUTO_EXPAND_REPLICAS, + SETTING_VERSION_UPGRADED ) ); - // It's OK to change some settings, but we shouldn't allow simply removing them - private static final Set USER_UNREMOVABLE_SETTINGS; private static final String REMOTE_STORE_INDEX_SETTINGS_REGEX = "index.remote_store.*"; - static { - Set unremovable = new HashSet<>(USER_UNMODIFIABLE_SETTINGS.size() + 4); - unremovable.addAll(USER_UNMODIFIABLE_SETTINGS); - unremovable.add(SETTING_NUMBER_OF_SHARDS); - unremovable.add(SETTING_NUMBER_OF_REPLICAS); - unremovable.add(SETTING_AUTO_EXPAND_REPLICAS); - unremovable.add(SETTING_VERSION_UPGRADED); - USER_UNREMOVABLE_SETTINGS = unmodifiableSet(unremovable); - } - private final ClusterService clusterService; private final RepositoriesService repositoriesService; @@ -874,9 +867,7 @@ private IndexMetadata updateIndexSettings( Settings.Builder settingsBuilder = Settings.builder() .put(settings.filter(settingsFilter)) .put(normalizedChangeSettings.filter(k -> { - if (USER_UNMODIFIABLE_SETTINGS.contains(k)) { - throw new SnapshotRestoreException(snapshot, "cannot modify setting [" + k + "] on restore"); - } else if (indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { + if (indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { throw new SnapshotRestoreException( snapshot, "cannot modify UnmodifiableOnRestore setting [" + k + "] on restore" From 252100cb1174316198044e93647c544aac1e4394 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Tue, 21 Jan 2025 09:51:12 -0800 Subject: [PATCH 6/9] fix test failures Signed-off-by: AnnTian Shao --- .../org/opensearch/cluster/metadata/IndexMetadata.java | 2 ++ .../java/org/opensearch/index/IndexSettingsTests.java | 6 +++--- .../java/org/opensearch/test/InternalSettingsPlugin.java | 9 --------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index f41e81177d06d..39a470a205511 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -582,7 +582,9 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final Setting SETTING_INDEX_CREATION_DATE = Setting.longSetting( SETTING_CREATION_DATE, -1L, + -1L, Property.IndexScope, + Property.NodeScope, Property.UnmodifiableOnRestore ); /** diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index 474ec73d5fe61..cbc9d8856a27e 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -608,7 +608,7 @@ public void testTranslogGenerationSizeThreshold() { } public void testPrivateSettingsValidation() { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, System.currentTimeMillis()).build(); + final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_UPGRADED, Version.V_EMPTY).build(); final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); { @@ -617,7 +617,7 @@ public void testPrivateSettingsValidation() { SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean()) ); - assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); + assertThat(e, hasToString(containsString("unknown setting [index.version.upgraded]"))); } { @@ -626,7 +626,7 @@ public void testPrivateSettingsValidation() { SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean(), false, randomBoolean()) ); - assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); + assertThat(e, hasToString(containsString("unknown setting [index.version.upgraded]"))); } // nothing should happen since we are ignoring private settings diff --git a/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java b/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java index 986cfd9c5b613..96919f65f88fc 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java @@ -31,7 +31,6 @@ package org.opensearch.test; -import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.unit.TimeValue; @@ -59,13 +58,6 @@ public final class InternalSettingsPlugin extends Plugin { Property.IndexScope, Property.NodeScope ); - public static final Setting INDEX_CREATION_DATE_SETTING = Setting.longSetting( - IndexMetadata.SETTING_CREATION_DATE, - -1, - -1, - Property.IndexScope, - Property.NodeScope - ); public static final Setting TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING = Setting.timeSetting( "index.translog.retention.check_interval", new TimeValue(10, TimeUnit.MINUTES), @@ -78,7 +70,6 @@ public final class InternalSettingsPlugin extends Plugin { public List> getSettings() { return Arrays.asList( MERGE_ENABLED, - INDEX_CREATION_DATE_SETTING, PROVIDED_NAME_SETTING, TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING, RemoteConnectionStrategy.REMOTE_MAX_PENDING_CONNECTION_LISTENERS, From e86049907d43dbfee5e25a4638954f6a88ef7d8d Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Tue, 21 Jan 2025 13:57:11 -0800 Subject: [PATCH 7/9] Revert "fix test failures" This reverts commit 252100cb1174316198044e93647c544aac1e4394. Signed-off-by: AnnTian Shao --- .../org/opensearch/cluster/metadata/IndexMetadata.java | 2 -- .../java/org/opensearch/index/IndexSettingsTests.java | 6 +++--- .../java/org/opensearch/test/InternalSettingsPlugin.java | 9 +++++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index 39a470a205511..f41e81177d06d 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -582,9 +582,7 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final Setting SETTING_INDEX_CREATION_DATE = Setting.longSetting( SETTING_CREATION_DATE, -1L, - -1L, Property.IndexScope, - Property.NodeScope, Property.UnmodifiableOnRestore ); /** diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index cbc9d8856a27e..474ec73d5fe61 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -608,7 +608,7 @@ public void testTranslogGenerationSizeThreshold() { } public void testPrivateSettingsValidation() { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_UPGRADED, Version.V_EMPTY).build(); + final Settings settings = Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, System.currentTimeMillis()).build(); final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); { @@ -617,7 +617,7 @@ public void testPrivateSettingsValidation() { SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean()) ); - assertThat(e, hasToString(containsString("unknown setting [index.version.upgraded]"))); + assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); } { @@ -626,7 +626,7 @@ public void testPrivateSettingsValidation() { SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean(), false, randomBoolean()) ); - assertThat(e, hasToString(containsString("unknown setting [index.version.upgraded]"))); + assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); } // nothing should happen since we are ignoring private settings diff --git a/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java b/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java index 96919f65f88fc..986cfd9c5b613 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java @@ -31,6 +31,7 @@ package org.opensearch.test; +import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.unit.TimeValue; @@ -58,6 +59,13 @@ public final class InternalSettingsPlugin extends Plugin { Property.IndexScope, Property.NodeScope ); + public static final Setting INDEX_CREATION_DATE_SETTING = Setting.longSetting( + IndexMetadata.SETTING_CREATION_DATE, + -1, + -1, + Property.IndexScope, + Property.NodeScope + ); public static final Setting TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING = Setting.timeSetting( "index.translog.retention.check_interval", new TimeValue(10, TimeUnit.MINUTES), @@ -70,6 +78,7 @@ public final class InternalSettingsPlugin extends Plugin { public List> getSettings() { return Arrays.asList( MERGE_ENABLED, + INDEX_CREATION_DATE_SETTING, PROVIDED_NAME_SETTING, TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING, RemoteConnectionStrategy.REMOTE_MAX_PENDING_CONNECTION_LISTENERS, From 6dc984006e82453bdd26f45879a749b18e7ac9d5 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Tue, 21 Jan 2025 15:22:02 -0800 Subject: [PATCH 8/9] fixes by adding back USER_UNMODIFIABLE_SETTINGS for settings without Setting implementation Signed-off-by: AnnTian Shao --- .../remotestore/RemoteRestoreSnapshotIT.java | 25 +++++++++++-- .../RestoreShallowSnapshotV2IT.java | 25 +++++++++++-- .../cluster/metadata/IndexMetadata.java | 22 ----------- .../common/settings/IndexScopedSettings.java | 3 -- .../opensearch/common/settings/Setting.java | 2 +- .../opensearch/snapshots/RestoreService.java | 37 +++++++++++-------- 6 files changed, 64 insertions(+), 50 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 8ab4be1ca9bde..837212badbf5b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -767,6 +767,23 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore")); + // try index restore with index.uuid setting modified + Settings uuidSetting = Settings.builder().put(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(uuidSetting) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.uuid]" + " on restore")); + // try index restore with index.number_of_shards setting modified Settings numberOfShardsSettingsDiff = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3).build(); @@ -824,9 +841,9 @@ public void testInvalidRestoreRequestScenarios() throws Exception { // try index restore with multiple UnmodifiableOnRestore settings on restore Settings unmodifiableSettings = Settings.builder() - .put(IndexMetadata.SETTING_CREATION_DATE, -1L) - .put(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) - .put(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) .build(); exception = expectThrows( @@ -841,7 +858,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .setRenameReplacement(restoredIndex) .get() ); - assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.creation_date]" + " on restore")); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); // try index restore with remote store repository and translog store repository disabled exception = expectThrows( diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java index f474a7ec0301b..65fbfdf2a09e0 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java @@ -834,6 +834,23 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore")); + // try index restore with index.uuid setting modified + Settings uuidSetting = Settings.builder().put(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(uuidSetting) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.uuid]" + " on restore")); + // try index restore with index.number_of_shards setting modified Settings numberOfShardsSettingsDiff = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3).build(); @@ -891,9 +908,9 @@ public void testInvalidRestoreRequestScenarios() throws Exception { // try index restore with multiple UnmodifiableOnRestore settings on restore Settings unmodifiableSettings = Settings.builder() - .put(IndexMetadata.SETTING_CREATION_DATE, -1L) - .put(IndexMetadata.SETTING_INDEX_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) - .put(IndexMetadata.SETTING_HISTORY_UUID, IndexMetadata.INDEX_UUID_NA_VALUE) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) .build(); exception = expectThrows( @@ -908,7 +925,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .setRenameReplacement(restoredIndex) .get() ); - assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.creation_date]" + " on restore")); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); // try index restore with remote store repository and translog store repository disabled exception = expectThrows( diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index f41e81177d06d..a9eddcce6b78c 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -579,12 +579,6 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final String SETTING_VERSION_UPGRADED_STRING = "index.version.upgraded_string"; public static final String SETTING_CREATION_DATE = "index.creation_date"; - public static final Setting SETTING_INDEX_CREATION_DATE = Setting.longSetting( - SETTING_CREATION_DATE, - -1L, - Property.IndexScope, - Property.UnmodifiableOnRestore - ); /** * The user provided name for an index. This is the plain string provided by the user when the index was created. * It might still contain date math expressions etc. (added in 5.0) @@ -611,22 +605,6 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final String INDEX_UUID_NA_VALUE = Strings.UNKNOWN_UUID_VALUE; - public static final Setting INDEX_UUID_SETTING = Setting.simpleString( - SETTING_INDEX_UUID, - INDEX_UUID_NA_VALUE, - Property.IndexScope, - Property.PrivateIndex, - Property.UnmodifiableOnRestore - ); - - public static final Setting SETTING_INDEX_HISTORY_UUID = Setting.simpleString( - SETTING_HISTORY_UUID, - INDEX_UUID_NA_VALUE, - Property.IndexScope, - Property.PrivateIndex, - Property.UnmodifiableOnRestore - ); - public static final String INDEX_ROUTING_REQUIRE_GROUP_PREFIX = "index.routing.allocation.require"; public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include"; public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude"; diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 8f59c928d758b..8d56a942c5d6e 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -91,9 +91,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings { MergeSchedulerConfig.MAX_MERGE_COUNT_SETTING, MergeSchedulerConfig.MAX_THREAD_COUNT_SETTING, IndexMetadata.SETTING_INDEX_VERSION_CREATED, - IndexMetadata.INDEX_UUID_SETTING, - IndexMetadata.SETTING_INDEX_CREATION_DATE, - IndexMetadata.SETTING_INDEX_HISTORY_UUID, IndexMetadata.INDEX_ROUTING_EXCLUDE_GROUP_SETTING, IndexMetadata.INDEX_ROUTING_INCLUDE_GROUP_SETTING, IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_SETTING, diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index f5f49e5c5c6ad..d061cae0f96a0 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -174,7 +174,7 @@ public enum Property { ExtensionScope, /** - * Mark this setting as immutable on restore snapshot + * Mark this setting as immutable on snapshot restore */ UnmodifiableOnRestore } diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index c27821274599d..8ee62ad83501f 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -163,25 +163,28 @@ public class RestoreService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RestoreService.class); - // It's OK to change some settings, but we shouldn't allow simply removing them - private static final Set USER_UNREMOVABLE_SETTINGS = unmodifiableSet( - newHashSet( - SETTING_VERSION_CREATED, - SETTING_INDEX_UUID, - SETTING_CREATION_DATE, - SETTING_HISTORY_UUID, - SETTING_REMOTE_STORE_ENABLED, - SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, - SETTING_NUMBER_OF_SHARDS, - SETTING_NUMBER_OF_REPLICAS, - SETTING_AUTO_EXPAND_REPLICAS, - SETTING_VERSION_UPGRADED - ) + private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( + newHashSet(SETTING_INDEX_UUID, SETTING_CREATION_DATE, SETTING_HISTORY_UUID) ); + // It's OK to change some settings, but we shouldn't allow simply removing them + private static final Set USER_UNREMOVABLE_SETTINGS; private static final String REMOTE_STORE_INDEX_SETTINGS_REGEX = "index.remote_store.*"; + static { + Set unremovable = new HashSet<>(USER_UNMODIFIABLE_SETTINGS.size() + 8); + unremovable.addAll(USER_UNMODIFIABLE_SETTINGS); + unremovable.add(SETTING_NUMBER_OF_SHARDS); + unremovable.add(SETTING_VERSION_CREATED); + unremovable.add(SETTING_REMOTE_STORE_ENABLED); + unremovable.add(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY); + unremovable.add(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY); + unremovable.add(SETTING_NUMBER_OF_REPLICAS); + unremovable.add(SETTING_AUTO_EXPAND_REPLICAS); + unremovable.add(SETTING_VERSION_UPGRADED); + USER_UNREMOVABLE_SETTINGS = unmodifiableSet(unremovable); + } + private final ClusterService clusterService; private final RepositoriesService repositoriesService; @@ -867,7 +870,9 @@ private IndexMetadata updateIndexSettings( Settings.Builder settingsBuilder = Settings.builder() .put(settings.filter(settingsFilter)) .put(normalizedChangeSettings.filter(k -> { - if (indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { + if (USER_UNMODIFIABLE_SETTINGS.contains(k)) { + throw new SnapshotRestoreException(snapshot, "cannot modify setting [" + k + "] on restore"); + } else if (indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { throw new SnapshotRestoreException( snapshot, "cannot modify UnmodifiableOnRestore setting [" + k + "] on restore" From 9ebab5a7ac9fa3a15b436168f227556bf18ace87 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Tue, 21 Jan 2025 18:41:27 -0800 Subject: [PATCH 9/9] testing CI config for registering plugin settings Signed-off-by: AnnTian Shao --- .../remotestore/RemoteRestoreSnapshotIT.java | 16 ++++++++++++++++ .../org/opensearch/snapshots/RestoreService.java | 5 ++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 837212badbf5b..a22312eb33ce7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -801,6 +801,22 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + Settings creationDate = Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, -1).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(creationDate) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.creation_date]" + " on restore")); + // try index restore with index.number_of_shards setting same Settings numberOfShardsSettingsSame = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).build(); diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 8ee62ad83501f..96204e83e39ba 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -164,7 +164,10 @@ public class RestoreService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RestoreService.class); private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( - newHashSet(SETTING_INDEX_UUID, SETTING_CREATION_DATE, SETTING_HISTORY_UUID) + newHashSet( + SETTING_INDEX_UUID, +// SETTING_CREATION_DATE, + SETTING_HISTORY_UUID) ); // It's OK to change some settings, but we shouldn't allow simply removing them