Skip to content

Commit

Permalink
Add a soft limit for the number of requested doc-value fields (#26574)
Browse files Browse the repository at this point in the history
Requesting to many docvalue_fields in a search request can potentially be costly
because it might incur a per-field per-document seek. This change introduces a
soft limit on the number of fields that can be retrieved. The setting can be
changed per index using the `index.max_docvalue_fields_search` setting.

Relates to #26390
  • Loading branch information
cbuescher committed Sep 13, 2017
1 parent c6f34c9 commit 9a98f07
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
IndexSettings.INDEX_REFRESH_INTERVAL_SETTING,
IndexSettings.MAX_RESULT_WINDOW_SETTING,
IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING,
IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING,
IndexSettings.MAX_RESCORE_WINDOW_SETTING,
IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING,
IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING,
Expand Down
21 changes: 21 additions & 0 deletions core/src/main/java/org/elasticsearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ public final class IndexSettings {
*/
public static final Setting<Integer> MAX_INNER_RESULT_WINDOW_SETTING =
Setting.intSetting("index.max_inner_result_window", 100, 1, 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
* doc values might incur a per-field per-document seek.
*/
public static final Setting<Integer> MAX_DOCVALUE_FIELDS_SEARCH_SETTING =
Setting.intSetting("index.max_docvalue_fields_search", 100, 0, 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.
Expand Down Expand Up @@ -234,6 +241,7 @@ public final class IndexSettings {
private volatile int maxInnerResultWindow;
private volatile int maxAdjacencyMatrixFilters;
private volatile int maxRescoreWindow;
private volatile int maxDocvalueFields;
private volatile boolean TTLPurgeDisabled;
/**
* The maximum number of refresh listeners allows on this shard.
Expand Down Expand Up @@ -335,6 +343,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
maxInnerResultWindow = scopedSettings.get(MAX_INNER_RESULT_WINDOW_SETTING);
maxAdjacencyMatrixFilters = scopedSettings.get(MAX_ADJACENCY_MATRIX_FILTERS_SETTING);
maxRescoreWindow = scopedSettings.get(MAX_RESCORE_WINDOW_SETTING);
maxDocvalueFields = scopedSettings.get(MAX_DOCVALUE_FIELDS_SEARCH_SETTING);
TTLPurgeDisabled = scopedSettings.get(INDEX_TTL_DISABLE_PURGE_SETTING);
maxRefreshListeners = scopedSettings.get(MAX_REFRESH_LISTENERS_PER_SHARD);
maxSlicesPerScroll = scopedSettings.get(MAX_SLICES_PER_SCROLL);
Expand Down Expand Up @@ -364,6 +373,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
scopedSettings.addSettingsUpdateConsumer(MAX_INNER_RESULT_WINDOW_SETTING, this::setMaxInnerResultWindow);
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(INDEX_WARMER_ENABLED_SETTING, this::setEnableWarmer);
scopedSettings.addSettingsUpdateConsumer(INDEX_GC_DELETES_SETTING, this::setGCDeletes);
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING, this::setTranslogFlushThresholdSize);
Expand Down Expand Up @@ -620,6 +630,17 @@ private void setMaxRescoreWindow(int maxRescoreWindow) {
this.maxRescoreWindow = maxRescoreWindow;
}

/**
* Returns the maximum number of allowed docvalue_fields to retrieve in a search request
*/
public int getMaxDocvalueFields() {
return this.maxDocvalueFields;
}

private void setMaxDocvalueFields(int maxDocvalueFields) {
this.maxDocvalueFields = maxDocvalueFields;
}

/**
* Returns the GC deletes cycle in milliseconds.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,13 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
context.fetchSourceContext(source.fetchSource());
}
if (source.docValueFields() != null) {
int maxAllowedDocvalueFields = context.mapperService().getIndexSettings().getMaxDocvalueFields();
if (source.docValueFields().size() > maxAllowedDocvalueFields) {
throw new IllegalArgumentException(
"Trying to retrieve too many docvalue_fields. Must be less than or equal to: [" + maxAllowedDocvalueFields
+ "] but was [" + source.docValueFields().size() + "]. This limit can be set by changing the ["
+ IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey() + "] index level setting.");
}
context.docValueFieldsContext(new DocValueFieldsContext(source.docValueFields()));
}
if (source.highlighter() != null) {
Expand Down
16 changes: 16 additions & 0 deletions core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,22 @@ public void testMaxInnerResultWindow() {
assertEquals(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxInnerResultWindow());
}

public void testMaxDocvalueFields() {
IndexMetaData metaData = newIndexMeta("index", Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey(), 200).build());
IndexSettings settings = new IndexSettings(metaData, Settings.EMPTY);
assertEquals(200, settings.getMaxDocvalueFields());
settings.updateIndexMetaData(
newIndexMeta("index", Settings.builder().put(IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey(), 50).build()));
assertEquals(50, settings.getMaxDocvalueFields());
settings.updateIndexMetaData(newIndexMeta("index", Settings.EMPTY));
assertEquals(IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxDocvalueFields());

metaData = newIndexMeta("index", Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build());
settings = new IndexSettings(metaData, Settings.EMPTY);
assertEquals(IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxDocvalueFields());
}

public void testMaxAdjacencyMatrixFiltersSetting() {
IndexMetaData metaData = newIndexMeta("index", Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchTask;
import org.elasticsearch.action.search.SearchType;
Expand Down Expand Up @@ -262,6 +261,35 @@ public void testTimeout() throws IOException {

}

/**
* test that getting more than the allowed number of docvalue_fields throws an exception
*/
public void testMaxDocvalueFieldsSearch() 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 docvalue_fields to retrieve
for (int i = 0; i < indexService.getIndexSettings().getMaxDocvalueFields(); i++) {
searchSourceBuilder.docValueField("field" + i);
}
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.docValueField("one_field_too_much");
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 docvalue_fields. Must be less than or equal to: [100] but was [101]. "
+ "This limit can be set by changing the [index.max_docvalue_fields_search] index level setting.",
ex.getMessage());
}
}

public static class FailOnRewriteQueryPlugin extends Plugin implements SearchPlugin {
@Override
public List<QuerySpec<?>> getQueries() {
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/index-modules.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ specific index module:
requests take heap memory and time proportional to
`max(window_size, from + size)` and this limits that memory.

`index.max_docvalue_fields_search`::

The maximum number of `docvalue_fields` that are allowed in a query.
Defaults to `100`. Doc-value fields are costly since they might incur
a per-field per-document seek.

`index.blocks.read_only`::

Set to `true` to make the index and index metadata read only, `false` to
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/migration/migrate_6_0/search.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ The `unified` highlighter outputs the same highlighting when `index_options` is

The deprecated `fielddata_fields` have now been removed. `docvalue_fields` should be used instead.

==== `docvalue_fields`

`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.

==== Inner hits

The source inside a hit of inner hits keeps its full path with respect to the entire source.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,25 @@ setup:
match_all: {}
query_weight: 1
rescore_query_weight: 2

---
"Docvalues_fields size limit":
- skip:
version: " - 5.99.99"
reason: soft limit for docvalue_fields only available as of 6.0.0

- do:
indices.create:
index: test_2
body:
settings:
index.max_docvalue_fields_search: 2

- do:
catch: /Trying to retrieve too many docvalue_fields\. Must be less than or equal to[:] \[2\] but was \[3\]\. This limit can be set by changing the \[index.max_docvalue_fields_search\] index level setting\./
search:
index: test_2
body:
query:
match_all: {}
docvalue_fields: ["one", "two", "three"]

0 comments on commit 9a98f07

Please sign in to comment.