Separate out validation of groups of settings (#34184)

Today, a setting can declare that its validity depends on the values of other
related settings. However, the validity of a setting is not always checked
against the correct values of its dependent settings because those settings'
correct values may not be available when the validator runs.

This commit separates the validation of a settings updates into two phases,
with separate methods on the `Setting.Validator` interface. In the first phase
the setting's validity is checked in isolation, and in the second phase it is
checked again against the values of its related settings. Most settings only
use the first phase, and only the few settings with dependencies make use of
the second phase.

plugins/examples/custom-settings/src/main/java/org/elasticsearch/example/customsettings/ExampleCustomSettingsConfig.java

@@ -49,7 +49,7 @@ public class ExampleCustomSettingsConfig {
     /**
      * A string setting that can be dynamically updated and that is validated by some logic
      */
-    static final Setting<String> VALIDATED_SETTING = Setting.simpleString("custom.validated", (value, settings) -> {
+    static final Setting<String> VALIDATED_SETTING = Setting.simpleString("custom.validated", value -> {
         if (value != null && value.contains("forbidden")) {
             throw new IllegalArgumentException("Setting must not contain [forbidden]");
         }

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

@@ -157,6 +157,10 @@ public class IndexMetaData implements Diffable<IndexMetaData>, ToXContentFragmen
     public static final Setting<Integer> INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING =
         Setting.intSetting("index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING,
                            1, new Setting.Validator<Integer>() {
+            @Override
+            public void validate(Integer value) {
+            }
+
             @Override
             public void validate(Integer numRoutingShards, Map<Setting<Integer>, Integer> settings) {
                 Integer numShards = settings.get(INDEX_NUMBER_OF_SHARDS_SETTING);
@@ -223,14 +227,14 @@ public class IndexMetaData implements Diffable<IndexMetaData>, ToXContentFragmen
     public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include";
     public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude";
     public static final Setting.AffixSetting<String> INDEX_ROUTING_REQUIRE_GROUP_SETTING =
-        Setting.prefixKeySetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", (key) ->
-            Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
+        Setting.prefixKeySetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", key ->
+            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
     public static final Setting.AffixSetting<String> INDEX_ROUTING_INCLUDE_GROUP_SETTING =
-        Setting.prefixKeySetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", (key) ->
-            Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
+        Setting.prefixKeySetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", key ->
+            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
     public static final Setting.AffixSetting<String> INDEX_ROUTING_EXCLUDE_GROUP_SETTING =
-        Setting.prefixKeySetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", (key) ->
-            Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
+        Setting.prefixKeySetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", key ->
+            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
     public static final Setting.AffixSetting<String> INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING =
         Setting.prefixKeySetting("index.routing.allocation.initial_recovery.", key -> Setting.simpleString(key));
         // this is only setable internally not a registered setting!!

server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java

@@ -93,6 +93,10 @@ public class DiskThresholdSettings {
 
     static final class LowDiskWatermarkValidator implements Setting.Validator<String> {
 
+        @Override
+        public void validate(String value) {
+        }
+
         @Override
         public void validate(String value, Map<Setting<String>, String> settings) {
             final String highWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING);
@@ -112,6 +116,10 @@ public class DiskThresholdSettings {
 
     static final class HighDiskWatermarkValidator implements Setting.Validator<String> {
 
+        @Override
+        public void validate(String value) {
+        }
+
         @Override
         public void validate(String value, Map<Setting<String>, String> settings) {
             final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);
@@ -131,6 +139,10 @@ public class DiskThresholdSettings {
 
     static final class FloodStageValidator implements Setting.Validator<String> {
 
+        @Override
+        public void validate(String value) {
+        }
+
         @Override
         public void validate(String value, Map<Setting<String>, String> settings) {
             final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING);

server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java

@@ -72,14 +72,14 @@ public class FilterAllocationDecider extends AllocationDecider {
     private static final String CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX = "cluster.routing.allocation.include";
     private static final String CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX = "cluster.routing.allocation.exclude";
     public static final Setting.AffixSetting<String> CLUSTER_ROUTING_REQUIRE_GROUP_SETTING =
-        Setting.prefixKeySetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", (key) ->
-            Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
+        Setting.prefixKeySetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", key ->
+            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
     public static final Setting.AffixSetting<String> CLUSTER_ROUTING_INCLUDE_GROUP_SETTING =
-        Setting.prefixKeySetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", (key) ->
-            Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
+        Setting.prefixKeySetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", key ->
+            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
     public static final Setting.AffixSetting<String>CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING =
-        Setting.prefixKeySetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", (key) ->
-            Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
+        Setting.prefixKeySetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", key ->
+            Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
 
     /**
      * The set of {@link RecoverySource.Type} values for which the

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -723,7 +723,7 @@ public abstract class AbstractScopedSettings {
             } else if (get(key) == null) {
                 throw new IllegalArgumentException(type + " setting [" + key + "], not recognized");
             } else if (isDelete == false && canUpdate.test(key)) {
-                validate(key, toApply, false); // we might not have a full picture here do to a dependency validation
+                get(key).validateWithoutDependencies(toApply); // we might not have a full picture here do to a dependency validation
                 settingsBuilder.copy(key, toApply);
                 updates.copy(key, toApply);
                 changed |= toApply.get(key).equals(target.get(key)) == false;

server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -186,7 +186,7 @@ public class Setting<T> implements ToXContentObject {
      * @param properties properties for this setting like scope, filtering...
      */
     public Setting(Key key, Function<Settings, String> defaultValue, Function<String, T> parser, Property... properties) {
-        this(key, defaultValue, parser, (v, s) -> {}, properties);
+        this(key, defaultValue, parser, v -> {}, properties);
     }
 
     /**
@@ -246,7 +246,7 @@ public class Setting<T> implements ToXContentObject {
      * @param properties properties for this setting like scope, filtering...
      */
     public Setting(Key key, Setting<T> fallbackSetting, Function<String, T> parser, Property... properties) {
-        this(key, fallbackSetting, fallbackSetting::getRaw, parser, (v, m) -> {}, properties);
+        this(key, fallbackSetting, fallbackSetting::getRaw, parser, v -> {}, properties);
     }
 
     /**
@@ -354,6 +354,14 @@ public class Setting<T> implements ToXContentObject {
         return isGroupSetting();
     }
 
+    /**
+     * Validate the current setting value only without dependencies with {@link Setting.Validator#validate(Object)}.
+     * @param settings a settings object for settings that has a default value depending on another setting if available
+     */
+    void validateWithoutDependencies(Settings settings) {
+        validator.validate(get(settings, false));
+    }
+
     /**
      * Returns the default value string representation for this setting.
      * @param settings a settings object for settings that has a default value depending on another setting if available
@@ -414,6 +422,7 @@ public class Setting<T> implements ToXContentObject {
                 } else {
                     map = Collections.emptyMap();
                 }
+                validator.validate(parsed);
                 validator.validate(parsed, map);
             }
             return parsed;
@@ -805,8 +814,10 @@ public class Setting<T> implements ToXContentObject {
     }
 
     /**
-     * Represents a validator for a setting. The {@link #validate(Object, Map)} method is invoked with the value of this setting and a map
-     * from the settings specified by {@link #settings()}} to their values. All these values come from the same {@link Settings} instance.
+     * Represents a validator for a setting. The {@link #validate(Object)} method is invoked early in the update setting process with the
+     * value of this setting for a fail-fast validation. Later on, the {@link #validate(Object, Map)} method is invoked with the value of
+     * this setting and a map from the settings specified by {@link #settings()}} to their values. All these values come from the same
+     * {@link Settings} instance.
      *
      * @param <T> the type of the {@link Setting}
      */
@@ -814,17 +825,28 @@ public class Setting<T> implements ToXContentObject {
     public interface Validator<T> {
 
         /**
-         * The validation routine for this validator.
+         * Validate this setting's value in isolation.
+         *
+         * @param value the value of this setting
+         */
+        void validate(T value);
+
+        /**
+         * Validate this setting against its dependencies, specified by {@link #settings()}. The default implementation does nothing,
+         * accepting any value as valid as long as it passes the validation in {@link #validate(Object)}.
          *
          * @param value    the value of this setting
          * @param settings a map from the settings specified by {@link #settings()}} to their values
          */
-        void validate(T value, Map<Setting<T>, T> settings);
+        default void validate(T value, Map<Setting<T>, T> settings) {
+        }
 
         /**
-         * The settings needed by this validator.
+         * The settings on which the validity of this setting depends. The values of the specified settings are passed to
+         * {@link #validate(Object, Map)}. By default this returns an empty iterator, indicating that this setting does not depend on any
+         * other settings.
          *
-         * @return the settings needed to validate; these can be used for cross-settings validation
+         * @return the settings on which the validity of this setting depends.
          */
         default Iterator<Setting<T>> settings() {
             return Collections.emptyIterator();
@@ -1021,8 +1043,8 @@ public class Setting<T> implements ToXContentObject {
         return new Setting<>(key, s -> "", Function.identity(), properties);
     }
 
-    public static Setting<String> simpleString(String key, Function<String, String> parser, Property... properties) {
-        return new Setting<>(key, s -> "", parser, properties);
+    public static Setting<String> simpleString(String key, Validator<String> validator, Property... properties) {
+        return new Setting<>(new SimpleKey(key), null, s -> "", Function.identity(), validator, properties);
     }
 
     public static Setting<String> simpleString(String key, Setting<String> fallback, Property... properties) {
@@ -1037,10 +1059,6 @@ public class Setting<T> implements ToXContentObject {
         return new Setting<>(key, fallback, parser, properties);
     }
 
-    public static Setting<String> simpleString(String key, Validator<String> validator, Property... properties) {
-        return new Setting<>(new SimpleKey(key), null, s -> "", Function.identity(), validator, properties);
-    }
-
     /**
      * Creates a new Setting instance with a String value
      *
@@ -1279,9 +1297,9 @@ public class Setting<T> implements ToXContentObject {
             super(
                     new ListKey(key),
                     fallbackSetting,
-                    (s) -> Setting.arrayToParsableString(defaultStringValue.apply(s)),
+                    s -> Setting.arrayToParsableString(defaultStringValue.apply(s)),
                     parser,
-                    (v,s) -> {},
+                    v -> {},
                     properties);
             this.defaultStringValue = defaultStringValue;
         }
@@ -1339,7 +1357,7 @@ public class Setting<T> implements ToXContentObject {
                 fallbackSetting,
                 fallbackSetting::getRaw,
                 minTimeValueParser(key, minValue),
-                (v, s) -> {},
+                v -> {},
                 properties);
     }
 

server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java

@@ -75,8 +75,12 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder<Aut
         this.minQueueSizeSetting = new Setting<>(
             minSizeKey,
             Integer.toString(minQueueSize),
-            (s) -> Setting.parseInt(s, 0, minSizeKey),
+            s -> Setting.parseInt(s, 0, minSizeKey),
             new Setting.Validator<Integer>() {
+                @Override
+                public void validate(Integer value) {
+                }
+
                 @Override
                 public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
                     if (value > settings.get(tempMaxQueueSizeSetting)) {
@@ -94,8 +98,12 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder<Aut
         this.maxQueueSizeSetting = new Setting<>(
                 maxSizeKey,
                 Integer.toString(maxQueueSize),
-                (s) -> Setting.parseInt(s, 0, maxSizeKey),
+                s -> Setting.parseInt(s, 0, maxSizeKey),
                 new Setting.Validator<Integer>() {
+                    @Override
+                    public void validate(Integer value) {
+                    }
+
                     @Override
                     public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
                         if (value < settings.get(tempMinQueueSizeSetting)) {

server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java

@@ -121,7 +121,6 @@ public abstract class RemoteClusterAware {
                         if (Strings.hasLength(s)) {
                             parsePort(s);
                         }
-                        return s;
                     },
                     Setting.Property.Deprecated,
                     Setting.Property.Dynamic,

server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java

@@ -29,12 +29,16 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 
 import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.util.Arrays.asList;
 import static org.elasticsearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
@@ -221,24 +225,11 @@ public class SettingsUpdaterTests extends ESTestCase {
 
         // these are invalid settings that exist as either persistent or transient settings
         final int numberOfInvalidSettings = randomIntBetween(0, 7);
-        final List<Setting<String>> invalidSettings = new ArrayList<>(numberOfInvalidSettings);
-        for (int i = 0; i < numberOfInvalidSettings; i++) {
-            final Setting<String> invalidSetting = Setting.simpleString(
-                    "invalid.setting" + i,
-                    (value, settings) -> {
-                        throw new IllegalArgumentException("invalid");
-                    },
-                    Property.NodeScope);
-            invalidSettings.add(invalidSetting);
-        }
+        final List<Setting<String>> invalidSettings = invalidSettings(numberOfInvalidSettings);
 
         // these are unknown settings that exist as either persistent or transient settings
         final int numberOfUnknownSettings = randomIntBetween(0, 7);
-        final List<Setting<String>> unknownSettings = new ArrayList<>(numberOfUnknownSettings);
-        for (int i = 0; i < numberOfUnknownSettings; i++) {
-            final Setting<String> unknownSetting = Setting.simpleString("unknown.setting" + i, Property.NodeScope);
-            unknownSettings.add(unknownSetting);
-        }
+        final List<Setting<String>> unknownSettings = unknownSettings(numberOfUnknownSettings);
 
         final Settings.Builder existingPersistentSettings = Settings.builder();
         final Settings.Builder existingTransientSettings = Settings.builder();
@@ -393,24 +384,11 @@ public class SettingsUpdaterTests extends ESTestCase {
 
         // these are invalid settings that exist as either persistent or transient settings
         final int numberOfInvalidSettings = randomIntBetween(0, 7);
-        final List<Setting<String>> invalidSettings = new ArrayList<>(numberOfInvalidSettings);
-        for (int i = 0; i < numberOfInvalidSettings; i++) {
-            final Setting<String> invalidSetting = Setting.simpleString(
-                    "invalid.setting" + i,
-                    (value, settings) -> {
-                        throw new IllegalArgumentException("invalid");
-                    },
-                    Property.NodeScope);
-            invalidSettings.add(invalidSetting);
-        }
+        final List<Setting<String>> invalidSettings = invalidSettings(numberOfInvalidSettings);
 
         // these are unknown settings that exist as either persistent or transient settings
         final int numberOfUnknownSettings = randomIntBetween(0, 7);
-        final List<Setting<String>> unknownSettings = new ArrayList<>(numberOfUnknownSettings);
-        for (int i = 0; i < numberOfUnknownSettings; i++) {
-            final Setting<String> unknownSetting = Setting.simpleString("unknown.setting" + i, Property.NodeScope);
-            unknownSettings.add(unknownSetting);
-        }
+        final List<Setting<String>> unknownSettings = unknownSettings(numberOfUnknownSettings);
 
         final Settings.Builder existingPersistentSettings = Settings.builder();
         final Settings.Builder existingTransientSettings = Settings.builder();
@@ -511,4 +489,120 @@ public class SettingsUpdaterTests extends ESTestCase {
         }
     }
 
+    private static List<Setting<String>> unknownSettings(int numberOfUnknownSettings) {
+        final List<Setting<String>> unknownSettings = new ArrayList<>(numberOfUnknownSettings);
+        for (int i = 0; i < numberOfUnknownSettings; i++) {
+            unknownSettings.add(Setting.simpleString("unknown.setting" + i, Property.NodeScope));
+        }
+        return unknownSettings;
+    }
+
+    private static List<Setting<String>> invalidSettings(int numberOfInvalidSettings) {
+        final List<Setting<String>> invalidSettings = new ArrayList<>(numberOfInvalidSettings);
+        for (int i = 0; i < numberOfInvalidSettings; i++) {
+            invalidSettings.add(randomBoolean() ? invalidInIsolationSetting(i) : invalidWithDependenciesSetting(i));
+        }
+        return invalidSettings;
+    }
+
+    private static Setting<String> invalidInIsolationSetting(int index) {
+        return Setting.simpleString("invalid.setting" + index,
+            new Setting.Validator<String>() {
+                @Override
+                public void validate(String value) {
+                    throw new IllegalArgumentException("Invalid in isolation setting");
+                }
+
+                @Override
+                public void validate(String value, Map<Setting<String>, String> settings) {
+                }
+            },
+            Property.NodeScope);
+    }
+
+    private static Setting<String> invalidWithDependenciesSetting(int index) {
+        return Setting.simpleString("invalid.setting" + index,
+            new Setting.Validator<String>() {
+                @Override
+                public void validate(String value) {
+                }
+
+                @Override
+                public void validate(String value, Map<Setting<String>, String> settings) {
+                    throw new IllegalArgumentException("Invalid with dependencies setting");
+                }
+            },
+            Property.NodeScope);
+    }
+
+    private static class FooLowSettingValidator implements Setting.Validator<Integer> {
+        @Override
+        public void validate(Integer value) {
+        }
+
+        @Override
+        public void validate(Integer low, Map<Setting<Integer>, Integer> settings) {
+            if (settings.containsKey(SETTING_FOO_HIGH) && low > settings.get(SETTING_FOO_HIGH)) {
+                throw new IllegalArgumentException("[low]=" + low + " is higher than [high]=" + settings.get(SETTING_FOO_HIGH));
+            }
+        }
+
+        @Override
+        public Iterator<Setting<Integer>> settings() {
+            return asList(SETTING_FOO_LOW, SETTING_FOO_HIGH).iterator();
+        }
+    }
+
+    private static class FooHighSettingValidator implements Setting.Validator<Integer> {
+        @Override
+        public void validate(Integer value) {
+        }
+
+        @Override
+        public void validate(Integer high, Map<Setting<Integer>, Integer> settings) {
+            if (settings.containsKey(SETTING_FOO_LOW) && high < settings.get(SETTING_FOO_LOW)) {
+                throw new IllegalArgumentException("[high]=" + high + " is lower than [low]=" + settings.get(SETTING_FOO_LOW));
+            }
+        }
+
+        @Override
+        public Iterator<Setting<Integer>> settings() {
+            return asList(SETTING_FOO_LOW, SETTING_FOO_HIGH).iterator();
+        }
+    }
+
+    private static final Setting<Integer> SETTING_FOO_LOW = new Setting<>("foo.low", "10",
+        Integer::valueOf, new FooLowSettingValidator(), Property.Dynamic, Setting.Property.NodeScope);
+    private static final Setting<Integer> SETTING_FOO_HIGH = new Setting<>("foo.high", "100",
+        Integer::valueOf, new FooHighSettingValidator(), Property.Dynamic, Setting.Property.NodeScope);
+
+    public void testUpdateOfValidationDependentSettings() {
+        final ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(asList(SETTING_FOO_LOW, SETTING_FOO_HIGH)));
+        final SettingsUpdater updater = new SettingsUpdater(settings);
+        final MetaData.Builder metaData = MetaData.builder().persistentSettings(Settings.EMPTY).transientSettings(Settings.EMPTY);
+
+        ClusterState cluster = ClusterState.builder(new ClusterName("cluster")).metaData(metaData).build();
+
+        cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_LOW.getKey(), 20).build(), Settings.EMPTY, logger);
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("20"));
+
+        cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 40).build(), Settings.EMPTY, logger);
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("20"));
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_HIGH.getKey()), equalTo("40"));
+
+        cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_LOW.getKey(), 5).build(), Settings.EMPTY, logger);
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("5"));
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_HIGH.getKey()), equalTo("40"));
+
+        cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 8).build(), Settings.EMPTY, logger);
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("5"));
+        assertThat(cluster.getMetaData().settings().get(SETTING_FOO_HIGH.getKey()), equalTo("8"));
+
+        final ClusterState finalCluster = cluster;
+        Exception exception = expectThrows(IllegalArgumentException.class, () ->
+            updater.updateSettings(finalCluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 2).build(), Settings.EMPTY, logger));
+
+        assertThat(exception.getMessage(), equalTo("[high]=2 is lower than [low]=5"));
+    }
+
 }

server/src/test/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettingsTests.java

@@ -26,6 +26,7 @@ import org.elasticsearch.test.ESTestCase;
 
 import java.util.Locale;
 
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.instanceOf;
@@ -203,4 +204,50 @@ public class DiskThresholdSettingsTests extends ESTestCase {
         assertThat(cause, hasToString(containsString("low disk watermark [85%] more than high disk watermark [75%]")));
     }
 
+    public void testSequenceOfUpdates() {
+        final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
+        new DiskThresholdSettings(Settings.EMPTY, clusterSettings); // this has the effect of registering the settings updater
+
+        final Settings.Builder target = Settings.builder();
+
+        {
+            final Settings settings = Settings.builder()
+                .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "99%")
+                .build();
+            final Settings.Builder updates = Settings.builder();
+            assertTrue(clusterSettings.updateSettings(settings, target, updates, "transient"));
+            assertNull(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey()));
+            assertNull(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey()));
+            assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey()),
+                equalTo("99%"));
+        }
+
+        {
+            final Settings settings = Settings.builder()
+                .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "97%")
+                .build();
+            final Settings.Builder updates = Settings.builder();
+            assertTrue(clusterSettings.updateSettings(settings, target, updates, "transient"));
+            assertNull(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey()));
+            assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey()),
+                equalTo("97%"));
+            assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey()),
+                equalTo("99%"));
+        }
+
+        {
+            final Settings settings = Settings.builder()
+                .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "95%")
+                .build();
+            final Settings.Builder updates = Settings.builder();
+            assertTrue(clusterSettings.updateSettings(settings, target, updates, "transient"));
+            assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey()),
+                equalTo("95%"));
+            assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey()),
+                equalTo("97%"));
+            assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey()),
+                equalTo("99%"));
+        }
+    }
+
 }

server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -37,7 +37,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -51,9 +50,7 @@ import java.util.stream.Collectors;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.startsWith;
-import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.sameInstance;
 
@@ -514,94 +511,6 @@ public class ScopedSettingsTests extends ESTestCase {
         assertEquals(15, bC.get());
     }
 
-    private static final Setting<Integer> FOO_BAR_LOW_SETTING = new Setting<>(
-            "foo.bar.low",
-            "1",
-            Integer::parseInt,
-            new FooBarLowValidator(),
-            Property.Dynamic,
-            Property.NodeScope);
-
-    private static final Setting<Integer> FOO_BAR_HIGH_SETTING = new Setting<>(
-            "foo.bar.high",
-            "2",
-            Integer::parseInt,
-            new FooBarHighValidator(),
-            Property.Dynamic,
-            Property.NodeScope);
-
-    static class FooBarLowValidator implements Setting.Validator<Integer> {
-        @Override
-        public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
-            final int high = settings.get(FOO_BAR_HIGH_SETTING);
-            if (value > high) {
-                throw new IllegalArgumentException("low [" + value + "] more than high [" + high + "]");
-            }
-        }
-
-        @Override
-        public Iterator<Setting<Integer>> settings() {
-            return Collections.singletonList(FOO_BAR_HIGH_SETTING).iterator();
-        }
-    }
-
-    static class FooBarHighValidator implements Setting.Validator<Integer> {
-        @Override
-        public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
-            final int low = settings.get(FOO_BAR_LOW_SETTING);
-            if (value < low) {
-                throw new IllegalArgumentException("high [" + value + "] less than low [" + low + "]");
-            }
-        }
-
-        @Override
-        public Iterator<Setting<Integer>> settings() {
-            return Collections.singletonList(FOO_BAR_LOW_SETTING).iterator();
-        }
-    }
-
-    public void testValidator() {
-        final AbstractScopedSettings service =
-                new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(FOO_BAR_LOW_SETTING, FOO_BAR_HIGH_SETTING)));
-
-        final AtomicInteger consumerLow = new AtomicInteger();
-        final AtomicInteger consumerHigh = new AtomicInteger();
-
-        service.addSettingsUpdateConsumer(FOO_BAR_LOW_SETTING, consumerLow::set);
-
-        service.addSettingsUpdateConsumer(FOO_BAR_HIGH_SETTING, consumerHigh::set);
-
-        final Settings newSettings = Settings.builder().put("foo.bar.low", 17).put("foo.bar.high", 13).build();
-        {
-            final IllegalArgumentException e =
-                    expectThrows(
-                            IllegalArgumentException.class,
-                            () -> service.validateUpdate(newSettings));
-            assertThat(e, hasToString(containsString("illegal value can't update [foo.bar.low] from [1] to [17]")));
-            assertNotNull(e.getCause());
-            assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
-            final IllegalArgumentException cause = (IllegalArgumentException) e.getCause();
-            assertThat(cause, hasToString(containsString("low [17] more than high [13]")));
-            assertThat(e.getSuppressed(), arrayWithSize(1));
-            assertThat(e.getSuppressed()[0], instanceOf(IllegalArgumentException.class));
-            final IllegalArgumentException suppressed = (IllegalArgumentException) e.getSuppressed()[0];
-            assertThat(suppressed, hasToString(containsString("illegal value can't update [foo.bar.high] from [2] to [13]")));
-            assertNotNull(suppressed.getCause());
-            assertThat(suppressed.getCause(), instanceOf(IllegalArgumentException.class));
-            final IllegalArgumentException suppressedCause = (IllegalArgumentException) suppressed.getCause();
-            assertThat(suppressedCause, hasToString(containsString("high [13] less than low [17]")));
-            assertThat(consumerLow.get(), equalTo(0));
-            assertThat(consumerHigh.get(), equalTo(0));
-        }
-
-        {
-            final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.applySettings(newSettings));
-            assertThat(e, hasToString(containsString("illegal value can't update [foo.bar.low] from [1] to [17]")));
-            assertThat(consumerLow.get(), equalTo(0));
-            assertThat(consumerHigh.get(), equalTo(0));
-        }
-    }
-
     public void testGet() {
         ClusterSettings settings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
         // affix setting - complex matcher

server/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -204,12 +204,18 @@ public class SettingTests extends ESTestCase {
 
     static class FooBarValidator implements Setting.Validator<String> {
 
-        public static boolean invoked;
+        public static boolean invokedInIsolation;
+        public static boolean invokedWithDependencies;
 
         @Override
-        public void validate(String value, Map<Setting<String>, String> settings) {
-            invoked = true;
+        public void validate(String value) {
+            invokedInIsolation = true;
             assertThat(value, equalTo("foo.bar value"));
+        }
+
+        @Override
+        public void validate(String value, Map<Setting<String>, String> settings) {
+            invokedWithDependencies = true;
             assertTrue(settings.keySet().contains(BAZ_QUX_SETTING));
             assertThat(settings.get(BAZ_QUX_SETTING), equalTo("baz.qux value"));
             assertTrue(settings.keySet().contains(QUUX_QUUZ_SETTING));
@@ -230,7 +236,8 @@ public class SettingTests extends ESTestCase {
                 .put("quux.quuz", "quux.quuz value")
                 .build();
         FOO_BAR_SETTING.get(settings);
-        assertTrue(FooBarValidator.invoked);
+        assertTrue(FooBarValidator.invokedInIsolation);
+        assertTrue(FooBarValidator.invokedWithDependencies);
     }
 
     public void testUpdateNotDynamic() {
@@ -934,7 +941,7 @@ public class SettingTests extends ESTestCase {
 
         final Setting.AffixSetting<String> affixSetting =
             Setting.prefixKeySetting("prefix" + ".",
-                (key) -> Setting.simpleString(key, (value, map) -> {}, Property.Dynamic, Property.NodeScope));
+                key -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope));
 
         final Consumer<Map<String, String>> consumer = (map) -> {};
         final BiConsumer<String, String> validator = (s1, s2) -> {};

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java

@@ -139,7 +139,7 @@ public class XPackSettings {
      * Do not allow insecure hashing algorithms to be used for password hashing
      */
     public static final Setting<String> PASSWORD_HASHING_ALGORITHM = new Setting<>(
-        "xpack.security.authc.password_hashing.algorithm", "bcrypt", Function.identity(), (v, s) -> {
+        "xpack.security.authc.password_hashing.algorithm", "bcrypt", Function.identity(), v -> {
         if (Hasher.getAvailableAlgoStoredHash().contains(v.toLowerCase(Locale.ROOT)) == false) {
             throw new IllegalArgumentException("Invalid algorithm: " + v + ". Valid values for password hashing are " +
                 Hasher.getAvailableAlgoStoredHash().toString());

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java

@@ -25,11 +25,11 @@ public abstract class Exporter implements AutoCloseable {
 
     private static final Setting.AffixSetting<Boolean> ENABLED_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","enabled",
-                    (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
+                    key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
 
     private static final Setting.AffixSetting<String> TYPE_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","type",
-                    (key) -> Setting.simpleString(key, (v, s) -> {
+                    key -> Setting.simpleString(key, v -> {
                         switch (v) {
                             case "":
                             case "http":
@@ -47,13 +47,13 @@ public abstract class Exporter implements AutoCloseable {
      */
     public static final Setting.AffixSetting<Boolean> USE_INGEST_PIPELINE_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","use_ingest",
-                    (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
+                    key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
     /**
      * Every {@code Exporter} allows users to explicitly disable cluster alerts.
      */
     public static final Setting.AffixSetting<Boolean> CLUSTER_ALERTS_MANAGEMENT_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.", "cluster_alerts.management.enabled",
-                    (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
+                    key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
     /**
      * Every {@code Exporter} allows users to explicitly disable specific cluster alerts.
      * <p>
@@ -61,14 +61,14 @@ public abstract class Exporter implements AutoCloseable {
      */
     public static final Setting.AffixSetting<List<String>> CLUSTER_ALERTS_BLACKLIST_SETTING = Setting
                 .affixKeySetting("xpack.monitoring.exporters.", "cluster_alerts.management.blacklist",
-                    (key) -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope));
+                    key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope));
 
     /**
      * Every {@code Exporter} allows users to use a different index time format.
      */
     private static final Setting.AffixSetting<String> INDEX_NAME_TIME_FORMAT_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","index.name.time_format",
-                    (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope));
+                    key -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope));
 
     private static final String INDEX_FORMAT = "YYYY.MM.dd";
 

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpSettings.java

@@ -34,7 +34,7 @@ public class HttpSettings {
     private static final String SSL_KEY_PREFIX = "xpack.http.ssl.";
 
     static final Setting<String> PROXY_HOST = Setting.simpleString(PROXY_HOST_KEY, Property.NodeScope);
-    static final Setting<String> PROXY_SCHEME = Setting.simpleString(PROXY_SCHEME_KEY, (v, s) -> Scheme.parse(v), Property.NodeScope);
+    static final Setting<String> PROXY_SCHEME = Setting.simpleString(PROXY_SCHEME_KEY, Scheme::parse, Property.NodeScope);
     static final Setting<Integer> PROXY_PORT = Setting.intSetting(PROXY_PORT_KEY, 0, 0, 0xFFFF, Property.NodeScope);
 
     static final Setting<ByteSizeValue> MAX_HTTP_RESPONSE_SIZE = Setting.byteSizeSetting("xpack.http.max_response_size",