Search optimisation - add canMatch early aborts for queries on "_inde…

…x" field (#48681) 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
elastic · Nov 15, 2019 · 82c00f0 · 82c00f0
1 parent 18c2aab
commit 82c00f0
Show file tree

Hide file tree

Showing 11 changed files with 333 additions and 4 deletions.
diff --git a/...lti-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml b/...lti-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/...luster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml b/...luster-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
@@ -168,6 +168,19 @@ public static PrefixQueryBuilder fromXContent(XContentParser parser) throws IOEx
     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
@@ -129,6 +129,19 @@ public static TermQueryBuilder fromXContent(XContentParser parser) throws IOExce
         }
         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
@@ -482,6 +482,21 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) {
             })));
             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
@@ -27,6 +27,7 @@
 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 static WildcardQueryBuilder fromXContent(XContentParser parser) throws IO
                 .rewrite(rewrite)
                 .boost(boost)
                 .queryName(queryName);
-    }
+    }    
+
+    @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/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java
@@ -141,4 +141,19 @@ public void testParseFailsWithMultipleFields() throws IOException {
         e = expectThrows(ParsingException.class, () -> 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
@@ -172,5 +172,19 @@ public void testTypeField() throws IOException {
         TermQueryBuilder builder = QueryBuilders.termQuery("_type", "value1");
         builder.doToQuery(createShardContext());
         assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE);
-    }
+    }   
+
+    public void testRewriteIndexQueryToMatchNone() throws IOException {
+        TermQueryBuilder query = QueryBuilders.termQuery("_index", "does_not_exist");
+        QueryShardContext queryShardContext = createShardContext();
+        QueryBuilder rewritten = query.rewrite(queryShardContext);
+        assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
+    }   
+
+    public void testRewriteIndexQueryToNotMatchNone() throws IOException {
+        TermQueryBuilder query = QueryBuilders.termQuery("_index", getIndex().getName());
+        QueryShardContext queryShardContext = createShardContext();
+        QueryBuilder rewritten = query.rewrite(queryShardContext);
+        assertThat(rewritten, instanceOf(TermQueryBuilder.class));
+    }      
 }
diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java
@@ -312,7 +312,22 @@ public void testTypeField() throws IOException {
         builder.doToQuery(createShardContext());
         assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE);
     }
-
+
+    public void testRewriteIndexQueryToMatchNone() throws IOException {
+        TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", "also_does_not_exist");
+        QueryShardContext queryShardContext = createShardContext();
+        QueryBuilder rewritten = query.rewrite(queryShardContext);
+        assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
+    }      
+
+    public void testRewriteIndexQueryToNotMatchNone() throws IOException {
+        // At least one name is good
+        TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", getIndex().getName());
+        QueryShardContext queryShardContext = createShardContext();
+        QueryBuilder rewritten = query.rewrite(queryShardContext);
+        assertThat(rewritten, instanceOf(TermsQueryBuilder.class));
+    }      
+
     @Override
     protected QueryBuilder parseQuery(XContentParser parser) throws IOException {
         QueryBuilder query = super.parseQuery(parser);

diff --git a/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java
@@ -138,4 +138,20 @@ public void testTypeField() throws IOException {
         builder.doToQuery(createShardContext());
         assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE);
     }
+
+    public void testRewriteIndexQueryToMatchNone() throws IOException {
+        WildcardQueryBuilder query = new WildcardQueryBuilder("_index", "does_not_exist");
+        QueryShardContext queryShardContext = createShardContext();
+        QueryBuilder rewritten = query.rewrite(queryShardContext);
+        assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
+    }   
+
+    public void testRewriteIndexQueryNotMatchNone() throws IOException {
+        String fullIndexName = getIndex().getName();
+        String firstHalfOfIndexName = fullIndexName.substring(0,fullIndexName.length()/2);
+        WildcardQueryBuilder query = new WildcardQueryBuilder("_index", firstHalfOfIndexName +"*");
+        QueryShardContext queryShardContext = createShardContext();
+        QueryBuilder rewritten = query.rewrite(queryShardContext);
+        assertThat(rewritten, instanceOf(WildcardQueryBuilder.class));
+    }      
 }