From 86a18a08fb5b3d8b4f000c71b0f69acea80b0b6f Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 10 Dec 2015 18:01:00 +0100 Subject: [PATCH] Make SettingsUpdater less stateful --- .../settings/AbstractScopedSettings.java | 111 +++++++------ .../common/settings/Setting.java | 157 +++++++----------- .../common/settings/SettingTests.java | 103 +++++------- 3 files changed, 159 insertions(+), 212 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 7fc8cebd31b..e3de252e083 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -32,7 +32,7 @@ import java.util.function.Consumer; * This service offers transactional application of updates settings. */ public abstract class AbstractScopedSettings extends AbstractComponent { - private Settings lastSettingsApplied; + private Settings lastSettingsApplied = Settings.EMPTY; private final List settingUpdaters = new ArrayList<>(); private final Map> groupSettings = new HashMap<>(); private final Map> keySettings = new HashMap<>(); @@ -62,29 +62,22 @@ public abstract class AbstractScopedSettings extends AbstractComponent { * method will not change any settings but will fail if any of the settings can't be applied. */ public synchronized Settings dryRun(Settings settings) { - final Settings build = Settings.builder().put(this.settings).put(settings).build(); - try { - List exceptions = new ArrayList<>(); - for (SettingUpdater settingUpdater : settingUpdaters) { - try { - settingUpdater.prepareApply(build); - } catch (RuntimeException ex) { - exceptions.add(ex); - logger.debug("failed to prepareCommit settings for [{}]", ex, settingUpdater); - } - } - // here we are exhaustive and record all settings that failed. - ExceptionsHelper.rethrowAndSuppress(exceptions); - } finally { - for (SettingUpdater settingUpdater : settingUpdaters) { - try { - settingUpdater.rollback(); - } catch (Exception e) { - logger.error("failed to rollback settings for [{}]", e, settingUpdater); + final Settings current = Settings.builder().put(this.settings).put(settings).build(); + final Settings previous = Settings.builder().put(this.settings).put(this.lastSettingsApplied).build(); + List exceptions = new ArrayList<>(); + for (SettingUpdater settingUpdater : settingUpdaters) { + try { + if (settingUpdater.hasChanged(current, previous)) { + settingUpdater.getValue(current, previous); } + } catch (RuntimeException ex) { + exceptions.add(ex); + logger.debug("failed to prepareCommit settings for [{}]", ex, settingUpdater); } } - return build; + // here we are exhaustive and record all settings that failed. + ExceptionsHelper.rethrowAndSuppress(exceptions); + return current; } /** @@ -99,34 +92,25 @@ public abstract class AbstractScopedSettings extends AbstractComponent { // nothing changed in the settings, ignore return newSettings; } - final Settings build = Settings.builder().put(this.settings).put(newSettings).build(); - boolean success = false; + final Settings current = Settings.builder().put(this.settings).put(newSettings).build(); + final Settings previous = Settings.builder().put(this.settings).put(this.lastSettingsApplied).build(); try { + List applyRunnables = new ArrayList<>(); for (SettingUpdater settingUpdater : settingUpdaters) { try { - settingUpdater.prepareApply(build); + applyRunnables.add(settingUpdater.updater(current, previous)); } catch (Exception ex) { logger.warn("failed to prepareCommit settings for [{}]", ex, settingUpdater); throw ex; } } - for (SettingUpdater settingUpdater : settingUpdaters) { - settingUpdater.apply(); + for (Runnable settingUpdater : applyRunnables) { + settingUpdater.run(); } - success = true; } catch (Exception ex) { logger.warn("failed to apply settings", ex); throw ex; } finally { - if (success == false) { - for (SettingUpdater settingUpdater : settingUpdaters) { - try { - settingUpdater.rollback(); - } catch (Exception e) { - logger.error("failed to refresh settings for [{}]", e, settingUpdater); - } - } - } } return lastSettingsApplied = newSettings; } @@ -141,7 +125,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { if (setting != get(setting.getKey())) { throw new IllegalArgumentException("Setting is not registered for key [" + setting.getKey() + "]"); } - this.settingUpdaters.add(setting.newUpdater(consumer, logger, settings, predicate)); + this.settingUpdaters.add(setting.newUpdater(consumer, logger, predicate)); } /** @@ -159,7 +143,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { if (b != get(b.getKey())) { throw new IllegalArgumentException("Setting is not registered for key [" + b.getKey() + "]"); } - this.settingUpdaters.add(Setting.compoundUpdater(consumer, a, b, logger, settings)); + this.settingUpdaters.add(Setting.compoundUpdater(consumer, a, b, logger)); } /** @@ -176,24 +160,51 @@ public abstract class AbstractScopedSettings extends AbstractComponent { * Transactional interface to update settings. * @see Setting */ - public interface SettingUpdater { - /** - * Prepares applying the given settings to this updater. All the heavy lifting like parsing and validation - * happens in this method. Yet the actual setting should not be changed by this call. - * @param settings the settings to apply - * @return true if this updater will update a setting on calling {@link #apply()} otherwise false - */ - boolean prepareApply(Settings settings); + public interface SettingUpdater { /** - * Applies the settings passed to {@link #prepareApply(Settings)} + * Returns true if this updaters setting has changed with the current update + * @param current the current settings + * @param previous the previous setting + * @return true if this updaters setting has changed with the current update */ - void apply(); + boolean hasChanged(Settings current, Settings previous); /** - * Rolls back to the state before {@link #prepareApply(Settings)} was called. All internal prepared state is cleared after this call. + * Returns the instance value for the current settings. This method is stateless and idempotent. */ - void rollback(); + T getValue(Settings current, Settings previous); + + /** + * Applies the given value to the updater. This methods will actually run the update. + */ + void apply(T value, Settings current, Settings previous); + + /** + * Updates this updaters value if it has changed. + * @return true iff the value has been updated. + */ + default boolean apply(Settings current, Settings previous) { + if (hasChanged(current, previous)) { + T value = getValue(current, previous); + apply(value, current, previous); + return true; + } + return false; + } + + /** + * Returns a callable runnable that calls {@link #apply(Object, Settings, Settings)} if the settings + * actually changed. This allows to defer the update to a later point in time while keeping type safety. + * If the value didn't change the returned runnable is a noop. + */ + default Runnable updater(Settings current, Settings previous) { + if (hasChanged(current, previous)) { + T value = getValue(current, previous); + return () -> { apply(value, current, previous);}; + } + return () -> {}; + } } /** diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index 1f4e8ed04b1..b164c906e7a 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -21,6 +21,7 @@ package org.elasticsearch.common.settings; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.Booleans; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.unit.ByteSizeValue; @@ -150,13 +151,13 @@ public class Setting extends ToXContentToBytes { INDEX; } - final AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Settings settings) { - return newUpdater(consumer, logger, settings, (s) -> {}); + final AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger) { + return newUpdater(consumer, logger, (s) -> {}); } - AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Settings settings, Consumer accept) { + AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Consumer accept) { if (isDynamic()) { - return new Updater(consumer, logger, settings, accept); + return new Updater(consumer, logger, accept); } else { throw new IllegalStateException("setting [" + getKey() + "] is not dynamic"); } @@ -166,39 +167,23 @@ public class Setting extends ToXContentToBytes { * this is used for settings that depend on each other... see {@link org.elasticsearch.common.settings.AbstractScopedSettings#addSettingsUpdateConsumer(Setting, Setting, BiConsumer)} and it's * usage for details. */ - static AbstractScopedSettings.SettingUpdater compoundUpdater(final BiConsumer consumer, final Setting aSettting, final Setting bSetting, ESLogger logger, Settings settings) { - final AtomicReference aRef = new AtomicReference<>(); - final AtomicReference bRef = new AtomicReference<>(); - final AbstractScopedSettings.SettingUpdater aSettingUpdater = aSettting.newUpdater(aRef::set, logger, settings); - final AbstractScopedSettings.SettingUpdater bSettingUpdater = bSetting.newUpdater(bRef::set, logger, settings); - return new AbstractScopedSettings.SettingUpdater() { - boolean aHasChanged = false; - boolean bHasChanged = false; + static AbstractScopedSettings.SettingUpdater> compoundUpdater(final BiConsumer consumer, final Setting aSettting, final Setting bSetting, ESLogger logger) { + final AbstractScopedSettings.SettingUpdater aSettingUpdater = aSettting.newUpdater(null, logger); + final AbstractScopedSettings.SettingUpdater bSettingUpdater = bSetting.newUpdater(null, logger); + return new AbstractScopedSettings.SettingUpdater>() { @Override - public boolean prepareApply(Settings settings) { - aHasChanged = aSettingUpdater.prepareApply(settings); - bHasChanged = bSettingUpdater.prepareApply(settings); - return aHasChanged || bHasChanged; + public boolean hasChanged(Settings current, Settings previous) { + return aSettingUpdater.hasChanged(current, previous) || bSettingUpdater.hasChanged(current, previous); } @Override - public void apply() { - aSettingUpdater.apply(); - bSettingUpdater.apply(); - if (aHasChanged || bHasChanged) { - consumer.accept(aRef.get(), bRef.get()); - } + public Tuple getValue(Settings current, Settings previous) { + return new Tuple<>(aSettingUpdater.getValue(current, previous), bSettingUpdater.getValue(current, previous)); } @Override - public void rollback() { - try { - aRef.set(null); - aSettingUpdater.rollback(); - } finally { - bRef.set(null); - bSettingUpdater.rollback(); - } + public void apply(Tuple value, Settings current, Settings previous) { + consumer.accept(value.v1(), value.v2()); } @Override @@ -209,63 +194,47 @@ public class Setting extends ToXContentToBytes { } - private class Updater implements AbstractScopedSettings.SettingUpdater { + private class Updater implements AbstractScopedSettings.SettingUpdater { private final Consumer consumer; private final ESLogger logger; private final Consumer accept; - private String value; - private boolean commitPending; - private String pendingValue; - private T valueInstance; - public Updater(Consumer consumer, ESLogger logger, Settings settings, Consumer accept) { + public Updater(Consumer consumer, ESLogger logger, Consumer accept) { this.consumer = consumer; this.logger = logger; - value = getRaw(settings); this.accept = accept; } - - public boolean prepareApply(Settings settings) { - final String newValue = getRaw(settings); - if (value.equals(newValue) == false) { - T inst = get(settings); - try { - accept.accept(inst); - } catch (Exception | AssertionError e) { - throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + value + "] to [" + getRaw(settings) + "]", e); - } - pendingValue = newValue; - valueInstance = inst; - commitPending = true; - - } else { - commitPending = false; - } - return commitPending; - } - - public void apply() { - if (commitPending) { - logger.info("update [{}] from [{}] to [{}]", key, value, pendingValue); - value = pendingValue; - consumer.accept(valueInstance); - } - commitPending = false; - valueInstance = null; - pendingValue = null; - } - - public void rollback() { - commitPending = false; - valueInstance = null; - pendingValue = null; - } - @Override public String toString() { return "Updater for: " + Setting.this.toString(); } + + @Override + public boolean hasChanged(Settings current, Settings previous) { + final String newValue = getRaw(current); + final String value = getRaw(previous); + return value.equals(newValue) == false; + } + + @Override + public T getValue(Settings current, Settings previous) { + final String newValue = getRaw(current); + final String value = getRaw(previous); + T inst = get(current); + try { + accept.accept(inst); + } catch (Exception | AssertionError e) { + throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + value + "] to [" + newValue + "]", e); + } + return inst; + } + + @Override + public void apply(T value, Settings current, Settings previous) { + logger.info("update [{}] from [{}] to [{}]", key, getRaw(previous), getRaw(current)); + consumer.accept(value); + } } @@ -329,43 +298,35 @@ public class Setting extends ToXContentToBytes { } @Override - public AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Settings settings, Consumer accept) { + public AbstractScopedSettings.SettingUpdater newUpdater(Consumer consumer, ESLogger logger, Consumer accept) { if (isDynamic() == false) { throw new IllegalStateException("setting [" + getKey() + "] is not dynamic"); } final Setting setting = this; - return new AbstractScopedSettings.SettingUpdater() { - private Settings pendingSettings; - private Settings committedSettings = get(settings); + return new AbstractScopedSettings.SettingUpdater() { @Override - public boolean prepareApply(Settings settings) { - Settings currentSettings = get(settings); - if (currentSettings.equals(committedSettings) == false) { - try { - accept.accept(currentSettings); - } catch (Exception | AssertionError e) { - throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + committedSettings.getAsMap() + "] to [" + currentSettings.getAsMap() + "]", e); - } - pendingSettings = currentSettings; - return true; - } else { - return false; - } + public boolean hasChanged(Settings current, Settings previous) { + Settings currentSettings = get(current); + Settings previousSettings = get(previous); + return currentSettings.equals(previousSettings) == false; } @Override - public void apply() { - if (pendingSettings != null) { - consumer.accept(pendingSettings); - committedSettings = pendingSettings; + public Settings getValue(Settings current, Settings previous) { + Settings currentSettings = get(current); + Settings previousSettings = get(previous); + try { + accept.accept(currentSettings); + } catch (Exception | AssertionError e) { + throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + previousSettings.getAsMap() + "] to [" + currentSettings.getAsMap() + "]", e); } - pendingSettings = null; + return currentSettings; } @Override - public void rollback() { - pendingSettings = null; + public void apply(Settings value, Settings current, Settings previous) { + consumer.accept(value); } @Override diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 3f048e454a5..d8ac616eccb 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; @@ -42,38 +43,33 @@ public class SettingTests extends ESTestCase { ByteSizeValue byteSizeValue = byteSizeValueSetting.get(Settings.EMPTY); assertEquals(byteSizeValue.bytes(), 1024); AtomicReference value = new AtomicReference<>(null); - ClusterSettings.SettingUpdater settingUpdater = byteSizeValueSetting.newUpdater(value::set, logger, Settings.EMPTY); + ClusterSettings.SettingUpdater settingUpdater = byteSizeValueSetting.newUpdater(value::set, logger); try { - settingUpdater.prepareApply(Settings.builder().put("a.byte.size", 12).build()); + settingUpdater.apply(Settings.builder().put("a.byte.size", 12).build(), Settings.EMPTY); fail("no unit"); } catch (ElasticsearchParseException ex) { assertEquals("failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized", ex.getMessage()); } - assertTrue(settingUpdater.prepareApply(Settings.builder().put("a.byte.size", "12b").build())); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(Settings.builder().put("a.byte.size", "12b").build(), Settings.EMPTY)); assertEquals(new ByteSizeValue(12), value.get()); } public void testSimpleUpdate() { Setting booleanSetting = Setting.boolSetting("foo.bar", false, true, Setting.Scope.CLUSTER); AtomicReference atomicBoolean = new AtomicReference<>(null); - ClusterSettings.SettingUpdater settingUpdater = booleanSetting.newUpdater(atomicBoolean::set, logger, Settings.EMPTY); + ClusterSettings.SettingUpdater settingUpdater = booleanSetting.newUpdater(atomicBoolean::set, logger); Settings build = Settings.builder().put("foo.bar", false).build(); - settingUpdater.prepareApply(build); - assertNull(atomicBoolean.get()); - settingUpdater.rollback(); + settingUpdater.apply(build, Settings.EMPTY); assertNull(atomicBoolean.get()); build = Settings.builder().put("foo.bar", true).build(); - settingUpdater.prepareApply(build); - assertNull(atomicBoolean.get()); - settingUpdater.apply(); + settingUpdater.apply(build, Settings.EMPTY); assertTrue(atomicBoolean.get()); // try update bogus value build = Settings.builder().put("foo.bar", "I am not a boolean").build(); try { - settingUpdater.prepareApply(build); + settingUpdater.apply(build, Settings.EMPTY); fail("not a boolean"); } catch (IllegalArgumentException ex) { assertEquals("Failed to parse value [I am not a boolean] for setting [foo.bar]", ex.getMessage()); @@ -85,7 +81,7 @@ public class SettingTests extends ESTestCase { assertFalse(booleanSetting.isGroupSetting()); AtomicReference atomicBoolean = new AtomicReference<>(null); try { - booleanSetting.newUpdater(atomicBoolean::set, logger, Settings.EMPTY); + booleanSetting.newUpdater(atomicBoolean::set, logger); fail("not dynamic"); } catch (IllegalStateException ex) { assertEquals("setting [foo.bar] is not dynamic", ex.getMessage()); @@ -96,11 +92,9 @@ public class SettingTests extends ESTestCase { Setting booleanSetting = Setting.boolSetting("foo.bar", false, true, Setting.Scope.CLUSTER); AtomicReference ab1 = new AtomicReference<>(null); AtomicReference ab2 = new AtomicReference<>(null); - ClusterSettings.SettingUpdater settingUpdater = booleanSetting.newUpdater(ab1::set, logger, Settings.EMPTY); - settingUpdater.prepareApply(Settings.builder().put("foo.bar", true).build()); - assertNull(ab1.get()); - assertNull(ab2.get()); - settingUpdater.apply(); + ClusterSettings.SettingUpdater settingUpdater = booleanSetting.newUpdater(ab1::set, logger); + ClusterSettings.SettingUpdater settingUpdater2 = booleanSetting.newUpdater(ab2::set, logger); + settingUpdater.apply(Settings.builder().put("foo.bar", true).build(), Settings.EMPTY); assertTrue(ab1.get()); assertNull(ab2.get()); } @@ -124,40 +118,22 @@ public class SettingTests extends ESTestCase { assertFalse(setting.isGroupSetting()); ref.set(setting.get(Settings.EMPTY)); ComplexType type = ref.get(); - ClusterSettings.SettingUpdater settingUpdater = setting.newUpdater(ref::set, logger, Settings.EMPTY); - assertFalse(settingUpdater.prepareApply(Settings.EMPTY)); - settingUpdater.apply(); + ClusterSettings.SettingUpdater settingUpdater = setting.newUpdater(ref::set, logger); + assertFalse(settingUpdater.apply(Settings.EMPTY, Settings.EMPTY)); assertSame("no update - type has not changed", type, ref.get()); // change from default - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.bar", "2").build())); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(Settings.builder().put("foo.bar", "2").build(), Settings.EMPTY)); assertNotSame("update - type has changed", type, ref.get()); assertEquals("2", ref.get().foo); // change back to default... - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.bar.baz", "2").build())); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(Settings.EMPTY, Settings.builder().put("foo.bar", "2").build())); assertNotSame("update - type has changed", type, ref.get()); assertEquals("", ref.get().foo); } - public void testRollback() { - Setting integerSetting = Setting.intSetting("foo.int.bar", 1, true, Setting.Scope.CLUSTER); - assertFalse(integerSetting.isGroupSetting()); - AtomicReference ref = new AtomicReference<>(null); - ClusterSettings.SettingUpdater settingUpdater = integerSetting.newUpdater(ref::set, logger, Settings.EMPTY); - assertNull(ref.get()); - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.int.bar", "2").build())); - settingUpdater.rollback(); - settingUpdater.apply(); - assertNull(ref.get()); - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.int.bar", "2").build())); - settingUpdater.apply(); - assertEquals(2, ref.get().intValue()); - } - public void testType() { Setting integerSetting = Setting.intSetting("foo.int.bar", 1, true, Setting.Scope.CLUSTER); assertEquals(integerSetting.getScope(), Setting.Scope.CLUSTER); @@ -169,10 +145,11 @@ public class SettingTests extends ESTestCase { AtomicReference ref = new AtomicReference<>(null); Setting setting = Setting.groupSetting("foo.bar.", true, Setting.Scope.CLUSTER); assertTrue(setting.isGroupSetting()); - ClusterSettings.SettingUpdater settingUpdater = setting.newUpdater(ref::set, logger, Settings.EMPTY); + ClusterSettings.SettingUpdater settingUpdater = setting.newUpdater(ref::set, logger); - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").put("foo.bar.3.value", "3").build())); - settingUpdater.apply(); + Settings currentInput = Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").put("foo.bar.3.value", "3").build(); + Settings previousInput = Settings.EMPTY; + assertTrue(settingUpdater.apply(currentInput, previousInput)); assertNotNull(ref.get()); Settings settings = ref.get(); Map asMap = settings.getAsGroups(); @@ -181,14 +158,16 @@ public class SettingTests extends ESTestCase { assertEquals(asMap.get("2").get("value"), "2"); assertEquals(asMap.get("3").get("value"), "3"); + previousInput = currentInput; + currentInput = Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").put("foo.bar.3.value", "3").build(); Settings current = ref.get(); - assertFalse(settingUpdater.prepareApply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").put("foo.bar.3.value", "3").build())); - settingUpdater.apply(); + assertFalse(settingUpdater.apply(currentInput, previousInput)); assertSame(current, ref.get()); + previousInput = currentInput; + currentInput = Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").build(); // now update and check that we got it - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").build())); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(currentInput, previousInput)); assertNotSame(current, ref.get()); asMap = ref.get().getAsGroups(); @@ -196,9 +175,10 @@ public class SettingTests extends ESTestCase { assertEquals(asMap.get("1").get("value"), "1"); assertEquals(asMap.get("2").get("value"), "2"); + previousInput = currentInput; + currentInput = Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "4").build(); // now update and check that we got it - assertTrue(settingUpdater.prepareApply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "4").build())); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(currentInput, previousInput)); assertNotSame(current, ref.get()); asMap = ref.get().getAsGroups(); @@ -209,9 +189,9 @@ public class SettingTests extends ESTestCase { assertTrue(setting.match("foo.bar.baz")); assertFalse(setting.match("foo.baz.bar")); - ClusterSettings.SettingUpdater predicateSettingUpdater = setting.newUpdater(ref::set, logger, Settings.EMPTY, (s) -> assertFalse(true)); + ClusterSettings.SettingUpdater predicateSettingUpdater = setting.newUpdater(ref::set, logger,(s) -> assertFalse(true)); try { - predicateSettingUpdater.prepareApply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").build()); + predicateSettingUpdater.apply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").build(), Settings.EMPTY); fail("not accepted"); } catch (IllegalArgumentException ex) { assertEquals(ex.getMessage(), "illegal value can't update [foo.bar.] from [{}] to [{1.value=1, 2.value=2}]"); @@ -243,32 +223,27 @@ public class SettingTests extends ESTestCase { Composite c = new Composite(); Setting a = Setting.intSetting("foo.int.bar.a", 1, true, Setting.Scope.CLUSTER); Setting b = Setting.intSetting("foo.int.bar.b", 1, true, Setting.Scope.CLUSTER); - ClusterSettings.SettingUpdater settingUpdater = Setting.compoundUpdater(c::set, a, b, logger, Settings.EMPTY); - assertFalse(settingUpdater.prepareApply(Settings.EMPTY)); - settingUpdater.apply(); + ClusterSettings.SettingUpdater> settingUpdater = Setting.compoundUpdater(c::set, a, b, logger); + assertFalse(settingUpdater.apply(Settings.EMPTY, Settings.EMPTY)); assertNull(c.a); assertNull(c.b); Settings build = Settings.builder().put("foo.int.bar.a", 2).build(); - assertTrue(settingUpdater.prepareApply(build)); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(build, Settings.EMPTY)); assertEquals(2, c.a.intValue()); - assertNull(c.b); + assertEquals(1, c.b.intValue()); Integer aValue = c.a; - assertFalse(settingUpdater.prepareApply(build)); - settingUpdater.apply(); + assertFalse(settingUpdater.apply(build, build)); assertSame(aValue, c.a); - + Settings previous = build; build = Settings.builder().put("foo.int.bar.a", 2).put("foo.int.bar.b", 5).build(); - assertTrue(settingUpdater.prepareApply(build)); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(build, previous)); assertEquals(2, c.a.intValue()); assertEquals(5, c.b.intValue()); // reset to default - assertTrue(settingUpdater.prepareApply(Settings.EMPTY)); - settingUpdater.apply(); + assertTrue(settingUpdater.apply(Settings.EMPTY, build)); assertEquals(1, c.a.intValue()); assertEquals(1, c.b.intValue());