瀏覽代碼

Search optimisation - add canMatch early aborts for queries on "_index" 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
markharwood 6 年之前
父節點
當前提交
82c00f0ff1

+ 163 - 0
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 }
+

+ 44 - 0
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 }

+ 13 - 0
server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java

@@ -168,6 +168,19 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder<PrefixQueryBuilder>
     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 {

+ 13 - 0
server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java

@@ -129,6 +129,19 @@ public class TermQueryBuilder extends BaseTermQueryBuilder<TermQueryBuilder> {
         }
         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 {

+ 15 - 0
server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java

@@ -482,6 +482,21 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
             })));
             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;
     }
 }

+ 15 - 1
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<WildcardQueryBuil
                 .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 {

+ 15 - 0
server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java

@@ -141,4 +141,19 @@ public class PrefixQueryBuilderTests extends AbstractQueryTestCase<PrefixQueryBu
         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));
+    }
+
 }

+ 15 - 1
server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java

@@ -172,5 +172,19 @@ public class TermQueryBuilderTests extends AbstractTermQueryTestCase<TermQueryBu
         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));
+    }      
 }

+ 16 - 1
server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java

@@ -312,7 +312,22 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase<TermsQueryBuil
         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);

+ 16 - 0
server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java

@@ -138,4 +138,20 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase<WildcardQue
         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));
+    }      
 }

+ 8 - 1
test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java

@@ -36,6 +36,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
@@ -92,6 +93,7 @@ import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.stream.Stream;
 
 import static java.util.Collections.emptyList;
@@ -398,6 +400,11 @@ public abstract class AbstractBuilderTestCase extends ESTestCase {
                 testCase.initializeAdditionalMappings(mapperService);
             }
         }
+                
+        public static Predicate<String> indexNameMatcher() {
+            // Simplistic index name matcher used for testing
+            return pattern -> Regex.simpleMatch(pattern, index.getName());
+        }                 
 
         @Override
         public void close() throws IOException {
@@ -406,7 +413,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<ScriptPlugin> scriptPlugins) {