Skip to content

Commit

Permalink
Allow removing replicas setting on closed indices (#56680)
Browse files Browse the repository at this point in the history
This is similar to a previous change that allowed removing the number of
replicas settings (so setting it to its default) on open indices. This
commit allows the same for closed indices.

It is unfortunate that we have separate branches for handling open and
closed indices here, but I do not see a clean way to merge these two
together without making a rather unnatural method (note that they invoke
different methods for doing the settings updates). For now, we leave
this as-is even though it led to the miss here.
  • Loading branch information
jasontedor committed May 13, 2020
1 parent e267df9 commit 15a1c5b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,15 @@ public void testNumberOfReplicasSettingsVersion() {
/*
* Test that we are able to set the setting index.number_of_replicas to the default.
*/
public void testDefaultNumberOfReplicas() {
public void testDefaultNumberOfReplicasOnOpenIndices() {
runTestDefaultNumberOfReplicasTest(false);
}

public void testDefaultNumberOfReplicasOnClosedIndices() {
runTestDefaultNumberOfReplicasTest(true);
}

private void runTestDefaultNumberOfReplicasTest(final boolean closeIndex) {
if (randomBoolean()) {
assertAcked(client().admin()
.indices()
Expand All @@ -658,6 +666,10 @@ public void testDefaultNumberOfReplicas() {
assertAcked(client().admin().indices().prepareCreate("test"));
}

if (closeIndex) {
assertAcked(client().admin().indices().prepareClose("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.
Expand All @@ -667,6 +679,11 @@ public void testDefaultNumberOfReplicas() {
.prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull(IndexMetaData.SETTING_NUMBER_OF_REPLICAS)));

final GetSettingsResponse response = client().admin().indices().prepareGetSettings("test").get();

// we removed the setting but it should still have an explicit value since index metadata requires this
assertTrue(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(response.getIndexToSettings().get("test")));
assertThat(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.get(response.getIndexToSettings().get("test")), equalTo(1));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,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 15a1c5b

Please sign in to comment.