Use default value when index.number_of_replicas is null (opensearch-p…

…roject#14812) * Use default value when index.number_of_replicas is null Signed-off-by: Liyun Xiu <[email protected]> * Add integration test Signed-off-by: Liyun Xiu <[email protected]> * Add changelog Signed-off-by: Liyun Xiu <[email protected]> --------- Signed-off-by: Liyun Xiu <[email protected]>
wdongyu · Aug 22, 2024 · 9ec4a9b · 9ec4a9b
1 parent 51db1db
commit 9ec4a9b
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 1 deletion.
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/),
 - Fix create or update alias API doesn't throw exception for unsupported parameters ([#14719](https://github.com/opensearch-project/OpenSearch/pull/14719))
 - Refactoring FilterPath.parse by using an iterative approach ([#14200](https://github.com/opensearch-project/OpenSearch/pull/14200))
 - Refactoring Grok.validatePatternBank by using an iterative approach ([#14206](https://github.com/opensearch-project/OpenSearch/pull/14206))
+- Fix NPE when creating index with index.number_of_replicas set to null ([#14812](https://github.com/opensearch-project/OpenSearch/pull/14812))
 - Update help output for _cat ([#14722](https://github.com/opensearch-project/OpenSearch/pull/14722))
 - Fix bulk upsert ignores the default_pipeline and final_pipeline when auto-created index matches the index template ([#12891](https://github.com/opensearch-project/OpenSearch/pull/12891))
 - Fix NPE in ReplicaShardAllocator ([#14385](https://github.com/opensearch-project/OpenSearch/pull/14385))

diff --git a/...rc/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java b/...rc/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java
@@ -406,4 +406,28 @@ public void testIndexNameInResponse() {
         assertEquals("Should have index name in response", "foo", response.index());
     }
 
+    public void testCreateIndexWithNullReplicaCountPickUpClusterReplica() {
+        int numReplicas = 3;
+        String indexName = "test-idx-1";
+        assertAcked(
+            client().admin()
+                .cluster()
+                .prepareUpdateSettings()
+                .setPersistentSettings(Settings.builder().put("cluster.default_number_of_replicas", numReplicas).build())
+                .get()
+        );
+        Settings settings = Settings.builder()
+            .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
+            .put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), (String) null)
+            .build();
+        assertAcked(client().admin().indices().prepareCreate(indexName).setSettings(settings).get());
+        IndicesService indicesService = internalCluster().getInstance(IndicesService.class, internalCluster().getClusterManagerName());
+        for (IndexService indexService : indicesService) {
+            assertEquals(indexName, indexService.index().getName());
+            assertEquals(
+                numReplicas,
+                (int) indexService.getIndexSettings().getSettings().getAsInt(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, null)
+            );
+        }
+    }
 }
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -946,7 +946,8 @@ static Settings aggregateIndexSettings(
         if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(indexSettingsBuilder) == false) {
             indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, INDEX_NUMBER_OF_SHARDS_SETTING.get(settings));
         }
-        if (INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettingsBuilder) == false) {
+        if (INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettingsBuilder) == false
+            || indexSettingsBuilder.get(SETTING_NUMBER_OF_REPLICAS) == null) {
             indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, DEFAULT_REPLICA_COUNT_SETTING.get(currentState.metadata().settings()));
         }
         if (settings.get(SETTING_AUTO_EXPAND_REPLICAS) != null && indexSettingsBuilder.get(SETTING_AUTO_EXPAND_REPLICAS) == null) {

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -2151,6 +2151,33 @@ public void testAsyncDurabilityThrowsExceptionWhenRestrictSettingTrue() {
         );
     }
 
+    public void testAggregateIndexSettingsIndexReplicaIsSetToNull() {
+        // This checks that aggregateIndexSettings works for the case when the index setting `index.number_of_replicas` is set to null
+        request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
+        request.settings(Settings.builder().putNull(SETTING_NUMBER_OF_REPLICAS).build());
+        Integer clusterDefaultReplicaNumber = 5;
+        Metadata metadata = new Metadata.Builder().persistentSettings(
+            Settings.builder().put("cluster.default_number_of_replicas", clusterDefaultReplicaNumber).build()
+        ).build();
+        ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
+            .metadata(metadata)
+            .build();
+        Settings settings = Settings.builder().put(CLUSTER_REMOTE_INDEX_RESTRICT_ASYNC_DURABILITY_SETTING.getKey(), true).build();
+        clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
+        Settings aggregatedSettings = aggregateIndexSettings(
+            clusterState,
+            request,
+            Settings.EMPTY,
+            null,
+            Settings.EMPTY,
+            IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
+            randomShardLimitService(),
+            Collections.emptySet(),
+            clusterSettings
+        );
+        assertEquals(clusterDefaultReplicaNumber.toString(), aggregatedSettings.get(SETTING_NUMBER_OF_REPLICAS));
+    }
+
     public void testRequestDurabilityWhenRestrictSettingTrue() {
         // This checks that aggregateIndexSettings works for the case when the cluster setting
         // cluster.remote_store.index.restrict.async-durability is false or not set, it allows all types of durability modes