Skip to content

Add a cluster setting to disallow expensive queries #51385

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 31 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
d9a14a6
Add a cluster setting to disallow slow queries
matriv Jan 24, 2020
181d838
address comments
matriv Jan 27, 2020
4ffc00f
Add script and script_score queries to the disallowed ones
matriv Jan 27, 2020
55200c7
Disallow joining queries
matriv Jan 28, 2020
16f6c8b
Rename setting to positive semantics
matriv Jan 28, 2020
126448f
Merge remote-tracking branch 'upstream/master' into impl-29050
matriv Feb 5, 2020
b1ca9cb
Merge remote-tracking branch 'upstream/master' into impl-29050
matriv Feb 5, 2020
2dd5d9a
Add percolator queries to the expensive ones
matriv Feb 5, 2020
65ff23e
Add legacy geo-shape queries to the disallowed ones
matriv Feb 5, 2020
14ef948
capitalize the first letter of error messages
matriv Feb 6, 2020
77a0eff
Add range queries on text/keyword to the expensive ones
matriv Feb 6, 2020
2b4d45a
Add index_prefixes suggestion to the error message
matriv Feb 6, 2020
f8e24e8
fix error msg
matriv Feb 6, 2020
0587709
fix assertions of error messages
matriv Feb 6, 2020
aaaf219
Enhance part of allow expensive in docs Query DSL
matriv Feb 6, 2020
b24ca0e
fix tests
matriv Feb 6, 2020
eab1b65
improve error messages
matriv Feb 6, 2020
16a7017
Merge remote-tracking branch 'upstream/master' into impl-29050
matriv Feb 6, 2020
51a8cdc
fix test
matriv Feb 6, 2020
7eccee2
fix test
matriv Feb 6, 2020
907207f
fix test
matriv Feb 6, 2020
5aa1a80
Check that allow_expensive_queries setting doesn't affect new geo-sha…
matriv Feb 6, 2020
ff9bb5c
revert the legacy quad tree test to the same method
matriv Feb 6, 2020
d1c5e1c
remove unused import
matriv Feb 6, 2020
78e3c9e
Remove duplicate constructors to ease up tests
matriv Feb 7, 2020
1c64502
fix tests
matriv Feb 7, 2020
6fdab2b
Merge remote-tracking branch 'upstream/master' into impl-29050
matriv Feb 7, 2020
623846b
Address comments
matriv Feb 10, 2020
97d2ce4
Merge remote-tracking branch 'upstream/master' into impl-29050
matriv Feb 10, 2020
1759a84
fix docs
matriv Feb 12, 2020
4774178
Merge remote-tracking branch 'upstream/master' into impl-29050
matriv Feb 12, 2020
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
4 changes: 4 additions & 0 deletions docs/reference/mapping/types/geo-shape.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ between index size and a reasonable level of precision of 50m at the
equator. This allows for indexing tens of millions of shapes without
overly bloating the resulting index too much relative to the input size.

[NOTE]
Geo-shape queries on geo-shapes implemented with PrefixTrees will not be executed if
<<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>> is set to false.

[[input-structure]]
[float]
==== Input Structure
Expand Down
23 changes: 22 additions & 1 deletion docs/reference/query-dsl.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,27 @@ or to alter their behaviour (such as the

Query clauses behave differently depending on whether they are used in
<<query-filter-context,query context or filter context>>.

[[query-dsl-allow-expensive-queries]]
Allow expensive queries::
Certain types of queries will generally execute slowly due to the way they are implemented, which can affect
the stability of the cluster. Those queries can be categorised as follows:
* Queries that need to do linear scans to identify matches:
** <<query-dsl-script-query, `script queries`>>
* Queries that have a high up-front cost:
** <<query-dsl-fuzzy-query,`fuzzy queries`>>
** <<query-dsl-regexp-query,`regexp queries`>>
** <<query-dsl-prefix-query,`prefix queries`>> without <<index-prefixes, `index_prefixes`>>
** <<query-dsl-wildcard-query, `wildcard queries`>>
** <<query-dsl-range-query, `range queries>> on <<text, `text`>> and <<keyword, `keyword`>> fields
* <<joining-queries, `Joining queries`>>
* Queries on <<prefix-trees, deprecated geo shapes>>
* Queries that may have a high per-document cost:
** <<query-dsl-script-score-query, `script score queries`>>
** <<query-dsl-percolate-query, `percolate queries`>>

The execution of such queries can be prevented by setting the value of the `search.allow_expensive_queries`
setting to `false` (defaults to `true`).
--

include::query-dsl/query_filter_context.asciidoc[]
Expand All @@ -51,4 +72,4 @@ include::query-dsl/minimum-should-match.asciidoc[]

include::query-dsl/multi-term-rewrite.asciidoc[]

include::query-dsl/regexp-syntax.asciidoc[]
include::query-dsl/regexp-syntax.asciidoc[]
6 changes: 5 additions & 1 deletion docs/reference/query-dsl/fuzzy-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,8 @@ adjacent characters (ab → ba). Defaults to `true`.

`rewrite`::
(Optional, string) Method used to rewrite the query. For valid values and more
information, see the <<query-dsl-multi-term-rewrite, `rewrite` parameter>>.
information, see the <<query-dsl-multi-term-rewrite, `rewrite` parameter>>.

==== Notes
Fuzzy queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.
4 changes: 4 additions & 0 deletions docs/reference/query-dsl/geo-shape-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,7 @@ and will not match any documents for this query. This can be useful when
querying multiple indexes which might have different mappings. When set to
`false` (the default value) the query will throw an exception if the field
is not mapped.

==== Notes
Geo-shape queries on geo-shapes implemented with <<prefix-trees, `PrefixTrees`>> will not be executed if
<<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>> is set to false.
5 changes: 4 additions & 1 deletion docs/reference/query-dsl/joining-queries.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,7 @@ include::has-parent-query.asciidoc[]

include::parent-id-query.asciidoc[]


=== Notes
==== Allow expensive queries
Joining queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.
5 changes: 5 additions & 0 deletions docs/reference/query-dsl/percolate-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -686,3 +686,8 @@ being percolated, as opposed to a single index as we do in examples. There are a
allows for fields to be stored in a denser, more efficient way.
- Percolate queries do not scale in the same way as other queries, so percolation performance may benefit from using
a different index configuration, like the number of primary shards.

=== Notes
==== Allow expensive queries
Percolate queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.
8 changes: 7 additions & 1 deletion docs/reference/query-dsl/prefix-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,10 @@ GET /_search
You can speed up prefix queries using the <<index-prefixes,`index_prefixes`>>
mapping parameter. If enabled, {es} indexes prefixes between 2 and 5
characters in a separate field. This lets {es} run prefix queries more
efficiently at the cost of a larger index.
efficiently at the cost of a larger index.

[[prefix-query-allow-expensive-queries]]
===== Allow expensive queries
Prefix queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, if <<index-prefixes, index_prefixes>> are enabled, an optimised query is built which is not considered slow, and will be executed in spite of this setting.

is set to false. However, if <<index-prefixes, `index_prefixes`>> are enabled, an optimised query is built which
is not considered slow, and will be executed in spite of this setting.
6 changes: 6 additions & 0 deletions docs/reference/query-dsl/query-string-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -537,3 +537,9 @@ The example above creates a boolean query:
`(blended(terms:[field2:this, field1:this]) blended(terms:[field2:that, field1:that]) blended(terms:[field2:thus, field1:thus]))~2`

that matches documents with at least two of the three per-term blended queries.

==== Notes
===== Allow expensive queries
Query string query can be internally be transformed to a <<query-dsl-prefix-query, `prefix query`>> which means
that if the prefix queries are disabled as explained <<prefix-query-allow-expensive-queries, here>> the query will not be
executed and an exception will be thrown.
5 changes: 5 additions & 0 deletions docs/reference/query-dsl/range-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ increases the relevance score.
[[range-query-notes]]
==== Notes

[[ranges-on-text-and-keyword]]
===== Using the `range` query with `text` and `keyword` fields
Range queries on <<text, `text`>> or <<keyword, `keyword`>> files will not be executed if
<<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>> is set to false.

[[ranges-on-dates]]
===== Using the `range` query with `date` fields

Expand Down
5 changes: 5 additions & 0 deletions docs/reference/query-dsl/regexp-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,8 @@ regular expressions.
`rewrite`::
(Optional, string) Method used to rewrite the query. For valid values and more
information, see the <<query-dsl-multi-term-rewrite, `rewrite` parameter>>.

==== Notes
===== Allow expensive queries
Regexp queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.
4 changes: 4 additions & 0 deletions docs/reference/query-dsl/script-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,7 @@ GET /_search
}
}
----

===== Allow expensive queries
Script queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.
4 changes: 4 additions & 0 deletions docs/reference/query-dsl/script-score-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ and default time zone. Also calculations with `now` are not supported.
<<vector-functions, Functions for vector fields>> are accessible through
`script_score` query.

===== Allow expensive queries
Script score queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.

[[script-score-faster-alt]]
===== Faster alternatives
The `script_score` query calculates the score for
Expand Down
7 changes: 6 additions & 1 deletion docs/reference/query-dsl/wildcard-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,9 @@ increases the relevance score.

`rewrite`::
(Optional, string) Method used to rewrite the query. For valid values and more information, see the
<<query-dsl-multi-term-rewrite, `rewrite` parameter>>.
<<query-dsl-multi-term-rewrite, `rewrite` parameter>>.

==== Notes
===== Allow expensive queries
Wildcard queries will not be executed if <<query-dsl-allow-expensive-queries, `search.allow_expensive_queries`>>
is set to false.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.lucene.search.TermInSetQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.Defaults;
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.PrefixFieldType;
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.SearchAsYouTypeFieldType;
Expand Down Expand Up @@ -100,14 +101,19 @@ public void testPrefixQuery() {

// this term should be a length that can be rewriteable to a term query on the prefix field
final String withinBoundsTerm = "foo";
assertThat(fieldType.prefixQuery(withinBoundsTerm, CONSTANT_SCORE_REWRITE, null),
assertThat(fieldType.prefixQuery(withinBoundsTerm, CONSTANT_SCORE_REWRITE, randomMockShardContext()),
equalTo(new ConstantScoreQuery(new TermQuery(new Term(PREFIX_NAME, withinBoundsTerm)))));

// our defaults don't allow a situation where a term can be too small

// this term should be too long to be rewriteable to a term query on the prefix field
final String longTerm = "toolongforourprefixfieldthistermis";
assertThat(fieldType.prefixQuery(longTerm, CONSTANT_SCORE_REWRITE, null),
assertThat(fieldType.prefixQuery(longTerm, CONSTANT_SCORE_REWRITE, MOCK_QSC),
equalTo(new PrefixQuery(new Term(NAME, longTerm))));

ElasticsearchException ee = expectThrows(ElasticsearchException.class,
() -> fieldType.prefixQuery(longTerm, CONSTANT_SCORE_REWRITE, MOCK_QSC_DISALLOW_EXPENSIVE));
assertEquals("[prefix] queries cannot be executed when 'search.allow_expensive_queries' is set to false. " +
"For optimised prefix queries on text fields please enable [index_prefixes].", ee.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.lucene.search.join.JoinUtil;
import org.apache.lucene.search.join.ScoreMode;
import org.apache.lucene.search.similarities.Similarity;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -53,6 +54,8 @@
import java.util.Map;
import java.util.Objects;

import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;

/**
* A query builder for {@code has_child} query.
*/
Expand Down Expand Up @@ -295,6 +298,11 @@ public String getWriteableName() {

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
if (context.allowExpensiveQueries() == false) {
throw new ElasticsearchException("[joining] queries cannot be executed when '" +
ALLOW_EXPENSIVE_QUERIES.getKey() + "' is set to false.");
}

ParentJoinFieldMapper joinFieldMapper = ParentJoinFieldMapper.getMapper(context.getMapperService());
if (joinFieldMapper == null) {
if (ignoreUnmapped) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -45,6 +46,8 @@
import java.util.Map;
import java.util.Objects;

import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;

/**
* Builder for the 'has_parent' query.
*/
Expand Down Expand Up @@ -158,6 +161,11 @@ public boolean ignoreUnmapped() {

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
if (context.allowExpensiveQueries() == false) {
throw new ElasticsearchException("[joining] queries cannot be executed when '" +
ALLOW_EXPENSIVE_QUERIES.getKey() + "' is set to false.");
}

ParentJoinFieldMapper joinFieldMapper = ParentJoinFieldMapper.getMapper(context.getMapperService());
if (joinFieldMapper == null) {
if (ignoreUnmapped) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -38,6 +39,8 @@
import java.io.IOException;
import java.util.Objects;

import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;

public final class ParentIdQueryBuilder extends AbstractQueryBuilder<ParentIdQueryBuilder> {
public static final String NAME = "parent_id";

Expand Down Expand Up @@ -153,6 +156,11 @@ public static ParentIdQueryBuilder fromXContent(XContentParser parser) throws IO

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
if (context.allowExpensiveQueries() == false) {
throw new ElasticsearchException("[joining] queries cannot be executed when '" +
ALLOW_EXPENSIVE_QUERIES.getKey() + "' is set to false.");
}

ParentJoinFieldMapper joinFieldMapper = ParentJoinFieldMapper.getMapper(context.getMapperService());
if (joinFieldMapper == null) {
if (ignoreUnmapped) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.elasticsearch.join.query;

import com.carrotsearch.randomizedtesting.generators.RandomPicks;

import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
Expand All @@ -32,6 +31,7 @@
import org.apache.lucene.search.join.ScoreMode;
import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper;
import org.apache.lucene.search.similarities.Similarity;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -70,6 +70,8 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQueryBuilder> {

Expand Down Expand Up @@ -361,5 +363,18 @@ public void testExtractInnerHitBuildersWithDuplicate() {
queryBuilder.innerHit(new InnerHitBuilder("some_name"));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> InnerHitContextBuilder.extractInnerHits(queryBuilder, Collections.singletonMap("some_name", null)));
assertEquals("[inner_hits] already contains an entry for key [some_name]", e.getMessage());
}

public void testDisallowExpensiveQueries() {
QueryShardContext queryShardContext = mock(QueryShardContext.class);
when(queryShardContext.allowExpensiveQueries()).thenReturn(false);

HasChildQueryBuilder queryBuilder =
hasChildQuery(CHILD_DOC, new TermQueryBuilder("custom_string", "value"), ScoreMode.None);
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> queryBuilder.toQuery(queryShardContext));
assertEquals("[joining] queries cannot be executed when 'search.allow_expensive_queries' is set to false.",
e.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -57,6 +58,8 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class HasParentQueryBuilderTests extends AbstractQueryTestCase<HasParentQueryBuilder> {
private static final String TYPE = "_doc";
Expand Down Expand Up @@ -261,5 +264,18 @@ public void testExtractInnerHitBuildersWithDuplicate() {
queryBuilder.innerHit(new InnerHitBuilder("some_name"));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> InnerHitContextBuilder.extractInnerHits(queryBuilder, Collections.singletonMap("some_name", null)));
assertEquals("[inner_hits] already contains an entry for key [some_name]", e.getMessage());
}

public void testDisallowExpensiveQueries() {
QueryShardContext queryShardContext = mock(QueryShardContext.class);
when(queryShardContext.allowExpensiveQueries()).thenReturn(false);

HasParentQueryBuilder queryBuilder = new HasParentQueryBuilder(
CHILD_DOC, new WrapperQueryBuilder(new MatchAllQueryBuilder().toString()), false);
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> queryBuilder.toQuery(queryShardContext));
assertEquals("[joining] queries cannot be executed when 'search.allow_expensive_queries' is set to false.",
e.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.Strings;
Expand All @@ -48,6 +49,8 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ParentIdQueryBuilderTests extends AbstractQueryTestCase<ParentIdQueryBuilder> {

Expand Down Expand Up @@ -154,4 +157,14 @@ public void testIgnoreUnmapped() throws IOException {
assertThat(e.getMessage(), containsString("[" + ParentIdQueryBuilder.NAME + "] no relation found for child [unmapped]"));
}

public void testDisallowExpensiveQueries() {
QueryShardContext queryShardContext = mock(QueryShardContext.class);
when(queryShardContext.allowExpensiveQueries()).thenReturn(false);

ParentIdQueryBuilder queryBuilder = doCreateTestQueryBuilder();
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> queryBuilder.toQuery(queryShardContext));
assertEquals("[joining] queries cannot be executed when 'search.allow_expensive_queries' is set to false.",
e.getMessage());
}
}
Loading