honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow removing replicas setting on closed indices (#56680) 

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.
This commit is contained in:
Jason Tedor 2020-05-13 15:55:09 -04:00
parent e3be18a443
commit 5ca2ea2dde
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
2 changed files with 28 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java
View File

@ -648,7 +648,15 @@ public class UpdateSettingsIT extends ESIntegTestCase {
/*
* 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()
@ -658,6 +666,10 @@ public class UpdateSettingsIT extends ESIntegTestCase {
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.
@ -667,6 +679,11 @@ public class UpdateSettingsIT extends ESIntegTestCase {
.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));
}
}

10
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java
View File

@ -217,6 +217,16 @@ public class MetadataUpdateSettingsService {
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);