Browse Source

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.
Jason Tedor 5 năm trước cách đây
mục cha
commit
a31a88d724

+ 18 - 1
server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java

@@ -649,7 +649,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()
@@ -659,6 +667,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.
@@ -668,6 +680,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 - 0
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java

@@ -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);