Skip to content

Commit 670e766

Browse files
authored
Fix alias resolution runtime complexity. (#40263) (#40788)
A user reported that the same query that takes ~900ms when querying an index pattern only takes ~50ms when only querying indices that have matches. The query is a date range query and we confirmed that the `can_match` phase works as expected. I was able to reproduce this issue locally with a single node: with 900 1-shard indices, a query to an index pattern that matches all indices runs in ~90ms while a query to the only index that has matches runs in 0-1ms. This ended up not being related to the `can_match` phase but to the cost of resolving aliases when querying an index pattern that matches lots of indices. In that case, we first resolve the index pattern to a list of concrete indices and then for each concrete index, we check whether it was matched through an alias, meaning we might have to apply alias filters. Unfortunately this second per-index operation runs in linear time with the number of matched concrete indices, which means that alias resolution runs in O(num_indices^2) overall. So queries get exponentially slower as an index pattern matches more indices. I reorganized alias resolution into a one-step operation that runs in linear time with the number of matches indices, and then a per-index operation that runs in linear time with the number of aliases of this index. This makes alias resolution run is O(num_indices * num_aliases_per_index) overall instead. When testing the scenario described above, the `took` went down from ~90ms to ~10ms. It is still more than the 0-1ms latency that one gets when only querying the single index that has data, but still much better than what we had before. Closes #40248
1 parent 65cca2e commit 670e766

File tree

11 files changed

+217
-66
lines changed

11 files changed

+217
-66
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search_shards/10_basic.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,9 @@
6464
- length: { shards: 1 }
6565
- match: { shards.0.0.index: test_index }
6666
- match: { indices.test_index.aliases: [test_alias_filter_1, test_alias_filter_2]}
67-
- match: { indices.test_index.filter.bool.should.0.term.field.value: value1 }
67+
- length: { indices.test_index.filter.bool.should: 2 }
6868
- lte: { indices.test_index.filter.bool.should.0.term.field.boost: 1.0 }
6969
- gte: { indices.test_index.filter.bool.should.0.term.field.boost: 1.0 }
70-
- match: { indices.test_index.filter.bool.should.1.term.field.value: value2}
7170
- lte: { indices.test_index.filter.bool.should.1.term.field.boost: 1.0 }
7271
- gte: { indices.test_index.filter.bool.should.1.term.field.boost: 1.0 }
7372
- match: { indices.test_index.filter.bool.adjust_pure_negative: true}

server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,11 @@ protected void masterOperation(final ClusterSearchShardsRequest request, final C
8888
String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request);
8989
Map<String, Set<String>> routingMap = indexNameExpressionResolver.resolveSearchRouting(state, request.routing(), request.indices());
9090
Map<String, AliasFilter> indicesAndFilters = new HashMap<>();
91+
Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
9192
for (String index : concreteIndices) {
92-
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, request.indices());
93+
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, indicesAndAliases);
9394
final String[] aliases = indexNameExpressionResolver.indexAliases(clusterState, index, aliasMetadata -> true, true,
94-
request.indices());
95+
indicesAndAliases);
9596
indicesAndFilters.put(index, new AliasFilter(aliasFilter.getQueryBuilder(), aliases));
9697
}
9798

server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ protected void doExecute(Task task, ValidateQueryRequest request, ActionListener
115115

116116
@Override
117117
protected ShardValidateQueryRequest newShardRequest(int numShards, ShardRouting shard, ValidateQueryRequest request) {
118-
final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterService.state(), shard.getIndexName(),
119-
request.indices());
118+
final ClusterState clusterState = clusterService.state();
119+
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
120+
final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, shard.getIndexName(), indicesAndAliases);
120121
return new ShardValidateQueryRequest(shard.shardId(), aliasFilter, request);
121122
}
122123

server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.elasticsearch.transport.TransportService;
5353

5454
import java.io.IOException;
55+
import java.util.Set;
5556

5657
/**
5758
* Explain transport action. Computes the explain on the targeted shard.
@@ -83,8 +84,8 @@ protected boolean resolveIndex(ExplainRequest request) {
8384

8485
@Override
8586
protected void resolveRequest(ClusterState state, InternalRequest request) {
86-
final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(),
87-
request.request().index());
87+
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(state, request.request().index());
88+
final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(), indicesAndAliases);
8889
request.request().filteringAlias(aliasFilter);
8990
// Fail fast on the node that received the request.
9091
if (request.request().routing() == null && state.getMetaData().routingRequired(request.concreteIndex())) {

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,10 @@ public TransportSearchAction(ThreadPool threadPool, TransportService transportSe
116116
private Map<String, AliasFilter> buildPerIndexAliasFilter(SearchRequest request, ClusterState clusterState,
117117
Index[] concreteIndices, Map<String, AliasFilter> remoteAliasMap) {
118118
final Map<String, AliasFilter> aliasFilterMap = new HashMap<>();
119+
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
119120
for (Index index : concreteIndices) {
120121
clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index.getName());
121-
AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), request.indices());
122+
AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), indicesAndAliases);
122123
assert aliasFilter != null;
123124
aliasFilterMap.put(index.getUUID(), aliasFilter);
124125
}

server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.cluster.ClusterState;
2626
import org.elasticsearch.common.Nullable;
2727
import org.elasticsearch.common.Strings;
28+
import org.elasticsearch.common.collect.ImmutableOpenMap;
2829
import org.elasticsearch.common.collect.Tuple;
2930
import org.elasticsearch.common.regex.Regex;
3031
import org.elasticsearch.common.time.DateFormatter;
@@ -42,15 +43,19 @@
4243
import java.time.ZoneOffset;
4344
import java.util.ArrayList;
4445
import java.util.Arrays;
46+
import java.util.Collection;
4547
import java.util.Collections;
4648
import java.util.HashMap;
4749
import java.util.HashSet;
4850
import java.util.List;
4951
import java.util.Map;
52+
import java.util.Objects;
5053
import java.util.Set;
5154
import java.util.SortedMap;
55+
import java.util.Spliterators;
5256
import java.util.function.Predicate;
5357
import java.util.stream.Collectors;
58+
import java.util.stream.StreamSupport;
5459

5560
import static java.util.Collections.unmodifiableList;
5661

@@ -322,71 +327,88 @@ public String resolveDateMathExpression(String dateExpression) {
322327
return dateMathExpressionResolver.resolveExpression(dateExpression, new Context(null, null));
323328
}
324329

330+
/**
331+
* Resolve an array of expressions to the set of indices and aliases that these expressions match.
332+
*/
333+
public Set<String> resolveExpressions(ClusterState state, String... expressions) {
334+
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false);
335+
List<String> resolvedExpressions = Arrays.asList(expressions);
336+
for (ExpressionResolver expressionResolver : expressionResolvers) {
337+
resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions);
338+
}
339+
return Collections.unmodifiableSet(new HashSet<>(resolvedExpressions));
340+
}
341+
325342
/**
326343
* Iterates through the list of indices and selects the effective list of filtering aliases for the
327344
* given index.
328345
* <p>Only aliases with filters are returned. If the indices list contains a non-filtering reference to
329346
* the index itself - null is returned. Returns {@code null} if no filtering is required.
347+
* <b>NOTE</b>: The provided expressions must have been resolved already via {@link #resolveExpressions}.
330348
*/
331-
public String[] filteringAliases(ClusterState state, String index, String... expressions) {
332-
return indexAliases(state, index, AliasMetaData::filteringRequired, false, expressions);
349+
public String[] filteringAliases(ClusterState state, String index, Set<String> resolvedExpressions) {
350+
return indexAliases(state, index, AliasMetaData::filteringRequired, false, resolvedExpressions);
351+
}
352+
353+
/**
354+
* Whether to generate the candidate set from index aliases, or from the set of resolved expressions.
355+
* @param indexAliasesSize the number of aliases of the index
356+
* @param resolvedExpressionsSize the number of resolved expressions
357+
*/
358+
// pkg-private for testing
359+
boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) {
360+
return indexAliasesSize <= resolvedExpressionsSize;
333361
}
334362

335363
/**
336364
* Iterates through the list of indices and selects the effective list of required aliases for the given index.
337365
* <p>Only aliases where the given predicate tests successfully are returned. If the indices list contains a non-required reference to
338366
* the index itself - null is returned. Returns {@code null} if no filtering is required.
367+
* <p><b>NOTE</b>: the provided expressions must have been resolved already via {@link #resolveExpressions}.
339368
*/
340369
public String[] indexAliases(ClusterState state, String index, Predicate<AliasMetaData> requiredAlias, boolean skipIdentity,
341-
String... expressions) {
342-
// expand the aliases wildcard
343-
List<String> resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList();
344-
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false);
345-
for (ExpressionResolver expressionResolver : expressionResolvers) {
346-
resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions);
347-
}
348-
370+
Set<String> resolvedExpressions) {
349371
if (isAllIndices(resolvedExpressions)) {
350372
return null;
351373
}
374+
352375
final IndexMetaData indexMetaData = state.metaData().getIndices().get(index);
353376
if (indexMetaData == null) {
354377
// Shouldn't happen
355378
throw new IndexNotFoundException(index);
356379
}
357-
// optimize for the most common single index/alias scenario
358-
if (resolvedExpressions.size() == 1) {
359-
String alias = resolvedExpressions.get(0);
360380

361-
AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias);
362-
if (aliasMetaData == null || requiredAlias.test(aliasMetaData) == false) {
363-
return null;
364-
}
365-
return new String[]{alias};
381+
if (skipIdentity == false && resolvedExpressions.contains(index)) {
382+
return null;
383+
}
384+
385+
final ImmutableOpenMap<String, AliasMetaData> indexAliases = indexMetaData.getAliases();
386+
final AliasMetaData[] aliasCandidates;
387+
if (iterateIndexAliases(indexAliases.size(), resolvedExpressions.size())) {
388+
// faster to iterate indexAliases
389+
aliasCandidates = StreamSupport.stream(Spliterators.spliteratorUnknownSize(indexAliases.values().iterator(), 0), false)
390+
.map(cursor -> cursor.value)
391+
.filter(aliasMetaData -> resolvedExpressions.contains(aliasMetaData.alias()))
392+
.toArray(AliasMetaData[]::new);
393+
} else {
394+
// faster to iterate resolvedExpressions
395+
aliasCandidates = resolvedExpressions.stream()
396+
.map(indexAliases::get)
397+
.filter(Objects::nonNull)
398+
.toArray(AliasMetaData[]::new);
366399
}
400+
367401
List<String> aliases = null;
368-
for (String alias : resolvedExpressions) {
369-
if (alias.equals(index)) {
370-
if (skipIdentity) {
371-
continue;
372-
} else {
373-
return null;
374-
}
375-
}
376-
AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias);
377-
// Check that this is an alias for the current index
378-
// Otherwise - skip it
379-
if (aliasMetaData != null) {
380-
if (requiredAlias.test(aliasMetaData)) {
381-
// If required - add it to the list of aliases
382-
if (aliases == null) {
383-
aliases = new ArrayList<>();
384-
}
385-
aliases.add(alias);
386-
} else {
387-
// If not, we have a non required alias for this index - no further checking needed
388-
return null;
402+
for (AliasMetaData aliasMetaData : aliasCandidates) {
403+
if (requiredAlias.test(aliasMetaData)) {
404+
// If required - add it to the list of aliases
405+
if (aliases == null) {
406+
aliases = new ArrayList<>();
389407
}
408+
aliases.add(aliasMetaData.alias());
409+
} else {
410+
// If not, we have a non required alias for this index - no further checking needed
411+
return null;
390412
}
391413
}
392414
if (aliases == null) {
@@ -513,7 +535,7 @@ public Map<String, Set<String>> resolveSearchRoutingAllIndices(MetaData metaData
513535
* @param aliasesOrIndices the array containing index names
514536
* @return true if the provided array maps to all indices, false otherwise
515537
*/
516-
public static boolean isAllIndices(List<String> aliasesOrIndices) {
538+
public static boolean isAllIndices(Collection<String> aliasesOrIndices) {
517539
return aliasesOrIndices == null || aliasesOrIndices.isEmpty() || isExplicitAllPattern(aliasesOrIndices);
518540
}
519541

@@ -524,8 +546,8 @@ public static boolean isAllIndices(List<String> aliasesOrIndices) {
524546
* @param aliasesOrIndices the array containing index names
525547
* @return true if the provided array explicitly maps to all indices, false otherwise
526548
*/
527-
static boolean isExplicitAllPattern(List<String> aliasesOrIndices) {
528-
return aliasesOrIndices != null && aliasesOrIndices.size() == 1 && MetaData.ALL.equals(aliasesOrIndices.get(0));
549+
static boolean isExplicitAllPattern(Collection<String> aliasesOrIndices) {
550+
return aliasesOrIndices != null && aliasesOrIndices.size() == 1 && MetaData.ALL.equals(aliasesOrIndices.iterator().next());
529551
}
530552

531553
/**
@@ -598,7 +620,7 @@ public long getStartTime() {
598620
/**
599621
* This is used to prevent resolving aliases to concrete indices but this also means
600622
* that we might return aliases that point to a closed index. This is currently only used
601-
* by {@link #filteringAliases(ClusterState, String, String...)} since it's the only one that needs aliases
623+
* by {@link #filteringAliases(ClusterState, String, Set)} since it's the only one that needs aliases
602624
*/
603625
boolean isPreserveAliases() {
604626
return preserveAliases;
@@ -642,6 +664,8 @@ public List<String> resolve(Context context, List<String> expressions) {
642664
return resolveEmptyOrTrivialWildcard(options, metaData);
643665
}
644666

667+
// TODO: Fix API to work with sets rather than lists since we need to convert to sets
668+
// internally anyway.
645669
Set<String> result = innerResolve(context, expressions, options, metaData);
646670

647671
if (result == null) {

server/src/main/java/org/elasticsearch/indices/IndicesService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,7 +1385,7 @@ interface IndexDeletionAllowedPredicate {
13851385
(Index index, IndexSettings indexSettings) -> canDeleteIndexContents(index, indexSettings);
13861386
private final IndexDeletionAllowedPredicate ALWAYS_TRUE = (Index index, IndexSettings indexSettings) -> true;
13871387

1388-
public AliasFilter buildAliasFilter(ClusterState state, String index, String... expressions) {
1388+
public AliasFilter buildAliasFilter(ClusterState state, String index, Set<String> resolvedExpressions) {
13891389
/* Being static, parseAliasFilter doesn't have access to whatever guts it needs to parse a query. Instead of passing in a bunch
13901390
* of dependencies we pass in a function that can perform the parsing. */
13911391
CheckedFunction<byte[], QueryBuilder, IOException> filterParser = bytes -> {
@@ -1394,8 +1394,8 @@ public AliasFilter buildAliasFilter(ClusterState state, String index, String...
13941394
return parseInnerQueryBuilder(parser);
13951395
}
13961396
};
1397-
String[] aliases = indexNameExpressionResolver.filteringAliases(state, index, expressions);
13981397
IndexMetaData indexMetaData = state.metaData().index(index);
1398+
String[] aliases = indexNameExpressionResolver.filteringAliases(state, index, resolvedExpressions);
13991399
return new AliasFilter(ShardSearchRequest.parseAliasFilter(filterParser, indexMetaData, aliases), aliases);
14001400
}
14011401

server/src/main/java/org/elasticsearch/search/SearchService.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
import java.util.List;
110110
import java.util.Map;
111111
import java.util.Optional;
112+
import java.util.Set;
112113
import java.util.concurrent.ExecutionException;
113114
import java.util.concurrent.Executor;
114115
import java.util.concurrent.atomic.AtomicInteger;
@@ -1004,8 +1005,8 @@ public void run() {
10041005
}
10051006
}
10061007

1007-
public AliasFilter buildAliasFilter(ClusterState state, String index, String... expressions) {
1008-
return indicesService.buildAliasFilter(state, index, expressions);
1008+
public AliasFilter buildAliasFilter(ClusterState state, String index, Set<String> resolvedExpressions) {
1009+
return indicesService.buildAliasFilter(state, index, resolvedExpressions);
10091010
}
10101011

10111012
/**
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.cluster.metadata;
21+
22+
public class IndexNameExpressionResolverAliasIterationTests extends IndexNameExpressionResolverTests {
23+
24+
protected IndexNameExpressionResolver getIndexNameExpressionResolver() {
25+
return new IndexNameExpressionResolver() {
26+
@Override
27+
boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) {
28+
return true;
29+
}
30+
};
31+
}
32+
33+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.cluster.metadata;
21+
22+
public class IndexNameExpressionResolverExpressionsIterationTests extends IndexNameExpressionResolverTests {
23+
24+
protected IndexNameExpressionResolver getIndexNameExpressionResolver() {
25+
return new IndexNameExpressionResolver() {
26+
@Override
27+
boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) {
28+
return false;
29+
}
30+
};
31+
}
32+
33+
}

0 commit comments

Comments
 (0)