Skip to content

Commit a41bf40

Browse files
chishuidbwiddis
authored and
akolarkunnu
committed
Update setting API honors cluster's replica setting as default opensearch-project#14810 (opensearch-project#14948)
* Update setting API honors cluster's replica setting as default opensearch-project#14810 Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Update changelog & add one more test case Signed-off-by: Liyun Xiu <xiliyun@amazon.com> --------- Signed-off-by: Liyun Xiu <xiliyun@amazon.com> Signed-off-by: Daniel Widdis <widdis@gmail.com> Co-authored-by: Daniel Widdis <widdis@gmail.com>
1 parent e51977e commit a41bf40

File tree

3 files changed

+74
-8
lines changed

3 files changed

+74
-8
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
6767
- Fix search_as_you_type not supporting multi-fields ([#15988](https://github.com/opensearch-project/OpenSearch/pull/15988))
6868
- Avoid infinite loop when `flat_object` field contains invalid token ([#15985](https://github.com/opensearch-project/OpenSearch/pull/15985))
6969
- Fix infinite loop in nested agg ([#15931](https://github.com/opensearch-project/OpenSearch/pull/15931))
70+
- Fix update settings with null replica not honoring cluster setting bug ([#14948](https://github.com/opensearch-project/OpenSearch/pull/14948))
7071
- Fix race condition in node-join and node-left ([#15521](https://github.com/opensearch-project/OpenSearch/pull/15521))
7172
- Streaming bulk request hangs ([#16158](https://github.com/opensearch-project/OpenSearch/pull/16158))
7273
- Fix warnings from SLF4J on startup when repository-s3 is installed ([#16194](https://github.com/opensearch-project/OpenSearch/pull/16194))

‎server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,69 @@ private void runTestDefaultNumberOfReplicasTest(final boolean closeIndex) {
895895
assertThat(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(response.getIndexToSettings().get("test")), equalTo(1));
896896
}
897897

898+
public void testNullReplicaUpdate() {
899+
internalCluster().ensureAtLeastNumDataNodes(2);
900+
901+
// cluster setting
902+
String defaultNumberOfReplica = "3";
903+
assertAcked(
904+
client().admin()
905+
.cluster()
906+
.prepareUpdateSettings()
907+
.setPersistentSettings(Settings.builder().put("cluster.default_number_of_replicas", defaultNumberOfReplica))
908+
.get()
909+
);
910+
// create index with number of replicas will ignore default value
911+
assertAcked(
912+
client().admin()
913+
.indices()
914+
.prepareCreate("test")
915+
.setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, "2"))
916+
);
917+
918+
String numberOfReplicas = client().admin()
919+
.indices()
920+
.prepareGetSettings("test")
921+
.get()
922+
.getSetting("test", IndexMetadata.SETTING_NUMBER_OF_REPLICAS);
923+
assertEquals("2", numberOfReplicas);
924+
// update setting with null replica will use cluster setting of replica number
925+
assertAcked(
926+
client().admin()
927+
.indices()
928+
.prepareUpdateSettings("test")
929+
.setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS))
930+
);
931+
932+
numberOfReplicas = client().admin()
933+
.indices()
934+
.prepareGetSettings("test")
935+
.get()
936+
.getSetting("test", IndexMetadata.SETTING_NUMBER_OF_REPLICAS);
937+
assertEquals(defaultNumberOfReplica, numberOfReplicas);
938+
939+
// Clean up cluster setting, then update setting with null replica should pick up default value of 1
940+
assertAcked(
941+
client().admin()
942+
.cluster()
943+
.prepareUpdateSettings()
944+
.setPersistentSettings(Settings.builder().putNull("cluster.default_number_of_replicas"))
945+
);
946+
assertAcked(
947+
client().admin()
948+
.indices()
949+
.prepareUpdateSettings("test")
950+
.setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS))
951+
);
952+
953+
numberOfReplicas = client().admin()
954+
.indices()
955+
.prepareGetSettings("test")
956+
.get()
957+
.getSetting("test", IndexMetadata.SETTING_NUMBER_OF_REPLICAS);
958+
assertEquals("1", numberOfReplicas);
959+
}
960+
898961
public void testNoopUpdate() {
899962
internalCluster().ensureAtLeastNumDataNodes(2);
900963
final ClusterService clusterService = internalCluster().getClusterManagerNodeInstance(ClusterService.class);

‎server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ public void updateSettings(
140140

141141
validateRefreshIntervalSettings(normalizedSettings, clusterService.getClusterSettings());
142142
validateTranslogDurabilitySettings(normalizedSettings, clusterService.getClusterSettings(), clusterService.getSettings());
143+
final int defaultReplicaCount = clusterService.getClusterSettings().get(Metadata.DEFAULT_REPLICA_COUNT_SETTING);
143144

144145
Settings.Builder settingsForClosedIndices = Settings.builder();
145146
Settings.Builder settingsForOpenIndices = Settings.builder();
@@ -248,7 +249,10 @@ public ClusterState execute(ClusterState currentState) {
248249
}
249250

250251
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(openSettings)) {
251-
final int updatedNumberOfReplicas = IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(openSettings);
252+
final int updatedNumberOfReplicas = openSettings.getAsInt(
253+
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
254+
defaultReplicaCount
255+
);
252256
if (preserveExisting == false) {
253257
for (Index index : request.indices()) {
254258
if (index.getName().charAt(0) != '.') {
@@ -329,15 +333,13 @@ public ClusterState execute(ClusterState currentState) {
329333
/*
330334
* The setting index.number_of_replicas is special; we require that this setting has a value in the index. When
331335
* creating the index, we ensure this by explicitly providing a value for the setting to the default (one) if
332-
* there is a not value provided on the source of the index creation. A user can update this setting though,
333-
* including updating it to null, indicating that they want to use the default value. In this case, we again
334-
* have to provide an explicit value for the setting to the default (one).
336+
* there is no value provided on the source of the index creation or "cluster.default_number_of_replicas" is
337+
* not set. A user can update this setting though, including updating it to null, indicating that they want to
338+
* use the value configured by cluster settings or a default value 1. In this case, we again have to provide
339+
* an explicit value for the setting.
335340
*/
336341
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) {
337-
indexSettings.put(
338-
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
339-
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY)
340-
);
342+
indexSettings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, defaultReplicaCount);
341343
}
342344
Settings finalSettings = indexSettings.build();
343345
indexScopedSettings.validate(

0 commit comments

Comments
 (0)