Separate out validation of groups of settings

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 @@ static Setting<Integer> buildNumberOfShardsSetting() {
     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 Iterator<Setting<Integer>> settings() {
     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 DiskThresholdSettings(Settings settings, ClusterSettings clusterSettings)
 
     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 Iterator<Setting<String>> settings() {
 
     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 Iterator<Setting<String>> settings() {
 
     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 @@ private boolean updateSettings(Settings toApply, Settings.Builder target, Settin
             } 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 @@ private void checkPropertyRequiresIndexScope(final EnumSet<Property> properties,
      * @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 Setting(String key, Function<Settings, String> defaultValue, Function<Str
      * @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 @@ boolean hasComplexMatcher() {
         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 @@ private T get(Settings settings, boolean validate) {
                 } else {
                     map = Collections.emptyMap();
                 }
+                validator.validate(parsed);
                 validator.validate(parsed, map);
             }
             return parsed;
@@ -805,26 +814,39 @@ public Map<String, T> getAsMap(Settings settings) {
     }
 
     /**
-     * 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}
      */
     @FunctionalInterface
     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 static Setting<String> simpleString(String key, Property... properties) {
         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 static Setting<String> simpleString(
         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 @@ private ListSetting(
             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 static Setting<TimeValue> timeSetting(
                 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 Iterator<Setting<Integer>> settings() {
         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 String getKey(final String key) {
                         if (Strings.hasLength(s)) {
                             parsePort(s);
                         }
-                        return s;
                     },
                     Setting.Property.Deprecated,
                     Setting.Property.Dynamic,