From 0e2dc290c16f481d8621e22dcf66b7e8e2d4b67c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 13 Sep 2017 17:22:16 +0200 Subject: [PATCH] Add soft limit on allowed number of script fields in request (#26598) Requesting to many script_fields in a search request can be costly because of script execution. This change introduces a soft limit on the number of script fields that are allowed per request. The setting can be changed per index using the index.max_script_fields setting. Relates to #26390 --- .../common/settings/IndexScopedSettings.java | 1 + .../elasticsearch/index/IndexSettings.java | 23 ++++++++ .../elasticsearch/search/SearchService.java | 7 +++ .../index/IndexSettingsTests.java | 16 ++++++ .../search/SearchServiceTests.java | 54 ++++++++++++++++++- docs/reference/index-modules.asciidoc | 5 ++ .../migration/migrate_6_0/search.asciidoc | 5 ++ .../rest-api-spec/test/search/30_limits.yml | 26 +++++++++ 8 files changed, 136 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 4454457b83261..43ac6365a6133 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -111,6 +111,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { IndexSettings.INDEX_REFRESH_INTERVAL_SETTING, IndexSettings.MAX_RESULT_WINDOW_SETTING, IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING, + IndexSettings.MAX_SCRIPT_FIELDS_SETTING, IndexSettings.MAX_RESCORE_WINDOW_SETTING, IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING, IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING, diff --git a/core/src/main/java/org/elasticsearch/index/IndexSettings.java b/core/src/main/java/org/elasticsearch/index/IndexSettings.java index 1382254c881a3..00648cdfb4e2d 100644 --- a/core/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/core/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -102,6 +102,15 @@ public final class IndexSettings { */ public static final Setting MAX_RESULT_WINDOW_SETTING = Setting.intSetting("index.max_result_window", 10000, 1, Property.Dynamic, Property.IndexScope); + + /** + * Index setting describing the maximum value of allowed `script_fields`that can be retrieved + * per search request. The default maximum of 32 is defensive for the reason that retrieving + * script fields is a costly operation. + */ + public static final Setting MAX_SCRIPT_FIELDS_SETTING = + Setting.intSetting("index.max_script_fields", 32, 0, Property.Dynamic, Property.IndexScope); + /** * Index setting describing the maximum value of allowed `docvalue_fields`that can be retrieved * per search request. The default maximum of 100 is defensive for the reason that retrieving @@ -232,6 +241,7 @@ public final class IndexSettings { private volatile int maxAdjacencyMatrixFilters; private volatile int maxRescoreWindow; private volatile int maxDocvalueFields; + private volatile int maxScriptFields; private volatile boolean TTLPurgeDisabled; /** * The maximum number of refresh listeners allows on this shard. @@ -329,6 +339,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti maxAdjacencyMatrixFilters = scopedSettings.get(MAX_ADJACENCY_MATRIX_FILTERS_SETTING); maxRescoreWindow = scopedSettings.get(MAX_RESCORE_WINDOW_SETTING); maxDocvalueFields = scopedSettings.get(MAX_DOCVALUE_FIELDS_SEARCH_SETTING); + maxScriptFields = scopedSettings.get(MAX_SCRIPT_FIELDS_SETTING); TTLPurgeDisabled = scopedSettings.get(INDEX_TTL_DISABLE_PURGE_SETTING); maxRefreshListeners = scopedSettings.get(MAX_REFRESH_LISTENERS_PER_SHARD); maxSlicesPerScroll = scopedSettings.get(MAX_SLICES_PER_SCROLL); @@ -358,6 +369,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti scopedSettings.addSettingsUpdateConsumer(MAX_ADJACENCY_MATRIX_FILTERS_SETTING, this::setMaxAdjacencyMatrixFilters); scopedSettings.addSettingsUpdateConsumer(MAX_RESCORE_WINDOW_SETTING, this::setMaxRescoreWindow); scopedSettings.addSettingsUpdateConsumer(MAX_DOCVALUE_FIELDS_SEARCH_SETTING, this::setMaxDocvalueFields); + scopedSettings.addSettingsUpdateConsumer(MAX_SCRIPT_FIELDS_SETTING, this::setMaxScriptFields); 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); @@ -613,6 +625,17 @@ private void setMaxDocvalueFields(int maxDocvalueFields) { this.maxDocvalueFields = maxDocvalueFields; } + /** + * Returns the maximum number of allowed script_fields to retrieve in a search request + */ + public int getMaxScriptFields() { + return this.maxScriptFields; + } + + private void setMaxScriptFields(int maxScriptFields) { + this.maxScriptFields = maxScriptFields; + } + /** * Returns the GC deletes cycle in milliseconds. */ diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index abcd2c5179643..0fb392d794808 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -755,6 +755,13 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc } } if (source.scriptFields() != null) { + int maxAllowedScriptFields = context.mapperService().getIndexSettings().getMaxScriptFields(); + if (source.scriptFields().size() > maxAllowedScriptFields) { + throw new IllegalArgumentException( + "Trying to retrieve too many script_fields. Must be less than or equal to: [" + maxAllowedScriptFields + + "] but was [" + source.scriptFields().size() + "]. This limit can be set by changing the [" + + IndexSettings.MAX_SCRIPT_FIELDS_SETTING.getKey() + "] index level setting."); + } for (org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField field : source.scriptFields()) { SearchScript.Factory factory = scriptService.compile(field.script(), SearchScript.CONTEXT); SearchScript.LeafFactory searchScript = factory.newFactory(field.script().getParams(), context.lookup()); diff --git a/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java b/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java index f3e39a3a5a43e..edb89bee5ac40 100644 --- a/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java @@ -305,6 +305,22 @@ public void testMaxDocvalueFields() { assertEquals(IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxDocvalueFields()); } + public void testMaxScriptFields() { + IndexMetaData metaData = newIndexMeta("index", Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexSettings.MAX_SCRIPT_FIELDS_SETTING.getKey(), 100).build()); + IndexSettings settings = new IndexSettings(metaData, Settings.EMPTY); + assertEquals(100, settings.getMaxScriptFields()); + settings.updateIndexMetaData( + newIndexMeta("index", Settings.builder().put(IndexSettings.MAX_SCRIPT_FIELDS_SETTING.getKey(), 20).build())); + assertEquals(20, settings.getMaxScriptFields()); + settings.updateIndexMetaData(newIndexMeta("index", Settings.EMPTY)); + assertEquals(IndexSettings.MAX_SCRIPT_FIELDS_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxScriptFields()); + + metaData = newIndexMeta("index", Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build()); + settings = new IndexSettings(metaData, Settings.EMPTY); + assertEquals(IndexSettings.MAX_SCRIPT_FIELDS_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxScriptFields()); + } + public void testMaxAdjacencyMatrixFiltersSetting() { IndexMetaData metaData = newIndexMeta("index", Settings.builder() .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) diff --git a/core/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/core/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 10af4b333ea8b..5d166aaa628ba 100644 --- a/core/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/core/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -47,6 +47,10 @@ import org.elasticsearch.indices.IndicesService; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.script.MockScriptEngine; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.aggregations.bucket.global.GlobalAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; @@ -60,11 +64,14 @@ import java.io.IOException; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; import static java.util.Collections.singletonList; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; @@ -83,7 +90,19 @@ protected boolean resetNodeAfterTest() { @Override protected Collection> getPlugins() { - return pluginList(FailOnRewriteQueryPlugin.class); + return pluginList(FailOnRewriteQueryPlugin.class, CustomScriptPlugin.class); + } + + public static class CustomScriptPlugin extends MockScriptPlugin { + + static final String DUMMY_SCRIPT = "dummyScript"; + + @Override + protected Map, Object>> pluginScripts() { + return Collections.singletonMap(DUMMY_SCRIPT, vars -> { + return "dummy"; + }); + } } @Override @@ -290,6 +309,39 @@ searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_A } } + /** + * test that getting more than the allowed number of script_fields throws an exception + */ + public void testMaxScriptFieldsSearch() throws IOException { + createIndex("index"); + final SearchService service = getInstanceFromNode(SearchService.class); + final IndicesService indicesService = getInstanceFromNode(IndicesService.class); + final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index")); + final IndexShard indexShard = indexService.getShard(0); + + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + // adding the maximum allowed number of script_fields to retrieve + int maxScriptFields = indexService.getIndexSettings().getMaxScriptFields(); + for (int i = 0; i < maxScriptFields; i++) { + searchSourceBuilder.scriptField("field" + i, + new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); + } + try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f), null)) { + assertNotNull(context); + searchSourceBuilder.scriptField("anotherScriptField", + new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f), null)); + assertEquals( + "Trying to retrieve too many script_fields. Must be less than or equal to: [" + maxScriptFields + "] but was [" + + (maxScriptFields + 1) + + "]. This limit can be set by changing the [index.max_script_fields] index level setting.", + ex.getMessage()); + } + } + public static class FailOnRewriteQueryPlugin extends Plugin implements SearchPlugin { @Override public List> getQueries() { diff --git a/docs/reference/index-modules.asciidoc b/docs/reference/index-modules.asciidoc index 82abb3e0a3b64..f25b2cf9059cc 100644 --- a/docs/reference/index-modules.asciidoc +++ b/docs/reference/index-modules.asciidoc @@ -134,6 +134,11 @@ specific index module: Defaults to `100`. Doc-value fields are costly since they might incur a per-field per-document seek. +`index.max_script_fields`:: + + The maximum number of `script_fields` that are allowed in a query. + Defaults to `32`. + `index.blocks.read_only`:: Set to `true` to make the index and index metadata read only, `false` to diff --git a/docs/reference/migration/migrate_6_0/search.asciidoc b/docs/reference/migration/migrate_6_0/search.asciidoc index ff41ae38d6871..cc81eff7764ce 100644 --- a/docs/reference/migration/migrate_6_0/search.asciidoc +++ b/docs/reference/migration/migrate_6_0/search.asciidoc @@ -142,6 +142,11 @@ The deprecated `fielddata_fields` have now been removed. `docvalue_fields` shoul `docvalue_fields` now have a default upper limit of 100 fields that can be requested. This limit can be overridden by using the `index.max_docvalue_fields_search` index setting. +==== `script_fields` + +`script_fields` now have a default upper limit of 32 script fields that can be requested. +This limit can be overridden by using the `index.max_script_fields` index setting. + ==== Inner hits The source inside a hit of inner hits keeps its full path with respect to the entire source. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml index 37ae1f4e0fffb..17927aabff585 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml @@ -72,3 +72,29 @@ setup: query: match_all: {} docvalue_fields: ["one", "two", "three"] + +--- +"Script_fields size limit": + - skip: + version: " - 5.99.99" + reason: soft limit for script_fields only available as of 6.0.0 + + - do: + indices.create: + index: test_3 + body: + settings: + index.max_script_fields: 2 + + - do: + catch: /Trying to retrieve too many script_fields\. Must be less than or equal to[:] \[2\] but was \[3\]\. This limit can be set by changing the \[index.max_script_fields\] index level setting\./ + search: + index: test_3 + body: + query: + match_all: {} + script_fields: { + "test1" : { "script" : { "lang": "painless", "source": "1" }}, + "test2" : { "script" : { "lang": "painless", "source": "1" }}, + "test3" : { "script" : { "lang": "painless", "source": "1" }} + }