From 6a3f8f686df2d988b86a5350bc30854baad67c1b Mon Sep 17 00:00:00 2001 From: luyuncheng Date: Wed, 25 Sep 2024 16:48:52 +0800 Subject: [PATCH 1/6] FIX Clone Index Replication Type Consistency Signed-off-by: luyuncheng --- .../metadata/MetadataCreateIndexService.java | 3 ++- .../metadata/MetadataCreateIndexServiceTests.java | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index 8b08927bc146a..d27912bf570c1 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1708,7 +1708,8 @@ static void prepareResizeIndexSettings( .put(builder.build()) .put(IndexMetadata.SETTING_ROUTING_PARTITION_SIZE, sourceMetadata.getRoutingPartitionSize()) .put(IndexMetadata.INDEX_RESIZE_SOURCE_NAME.getKey(), resizeSourceIndex.getName()) - .put(IndexMetadata.INDEX_RESIZE_SOURCE_UUID.getKey(), resizeSourceIndex.getUUID()); + .put(IndexMetadata.INDEX_RESIZE_SOURCE_UUID.getKey(), resizeSourceIndex.getUUID()) + .put(INDEX_REPLICATION_TYPE_SETTING.getKey(), INDEX_REPLICATION_TYPE_SETTING.get(sourceMetadata.getSettings())); } /** diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 3f223706819b7..10b881abbae34 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -611,6 +611,20 @@ public void testPrepareResizeIndexSettingsCopySettings() { ); } + public void testPrepareResizeIndexSettingsReplicationType() { + runPrepareResizeIndexSettingsTest( + Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT.toString()).build(), + Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT.toString()).build(), + emptyList(), + true, + settings -> assertThat( + "Segment Replication must be equal to source type", + settings.get(SETTING_REPLICATION_TYPE), + equalTo(ReplicationType.SEGMENT.toString()) + ) + ); + } + public void testPrepareResizeIndexSettingsAnalysisSettings() { // analysis settings from the request are not overwritten runPrepareResizeIndexSettingsTest( From 2353b72903477cad4fd881f1a6a870a2475f188f Mon Sep 17 00:00:00 2001 From: luyuncheng Date: Wed, 25 Sep 2024 16:58:35 +0800 Subject: [PATCH 2/6] FIX Clone Index Replication Type Consistency Signed-off-by: luyuncheng --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfec81c070bab..a0f837c0683a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix search_as_you_type not supporting multi-fields ([#15988](https://github.com/opensearch-project/OpenSearch/pull/15988)) - Avoid infinite loop when `flat_object` field contains invalid token ([#15985](https://github.com/opensearch-project/OpenSearch/pull/15985)) - Fix infinite loop in nested agg ([#15931](https://github.com/opensearch-project/OpenSearch/pull/15931)) +- Fix Clone Index setting override for replication type ([#16076](https://github.com/opensearch-project/OpenSearch/pull/16076)) ### Security From 81aec5e113bd19a9228133ecd671a7a021421eba Mon Sep 17 00:00:00 2001 From: luyuncheng Date: Thu, 26 Sep 2024 20:01:32 +0800 Subject: [PATCH 3/6] Add IllegalArgumentException AND YAML RestTest Signed-off-by: luyuncheng --- .../test/indices.split/30_copy_settings.yml | 3 +++ .../indices/shrink/TransportResizeAction.java | 4 ++++ .../shrink/TransportResizeActionTests.java | 21 +++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml index b8e6b54a393b9..dce1eec15b9e5 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml @@ -20,6 +20,7 @@ index.number_of_shards: 1 index.number_of_routing_shards: 4 index.merge.scheduler.max_merge_count: 4 + index.replication.type: SEGMENT # make it read-only - do: @@ -64,6 +65,7 @@ - match: { copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - match: { copy-settings-target.settings.index.blocks.write: "true" } - match: { copy-settings-target.settings.index.routing.allocation.include._id: $node_id } + - match: { copy-settings-target.settings.index.replication.type: "SEGMENT" } # now we do a actual shrink and copy settings (by default) - do: @@ -93,6 +95,7 @@ - match: { default-copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - match: { default-copy-settings-target.settings.index.blocks.write: "true" } - match: { default-copy-settings-target.settings.index.routing.allocation.include._id: $node_id } + - match: { default-copy-settings-target.settings.index.replication.type: "SEGMENT" } - do: catch: /illegal_argument_exception/ diff --git a/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java b/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java index cb41325c18a22..ca947f6bc49b7 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java @@ -70,6 +70,7 @@ import java.util.Set; import java.util.function.IntFunction; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; @@ -311,6 +312,9 @@ static CreateIndexClusterStateUpdateRequest prepareCreateIndexRequest( && IndexSettings.INDEX_SOFT_DELETES_SETTING.get(targetIndexSettings) == false) { throw new IllegalArgumentException("Can't disable [index.soft_deletes.enabled] setting on resize"); } + if (IndexMetadata.INDEX_REPLICATION_TYPE_SETTING.exists(targetIndexSettings)) { + throw new IllegalArgumentException("cannot provide [index.replication.type] setting on resize"); + } String cause = resizeRequest.getResizeType().name().toLowerCase(Locale.ROOT) + "_index"; targetIndex.cause(cause); Settings.Builder settingsBuilder = Settings.builder().put(targetIndexSettings); diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java index 5bab2ceca0988..59db3c2bd97c3 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java @@ -175,6 +175,27 @@ public void testErrorCondition() { }); assertThat(softDeletesError.getMessage(), equalTo("Can't disable [index.soft_deletes.enabled] setting on resize")); + IllegalArgumentException resizeError = expectThrows(IllegalArgumentException.class, () -> { + ResizeRequest req = new ResizeRequest("target", "source"); + req.getTargetIndexRequest().settings(Settings.builder().put("index.replication.type", "SEGMENT")); + ClusterState clusterState = createClusterState( + "source", + 8, + 1, + Settings.builder().put("index.blocks.write", true).build() + ); + TransportResizeAction.prepareCreateIndexRequest( + req, + clusterState, + (i) -> new DocsStats(between(10, 1000), between(1, 10), between(1, 10000)), + new StoreStats(between(1, 10000), between(1, 10000)), + clusterSettings, + "source", + "target" + ); + }); + assertThat(resizeError.getMessage(), equalTo("cannot provide [index.replication.type] setting on resize")); + // create one that won't fail ClusterState clusterState = ClusterState.builder( createClusterState("source", randomIntBetween(2, 10), 0, Settings.builder().put("index.blocks.write", true).build()) From ba11329d2b272b16949afcecdb7df5a3890644ca Mon Sep 17 00:00:00 2001 From: luyuncheng Date: Mon, 30 Sep 2024 11:52:18 +0800 Subject: [PATCH 4/6] Add IllegalArgumentException AND YAML RestTest Signed-off-by: luyuncheng --- .../test/indices.clone/30_copy_settings.yml | 20 ++++++++++++++++++ .../test/indices.shrink/30_copy_settings.yml | 21 +++++++++++++++++++ .../test/indices.split/30_copy_settings.yml | 18 ++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.clone/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.clone/30_copy_settings.yml index b0bd8056cb004..6c9aa8c48a079 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.clone/30_copy_settings.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.clone/30_copy_settings.yml @@ -20,6 +20,7 @@ index.number_of_replicas: 0 index.number_of_shards: 1 index.merge.scheduler.max_merge_count: 4 + index.replication.type: SEGMENT # make it read-only - do: @@ -61,3 +62,22 @@ - match: { copy-settings-target.settings.index.merge.scheduler.max_merge_count: "4" } - match: { copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - match: { copy-settings-target.settings.index.blocks.write: "true" } + - match: { copy-settings-target.settings.index.replication.type: "SEGMENT" } + + - do: + catch: /illegal_argument_exception/ + allowed_warnings: + - "Parameter [master_timeout] is deprecated and will be removed in 3.0. To support inclusive language, please use [cluster_manager_timeout] instead." + indices.clone: + index: "source" + target: "explicit-illegal-settings-target" + wait_for_active_shards: 1 + master_timeout: 10s + body: + settings: + index.number_of_replicas: 0 + index.number_of_shards: 1 + index.replication.type: DOCUMENT + + - match: { error.type: illegal_argument_exception } + - match: { error.reason: "cannot provide [index.replication.type] setting on resize" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml index 07d5076550889..1563b2de83570 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml @@ -19,6 +19,7 @@ index.number_of_shards: 2 index.number_of_replicas: 0 index.merge.scheduler.max_merge_count: 4 + index.replication.type: SEGMENT # make it read-only - do: @@ -62,6 +63,7 @@ - match: { copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - match: { copy-settings-target.settings.index.blocks.write: "true" } - match: { copy-settings-target.settings.index.routing.allocation.include._id: $node_id } + - match: { copy-settings-target.settings.index.replication.type: "SEGMENT" } # now we do a actual shrink and copy settings (by default) - do: @@ -90,6 +92,7 @@ - match: { default-copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - match: { default-copy-settings-target.settings.index.blocks.write: "true" } - match: { default-copy-settings-target.settings.index.routing.allocation.include._id: $node_id } + - match: { default-copy-settings-target.settings.index.replication.type: "SEGMENT" } # now we do a actual shrink and try to set no copy settings - do: @@ -105,3 +108,21 @@ body: settings: index.number_of_replicas: 0 + + - do: + catch: /illegal_argument_exception/ + allowed_warnings: + - "Parameter [master_timeout] is deprecated and will be removed in 3.0. To support inclusive language, please use [cluster_manager_timeout] instead." + indices.shrink: + index: "source" + target: "explicit-illegal-settings-target" + wait_for_active_shards: 1 + master_timeout: 10s + body: + settings: + index.number_of_replicas: 0 + index.number_of_shards: 1 + index.replication.type: DOCUMENT + + - match: { error.type: illegal_argument_exception } + - match: { error.reason: "cannot provide [index.replication.type] setting on resize" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml index dce1eec15b9e5..0810941bf6405 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml @@ -110,3 +110,21 @@ body: settings: index.number_of_replicas: 0 + + - do: + catch: /illegal_argument_exception/ + allowed_warnings: + - "Parameter [master_timeout] is deprecated and will be removed in 3.0. To support inclusive language, please use [cluster_manager_timeout] instead." + indices.split: + index: "source" + target: "explicit-illegal-settings-target" + wait_for_active_shards: 1 + master_timeout: 10s + body: + settings: + index.number_of_replicas: 0 + index.number_of_shards: 1 + index.replication.type: DOCUMENT + + - match: { error.type: illegal_argument_exception } + - match: { error.reason: "cannot provide [index.replication.type] setting on resize" } From 9e2205ae36fd56e927b1d00253d48a56c5445617 Mon Sep 17 00:00:00 2001 From: luyuncheng Date: Mon, 30 Sep 2024 14:57:40 +0800 Subject: [PATCH 5/6] Add IllegalArgumentException AND YAML RestTest Signed-off-by: luyuncheng --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5d6b6efd0eee..63185751c7a55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix race condition in node-join and node-left ([#15521](https://github.com/opensearch-project/OpenSearch/pull/15521)) - Fix Clone Index setting override for replication type ([#16076](https://github.com/opensearch-project/OpenSearch/pull/16076)) - ### Security [Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.17...2.x From 16820016197a7ad61f05860988318aab925e801a Mon Sep 17 00:00:00 2001 From: luyuncheng Date: Mon, 30 Sep 2024 15:21:11 +0800 Subject: [PATCH 6/6] Spotless apply Signed-off-by: luyuncheng --- .../action/admin/indices/shrink/TransportResizeAction.java | 1 - .../admin/indices/shrink/TransportResizeActionTests.java | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java b/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java index ca947f6bc49b7..8d0d532f305e3 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java @@ -70,7 +70,6 @@ import java.util.Set; import java.util.function.IntFunction; -import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java index 59db3c2bd97c3..ecbcd43db3994 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java @@ -178,12 +178,7 @@ public void testErrorCondition() { IllegalArgumentException resizeError = expectThrows(IllegalArgumentException.class, () -> { ResizeRequest req = new ResizeRequest("target", "source"); req.getTargetIndexRequest().settings(Settings.builder().put("index.replication.type", "SEGMENT")); - ClusterState clusterState = createClusterState( - "source", - 8, - 1, - Settings.builder().put("index.blocks.write", true).build() - ); + ClusterState clusterState = createClusterState("source", 8, 1, Settings.builder().put("index.blocks.write", true).build()); TransportResizeAction.prepareCreateIndexRequest( req, clusterState,