From a24b673ce3b3c7a231e10be56acf8d39041dd425 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Fri, 17 Jan 2025 13:03:33 -0800 Subject: [PATCH] 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"