Fix get index settings API doesn't show number_of_routing_shards when…

… it was explicitly set on index creation (#16294) * Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation Signed-off-by: Gao Binlong <[email protected]> * Update skip version in rest yaml test file Signed-off-by: Gao Binlong <[email protected]> * Fix test failure Signed-off-by: Gao Binlong <[email protected]> --------- Signed-off-by: Gao Binlong <[email protected]>
opensearch-project · Oct 23, 2024 · 5941a7e · 5941a7e
1 parent 15607b1
commit 5941a7e
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -85,6 +85,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Streaming bulk request hangs ([#16158](https://github.com/opensearch-project/OpenSearch/pull/16158))
 - 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 get index settings API doesn't show `number_of_routing_shards` setting when it was explicitly set ([#16294](https://github.com/opensearch-project/OpenSearch/pull/16294))
 - Fix multi-search with template doesn't return status code ([#16265](https://github.com/opensearch-project/OpenSearch/pull/16265))
 - [Streaming Indexing] Fix intermittent 'The bulk request must be terminated by a newline [\n]' failures [#16337](https://github.com/opensearch-project/OpenSearch/pull/16337))
 - 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))

diff --git a/...rc/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml b/...rc/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml
@@ -0,0 +1,41 @@
+---
+setup:
+  - do:
+      indices.create:
+        body:
+          settings:
+            index:
+              number_of_routing_shards: 4
+              number_of_shards: 2
+              number_of_replicas: 1
+        index: test-index
+
+  - do:
+      indices.create:
+        body:
+          settings:
+            index:
+              number_of_shards: 2
+              number_of_replicas: 1
+        index: test-index1
+
+---
+Test retrieval of number_routing_shards settings:
+  - skip:
+      version: " - 2.99.99"
+      reason: "introduced in 3.0.0" # TODO: change it to 2.18.0 after backport to 2.x branch
+  - do:
+      indices.get_settings:
+        flat_settings: true
+        index: test-index
+  # show `index.number_of_routing_shards` if it was explicitly set when creating
+  - match:
+      test-index.settings.index\.number_of_routing_shards: "4"
+
+  - do:
+      indices.get_settings:
+        flat_settings: true
+        index: test-index1
+  # do not show `index.number_of_routing_shards` if it was not explicitly set when creating
+  - match:
+      test-index1.settings.index\.number_of_routing_shards: null
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
@@ -300,7 +300,8 @@ public Iterator<Setting<?>> settings() {
             }
 
         },
-        Property.IndexScope
+        Property.IndexScope,
+        Property.NotCopyableOnResize
     );
 
     /**

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -626,14 +626,9 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
         final boolean isHiddenAfterTemplates = IndexMetadata.INDEX_HIDDEN_SETTING.get(aggregatedIndexSettings);
         final boolean isSystem = validateDotIndex(request.index(), isHiddenAfterTemplates);
 
-        // remove the setting it's temporary and is only relevant once we create the index
-        final Settings.Builder settingsBuilder = Settings.builder().put(aggregatedIndexSettings);
-        settingsBuilder.remove(IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey());
-        final Settings indexSettings = settingsBuilder.build();
-
         final IndexMetadata.Builder tmpImdBuilder = IndexMetadata.builder(request.index());
         tmpImdBuilder.setRoutingNumShards(routingNumShards);
-        tmpImdBuilder.settings(indexSettings);
+        tmpImdBuilder.settings(aggregatedIndexSettings);
         tmpImdBuilder.system(isSystem);
         addRemoteStoreCustomMetadata(tmpImdBuilder, true);
 

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -136,6 +136,7 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
@@ -1821,6 +1822,42 @@ private void validateRemoteCustomData(Map<String, String> customData, String exp
         assertEquals(expectedValue, customData.get(expectedKey));
     }
 
+    public void testNumberOfRoutingShardsShowsInIndexSettings() {
+        withTemporaryClusterService(((clusterService, threadPool) -> {
+            MetadataCreateIndexService checkerService = new MetadataCreateIndexService(
+                Settings.EMPTY,
+                clusterService,
+                indicesServices,
+                null,
+                null,
+                createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
+                null,
+                null,
+                threadPool,
+                null,
+                new SystemIndices(Collections.emptyMap()),
+                false,
+                new AwarenessReplicaBalance(Settings.EMPTY, clusterService.getClusterSettings()),
+                DefaultRemoteStoreSettings.INSTANCE,
+                repositoriesServiceSupplier
+            );
+            final int routingNumberOfShards = 4;
+            Settings indexSettings = Settings.builder()
+                .put("index.version.created", Version.CURRENT)
+                .put(INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 2)
+                .put(INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0)
+                .put(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey(), routingNumberOfShards)
+                .build();
+            CreateIndexClusterStateUpdateRequest request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
+            IndexMetadata indexMetadata = checkerService.buildAndValidateTemporaryIndexMetadata(
+                indexSettings,
+                request,
+                routingNumberOfShards
+            );
+            assertEquals(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.get(indexMetadata.getSettings()).intValue(), routingNumberOfShards);
+        }));
+    }
+
     public void testGetIndexNumberOfRoutingShardsWithNullSourceIndex() {
         Settings indexSettings = Settings.builder()
             .put("index.version.created", Version.CURRENT)
-Original file line number
+Diff line change
@@ Expand Up / @@ -300,7 +300,8 @@ public Iterator<Setting<?>> settings() { @@
                 }
             },
-            Property.IndexScope
+            Property.IndexScope,
+            Property.NotCopyableOnResize
         );
         /**
@@ Expand Down @@