Browse Source

Fail request if rescore window > 10,000

The setting is named `index.max_rescore_window` and defaults to
`index.max_result_window` which defaults to 10,000.
Nik Everett 9 years ago
parent
commit
1c2e84ba46

+ 1 - 0
core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

@@ -109,6 +109,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
         IndexSettings.INDEX_WARMER_ENABLED_SETTING,
         IndexSettings.INDEX_REFRESH_INTERVAL_SETTING,
         IndexSettings.MAX_RESULT_WINDOW_SETTING,
+        IndexSettings.MAX_RESCORE_WINDOW_SETTING,
         IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING,
         IndexSettings.DEFAULT_FIELD_SETTING,
         IndexSettings.QUERY_STRING_LENIENT_SETTING,

+ 4 - 0
core/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -454,6 +454,10 @@ public class Setting<T> extends ToXContentToBytes {
         return new Setting<>(key, (s) -> Integer.toString(defaultValue), (s) -> parseInt(s, minValue, key), properties);
     }
 
+    public static Setting<Integer> intSetting(String key, Setting<Integer> fallbackSetting, int minValue, Property... properties) {
+        return new Setting<>(key, fallbackSetting, (s) -> parseInt(s, minValue, key), properties);
+    }
+
     public static Setting<Long> longSetting(String key, long defaultValue, long minValue, Property... properties) {
         return new Setting<>(key, (s) -> Long.toString(defaultValue), (s) -> parseLong(s, minValue, key), properties);
     }

+ 19 - 0
core/src/main/java/org/elasticsearch/index/IndexSettings.java

@@ -92,6 +92,12 @@ public final class IndexSettings {
      */
     public static final Setting<Integer> MAX_RESULT_WINDOW_SETTING =
         Setting.intSetting("index.max_result_window", 10000, 1, Property.Dynamic, Property.IndexScope);
+    /**
+     * Index setting describing the maximum size of the rescore window. Defaults to {@link #MAX_RESULT_WINDOW_SETTING}
+     * because they both do the same thing: control the size of the heap of hits.
+     */
+    public static final Setting<Integer> MAX_RESCORE_WINDOW_SETTING =
+            Setting.intSetting("index.max_rescore_window", MAX_RESULT_WINDOW_SETTING, 1, Property.Dynamic, Property.IndexScope);
     public static final TimeValue DEFAULT_REFRESH_INTERVAL = new TimeValue(1, TimeUnit.SECONDS);
     public static final Setting<TimeValue> INDEX_REFRESH_INTERVAL_SETTING =
         Setting.timeSetting("index.refresh_interval", DEFAULT_REFRESH_INTERVAL, new TimeValue(-1, TimeUnit.MILLISECONDS),
@@ -137,6 +143,7 @@ public final class IndexSettings {
     private long gcDeletesInMillis = DEFAULT_GC_DELETES.millis();
     private volatile boolean warmerEnabled;
     private volatile int maxResultWindow;
+    private volatile int maxRescoreWindow;
     private volatile boolean TTLPurgeDisabled;
 
     /**
@@ -220,6 +227,7 @@ public final class IndexSettings {
         gcDeletesInMillis = scopedSettings.get(INDEX_GC_DELETES_SETTING).getMillis();
         warmerEnabled = scopedSettings.get(INDEX_WARMER_ENABLED_SETTING);
         maxResultWindow = scopedSettings.get(MAX_RESULT_WINDOW_SETTING);
+        maxRescoreWindow = scopedSettings.get(MAX_RESCORE_WINDOW_SETTING);
         TTLPurgeDisabled = scopedSettings.get(INDEX_TTL_DISABLE_PURGE_SETTING);
         this.mergePolicyConfig = new MergePolicyConfig(logger, this);
         assert indexNameMatcher.test(indexMetaData.getIndex().getName());
@@ -238,6 +246,7 @@ public final class IndexSettings {
         scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_DURABILITY_SETTING, this::setTranslogDurability);
         scopedSettings.addSettingsUpdateConsumer(INDEX_TTL_DISABLE_PURGE_SETTING, this::setTTLPurgeDisabled);
         scopedSettings.addSettingsUpdateConsumer(MAX_RESULT_WINDOW_SETTING, this::setMaxResultWindow);
+        scopedSettings.addSettingsUpdateConsumer(MAX_RESCORE_WINDOW_SETTING, this::setMaxRescoreWindow);
         scopedSettings.addSettingsUpdateConsumer(INDEX_WARMER_ENABLED_SETTING, this::setEnableWarmer);
         scopedSettings.addSettingsUpdateConsumer(INDEX_GC_DELETES_SETTING, this::setGCDeletes);
         scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING, this::setTranslogFlushThresholdSize);
@@ -449,6 +458,16 @@ public final class IndexSettings {
         this.maxResultWindow = maxResultWindow;
     }
 
+    /**
+     * Returns the maximum rescore window for search requests.
+     */
+    public int getMaxRescoreWindow() {
+        return maxRescoreWindow;
+    }
+
+    private void setMaxRescoreWindow(int maxRescoreWindow) {
+        this.maxRescoreWindow = maxRescoreWindow;
+    }
 
     /**
      * Returns the GC deletes cycle in milliseconds.

+ 13 - 3
core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java

@@ -198,8 +198,6 @@ public class DefaultSearchContext extends SearchContext {
             long from = from() == -1 ? 0 : from();
             long size = size() == -1 ? 10 : size();
             long resultWindow = from + size;
-            // We need settingsService's view of the settings because its dynamic.
-            // indexService's isn't.
             int maxResultWindow = indexService.getIndexSettings().getMaxResultWindow();
 
             if (resultWindow > maxResultWindow) {
@@ -207,7 +205,19 @@ public class DefaultSearchContext extends SearchContext {
                         "Result window is too large, from + size must be less than or equal to: [" + maxResultWindow + "] but was ["
                                 + resultWindow + "]. See the scroll api for a more efficient way to request large data sets. "
                                 + "This limit can be set by changing the [" + IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey()
-                                + "] index level parameter.");
+                                + "] index level setting.");
+            }
+        }
+        if (rescore != null) {
+            int maxWindow = indexService.getIndexSettings().getMaxRescoreWindow();
+            for (RescoreSearchContext rescoreContext: rescore) {
+                if (rescoreContext.window() > maxWindow) {
+                    throw new QueryPhaseExecutionException(this, "Rescore window [" + rescoreContext.window() + "] is too large. It must "
+                            + "be less than [" + maxWindow + "]. This prevents allocating massive heaps for storing the results to be "
+                            + "rescored. This limit can be set by chaning the [" + IndexSettings.MAX_RESCORE_WINDOW_SETTING.getKey()
+                            + "] index level setting.");
+                
+                }
             }
         }
 

+ 59 - 16
core/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java

@@ -19,6 +19,10 @@
 
 package org.elasticsearch.search.simple;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+
 import org.apache.lucene.util.Constants;
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
@@ -29,17 +33,14 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.search.rescore.QueryRescorerBuilder;
 import org.elasticsearch.test.ESIntegTestCase;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-
-import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
+import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery;
 import static org.elasticsearch.index.query.QueryBuilders.rangeQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
@@ -47,7 +48,8 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFail
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
+
+import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
 
 public class SimpleSearchIT extends ESIntegTestCase {
     public void testSearchNullIndex() {
@@ -326,7 +328,8 @@ public class SimpleSearchIT extends ESIntegTestCase {
     }
 
     public void testTooLargeFromAndSizeOkBySetting() throws Exception {
-        prepareCreate("idx").setSettings(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(), IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY) * 2).get();
+        prepareCreate("idx").setSettings(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(),
+                IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY) * 2).get();
         indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
 
         assertHitCount(client().prepareSearch("idx").setFrom(IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY)).get(), 1);
@@ -339,7 +342,8 @@ public class SimpleSearchIT extends ESIntegTestCase {
         createIndex("idx");
         assertAcked(client().admin().indices().prepareUpdateSettings("idx")
                 .setSettings(
-                        Settings.builder().put(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(), IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY) * 2))
+                        Settings.builder().put(IndexSettings.MAX_RESULT_WINDOW_SETTING.getKey(),
+                                IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY) * 2))
                 .get());
         indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
 
@@ -359,6 +363,39 @@ public class SimpleSearchIT extends ESIntegTestCase {
                 .setFrom(IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY) * 10).get(), 1);
     }
 
+    public void testTooLargeRescoreWindow() throws Exception {
+        createIndex("idx");
+        indexRandom(true, client().prepareIndex("idx", "type").setSource("{}"));
+
+        assertRescoreWindowFails(Integer.MAX_VALUE);
+        assertRescoreWindowFails(IndexSettings.MAX_RESCORE_WINDOW_SETTING.get(Settings.EMPTY) + 1);
+    }
+
+    public void testTooLargeRescoreOkBySetting() throws Exception {
+        int defaultMaxWindow = IndexSettings.MAX_RESCORE_WINDOW_SETTING.get(Settings.EMPTY);
+        prepareCreate("idx").setSettings(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);
+    }
+
+    public void testTooLargeRescoreOkByDynamicSetting() throws Exception {
+        int defaultMaxWindow = IndexSettings.MAX_RESCORE_WINDOW_SETTING.get(Settings.EMPTY);
+        createIndex("idx");
+        assertAcked(client().admin().indices().prepareUpdateSettings("idx")
+                .setSettings(
+                        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)).get(), 1);
+    }
+
+    // DODO assertRescoreWindowFails
+
     public void testQueryNumericFieldWithRegex() throws Exception {
         assertAcked(prepareCreate("idx").addMapping("type", "num", "type=integer"));
         ensureGreen("idx");
@@ -372,13 +409,19 @@ public class SimpleSearchIT extends ESIntegTestCase {
     }
 
     private void assertWindowFails(SearchRequestBuilder search) {
-        try {
-            search.get();
-            fail();
-        } catch (SearchPhaseExecutionException e) {
-            assertThat(e.toString(), containsString("Result window is too large, from + size must be less than or equal to: ["
-                    + IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY)));
-            assertThat(e.toString(), containsString("See the scroll api for a more efficient way to request large data sets"));
-        }
+        SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () -> search.get());
+        assertThat(e.toString(), containsString("Result window is too large, from + size must be less than or equal to: ["
+                + IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY)));
+        assertThat(e.toString(), containsString("See the scroll api for a more efficient way to request large data sets"));
+    }
+
+    private void assertRescoreWindowFails(int windowSize) {
+        SearchRequestBuilder search = client().prepareSearch("idx")
+                .addRescorer(new QueryRescorerBuilder(matchAllQuery()).windowSize(windowSize));
+        SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () -> search.get());
+        assertThat(e.toString(), containsString("Rescore window [" + windowSize + "] is too large. It must "
+                + "be less than [" + IndexSettings.MAX_RESCORE_WINDOW_SETTING.get(Settings.EMPTY)));
+        assertThat(e.toString(), containsString(
+                "This limit can be set by chaning the [" + IndexSettings.MAX_RESCORE_WINDOW_SETTING.getKey() + "] index level setting."));
     }
 }

+ 35 - 0
rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yaml

@@ -0,0 +1,35 @@
+setup:
+  - do:
+      index:
+          index:  test_1
+          type:   test
+          id:     1
+          body:   { foo: bar }
+
+  - do:
+      indices.refresh: {}
+
+---
+"Request window limits":
+  - do:
+      catch:      /Result window is too large, from \+ size must be less than or equal to[:] \[10000\] but was \[10010\]/
+      search:
+        index: test_1
+        from: 10000
+
+---
+"Rescore window limits":
+  - do:
+      catch:      /Rescore window \[10001\] is too large\. It must be less than \[10000\]\./
+      search:
+        index: test_1
+        body:
+          query:
+            match_all: {}
+          rescore:
+              - window_size: 10001
+                query:
+                  rescore_query:
+                    match_all: {}
+                  query_weight: 1
+                  rescore_query_weight: 2