Skip to content

Commit

Permalink
Allow removing index.number_of_replicas setting (#56656)
Browse files Browse the repository at this point in the history
Today a user can create an index without setting the
index.number_of_replicas setting even though the index metadata requires
that the setting has a value. We do this when creating an index by
explicitly settings index.number_of_replicas to a default value if one
is not provided. However, if a user updates the number of replicas, and
then let wants to return to the default value, they are naturally
inclined to try setting this setting to null, as the agreed upon way to
return a setting to its default. Since the index metadata requires that
this setting has a non-null value, we blow up when a user attempts to
make this change. This is because we are not taking the same action when
updating a setting on an index that we take when create an
index. Namely, we are not explicitly setting index.number_of_replicas if
the request does not carry a value for this setting. This would happen
when nulling the setting, which we want to support. This commit
addresses this by setting index.number_of_replicas to the default if the
value for this setting is null when updating the settings for an index.
  • Loading branch information
jasontedor authored May 13, 2020
1 parent f204eb9 commit e09e443
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -646,4 +646,28 @@ public void testNumberOfReplicasSettingsVersion() {
assertThat(newSettingsVersion, equalTo(1 + settingsVersion));
}

/*
* Test that we are able to set the setting index.number_of_replicas to the default.
*/
public void testDefaultNumberOfReplicas() {
if (randomBoolean()) {
assertAcked(client().admin()
.indices()
.prepareCreate("test")
.setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, randomIntBetween(1, 8))));
} else {
assertAcked(client().admin().indices().prepareCreate("test"));
}

/*
* Previous versions of Elasticsearch would throw an exception that the number of replicas had to have a value, and could not be
* null. In the update settings logic, we ensure this by providing an explicit default value if the setting is set to null.
*/
assertAcked(client().admin()
.indices()
.prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS)));

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,16 @@ public ClusterState execute(ClusterState currentState) {
if (preserveExisting) {
indexSettings.put(indexMetadata.getSettings());
}
/*
* 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).
*/
if (indexSettings.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS) == null) {
indexSettings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1);
}
Settings finalSettings = indexSettings.build();
indexScopedSettings.validate(
finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false), true);
Expand Down

0 comments on commit e09e443

Please sign in to comment.