Skip to content

Commit

Permalink
SETTINGS: Correctly Identify Noop Updates (#36560)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
original-brownbear committed Dec 13, 2018
1 parent c650be7 commit 34d7cc1
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ private boolean updateSettings(Settings toApply, Settings.Builder target, Settin
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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ public void testResetSettingWithIPValidator() {
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);
Expand Down

0 comments on commit 34d7cc1

Please sign in to comment.