Fix fallback setting for two get/2

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

@@ -18,11 +18,24 @@
  */
 package org.elasticsearch.common.settings;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.action.support.ToXContentToBytes;
 import org.elasticsearch.common.Booleans;
+import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.logging.DeprecationLogger;
@@ -37,22 +50,6 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 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.Enumeration;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.function.BiConsumer;
-import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.function.Predicate;
-import java.util.regex.Pattern;
-import java.util.stream.Collectors;
-
 /**
  * A setting. Encapsulates typical stuff like default value, parsing, and scope.
  * Some (SettingsProperty.Dynamic) can by modified at run time using the API.
@@ -110,21 +107,18 @@ public class Setting<T> extends ToXContentToBytes {
 
     private final Key key;
     protected final Function<Settings, String> defaultValue;
+    @Nullable
+    private final Setting<T> fallbackSetting;
     private final Function<String, T> parser;
     private final EnumSet<Property> properties;
 
     private static final EnumSet<Property> EMPTY_PROPERTIES = EnumSet.noneOf(Property.class);
 
-    /**
-     * Creates a new Setting instance. When no scope is provided, we default to {@link Property#NodeScope}.
-     * @param key the settings key for this setting.
-     * @param defaultValue a default value function that returns the default values string representation.
-     * @param parser a parser that parses the string rep into a complex datatype.
-     * @param properties properties for this setting like scope, filtering...
-     */
-    public Setting(Key key, Function<Settings, String> defaultValue, Function<String, T> parser, Property... properties) {
+    private Setting(Key key, @Nullable Setting<T> fallbackSetting, Function<Settings, String> defaultValue, Function<String, T> parser,
+            Property... properties) {
         assert parser.apply(defaultValue.apply(Settings.EMPTY)) != null || this.isGroupSetting(): "parser returned null";
         this.key = key;
+        this.fallbackSetting = fallbackSetting;
         this.defaultValue = defaultValue;
         this.parser = parser;
         if (properties == null) {
@@ -137,6 +131,17 @@ public class Setting<T> extends ToXContentToBytes {
         }
     }
 
+    /**
+     * Creates a new Setting instance. When no scope is provided, we default to {@link Property#NodeScope}.
+     * @param key the settings key for this setting.
+     * @param defaultValue a default value function that returns the default values string representation.
+     * @param parser a parser that parses the string rep into a complex datatype.
+     * @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);
+    }
+
     /**
      * Creates a new Setting instance
      * @param key the settings key for this setting.
@@ -159,6 +164,17 @@ public class Setting<T> extends ToXContentToBytes {
         this(new SimpleKey(key), defaultValue, parser, properties);
     }
 
+    /**
+     * Creates a new Setting instance. When no scope is provided, we default to {@link Property#NodeScope}.
+     * @param key the settings key for this setting.
+     * @param fallbackSetting a setting who's value to fallback on if this setting is not defined
+     * @param parser a parser that parses the string rep into a complex datatype.
+     * @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);
+    }
+
     /**
      * Creates a new Setting instance
      * @param key the settings key for this setting.
@@ -167,7 +183,7 @@ public class Setting<T> extends ToXContentToBytes {
      * @param properties properties for this setting like scope, filtering...
      */
     public Setting(String key, Setting<T> fallBackSetting, Function<String, T> parser, Property... properties) {
-        this(key, fallBackSetting::getRaw, parser, properties);
+        this(new SimpleKey(key), fallBackSetting, parser, properties);
     }
 
     /**
@@ -327,7 +343,16 @@ public class Setting<T> extends ToXContentToBytes {
         if (exists(primary)) {
             return get(primary);
         }
-        return get(secondary);
+        if (fallbackSetting == null) {
+            return get(secondary); 
+        }
+        if (exists(secondary)) {
+            return get(secondary);
+        }
+        if (fallbackSetting.exists(primary)) {
+            return fallbackSetting.get(primary);
+        }
+        return fallbackSetting.get(secondary);
     }
 
     public Setting<T> getConcreteSetting(String key) {
@@ -517,9 +542,9 @@ public class Setting<T> extends ToXContentToBytes {
         return byteSizeSetting(key, (s) -> value.toString(), properties);
     }
 
-    public static Setting<ByteSizeValue> byteSizeSetting(String key, Setting<ByteSizeValue> fallbackSettings,
+    public static Setting<ByteSizeValue> byteSizeSetting(String key, Setting<ByteSizeValue> fallbackSetting,
                                                          Property... properties) {
-        return byteSizeSetting(key, fallbackSettings::getRaw, properties);
+        return new Setting<>(key, fallbackSetting, (s) -> ByteSizeValue.parseBytesSizeValue(s, key), properties);
     }
 
     public static Setting<ByteSizeValue> byteSizeSetting(String key, Function<Settings, String> defaultValue,
@@ -558,6 +583,7 @@ public class Setting<T> extends ToXContentToBytes {
         return listSetting(key, (s) -> defaultStringValue, singleValueParser, properties);
     }
 
+    // TODO this one's two argument get is still broken
     public static <T> Setting<List<T>> listSetting(String key, Setting<List<T>> fallbackSetting, Function<String, T> singleValueParser,
                                                    Property... properties) {
         return listSetting(key, (s) -> parseableStringToList(fallbackSetting.getRaw(s)), singleValueParser, properties);
@@ -720,7 +746,7 @@ public class Setting<T> extends ToXContentToBytes {
     }
 
     public static Setting<TimeValue> timeSetting(String key, Setting<TimeValue> fallbackSetting, Property... properties) {
-        return new Setting<>(key, fallbackSetting::getRaw, (s) -> TimeValue.parseTimeValue(s, key), properties);
+        return new Setting<>(key, fallbackSetting, (s) -> TimeValue.parseTimeValue(s, key), properties);
     }
 
     public static Setting<Double> doubleSetting(String key, double defaultValue, double minValue, Property... properties) {

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

@@ -113,23 +113,37 @@ public class SettingTests extends ESTestCase {
     }
 
     public void testDefault() {
-        TimeValue defautlValue = TimeValue.timeValueMillis(randomIntBetween(0, 1000000));
+        TimeValue defaultValue = TimeValue.timeValueMillis(randomIntBetween(0, 1000000));
         Setting<TimeValue> setting =
-            Setting.positiveTimeSetting("my.time.value", defautlValue, Property.NodeScope);
+            Setting.positiveTimeSetting("my.time.value", defaultValue, Property.NodeScope);
         assertFalse(setting.isGroupSetting());
         String aDefault = setting.getDefaultRaw(Settings.EMPTY);
-        assertEquals(defautlValue.millis() + "ms", aDefault);
-        assertEquals(defautlValue.millis(), setting.get(Settings.EMPTY).millis());
-        assertEquals(defautlValue, setting.getDefault(Settings.EMPTY));
+        assertEquals(defaultValue.millis() + "ms", aDefault);
+        assertEquals(defaultValue.millis(), setting.get(Settings.EMPTY).millis());
+        assertEquals(defaultValue, setting.getDefault(Settings.EMPTY));
 
         Setting<String> secondaryDefault =
             new Setting<>("foo.bar", (s) -> s.get("old.foo.bar", "some_default"), Function.identity(), Property.NodeScope);
         assertEquals("some_default", secondaryDefault.get(Settings.EMPTY));
         assertEquals("42", secondaryDefault.get(Settings.builder().put("old.foo.bar", 42).build()));
+
         Setting<String> secondaryDefaultViaSettings =
             new Setting<>("foo.bar", secondaryDefault, Function.identity(), Property.NodeScope);
         assertEquals("some_default", secondaryDefaultViaSettings.get(Settings.EMPTY));
         assertEquals("42", secondaryDefaultViaSettings.get(Settings.builder().put("old.foo.bar", 42).build()));
+
+        // It gets more complicated when there are two settings objects....
+        Settings hasFallback = Settings.builder().put("foo.bar", "o").build();
+        Setting<String> fallsback =
+                new Setting<>("foo.baz", secondaryDefault, Function.identity(), Property.NodeScope);
+        assertEquals("o", fallsback.get(hasFallback));
+        assertEquals("some_default", fallsback.get(Settings.EMPTY));
+        assertEquals("some_default", fallsback.get(Settings.EMPTY, Settings.EMPTY));
+        assertEquals("o", fallsback.get(Settings.EMPTY, hasFallback));
+        assertEquals("o", fallsback.get(hasFallback, Settings.EMPTY));
+        assertEquals("a", fallsback.get(
+                Settings.builder().put("foo.bar", "a").build(),
+                Settings.builder().put("foo.bar", "b").build()));
     }
 
     public void testComplexType() {

core/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java

@@ -378,7 +378,19 @@ public class SimpleSearchIT extends ESIntegTestCase {
         indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
 
         assertHitCount(
-                client().prepareSearch("idx").addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(defaultMaxWindow)).get(), 1);
+                client().prepareSearch("idx").addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(defaultMaxWindow + 1)).get(),
+                1);
+    }
+
+    public void testTooLargeRescoreOkByResultWindowSetting() throws Exception {
+        int defaultMaxWindow = IndexSettings.MAX_RESCORE_WINDOW_SETTING.get(Settings.EMPTY);
+        prepareCreate("idx").setSettings(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(), // Note that this is the RESULT window.
+                defaultMaxWindow * 2).get();
+        indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
+
+        assertHitCount(
+                client().prepareSearch("idx").addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(defaultMaxWindow + 1)).get(),
+                1);
     }
 
     public void testTooLargeRescoreOkByDynamicSetting() throws Exception {
@@ -386,16 +398,29 @@ public class SimpleSearchIT extends ESIntegTestCase {
         createIndex("idx");
         assertAcked(client().admin().indices().prepareUpdateSettings("idx")
                 .setSettings(
-                        Settings.builder().put(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(), defaultMaxWindow * 2))
+                        Settings.builder().put(IndexSettings.MAX_RESCORE_WINDOW_SETTING.getKey(), defaultMaxWindow * 2))
                 .get());
         indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
 
         assertHitCount(
-                client().prepareSearch("idx").addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(defaultMaxWindow)).get(), 1);
+                client().prepareSearch("idx").addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(defaultMaxWindow + 1)).get(),
+                1);
     }
 
-    // DODO assertRescoreWindowFails
+    public void testTooLargeRescoreOkByDynamicResultWindowSetting() throws Exception {
+        int defaultMaxWindow = IndexSettings.MAX_RESCORE_WINDOW_SETTING.get(Settings.EMPTY);
+        createIndex("idx");
+        assertAcked(client().admin().indices().prepareUpdateSettings("idx")
+                .setSettings(
+                        // Note that this is the RESULT window
+                        Settings.builder().put(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(), defaultMaxWindow * 2))
+                .get());
+        indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
 
+        assertHitCount(
+                client().prepareSearch("idx").addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(defaultMaxWindow + 1)).get(),
+                1);
+    }
     public void testQueryNumericFieldWithRegex() throws Exception {
         assertAcked(prepareCreate("idx").addMapping("type", "num", "type=integer"));
         ensureGreen("idx");

docs/reference/index-modules.asciidoc

@@ -107,6 +107,13 @@ specific index module:
     <<search-request-scroll,Scroll>> or <<search-request-search-after,Search After>> for a more efficient alternative
     to raising this.
 
+`index.max_rescore_window`::
+
+    The maximum value of `window_size` for `rescore`s in searches of this index.
+    Defaults to `index.max_result_window` which defaults to `10000`. Search
+    requests take heap memory and time proportional to
+    `max(window_size, from + size)` and this limits that memory.
+
 `index.blocks.read_only`::
 
     Set to `true` to make the index and index metadata read only, `false` to