Browse Source

SETTINGS: Correctly Identify Noop Updates (#36560)

* We should compare the target value with the to be applied value before interpreting the update as a change
* This speeds up the test failing in #36496 considerably by preventing state updates on noop setting updates
Armin Braun 6 years ago
parent
commit
34d7cc13c1

+ 1 - 1
server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -726,7 +726,7 @@ public abstract class AbstractScopedSettings {
                 validate(key, toApply, false); // we might not have a full picture here do to a dependency validation
                 settingsBuilder.copy(key, toApply);
                 updates.copy(key, toApply);
-                changed = true;
+                changed |= toApply.get(key).equals(target.get(key)) == false;
             } else {
                 if (isFinalSetting(key)) {
                     throw new IllegalArgumentException("final " + type + " setting [" + key + "], not updateable");

+ 11 - 0
server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -136,6 +136,17 @@ public class ScopedSettingsTests extends ESTestCase {
         assertEquals(1, currentSettings.size());
     }
 
+    public void testNoopSettingsUpdate() {
+        String value = "192.168.0.1,127.0.0.1";
+        String setting = "index.routing.allocation.require._ip";
+        Settings currentSettings = Settings.builder().put(setting, value)
+            .build();
+        Settings updates = Settings.builder().put(setting, value).build();
+        IndexScopedSettings settings = new IndexScopedSettings(currentSettings,
+            new HashSet<>(Collections.singletonList(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING)));
+        assertFalse(settings.updateSettings(updates, Settings.builder().put(currentSettings), Settings.builder(), ""));
+    }
+
     public void testAddConsumer() {
         Setting<Integer> testSetting = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope);
         Setting<Integer> testSetting2 = Setting.intSetting("foo.bar.baz", 1, Property.Dynamic, Property.NodeScope);