diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a68808101ca5..d238936748f19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,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 update settings with null replica not honoring cluster setting bug ([#14948](https://github.com/opensearch-project/OpenSearch/pull/14948)) - Fix race condition in node-join and node-left ([#15521](https://github.com/opensearch-project/OpenSearch/pull/15521)) - 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)) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java index f9b818b210464..e84dab206057c 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java @@ -894,6 +894,69 @@ private void runTestDefaultNumberOfReplicasTest(final boolean closeIndex) { assertThat(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(response.getIndexToSettings().get("test")), equalTo(1)); } + public void testNullReplicaUpdate() { + internalCluster().ensureAtLeastNumDataNodes(2); + + // cluster setting + String defaultNumberOfReplica = "3"; + assertAcked( + client().admin() + .cluster() + .prepareUpdateSettings() + .setPersistentSettings(Settings.builder().put("cluster.default_number_of_replicas", defaultNumberOfReplica)) + .get() + ); + // create index with number of replicas will ignore default value + assertAcked( + client().admin() + .indices() + .prepareCreate("test") + .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, "2")) + ); + + String numberOfReplicas = client().admin() + .indices() + .prepareGetSettings("test") + .get() + .getSetting("test", IndexMetadata.SETTING_NUMBER_OF_REPLICAS); + assertEquals("2", numberOfReplicas); + // update setting with null replica will use cluster setting of replica number + assertAcked( + client().admin() + .indices() + .prepareUpdateSettings("test") + .setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS)) + ); + + numberOfReplicas = client().admin() + .indices() + .prepareGetSettings("test") + .get() + .getSetting("test", IndexMetadata.SETTING_NUMBER_OF_REPLICAS); + assertEquals(defaultNumberOfReplica, numberOfReplicas); + + // Clean up cluster setting, then update setting with null replica should pick up default value of 1 + assertAcked( + client().admin() + .cluster() + .prepareUpdateSettings() + .setPersistentSettings(Settings.builder().putNull("cluster.default_number_of_replicas")) + ); + assertAcked( + client().admin() + .indices() + .prepareUpdateSettings("test") + .setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS)) + ); + + numberOfReplicas = client().admin() + .indices() + .prepareGetSettings("test") + .get() + .getSetting("test", IndexMetadata.SETTING_NUMBER_OF_REPLICAS); + assertEquals("1", numberOfReplicas); + } + public void testNoopUpdate() { internalCluster().ensureAtLeastNumDataNodes(2); final ClusterService clusterService = internalCluster().getClusterManagerNodeInstance(ClusterService.class); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java index 7957a808970eb..4f0c0f3aa2ab4 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java @@ -140,6 +140,7 @@ public void updateSettings( validateRefreshIntervalSettings(normalizedSettings, clusterService.getClusterSettings()); validateTranslogDurabilitySettings(normalizedSettings, clusterService.getClusterSettings(), clusterService.getSettings()); + final int defaultReplicaCount = clusterService.getClusterSettings().get(Metadata.DEFAULT_REPLICA_COUNT_SETTING); Settings.Builder settingsForClosedIndices = Settings.builder(); Settings.Builder settingsForOpenIndices = Settings.builder(); @@ -248,7 +249,10 @@ public ClusterState execute(ClusterState currentState) { } if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(openSettings)) { - final int updatedNumberOfReplicas = IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(openSettings); + final int updatedNumberOfReplicas = openSettings.getAsInt( + IndexMetadata.SETTING_NUMBER_OF_REPLICAS, + defaultReplicaCount + ); if (preserveExisting == false) { for (Index index : request.indices()) { if (index.getName().charAt(0) != '.') { @@ -329,15 +333,13 @@ public ClusterState execute(ClusterState currentState) { /* * The setting index.number_of_replicas is special; we require that this setting has a value in the index. When * creating the index, we ensure this by explicitly providing a value for the setting to the default (one) if - * there is a not value provided on the source of the index creation. A user can update this setting though, - * including updating it to null, indicating that they want to use the default value. In this case, we again - * have to provide an explicit value for the setting to the default (one). + * there is no value provided on the source of the index creation or "cluster.default_number_of_replicas" is + * not set. A user can update this setting though, including updating it to null, indicating that they want to + * use the value configured by cluster settings or a default value 1. In this case, we again have to provide + * an explicit value for the setting. */ if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) { - indexSettings.put( - IndexMetadata.SETTING_NUMBER_OF_REPLICAS, - IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY) - ); + indexSettings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, defaultReplicaCount); } Settings finalSettings = indexSettings.build(); indexScopedSettings.validate(