Skip to content

Commit

Permalink
Count only mapped fields towards docvalue_fields limit (elastic#63806)
Browse files Browse the repository at this point in the history
Currently we count every field requested in the search request bodies
'docvalue_fields' section towards the limit defined by the
'max_docvalue_fields_search' index setting which defaults to 100. This can be a
problem e.g. if the user searches across several indices with some fields
present in one index but not the other and has to add the joint set of field
names to the query. We currently trip the limit even if the number of actually
mapped fields in each index is below the limit.
This change adds a step to distiguish between mappend and unmapped fields and
only count the former towards the limit.

Closes elastic#63730
  • Loading branch information
Christoph Büscher committed Oct 21, 2020
1 parent a685d96 commit 8c81ede
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ setup:
index:
index: test_1
id: 1
body: { foo: bar }
body: { foo: bar, foo2: bar, foo3: bar }

- do:
indices.refresh: {}
Expand Down Expand Up @@ -77,9 +77,9 @@ setup:
query:
match_all: {}
docvalue_fields:
- "one"
- "two"
- "three"
- "foo"
- "foo2"
- "foo3"

---
"Script_fields size limit":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ protected void setupInnerHitsContext(QueryShardContext queryShardContext,
innerHitsContext.storedFieldsContext(innerHitBuilder.getStoredFieldsContext());
}
if (innerHitBuilder.getDocValueFields() != null) {
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(queryShardContext::simpleMatchToIndexNames,
queryShardContext.getIndexSettings().getMaxDocvalueFields(), innerHitBuilder.getDocValueFields());
FetchDocValuesContext docValuesContext = new FetchDocValuesContext(queryShardContext, innerHitBuilder.getDocValueFields());
innerHitsContext.docValuesContext(docValuesContext);
}
if (innerHitBuilder.getFetchFields() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -967,8 +967,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
context.fetchSourceContext(source.fetchSource());
}
if (source.docValueFields() != null) {
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(context.mapperService()::simpleMatchToFullName,
context.mapperService().getIndexSettings().getMaxDocvalueFields(), source.docValueFields());
FetchDocValuesContext docValuesContext = new FetchDocValuesContext(context.getQueryShardContext(), source.docValueFields());
context.docValuesContext(docValuesContext);
}
if (source.fetchFields() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ public Aggregator createInternal(SearchContext searchContext,
subSearchContext.storedFieldsContext(storedFieldsContext);
}
if (docValueFields != null) {
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(searchContext.mapperService()::simpleMatchToFullName,
searchContext.mapperService().getIndexSettings().getMaxDocvalueFields(), docValueFields);
FetchDocValuesContext docValuesContext = new FetchDocValuesContext(searchContext.getQueryShardContext(), docValueFields);
subSearchContext.docValuesContext(docValuesContext);
}
if (fetchFields != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ public FetchDocValuesContext docValuesContext() {
// retrieve the `doc_value` associated with the collapse field
String name = searchContext.collapse().getFieldName();
if (dvContext == null) {
return new FetchDocValuesContext(Collections.singletonList(new FieldAndFormat(name, null)));
return new FetchDocValuesContext(
searchContext.getQueryShardContext(),
Collections.singletonList(new FieldAndFormat(name, null))
);
} else if (searchContext.docValuesContext().fields().stream().map(ff -> ff.field).anyMatch(name::equals) == false) {
dvContext.fields().add(new FieldAndFormat(name, null));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,45 +19,48 @@
package org.elasticsearch.search.fetch.subphase;

import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryShardContext;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.Function;

/**
* All the required context to pull a field from the doc values.
* This contains:
* <ul>
* <li>a list of field names and its format.
* </ul>
*/
public class FetchDocValuesContext {
private final List<FieldAndFormat> fields;

public static FetchDocValuesContext create(Function<String, Set<String>> simpleMatchToFullName,
int maxAllowedDocvalueFields,
List<FieldAndFormat> fieldPatterns) {
List<FieldAndFormat> fields = new ArrayList<>();
private final List<FieldAndFormat> fields = new ArrayList<>();

/**
* Create a new FetchDocValuesContext using the provided input list.
* Field patterns containing wildcards are resolved and unmapped fields are filtered out.
*/
public FetchDocValuesContext(QueryShardContext shardContext, List<FieldAndFormat> fieldPatterns) {
for (FieldAndFormat field : fieldPatterns) {
Collection<String> fieldNames = simpleMatchToFullName.apply(field.field);
for (String fieldName: fieldNames) {
fields.add(new FieldAndFormat(fieldName, field.format));
Collection<String> fieldNames = shardContext.simpleMatchToIndexNames(field.field);
for (String fieldName : fieldNames) {
if (shardContext.isFieldMapped(fieldName)) {
fields.add(new FieldAndFormat(fieldName, field.format));
}
}
}

int maxAllowedDocvalueFields = shardContext.getIndexSettings().getMaxDocvalueFields();
if (fields.size() > maxAllowedDocvalueFields) {
throw new IllegalArgumentException(
"Trying to retrieve too many docvalue_fields. Must be less than or equal to: [" + maxAllowedDocvalueFields
+ "] but was [" + fields.size() + "]. This limit can be set by changing the ["
+ IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey() + "] index level setting.");
}

return new FetchDocValuesContext(fields);
}

public FetchDocValuesContext(List<FieldAndFormat> fields) {
this.fields = fields;
}

/**
* Returns the required docvalue fields
* Returns the required docvalue fields.
*/
public List<FieldAndFormat> fields() {
return this.fields;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.search;

import com.carrotsearch.hppc.IntArrayList;

import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.FilterDirectoryReader;
import org.apache.lucene.index.LeafReader;
Expand Down Expand Up @@ -441,7 +442,12 @@ 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 Settings settings = Settings.builder()
.put(IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey(), 1)
.build();
createIndex("index", settings, null, "field1", "keyword", "field2", "keyword");
client().prepareIndex("index", "_doc", "1").setSource("field1", "value1", "field2", "value2").setRefreshPolicy(IMMEDIATE).get();

final SearchService service = getInstanceFromNode(SearchService.class);
final IndicesService indicesService = getInstanceFromNode(IndicesService.class);
final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index"));
Expand All @@ -450,22 +456,27 @@ public void testMaxDocvalueFieldsSearch() throws IOException {
SearchRequest searchRequest = new SearchRequest().allowPartialSearchResults(true);
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
searchRequest.source(searchSourceBuilder);
// adding the maximum allowed number of docvalue_fields to retrieve
for (int i = 0; i < indexService.getIndexSettings().getMaxDocvalueFields(); i++) {
searchSourceBuilder.docValueField("field" + i);
}
searchSourceBuilder.docValueField("field1");

final ShardSearchRequest request = new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 1,
new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, -1, null, null);
try (ReaderContext reader = createReaderContext(indexService, indexShard);
SearchContext context = service.createContext(reader, request, null, randomBoolean())) {
assertNotNull(context);
}
searchSourceBuilder.docValueField("one_field_too_much");

searchSourceBuilder.docValueField("unmapped_field");
try (ReaderContext reader = createReaderContext(indexService, indexShard);
SearchContext context = service.createContext(reader, request, null, randomBoolean())) {
assertNotNull(context);
}

searchSourceBuilder.docValueField("field2");
try (ReaderContext reader = createReaderContext(indexService, indexShard)) {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
() -> service.createContext(reader, request, null, randomBoolean()));
assertEquals(
"Trying to retrieve too many docvalue_fields. Must be less than or equal to: [100] but was [101]. "
"Trying to retrieve too many docvalue_fields. Must be less than or equal to: [1] but was [2]. "
+ "This limit can be set by changing the [index.max_docvalue_fields_search] index level setting.",
ex.getMessage());
}
Expand Down

0 comments on commit 8c81ede

Please sign in to comment.