Skip to content

Add a limit to from + size in top_hits and inner hits. #26492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package org.elasticsearch.common.settings;

import org.elasticsearch.index.IndexSortConfig;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.routing.UnassignedInfo;
import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider;
Expand All @@ -27,6 +26,7 @@
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexSortConfig;
import org.elasticsearch.index.IndexingSlowLog;
import org.elasticsearch.index.MergePolicyConfig;
import org.elasticsearch.index.MergeSchedulerConfig;
Expand Down Expand Up @@ -110,6 +110,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
IndexSettings.INDEX_WARMER_ENABLED_SETTING,
IndexSettings.INDEX_REFRESH_INTERVAL_SETTING,
IndexSettings.MAX_RESULT_WINDOW_SETTING,
IndexSettings.MAX_INNER_RESULT_WINDOW_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 @@ -91,6 +91,13 @@ 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 value of from + size on an individual inner hit definition or
* top hits aggregation. The default maximum of 100 is defensive for the reason that the number of inner hit responses
* and number of top hits buckets returned is unbounded. Profile your cluster when increasing this setting.
*/
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 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 @@ -211,6 +218,7 @@ public final class IndexSettings {
private long gcDeletesInMillis = DEFAULT_GC_DELETES.millis();
private volatile boolean warmerEnabled;
private volatile int maxResultWindow;
private volatile int maxInnerResultWindow;
private volatile int maxAdjacencyMatrixFilters;
private volatile int maxRescoreWindow;
private volatile boolean TTLPurgeDisabled;
Expand Down Expand Up @@ -311,6 +319,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
gcDeletesInMillis = scopedSettings.get(INDEX_GC_DELETES_SETTING).getMillis();
warmerEnabled = scopedSettings.get(INDEX_WARMER_ENABLED_SETTING);
maxResultWindow = scopedSettings.get(MAX_RESULT_WINDOW_SETTING);
maxInnerResultWindow = scopedSettings.get(MAX_INNER_RESULT_WINDOW_SETTING);
maxAdjacencyMatrixFilters = scopedSettings.get(MAX_ADJACENCY_MATRIX_FILTERS_SETTING);
maxRescoreWindow = scopedSettings.get(MAX_RESCORE_WINDOW_SETTING);
TTLPurgeDisabled = scopedSettings.get(INDEX_TTL_DISABLE_PURGE_SETTING);
Expand Down Expand Up @@ -339,6 +348,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
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_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(INDEX_WARMER_ENABLED_SETTING, this::setEnableWarmer);
Expand Down Expand Up @@ -564,6 +574,17 @@ private void setMaxResultWindow(int maxResultWindow) {
this.maxResultWindow = maxResultWindow;
}

/**
* Returns the max result window for an individual inner hit definition or top hits aggregation.
*/
public int getMaxInnerResultWindow() {
return maxInnerResultWindow;
}

private void setMaxInnerResultWindow(int maxInnerResultWindow) {
this.maxInnerResultWindow = maxInnerResultWindow;
}

/**
* Returns the max number of filters in adjacency_matrix aggregation search requests
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

package org.elasticsearch.index.query;

import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.script.SearchScript;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
Expand Down Expand Up @@ -47,8 +47,21 @@ protected InnerHitContextBuilder(QueryBuilder query, InnerHitBuilder innerHitBui
this.query = query;
}

public abstract void build(SearchContext parentSearchContext,
InnerHitsContext innerHitsContext) throws IOException;
public final void build(SearchContext parentSearchContext, InnerHitsContext innerHitsContext) throws IOException {
long innerResultWindow = innerHitBuilder.getFrom() + innerHitBuilder.getSize();
int maxInnerResultWindow = parentSearchContext.mapperService().getIndexSettings().getMaxInnerResultWindow();
if (innerResultWindow > maxInnerResultWindow) {
throw new IllegalArgumentException(
"Inner result window is too large, the inner hit definition's [" + innerHitBuilder.getName() +
"]'s from + size must be less than or equal to: [" + maxInnerResultWindow + "] but was [" + innerResultWindow +
"]. This limit can be set by changing the [" + IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey() +
"] index level setting."
);
}
doBuild(parentSearchContext, innerHitsContext);
}

protected abstract void doBuild(SearchContext parentSearchContext, InnerHitsContext innerHitsContext) throws IOException;

public static void extractInnerHits(QueryBuilder query, Map<String, InnerHitContextBuilder> innerHitBuilders) {
if (query instanceof AbstractQueryBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ static class NestedInnerHitContextBuilder extends InnerHitContextBuilder {
}

@Override
public void build(SearchContext parentSearchContext,
protected void doBuild(SearchContext parentSearchContext,
InnerHitsContext innerHitsContext) throws IOException {
QueryShardContext queryShardContext = parentSearchContext.getQueryShardContext();
ObjectMapper nestedObjectMapper = queryShardContext.getObjectMapper(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.SearchScript;
Expand Down Expand Up @@ -529,6 +530,17 @@ public TopHitsAggregationBuilder subAggregations(Builder subFactories) {
@Override
protected TopHitsAggregatorFactory doBuild(SearchContext context, AggregatorFactory<?> parent, Builder subfactoriesBuilder)
throws IOException {
long innerResultWindow = from() + size();
int maxInnerResultWindow = context.mapperService().getIndexSettings().getMaxInnerResultWindow();
if (innerResultWindow > maxInnerResultWindow) {
throw new IllegalArgumentException(
"Top hits result window is too large, the top hits aggregator [" + name + "]'s from + size must be less " +
"than or equal to: [" + maxInnerResultWindow + "] but was [" + innerResultWindow +
"]. This limit can be set by changing the [" + IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey() +
"] index level setting."
);
}

List<ScriptFieldsContext.ScriptField> fields = new ArrayList<>();
if (scriptFields != null) {
for (ScriptField field : scriptFields) {
Expand Down
20 changes: 20 additions & 0 deletions core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,26 @@ public void testMaxResultWindow() {
assertEquals(IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxResultWindow());
}

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

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

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 @@ -139,8 +139,8 @@ public void testEqualsAndHashcode() {
public static InnerHitBuilder randomInnerHits() {
InnerHitBuilder innerHits = new InnerHitBuilder();
innerHits.setName(randomAlphaOfLengthBetween(1, 16));
innerHits.setFrom(randomIntBetween(0, 128));
innerHits.setSize(randomIntBetween(0, 128));
innerHits.setFrom(randomIntBetween(0, 32));
innerHits.setSize(randomIntBetween(0, 32));
innerHits.setExplain(randomBoolean());
innerHits.setVersion(randomBoolean());
innerHits.setTrackScores(randomBoolean());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
import org.elasticsearch.index.search.ESToParentBlockJoinQuery;
Expand All @@ -41,6 +43,7 @@
import java.util.HashMap;
import java.util.Map;

import static org.elasticsearch.index.IndexSettingsTests.newIndexMeta;
import static org.elasticsearch.index.query.InnerHitBuilderTests.randomInnerHits;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
Expand Down Expand Up @@ -325,6 +328,11 @@ public void testBuildIgnoreUnmappedNestQuery() throws Exception {
SearchContext searchContext = mock(SearchContext.class);
when(searchContext.getQueryShardContext()).thenReturn(queryShardContext);

MapperService mapperService = mock(MapperService.class);
IndexSettings settings = new IndexSettings(newIndexMeta("index", Settings.EMPTY), Settings.EMPTY);
when(mapperService.getIndexSettings()).thenReturn(settings);
when(searchContext.mapperService()).thenReturn(mapperService);

InnerHitBuilder leafInnerHits = randomInnerHits();
NestedQueryBuilder query1 = new NestedQueryBuilder("path", new MatchAllQueryBuilder(), ScoreMode.None);
query1.innerHit(leafInnerHits);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.common.document.DocumentField;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.MatchAllQueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.plugins.Plugin;
Expand Down Expand Up @@ -942,7 +943,10 @@ public void testTopHitsInNested() throws Exception {
}
}

public void testDontExplode() throws Exception {
public void testUseMaxDocInsteadOfSize() throws Exception {
client().admin().indices().prepareUpdateSettings("idx")
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), ArrayUtil.MAX_ARRAY_LENGTH))
.get();
SearchResponse response = client()
.prepareSearch("idx")
.addAggregation(terms("terms")
Expand All @@ -954,6 +958,67 @@ public void testDontExplode() throws Exception {
)
.get();
assertNoFailures(response);
client().admin().indices().prepareUpdateSettings("idx")
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), null))
.get();
}

public void testTooHighResultWindow() throws Exception {
SearchResponse response = client()
.prepareSearch("idx")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").from(50).size(10).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
)
)
.get();
assertNoFailures(response);

Exception e = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch("idx")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").from(100).size(10).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
)
).get());
assertThat(e.getCause().getMessage(),
containsString("the top hits aggregator [hits]'s from + size must be less than or equal to: [100] but was [110]"));
e = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch("idx")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").from(10).size(100).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
)
).get());
assertThat(e.getCause().getMessage(),
containsString("the top hits aggregator [hits]'s from + size must be less than or equal to: [100] but was [110]"));

client().admin().indices().prepareUpdateSettings("idx")
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), 110))
.get();
response = client().prepareSearch("idx")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").from(100).size(10).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
)).get();
assertNoFailures(response);
response = client().prepareSearch("idx")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").from(10).size(100).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
)).get();
assertNoFailures(response);
client().admin().indices().prepareUpdateSettings("idx")
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), null))
.get();
}

public void testNoStoredFields() throws Exception {
Expand Down
Loading