Search optimisation - add canMatch early aborts for queries on "_index" field (#49158)

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
This commit is contained in:
markharwood 2019-11-15 16:50:32 +00:00 committed by GitHub
parent b6ec066ca9
commit c3745b03ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 333 additions and 4 deletions

View File

@ -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 }

View File

@ -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 }

View File

@ -169,6 +169,19 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder<PrefixQueryBuilder>
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 {
MultiTermQuery.RewriteMethod method = QueryParsers.parseRewriteMethod(rewrite, null, LoggingDeprecationHandler.INSTANCE);

View File

@ -130,6 +130,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 {
Query query = null;

View File

@ -499,6 +499,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;
}
}

View File

@ -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;
@ -179,6 +180,19 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder<WildcardQueryBuil
.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 {
MappedFieldType fieldType = context.fieldMapper(fieldName);

View File

@ -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));
}
}

View File

@ -173,4 +173,18 @@ public class TermQueryBuilderTests extends AbstractTermQueryTestCase<TermQueryBu
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));
}
}

View File

@ -317,6 +317,21 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase<TermsQueryBuil
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);

View File

@ -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));
}
}

View File

@ -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;
@ -400,6 +402,11 @@ public abstract class AbstractBuilderTestCase extends ESTestCase {
}
}
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 {
}
@ -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<ScriptPlugin> scriptPlugins) {