From c3745b03ee032d77290471e391a1336d038b7f66 Mon Sep 17 00:00:00 2001 From: markharwood Date: Fri, 15 Nov 2019 16:50:32 +0000 Subject: [PATCH] Search optimisation - add canMatch early aborts for queries on "_index" field (#49158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make queries on the “_index” field fast-fail if the target shard is an index that doesn’t match the query expression. Part of the “canMatch” phase optimisations. Closes #48473 --- .../test/multi_cluster/70_skip_shards.yml | 163 ++++++++++++++++++ .../multi_cluster/90_index_name_query.yml | 44 +++++ .../index/query/PrefixQueryBuilder.java | 13 ++ .../index/query/TermQueryBuilder.java | 13 ++ .../index/query/TermsQueryBuilder.java | 15 ++ .../index/query/WildcardQueryBuilder.java | 16 +- .../index/query/PrefixQueryBuilderTests.java | 15 ++ .../index/query/TermQueryBuilderTests.java | 16 +- .../index/query/TermsQueryBuilderTests.java | 17 +- .../query/WildcardQueryBuilderTests.java | 16 ++ .../test/AbstractBuilderTestCase.java | 9 +- 11 files changed, 333 insertions(+), 4 deletions(-) diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml index 9242664d9f2..92ae11c712b 100644 --- a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml @@ -58,3 +58,166 @@ - match: { _shards.failed: 0 } - match: { hits.total: 1 } +--- +"Test that queries on _index field that don't match alias are skipped": + + - do: + indices.create: + index: skip_shards_local_index + body: + settings: + index: + number_of_shards: 2 + number_of_replicas: 0 + mappings: + properties: + created_at: + type: date + format: "yyyy-MM-dd" + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "skip_shards_local_index"}}' + - '{"f1": "local_cluster", "sort_field": 0, "created_at" : "2017-01-01"}' + - '{"index": {"_index": "skip_shards_local_index"}}' + - '{"f1": "local_cluster", "sort_field": 1, "created_at" : "2017-01-02"}' + - do: + indices.put_alias: + index: skip_shards_local_index + name: test_skip_alias + + # check that we match the alias with term query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "term" : { "_index" : "test_skip_alias" } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + # check that we match the alias with terms query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "terms" : { "_index" : ["test_skip_alias", "does_not_match"] } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + # check that we match the alias with prefix query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "prefix" : { "_index" : "test_skip_ali" } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + # check that we match the alias with wildcard query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "wildcard" : { "_index" : "test_skip_ali*" } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + + # check that skipped when we don't match the alias with a term query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "term" : { "_index" : "does_not_match" } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + # check that skipped when we don't match the alias with a terms query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "terms" : { "_index" : ["does_not_match", "also_does_not_match"] } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + # check that skipped when we don't match the alias with a prefix query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "prefix" : { "_index" : "does_not_matc" } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + # check that skipped when we don't match the alias with a wildcard query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "wildcard" : { "_index" : "does_not_matc*" } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml index 030dad662df..a60a1b0d812 100644 --- a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml @@ -56,3 +56,47 @@ teardown: - match: { _shards.successful: 2 } - match: { _shards.skipped : 0} - match: { _shards.failed: 0 } + +--- +"Test that queries on _index that don't match are skipped": + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "single_doc_index"}}' + - '{"f1": "local_cluster", "sort_field": 0}' + + - do: + search: + ccs_minimize_roundtrips: false + track_total_hits: true + index: "single_doc_index,my_remote_cluster:single_doc_index" + pre_filter_shard_size: 1 + body: + query: + term: + "_index": "does_not_match" + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + - do: + search: + ccs_minimize_roundtrips: false + track_total_hits: true + index: "single_doc_index,my_remote_cluster:single_doc_index" + pre_filter_shard_size: 1 + body: + query: + term: + "_index": "my_remote_cluster:does_not_match" + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } diff --git a/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java index eacb2be100c..db596e2ecfc 100644 --- a/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java @@ -168,6 +168,19 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder public String getWriteableName() { return NAME; } + + @Override + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + if ("_index".equals(fieldName)) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match the value of this query on the "_index" field. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null && shardContext.indexMatches(value + "*") == false) { + return new MatchNoneQueryBuilder(); + } + } + return super.doRewrite(queryRewriteContext); + } @Override protected Query doToQuery(QueryShardContext context) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java index c35aa9b03d5..262bfb2c6b5 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java @@ -129,6 +129,19 @@ public class TermQueryBuilder extends BaseTermQueryBuilder { } return termQuery; } + + @Override + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + if ("_index".equals(fieldName)) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match the value of this query on the "_index" field. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null && shardContext.indexMatches(BytesRefs.toString(value)) == false) { + return new MatchNoneQueryBuilder(); + } + } + return super.doRewrite(queryRewriteContext); + } @Override protected Query doToQuery(QueryShardContext context) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index 2f4f934727d..4cf205ea5b2 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -499,6 +499,21 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { }))); return new TermsQueryBuilder(this.fieldName, supplier::get); } + if ("_index".equals(this.fieldName) && values != null) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match any of the search terms. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null) { + for (Object localValue : values) { + if (shardContext.indexMatches(BytesRefs.toString(localValue))) { + // We can match - at least one index name matches + return this; + } + } + // all index names are invalid - no possibility of a match on this shard. + return new MatchNoneQueryBuilder(); + } + } return this; } } diff --git a/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java index 0b855bd50a4..115fa8d476d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -177,7 +178,20 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder parseQuery(shortJson)); assertEquals("[prefix] query doesn't support multiple fields, found [user1] and [user2]", e.getMessage()); } + + public void testRewriteIndexQueryToMatchNone() throws Exception { + PrefixQueryBuilder query = prefixQuery("_index", "does_not_exist"); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class)); + } + + public void testRewriteIndexQueryToNotMatchNone() throws Exception { + PrefixQueryBuilder query = prefixQuery("_index", getIndex().getName()); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(PrefixQueryBuilder.class)); + } + } diff --git a/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java index 48137b2726f..0bf6ddbc574 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java @@ -172,5 +172,19 @@ public class TermQueryBuilderTests extends AbstractTermQueryTestCase indexNameMatcher() { + // Simplistic index name matcher used for testing + return pattern -> Regex.simpleMatch(pattern, index.getName()); + } @Override public void close() throws IOException { @@ -407,7 +414,7 @@ public abstract class AbstractBuilderTestCase extends ESTestCase { QueryShardContext createShardContext(IndexSearcher searcher) { return new QueryShardContext(0, idxSettings, BigArrays.NON_RECYCLING_INSTANCE, bitsetFilterCache, indexFieldDataService::getForField, mapperService, similarityService, scriptService, xContentRegistry, - namedWriteableRegistry, this.client, searcher, () -> nowInMillis, null, null); + namedWriteableRegistry, this.client, searcher, () -> nowInMillis, null, indexNameMatcher()); } ScriptModule createScriptModule(List scriptPlugins) {