Enable cross-setting validation

This commit introduces a framework for settings validation and enables
cross-setting validation.

Relates #25560

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

@@ -42,8 +42,11 @@ import org.elasticsearch.common.xcontent.XContentType;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.EnumSet;
+import java.util.HashMap;
 import java.util.IdentityHashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -118,18 +121,20 @@ public class Setting<T> extends ToXContentToBytes {
     @Nullable
     private final Setting<T> fallbackSetting;
     private final Function<String, T> parser;
+    private final Validator<T> validator;
     private final EnumSet<Property> properties;
 
     private static final EnumSet<Property> EMPTY_PROPERTIES = EnumSet.noneOf(Property.class);
 
     private Setting(Key key, @Nullable Setting<T> fallbackSetting, Function<Settings, String> defaultValue, Function<String, T> parser,
-            Property... properties) {
+            Validator<T> validator, Property... properties) {
         assert this instanceof SecureSetting || this.isGroupSetting() || parser.apply(defaultValue.apply(Settings.EMPTY)) != null
                : "parser returned null";
         this.key = key;
         this.fallbackSetting = fallbackSetting;
         this.defaultValue = defaultValue;
         this.parser = parser;
+        this.validator = validator;
         if (properties == null) {
             throw new IllegalArgumentException("properties cannot be null for setting [" + key + "]");
         }
@@ -151,7 +156,21 @@ public class Setting<T> extends ToXContentToBytes {
      * @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, null, defaultValue, parser, properties);
+        this(key, defaultValue, parser, (v, s) -> {}, properties);
+    }
+
+    /**
+     * Creates a new {@code Setting} instance.
+     *
+     * @param key          the settings key for this setting
+     * @param defaultValue a default value function that results a string representation of the default value
+     * @param parser       a parser that parses a string representation into the concrete type for this setting
+     * @param validator    a {@link Validator} for validating this setting
+     * @param properties   properties for this setting
+     */
+    public Setting(
+            Key key, Function<Settings, String> defaultValue, Function<String, T> parser, Validator<T> validator, Property... properties) {
+        this(key, null, defaultValue, parser, validator, properties);
     }
 
     /**
@@ -165,6 +184,19 @@ public class Setting<T> extends ToXContentToBytes {
         this(key, s -> defaultValue, parser, properties);
     }
 
+    /**
+     * Creates a new {@code Setting} instance.
+     *
+     * @param key          the settings key for this setting
+     * @param defaultValue a default value function that results a string representation of the default value
+     * @param parser       a parser that parses a string representation into the concrete type for this setting
+     * @param validator    a {@link Validator} for validating this setting
+     * @param properties   properties for this setting
+     */
+    public Setting(String key, String defaultValue, Function<String, T> parser, Validator<T> validator, Property... properties) {
+        this(new SimpleKey(key), s -> defaultValue, parser, validator, properties);
+    }
+
     /**
      * Creates a new Setting instance
      * @param key the settings key for this setting.
@@ -184,7 +216,7 @@ public class Setting<T> extends ToXContentToBytes {
      * @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, properties);
+        this(key, fallbackSetting, fallbackSetting::getRaw, parser, (v, m) -> {}, properties);
     }
 
     /**
@@ -307,9 +339,28 @@ public class Setting<T> extends ToXContentToBytes {
      * instead.
      */
     public T get(Settings settings) {
+        return get(settings, true);
+    }
+
+    private T get(Settings settings, boolean validate) {
         String value = getRaw(settings);
         try {
-            return parser.apply(value);
+            T parsed = parser.apply(value);
+            if (validate) {
+                final Iterator<Setting<T>> it = validator.settings();
+                final Map<Setting<T>, T> map;
+                if (it.hasNext()) {
+                    map = new HashMap<>();
+                    while (it.hasNext()) {
+                        final Setting<T> setting = it.next();
+                        map.put(setting, setting.get(settings, false)); // we have to disable validation or we will stack overflow
+                    }
+                } else {
+                    map = Collections.emptyMap();
+                }
+                validator.validate(parsed, map);
+            }
+            return parsed;
         } catch (ElasticsearchParseException ex) {
             throw new IllegalArgumentException(ex.getMessage(), ex);
         } catch (NumberFormatException ex) {
@@ -574,6 +625,33 @@ public class Setting<T> extends ToXContentToBytes {
         }
     }
 
+    /**
+     * 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.
+     *
+     * @param <T> the type of the {@link Setting}
+     */
+    @FunctionalInterface
+    public interface Validator<T> {
+
+        /**
+         * The validation routine for this validator.
+         *
+         * @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);
+
+        /**
+         * The settings needed by this validator.
+         *
+         * @return the settings needed to validate; these can be used for cross-settings validation
+         */
+        default Iterator<Setting<T>> settings() {
+            return Collections.emptyIterator();
+        }
+
+    }
 
     private final class Updater implements AbstractScopedSettings.SettingUpdater<T> {
         private final Consumer<T> consumer;
@@ -605,14 +683,14 @@ public class Setting<T> extends ToXContentToBytes {
         public T getValue(Settings current, Settings previous) {
             final String newValue = getRaw(current);
             final String value = getRaw(previous);
-            T inst = get(current);
             try {
+                T inst = get(current);
                 accept.accept(inst);
+                return inst;
             } catch (Exception | AssertionError e) {
                 throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + value + "] to [" + newValue + "]",
                         e);
             }
-            return inst;
         }
 
         @Override

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

@@ -35,6 +35,7 @@ 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;
@@ -46,7 +47,10 @@ import java.util.function.Function;
 
 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;
 
 public class ScopedSettingsTests extends ESTestCase {
 
@@ -253,6 +257,94 @@ 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);
 

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

@@ -27,6 +27,7 @@ import org.elasticsearch.test.ESTestCase;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
@@ -36,6 +37,8 @@ import java.util.stream.Stream;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 
 public class SettingTests extends ESTestCase {
@@ -64,8 +67,13 @@ public class SettingTests extends ESTestCase {
             settingUpdater.apply(Settings.builder().put("a.byte.size", 12).build(), Settings.EMPTY);
             fail("no unit");
         } catch (IllegalArgumentException ex) {
-            assertEquals("failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized",
+            assertThat(ex, hasToString(containsString("illegal value can't update [a.byte.size] from [2048b] to [12]")));
-                    ex.getMessage());
+            assertNotNull(ex.getCause());
+            assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
+            final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
+            final String expected =
+                    "failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized";
+            assertThat(cause, hasToString(containsString(expected)));
         }
 
         assertTrue(settingUpdater.apply(Settings.builder().put("a.byte.size", "12b").build(), Settings.EMPTY));
@@ -99,8 +107,13 @@ public class SettingTests extends ESTestCase {
             settingUpdater.apply(Settings.builder().put("a.byte.size", 12).build(), Settings.EMPTY);
             fail("no unit");
         } catch (IllegalArgumentException ex) {
-            assertEquals("failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized",
+            assertThat(ex, hasToString(containsString("illegal value can't update [a.byte.size] from [25%] to [12]")));
-                    ex.getMessage());
+            assertNotNull(ex.getCause());
+            assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
+            final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
+            final String expected =
+                    "failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized";
+            assertThat(cause, hasToString(containsString(expected)));
         }
 
         assertTrue(settingUpdater.apply(Settings.builder().put("a.byte.size", "12b").build(), Settings.EMPTY));
@@ -127,11 +140,58 @@ public class SettingTests extends ESTestCase {
             settingUpdater.apply(build, Settings.EMPTY);
             fail("not a boolean");
         } catch (IllegalArgumentException ex) {
-            assertEquals("Failed to parse value [I am not a boolean] as only [true] or [false] are allowed.",
+            assertThat(ex, hasToString(containsString("illegal value can't update [foo.bar] from [false] to [I am not a boolean]")));
-                    ex.getMessage());
+            assertNotNull(ex.getCause());
+            assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
+            final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
+            assertThat(
+                    cause,
+                    hasToString(containsString("Failed to parse value [I am not a boolean] as only [true] or [false] are allowed.")));
         }
     }
 
+    private static final Setting<String> FOO_BAR_SETTING = new Setting<>(
+            "foo.bar",
+            "foobar",
+            Function.identity(),
+            new FooBarValidator(),
+            Property.Dynamic,
+            Property.NodeScope);
+
+    private static final Setting<String> BAZ_QUX_SETTING = Setting.simpleString("baz.qux", Property.NodeScope);
+    private static final Setting<String> QUUX_QUUZ_SETTING = Setting.simpleString("quux.quuz", Property.NodeScope);
+
+    static class FooBarValidator implements Setting.Validator<String> {
+
+        public static boolean invoked;
+
+        @Override
+        public void validate(String value, Map<Setting<String>, String> settings) {
+            invoked = true;
+            assertThat(value, equalTo("foo.bar value"));
+            assertTrue(settings.keySet().contains(BAZ_QUX_SETTING));
+            assertThat(settings.get(BAZ_QUX_SETTING), equalTo("baz.qux value"));
+            assertTrue(settings.keySet().contains(QUUX_QUUZ_SETTING));
+            assertThat(settings.get(QUUX_QUUZ_SETTING), equalTo("quux.quuz value"));
+        }
+
+        @Override
+        public Iterator<Setting<String>> settings() {
+            return Arrays.asList(BAZ_QUX_SETTING, QUUX_QUUZ_SETTING).iterator();
+        }
+    }
+
+    // the purpose of this test is merely to ensure that a validator is invoked with the appropriate values
+    public void testValidator() {
+        final Settings settings = Settings.builder()
+                .put("foo.bar", "foo.bar value")
+                .put("baz.qux", "baz.qux value")
+                .put("quux.quuz", "quux.quuz value")
+                .build();
+        FOO_BAR_SETTING.get(settings);
+        assertTrue(FooBarValidator.invoked);
+    }
+
     public void testUpdateNotDynamic() {
         Setting<Boolean> booleanSetting = Setting.boolSetting("foo.bar", false, Property.NodeScope);
         assertFalse(booleanSetting.isGroupSetting());

core/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java

@@ -35,6 +35,8 @@ import org.elasticsearch.test.ESTestCase;
 import java.io.IOException;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.startsWith;
 
@@ -89,7 +91,12 @@ public class IndexingSlowLogTests extends ESTestCase {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_REFORMAT_SETTING.getKey(), "NOT A BOOLEAN").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "Failed to parse value [NOT A BOOLEAN] as only [true] or [false] are allowed.");
+            final String expected = "illegal value can't update [index.indexing.slowlog.reformat] from [true] to [NOT A BOOLEAN]";
+            assertThat(ex, hasToString(containsString(expected)));
+            assertNotNull(ex.getCause());
+            assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
+            final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
+            assertThat(cause, hasToString(containsString("Failed to parse value [NOT A BOOLEAN] as only [true] or [false] are allowed.")));
         }
         assertTrue(log.isReformat());
     }
@@ -127,7 +134,12 @@ public class IndexingSlowLogTests extends ESTestCase {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_LEVEL_SETTING.getKey(), "NOT A LEVEL").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "No enum constant org.elasticsearch.index.SlowLogLevel.NOT A LEVEL");
+            final String expected = "illegal value can't update [index.indexing.slowlog.level] from [TRACE] to [NOT A LEVEL]";
+            assertThat(ex, hasToString(containsString(expected)));
+            assertNotNull(ex.getCause());
+            assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
+            final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
+            assertThat(cause, hasToString(containsString("No enum constant org.elasticsearch.index.SlowLogLevel.NOT A LEVEL")));
         }
         assertEquals(SlowLogLevel.TRACE, log.getLevel());
     }
@@ -178,31 +190,42 @@ public class IndexingSlowLogTests extends ESTestCase {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_THRESHOLD_INDEX_TRACE_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.indexing.slowlog.threshold.index.trace] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.indexing.slowlog.threshold.index.trace");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_THRESHOLD_INDEX_DEBUG_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.indexing.slowlog.threshold.index.debug] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.indexing.slowlog.threshold.index.debug");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_THRESHOLD_INDEX_INFO_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.indexing.slowlog.threshold.index.info] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.indexing.slowlog.threshold.index.info");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_THRESHOLD_INDEX_WARN_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.indexing.slowlog.threshold.index.warn] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.indexing.slowlog.threshold.index.warn");
         }
     }
 
+    private void assertTimeValueException(final IllegalArgumentException e, final String key) {
+        final String expected = "illegal value can't update [" + key + "] from [-1] to [NOT A TIME VALUE]";
+        assertThat(e, hasToString(containsString(expected)));
+        assertNotNull(e.getCause());
+        assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+        final IllegalArgumentException cause = (IllegalArgumentException) e.getCause();
+        final String causeExpected =
+                "failed to parse setting [" + key + "] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized";
+        assertThat(cause, hasToString(containsString(causeExpected)));
+    }
+
     private IndexMetaData newIndexMeta(String name, Settings indexSettings) {
         Settings build = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
             .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)

core/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java

@@ -40,6 +40,8 @@ import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.IOException;
 
+import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.startsWith;
@@ -176,7 +178,12 @@ public class SearchSlowLogTests extends ESSingleNodeTestCase {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_LEVEL.getKey(), "NOT A LEVEL").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "No enum constant org.elasticsearch.index.SlowLogLevel.NOT A LEVEL");
+            final String expected = "illegal value can't update [index.search.slowlog.level] from [TRACE] to [NOT A LEVEL]";
+            assertThat(ex, hasToString(containsString(expected)));
+            assertNotNull(ex.getCause());
+            assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
+            final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
+            assertThat(cause, hasToString(containsString("No enum constant org.elasticsearch.index.SlowLogLevel.NOT A LEVEL")));
         }
         assertEquals(SlowLogLevel.TRACE, log.getLevel());
     }
@@ -227,28 +234,28 @@ public class SearchSlowLogTests extends ESSingleNodeTestCase {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_TRACE_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.query.trace] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.query.trace");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_DEBUG_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.query.debug] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.query.debug");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_INFO_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.query.info] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.query.info");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_WARN_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.query.warn] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.query.warn");
         }
     }
 
@@ -298,31 +305,41 @@ public class SearchSlowLogTests extends ESSingleNodeTestCase {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_FETCH_TRACE_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.fetch.trace] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.fetch.trace");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_FETCH_DEBUG_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.fetch.debug] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.fetch.debug");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_FETCH_INFO_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.fetch.info] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.fetch.info");
         }
 
         try {
             settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_FETCH_WARN_SETTING.getKey(), "NOT A TIME VALUE").build()));
             fail();
         } catch (IllegalArgumentException ex) {
-            assertEquals(ex.getMessage(), "failed to parse setting [index.search.slowlog.threshold.fetch.warn] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized");
+            assertTimeValueException(ex, "index.search.slowlog.threshold.fetch.warn");
         }
     }
 
+    private void assertTimeValueException(final IllegalArgumentException e, final String key) {
+        final String expected = "illegal value can't update [" + key + "] from [-1] to [NOT A TIME VALUE]";
+        assertThat(e, hasToString(containsString(expected)));
+        assertNotNull(e.getCause());
+        assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+        final IllegalArgumentException cause = (IllegalArgumentException) e.getCause();
+        final String causeExpected =
+                "failed to parse setting [" + key + "] with value [NOT A TIME VALUE] as a time value: unit is missing or unrecognized";
+        assertThat(cause, hasToString(containsString(causeExpected)));
+    }
 
     private IndexMetaData newIndexMeta(String name, Settings indexSettings) {
         Settings build = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)