Skip to content

Commit

Permalink
Add a cluster setting to disallow expensive queries (#51385) (#52279)
Browse files Browse the repository at this point in the history
Add a new cluster setting `search.allow_expensive_queries` which by
default is `true`. If set to `false`, certain queries that have
usually slow performance cannot be executed and an error message
is returned.

- Queries that need to do linear scans to identify matches:
  - Script queries
- Queries that have a high up-front cost:
  - Fuzzy queries
  - Regexp queries
  - Prefix queries (without index_prefixes enabled
  - Wildcard queries
  - Range queries on text and keyword fields
- Joining queries
  - HasParent queries
  - HasChild queries
  - ParentId queries
  - Nested queries
- Queries on deprecated 6.x geo shapes (using PrefixTree implementation)
- Queries that may have a high per-document cost:
  - Script score queries
  - Percolate queries

Closes: #29050
(cherry picked from commit a8b39ed)
  • Loading branch information
matriv authored Feb 12, 2020
1 parent 40b58e6 commit dac720d
Show file tree
Hide file tree
Showing 83 changed files with 1,371 additions and 152 deletions.
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 @@ -693,3 +693,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`>>
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 Down Expand Up @@ -55,6 +56,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 @@ -302,6 +305,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 @@ -31,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 @@ -69,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 @@ -371,5 +374,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 @@ -265,5 +268,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

0 comments on commit dac720d

Please sign in to comment.