Skip to content

Commit df55dba

Browse files
martijnvgmartijnvg
committed
Added a limit to from + size in top_hits and inner hits.
Relates to #11511
1 parent 5798af3 commit df55dba

File tree

13 files changed

+264
-11
lines changed

13 files changed

+264
-11
lines changed

‎core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919
package org.elasticsearch.common.settings;
2020

21-
import org.elasticsearch.index.IndexSortConfig;
2221
import org.elasticsearch.cluster.metadata.IndexMetaData;
2322
import org.elasticsearch.cluster.routing.UnassignedInfo;
2423
import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider;
@@ -27,6 +26,7 @@
2726
import org.elasticsearch.common.settings.Setting.Property;
2827
import org.elasticsearch.index.IndexModule;
2928
import org.elasticsearch.index.IndexSettings;
29+
import org.elasticsearch.index.IndexSortConfig;
3030
import org.elasticsearch.index.IndexingSlowLog;
3131
import org.elasticsearch.index.MergePolicyConfig;
3232
import org.elasticsearch.index.MergeSchedulerConfig;
@@ -110,6 +110,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
110110
IndexSettings.INDEX_WARMER_ENABLED_SETTING,
111111
IndexSettings.INDEX_REFRESH_INTERVAL_SETTING,
112112
IndexSettings.MAX_RESULT_WINDOW_SETTING,
113+
IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING,
113114
IndexSettings.MAX_RESCORE_WINDOW_SETTING,
114115
IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING,
115116
IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING,

‎core/src/main/java/org/elasticsearch/index/IndexSettings.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ public final class IndexSettings {
104104
*/
105105
public static final Setting<Integer> MAX_RESULT_WINDOW_SETTING =
106106
Setting.intSetting("index.max_result_window", 10000, 1, Property.Dynamic, Property.IndexScope);
107+
/**
108+
* Index setting describing the maximum value of from + size on an individual inner hit definition or
109+
* top hits aggregation. The default maximum of 100 is defensive for the reason that the number of inner hit responses
110+
* and number of top hits buckets returned is unbounded. Profile your cluster when increasing this setting.
111+
*/
112+
public static final Setting<Integer> MAX_INNER_RESULT_WINDOW_SETTING =
113+
Setting.intSetting("index.max_inner_result_window", 100, 1, Property.Dynamic, Property.IndexScope);
107114
/**
108115
* Index setting describing the maximum size of the rescore window. Defaults to {@link #MAX_RESULT_WINDOW_SETTING}
109116
* because they both do the same thing: control the size of the heap of hits.
@@ -224,6 +231,7 @@ public final class IndexSettings {
224231
private long gcDeletesInMillis = DEFAULT_GC_DELETES.millis();
225232
private volatile boolean warmerEnabled;
226233
private volatile int maxResultWindow;
234+
private volatile int maxInnerResultWindow;
227235
private volatile int maxAdjacencyMatrixFilters;
228236
private volatile int maxRescoreWindow;
229237
private volatile boolean TTLPurgeDisabled;
@@ -324,6 +332,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
324332
gcDeletesInMillis = scopedSettings.get(INDEX_GC_DELETES_SETTING).getMillis();
325333
warmerEnabled = scopedSettings.get(INDEX_WARMER_ENABLED_SETTING);
326334
maxResultWindow = scopedSettings.get(MAX_RESULT_WINDOW_SETTING);
335+
maxInnerResultWindow = scopedSettings.get(MAX_INNER_RESULT_WINDOW_SETTING);
327336
maxAdjacencyMatrixFilters = scopedSettings.get(MAX_ADJACENCY_MATRIX_FILTERS_SETTING);
328337
maxRescoreWindow = scopedSettings.get(MAX_RESCORE_WINDOW_SETTING);
329338
TTLPurgeDisabled = scopedSettings.get(INDEX_TTL_DISABLE_PURGE_SETTING);
@@ -352,6 +361,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
352361
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_DURABILITY_SETTING, this::setTranslogDurability);
353362
scopedSettings.addSettingsUpdateConsumer(INDEX_TTL_DISABLE_PURGE_SETTING, this::setTTLPurgeDisabled);
354363
scopedSettings.addSettingsUpdateConsumer(MAX_RESULT_WINDOW_SETTING, this::setMaxResultWindow);
364+
scopedSettings.addSettingsUpdateConsumer(MAX_INNER_RESULT_WINDOW_SETTING, this::setMaxInnerResultWindow);
355365
scopedSettings.addSettingsUpdateConsumer(MAX_ADJACENCY_MATRIX_FILTERS_SETTING, this::setMaxAdjacencyMatrixFilters);
356366
scopedSettings.addSettingsUpdateConsumer(MAX_RESCORE_WINDOW_SETTING, this::setMaxRescoreWindow);
357367
scopedSettings.addSettingsUpdateConsumer(INDEX_WARMER_ENABLED_SETTING, this::setEnableWarmer);
@@ -577,6 +587,17 @@ private void setMaxResultWindow(int maxResultWindow) {
577587
this.maxResultWindow = maxResultWindow;
578588
}
579589

590+
/**
591+
* Returns the max result window for an individual inner hit definition or top hits aggregation.
592+
*/
593+
public int getMaxInnerResultWindow() {
594+
return maxInnerResultWindow;
595+
}
596+
597+
private void setMaxInnerResultWindow(int maxInnerResultWindow) {
598+
this.maxInnerResultWindow = maxInnerResultWindow;
599+
}
600+
580601
/**
581602
* Returns the max number of filters in adjacency_matrix aggregation search requests
582603
*/

‎core/src/main/java/org/elasticsearch/index/query/InnerHitContextBuilder.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
package org.elasticsearch.index.query;
2121

22-
import org.elasticsearch.script.ScriptContext;
22+
import org.elasticsearch.index.IndexSettings;
2323
import org.elasticsearch.script.SearchScript;
2424
import org.elasticsearch.search.builder.SearchSourceBuilder;
2525
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
@@ -47,8 +47,21 @@ protected InnerHitContextBuilder(QueryBuilder query, InnerHitBuilder innerHitBui
4747
this.query = query;
4848
}
4949

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

5366
public static void extractInnerHits(QueryBuilder query, Map<String, InnerHitContextBuilder> innerHitBuilders) {
5467
if (query instanceof AbstractQueryBuilder) {

‎core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ static class NestedInnerHitContextBuilder extends InnerHitContextBuilder {
336336
}
337337

338338
@Override
339-
public void build(SearchContext parentSearchContext,
339+
protected void doBuild(SearchContext parentSearchContext,
340340
InnerHitsContext innerHitsContext) throws IOException {
341341
QueryShardContext queryShardContext = parentSearchContext.getQueryShardContext();
342342
ObjectMapper nestedObjectMapper = queryShardContext.getObjectMapper(path);

‎core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.elasticsearch.common.io.stream.StreamOutput;
2727
import org.elasticsearch.common.xcontent.XContentBuilder;
2828
import org.elasticsearch.common.xcontent.XContentParser;
29+
import org.elasticsearch.index.IndexSettings;
2930
import org.elasticsearch.index.query.QueryShardContext;
3031
import org.elasticsearch.script.Script;
3132
import org.elasticsearch.script.SearchScript;
@@ -529,6 +530,17 @@ public TopHitsAggregationBuilder subAggregations(Builder subFactories) {
529530
@Override
530531
protected TopHitsAggregatorFactory doBuild(SearchContext context, AggregatorFactory<?> parent, Builder subfactoriesBuilder)
531532
throws IOException {
533+
long innerResultWindow = from() + size();
534+
int maxInnerResultWindow = context.mapperService().getIndexSettings().getMaxInnerResultWindow();
535+
if (innerResultWindow > maxInnerResultWindow) {
536+
throw new IllegalArgumentException(
537+
"Top hits result window is too large, the top hits aggregator [" + name + "]'s from + size must be less " +
538+
"than or equal to: [" + maxInnerResultWindow + "] but was [" + innerResultWindow +
539+
"]. This limit can be set by changing the [" + IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey() +
540+
"] index level setting."
541+
);
542+
}
543+
532544
List<ScriptFieldsContext.ScriptField> fields = new ArrayList<>();
533545
if (scriptFields != null) {
534546
for (ScriptField field : scriptFields) {

‎core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,26 @@ public void testMaxResultWindow() {
290290
assertEquals(IndexSettings.MAX_RESULT_WINDOW_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxResultWindow());
291291
}
292292

293+
public void testMaxInnerResultWindow() {
294+
IndexMetaData metaData = newIndexMeta("index", Settings.builder()
295+
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
296+
.put(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), 200)
297+
.build());
298+
IndexSettings settings = new IndexSettings(metaData, Settings.EMPTY);
299+
assertEquals(200, settings.getMaxInnerResultWindow());
300+
settings.updateIndexMetaData(newIndexMeta("index", Settings.builder().put(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(),
301+
50).build()));
302+
assertEquals(50, settings.getMaxInnerResultWindow());
303+
settings.updateIndexMetaData(newIndexMeta("index", Settings.EMPTY));
304+
assertEquals(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxInnerResultWindow());
305+
306+
metaData = newIndexMeta("index", Settings.builder()
307+
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
308+
.build());
309+
settings = new IndexSettings(metaData, Settings.EMPTY);
310+
assertEquals(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxInnerResultWindow());
311+
}
312+
293313
public void testMaxAdjacencyMatrixFiltersSetting() {
294314
IndexMetaData metaData = newIndexMeta("index", Settings.builder()
295315
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)

‎core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ public void testEqualsAndHashcode() {
139139
public static InnerHitBuilder randomInnerHits() {
140140
InnerHitBuilder innerHits = new InnerHitBuilder();
141141
innerHits.setName(randomAlphaOfLengthBetween(1, 16));
142-
innerHits.setFrom(randomIntBetween(0, 128));
143-
innerHits.setSize(randomIntBetween(0, 128));
142+
innerHits.setFrom(randomIntBetween(0, 32));
143+
innerHits.setSize(randomIntBetween(0, 32));
144144
innerHits.setExplain(randomBoolean());
145145
innerHits.setVersion(randomBoolean());
146146
innerHits.setTrackScores(randomBoolean());

‎core/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.elasticsearch.Version;
2727
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
2828
import org.elasticsearch.common.compress.CompressedXContent;
29+
import org.elasticsearch.common.settings.Settings;
30+
import org.elasticsearch.index.IndexSettings;
2931
import org.elasticsearch.index.mapper.MapperService;
3032
import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
3133
import org.elasticsearch.index.search.ESToParentBlockJoinQuery;
@@ -41,6 +43,7 @@
4143
import java.util.HashMap;
4244
import java.util.Map;
4345

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

331+
MapperService mapperService = mock(MapperService.class);
332+
IndexSettings settings = new IndexSettings(newIndexMeta("index", Settings.EMPTY), Settings.EMPTY);
333+
when(mapperService.getIndexSettings()).thenReturn(settings);
334+
when(searchContext.mapperService()).thenReturn(mapperService);
335+
328336
InnerHitBuilder leafInnerHits = randomInnerHits();
329337
NestedQueryBuilder query1 = new NestedQueryBuilder("path", new MatchAllQueryBuilder(), ScoreMode.None);
330338
query1.innerHit(leafInnerHits);

‎core/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.common.document.DocumentField;
2929
import org.elasticsearch.common.settings.Settings;
3030
import org.elasticsearch.common.xcontent.XContentBuilder;
31+
import org.elasticsearch.index.IndexSettings;
3132
import org.elasticsearch.index.query.MatchAllQueryBuilder;
3233
import org.elasticsearch.index.query.QueryBuilders;
3334
import org.elasticsearch.plugins.Plugin;
@@ -942,7 +943,10 @@ public void testTopHitsInNested() throws Exception {
942943
}
943944
}
944945

945-
public void testDontExplode() throws Exception {
946+
public void testUseMaxDocInsteadOfSize() throws Exception {
947+
client().admin().indices().prepareUpdateSettings("idx")
948+
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), ArrayUtil.MAX_ARRAY_LENGTH))
949+
.get();
946950
SearchResponse response = client()
947951
.prepareSearch("idx")
948952
.addAggregation(terms("terms")
@@ -954,6 +958,67 @@ public void testDontExplode() throws Exception {
954958
)
955959
.get();
956960
assertNoFailures(response);
961+
client().admin().indices().prepareUpdateSettings("idx")
962+
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), null))
963+
.get();
964+
}
965+
966+
public void testTooHighResultWindow() throws Exception {
967+
SearchResponse response = client()
968+
.prepareSearch("idx")
969+
.addAggregation(terms("terms")
970+
.executionHint(randomExecutionHint())
971+
.field(TERMS_AGGS_FIELD)
972+
.subAggregation(
973+
topHits("hits").from(50).size(10).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
974+
)
975+
)
976+
.get();
977+
assertNoFailures(response);
978+
979+
Exception e = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch("idx")
980+
.addAggregation(terms("terms")
981+
.executionHint(randomExecutionHint())
982+
.field(TERMS_AGGS_FIELD)
983+
.subAggregation(
984+
topHits("hits").from(100).size(10).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
985+
)
986+
).get());
987+
assertThat(e.getCause().getMessage(),
988+
containsString("the top hits aggregator [hits]'s from + size must be less than or equal to: [100] but was [110]"));
989+
e = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch("idx")
990+
.addAggregation(terms("terms")
991+
.executionHint(randomExecutionHint())
992+
.field(TERMS_AGGS_FIELD)
993+
.subAggregation(
994+
topHits("hits").from(10).size(100).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
995+
)
996+
).get());
997+
assertThat(e.getCause().getMessage(),
998+
containsString("the top hits aggregator [hits]'s from + size must be less than or equal to: [100] but was [110]"));
999+
1000+
client().admin().indices().prepareUpdateSettings("idx")
1001+
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), 110))
1002+
.get();
1003+
response = client().prepareSearch("idx")
1004+
.addAggregation(terms("terms")
1005+
.executionHint(randomExecutionHint())
1006+
.field(TERMS_AGGS_FIELD)
1007+
.subAggregation(
1008+
topHits("hits").from(100).size(10).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
1009+
)).get();
1010+
assertNoFailures(response);
1011+
response = client().prepareSearch("idx")
1012+
.addAggregation(terms("terms")
1013+
.executionHint(randomExecutionHint())
1014+
.field(TERMS_AGGS_FIELD)
1015+
.subAggregation(
1016+
topHits("hits").from(10).size(100).sort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC))
1017+
)).get();
1018+
assertNoFailures(response);
1019+
client().admin().indices().prepareUpdateSettings("idx")
1020+
.setSettings(Collections.singletonMap(IndexSettings.MAX_INNER_RESULT_WINDOW_SETTING.getKey(), null))
1021+
.get();
9571022
}
9581023

9591024
public void testNoStoredFields() throws Exception {

0 commit comments

Comments
 (0)