From 3e37dfddad497e7e2b378114fb8d51e0dd5adb3f Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Wed, 16 Oct 2024 10:44:49 +0800 Subject: [PATCH] Fix wrong default value when setting index.number_of_routing_shards to null on index creation (#16331) * Fix wrong value when setting index.number_of_routing_shards to null on index creation Signed-off-by: Gao Binlong * Modify change log Signed-off-by: Gao Binlong --------- Signed-off-by: Gao Binlong --- CHANGELOG.md | 1 + .../test/indices.create/10_basic.yml | 30 +++++++++++++++++++ .../metadata/MetadataCreateIndexService.java | 2 +- .../MetadataCreateIndexServiceTests.java | 18 +++++++++++ 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bf90a75d0148..73db9bb9ed7af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix warnings from SLF4J on startup when repository-s3 is installed ([#16194](https://github.com/opensearch-project/OpenSearch/pull/16194)) - Fix protobuf-java leak through client library dependencies ([#16254](https://github.com/opensearch-project/OpenSearch/pull/16254)) - Fix multi-search with template doesn't return status code ([#16265](https://github.com/opensearch-project/OpenSearch/pull/16265)) +- Fix wrong default value when setting `index.number_of_routing_shards` to null on index creation ([#16331](https://github.com/opensearch-project/OpenSearch/pull/16331)) ### Security diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml index 0f8c7a7a68f07..077ea98ccf8cc 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/10_basic.yml @@ -112,3 +112,33 @@ properties: "": type: keyword + +--- +"Create index with setting index.number_of_routing_shards to null": + - skip: + version: " - 2.99.99" + reason: "fixed in 3.0.0" + - do: + indices.create: + index: test_index + body: + settings: + number_of_routing_shards: null + - do: + cluster.state: + metric: [ metadata ] + index: test_index + - match : { metadata.indices.test_index.routing_num_shards: 1024 } + + - do: + indices.create: + index: test_index1 + body: + settings: + number_of_routing_shards: null + number_of_shards: 3 + - do: + cluster.state: + metric: [ metadata ] + index: test_index1 + - match : { metadata.indices.test_index1.routing_num_shards: 768 } 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..abda5dad25e4e 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1210,7 +1210,7 @@ static int getIndexNumberOfRoutingShards(Settings indexSettings, @Nullable Index // in this case we either have no index to recover from or // we have a source index with 1 shard and without an explicit split factor // or one that is valid in that case we can split into whatever and auto-generate a new factor. - if (IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.exists(indexSettings)) { + if (indexSettings.get(IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey()) != null) { routingNumShards = IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.get(indexSettings); } else { routingNumShards = calculateNumRoutingShards(numTargetShards, indexVersionCreated); 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..1fdd038053eb6 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1857,6 +1857,24 @@ public void testGetIndexNumberOfRoutingShardsYieldsSourceNumberOfShards() { assertThat(targetRoutingNumberOfShards, is(6)); } + public void testGetIndexNumberOfRoutingShardsWhenExplicitlySetToNull() { + String nullValue = null; + Settings indexSettings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey(), nullValue) + .build(); + int targetRoutingNumberOfShards = getIndexNumberOfRoutingShards(indexSettings, null); + assertThat(targetRoutingNumberOfShards, is(1024)); + + indexSettings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put(INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 3) + .put(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey(), nullValue) + .build(); + targetRoutingNumberOfShards = getIndexNumberOfRoutingShards(indexSettings, null); + assertThat(targetRoutingNumberOfShards, is(768)); + } + public void testSoftDeletesDisabledIsRejected() { final IllegalArgumentException error = expectThrows(IllegalArgumentException.class, () -> { request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");