Skip to content

Commit 40dd85f

Browse files
authored
Fix alias resolution runtime complexity. (#40263) (#40791)
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 b824652 commit 40dd85f

File tree

11 files changed

+221
-66
lines changed

11 files changed

+221
-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
@@ -69,10 +69,9 @@
6969
- length: { shards: 1 }
7070
- match: { shards.0.0.index: test_index }
7171
- match: { indices.test_index.aliases: [test_alias_filter_1, test_alias_filter_2]}
72-
- match: { indices.test_index.filter.bool.should.0.term.field.value: value1 }
72+
- length: { indices.test_index.filter.bool.should: 2 }
7373
- lte: { indices.test_index.filter.bool.should.0.term.field.boost: 1.0 }
7474
- gte: { indices.test_index.filter.bool.should.0.term.field.boost: 1.0 }
75-
- match: { indices.test_index.filter.bool.should.1.term.field.value: value2}
7675
- lte: { indices.test_index.filter.bool.should.1.term.field.boost: 1.0 }
7776
- gte: { indices.test_index.filter.bool.should.1.term.field.boost: 1.0 }
7877
- 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
@@ -89,10 +89,11 @@ protected void masterOperation(final ClusterSearchShardsRequest request, final C
8989
String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request);
9090
Map<String, Set<String>> routingMap = indexNameExpressionResolver.resolveSearchRouting(state, request.routing(), request.indices());
9191
Map<String, AliasFilter> indicesAndFilters = new HashMap<>();
92+
Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
9293
for (String index : concreteIndices) {
93-
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, request.indices());
94+
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, indicesAndAliases);
9495
final String[] aliases = indexNameExpressionResolver.indexAliases(clusterState, index, aliasMetadata -> true, true,
95-
request.indices());
96+
indicesAndAliases);
9697
indicesAndFilters.put(index, new AliasFilter(aliasFilter.getQueryBuilder(), aliases));
9798
}
9899

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
@@ -116,8 +116,9 @@ protected void doExecute(Task task, ValidateQueryRequest request, ActionListener
116116

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

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

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

4949
import java.io.IOException;
50+
import java.util.Set;
5051

5152
/**
5253
* Explain transport action. Computes the explain on the targeted shard.
@@ -78,8 +79,8 @@ protected boolean resolveIndex(ExplainRequest request) {
7879

7980
@Override
8081
protected void resolveRequest(ClusterState state, InternalRequest request) {
81-
final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(),
82-
request.request().index());
82+
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(state, request.request().index());
83+
final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(), indicesAndAliases);
8384
request.request().filteringAlias(aliasFilter);
8485
// Fail fast on the node that received the request.
8586
if (request.request().routing() == null && state.getMetaData().routingRequired(request.concreteIndex(), request.request().type())) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,10 @@ public TransportSearchAction(Settings settings, ThreadPool threadPool, Transport
9696
private Map<String, AliasFilter> buildPerIndexAliasFilter(SearchRequest request, ClusterState clusterState,
9797
Index[] concreteIndices, Map<String, AliasFilter> remoteAliasMap) {
9898
final Map<String, AliasFilter> aliasFilterMap = new HashMap<>();
99+
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
99100
for (Index index : concreteIndices) {
100101
clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index.getName());
101-
AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), request.indices());
102+
AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), indicesAndAliases);
102103
assert aliasFilter != null;
103104
aliasFilterMap.put(index.getUUID(), aliasFilter);
104105
}

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.joda.JodaDateFormatter;
3031
import org.elasticsearch.common.regex.Regex;
@@ -43,16 +44,20 @@
4344

4445
import java.util.ArrayList;
4546
import java.util.Arrays;
47+
import java.util.Collection;
4648
import java.util.Collections;
4749
import java.util.HashMap;
4850
import java.util.HashSet;
4951
import java.util.List;
5052
import java.util.Locale;
5153
import java.util.Map;
54+
import java.util.Objects;
5255
import java.util.Set;
5356
import java.util.SortedMap;
57+
import java.util.Spliterators;
5458
import java.util.function.Predicate;
5559
import java.util.stream.Collectors;
60+
import java.util.stream.StreamSupport;
5661

5762
public class IndexNameExpressionResolver {
5863
private final List<ExpressionResolver> expressionResolvers;
@@ -313,71 +318,88 @@ public String resolveDateMathExpression(String dateExpression) {
313318
return dateMathExpressionResolver.resolveExpression(dateExpression, new Context(null, null));
314319
}
315320

321+
/**
322+
* Resolve an array of expressions to the set of indices and aliases that these expressions match.
323+
*/
324+
public Set<String> resolveExpressions(ClusterState state, String... expressions) {
325+
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false);
326+
List<String> resolvedExpressions = Arrays.asList(expressions);
327+
for (ExpressionResolver expressionResolver : expressionResolvers) {
328+
resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions);
329+
}
330+
return Collections.unmodifiableSet(new HashSet<>(resolvedExpressions));
331+
}
332+
316333
/**
317334
* Iterates through the list of indices and selects the effective list of filtering aliases for the
318335
* given index.
319336
* <p>Only aliases with filters are returned. If the indices list contains a non-filtering reference to
320337
* the index itself - null is returned. Returns {@code null} if no filtering is required.
338+
* <b>NOTE</b>: The provided expressions must have been resolved already via {@link #resolveExpressions}.
321339
*/
322-
public String[] filteringAliases(ClusterState state, String index, String... expressions) {
323-
return indexAliases(state, index, AliasMetaData::filteringRequired, false, expressions);
340+
public String[] filteringAliases(ClusterState state, String index, Set<String> resolvedExpressions) {
341+
return indexAliases(state, index, AliasMetaData::filteringRequired, false, resolvedExpressions);
342+
}
343+
344+
/**
345+
* Whether to generate the candidate set from index aliases, or from the set of resolved expressions.
346+
* @param indexAliasesSize the number of aliases of the index
347+
* @param resolvedExpressionsSize the number of resolved expressions
348+
*/
349+
// pkg-private for testing
350+
boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) {
351+
return indexAliasesSize <= resolvedExpressionsSize;
324352
}
325353

326354
/**
327355
* Iterates through the list of indices and selects the effective list of required aliases for the given index.
328356
* <p>Only aliases where the given predicate tests successfully are returned. If the indices list contains a non-required reference to
329357
* the index itself - null is returned. Returns {@code null} if no filtering is required.
358+
* <p><b>NOTE</b>: the provided expressions must have been resolved already via {@link #resolveExpressions}.
330359
*/
331360
public String[] indexAliases(ClusterState state, String index, Predicate<AliasMetaData> requiredAlias, boolean skipIdentity,
332-
String... expressions) {
333-
// expand the aliases wildcard
334-
List<String> resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList();
335-
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false);
336-
for (ExpressionResolver expressionResolver : expressionResolvers) {
337-
resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions);
338-
}
339-
361+
Set<String> resolvedExpressions) {
340362
if (isAllIndices(resolvedExpressions)) {
341363
return null;
342364
}
365+
343366
final IndexMetaData indexMetaData = state.metaData().getIndices().get(index);
344367
if (indexMetaData == null) {
345368
// Shouldn't happen
346369
throw new IndexNotFoundException(index);
347370
}
348-
// optimize for the most common single index/alias scenario
349-
if (resolvedExpressions.size() == 1) {
350-
String alias = resolvedExpressions.get(0);
351371

352-
AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias);
353-
if (aliasMetaData == null || requiredAlias.test(aliasMetaData) == false) {
354-
return null;
355-
}
356-
return new String[]{alias};
372+
if (skipIdentity == false && resolvedExpressions.contains(index)) {
373+
return null;
374+
}
375+
376+
final ImmutableOpenMap<String, AliasMetaData> indexAliases = indexMetaData.getAliases();
377+
final AliasMetaData[] aliasCandidates;
378+
if (iterateIndexAliases(indexAliases.size(), resolvedExpressions.size())) {
379+
// faster to iterate indexAliases
380+
aliasCandidates = StreamSupport.stream(Spliterators.spliteratorUnknownSize(indexAliases.values().iterator(), 0), false)
381+
.map(cursor -> cursor.value)
382+
.filter(aliasMetaData -> resolvedExpressions.contains(aliasMetaData.alias()))
383+
.toArray(AliasMetaData[]::new);
384+
} else {
385+
// faster to iterate resolvedExpressions
386+
aliasCandidates = resolvedExpressions.stream()
387+
.map(indexAliases::get)
388+
.filter(Objects::nonNull)
389+
.toArray(AliasMetaData[]::new);
357390
}
391+
358392
List<String> aliases = null;
359-
for (String alias : resolvedExpressions) {
360-
if (alias.equals(index)) {
361-
if (skipIdentity) {
362-
continue;
363-
} else {
364-
return null;
365-
}
366-
}
367-
AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias);
368-
// Check that this is an alias for the current index
369-
// Otherwise - skip it
370-
if (aliasMetaData != null) {
371-
if (requiredAlias.test(aliasMetaData)) {
372-
// If required - add it to the list of aliases
373-
if (aliases == null) {
374-
aliases = new ArrayList<>();
375-
}
376-
aliases.add(alias);
377-
} else {
378-
// If not, we have a non required alias for this index - no further checking needed
379-
return null;
393+
for (AliasMetaData aliasMetaData : aliasCandidates) {
394+
if (requiredAlias.test(aliasMetaData)) {
395+
// If required - add it to the list of aliases
396+
if (aliases == null) {
397+
aliases = new ArrayList<>();
380398
}
399+
aliases.add(aliasMetaData.alias());
400+
} else {
401+
// If not, we have a non required alias for this index - no further checking needed
402+
return null;
381403
}
382404
}
383405
if (aliases == null) {
@@ -504,7 +526,7 @@ public Map<String, Set<String>> resolveSearchRoutingAllIndices(MetaData metaData
504526
* @param aliasesOrIndices the array containing index names
505527
* @return true if the provided array maps to all indices, false otherwise
506528
*/
507-
public static boolean isAllIndices(List<String> aliasesOrIndices) {
529+
public static boolean isAllIndices(Collection<String> aliasesOrIndices) {
508530
return aliasesOrIndices == null || aliasesOrIndices.isEmpty() || isExplicitAllPattern(aliasesOrIndices);
509531
}
510532

@@ -515,8 +537,8 @@ public static boolean isAllIndices(List<String> aliasesOrIndices) {
515537
* @param aliasesOrIndices the array containing index names
516538
* @return true if the provided array explicitly maps to all indices, false otherwise
517539
*/
518-
static boolean isExplicitAllPattern(List<String> aliasesOrIndices) {
519-
return aliasesOrIndices != null && aliasesOrIndices.size() == 1 && MetaData.ALL.equals(aliasesOrIndices.get(0));
540+
static boolean isExplicitAllPattern(Collection<String> aliasesOrIndices) {
541+
return aliasesOrIndices != null && aliasesOrIndices.size() == 1 && MetaData.ALL.equals(aliasesOrIndices.iterator().next());
520542
}
521543

522544
/**
@@ -589,7 +611,7 @@ public long getStartTime() {
589611
/**
590612
* This is used to prevent resolving aliases to concrete indices but this also means
591613
* that we might return aliases that point to a closed index. This is currently only used
592-
* by {@link #filteringAliases(ClusterState, String, String...)} since it's the only one that needs aliases
614+
* by {@link #filteringAliases(ClusterState, String, Set)} since it's the only one that needs aliases
593615
*/
594616
boolean isPreserveAliases() {
595617
return preserveAliases;
@@ -633,6 +655,8 @@ public List<String> resolve(Context context, List<String> expressions) {
633655
return resolveEmptyOrTrivialWildcard(options, metaData);
634656
}
635657

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

638662
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
@@ -1352,7 +1352,7 @@ interface IndexDeletionAllowedPredicate {
13521352
(Index index, IndexSettings indexSettings) -> canDeleteIndexContents(index, indexSettings);
13531353
private final IndexDeletionAllowedPredicate ALWAYS_TRUE = (Index index, IndexSettings indexSettings) -> true;
13541354

1355-
public AliasFilter buildAliasFilter(ClusterState state, String index, String... expressions) {
1355+
public AliasFilter buildAliasFilter(ClusterState state, String index, Set<String> resolvedExpressions) {
13561356
/* Being static, parseAliasFilter doesn't have access to whatever guts it needs to parse a query. Instead of passing in a bunch
13571357
* of dependencies we pass in a function that can perform the parsing. */
13581358
CheckedFunction<byte[], QueryBuilder, IOException> filterParser = bytes -> {
@@ -1361,8 +1361,8 @@ public AliasFilter buildAliasFilter(ClusterState state, String index, String...
13611361
return parseInnerQueryBuilder(parser);
13621362
}
13631363
};
1364-
String[] aliases = indexNameExpressionResolver.filteringAliases(state, index, expressions);
13651364
IndexMetaData indexMetaData = state.metaData().index(index);
1365+
String[] aliases = indexNameExpressionResolver.filteringAliases(state, index, resolvedExpressions);
13661366
return new AliasFilter(ShardSearchRequest.parseAliasFilter(filterParser, indexMetaData, aliases), aliases);
13671367
}
13681368

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@
111111
import java.util.List;
112112
import java.util.Map;
113113
import java.util.Optional;
114+
import java.util.Set;
114115
import java.util.concurrent.ExecutionException;
115116
import java.util.concurrent.Executor;
116117
import java.util.concurrent.atomic.AtomicInteger;
@@ -1019,8 +1020,8 @@ public void run() {
10191020
}
10201021
}
10211022

1022-
public AliasFilter buildAliasFilter(ClusterState state, String index, String... expressions) {
1023-
return indicesService.buildAliasFilter(state, index, expressions);
1023+
public AliasFilter buildAliasFilter(ClusterState state, String index, Set<String> resolvedExpressions) {
1024+
return indicesService.buildAliasFilter(state, index, resolvedExpressions);
10241025
}
10251026

10261027
/**
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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+
import org.elasticsearch.common.settings.Settings;
23+
24+
public class IndexNameExpressionResolverAliasIterationTests extends IndexNameExpressionResolverTests {
25+
26+
protected IndexNameExpressionResolver getIndexNameExpressionResolver() {
27+
return new IndexNameExpressionResolver(Settings.EMPTY) {
28+
@Override
29+
boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) {
30+
return true;
31+
}
32+
};
33+
}
34+
35+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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+
import org.elasticsearch.common.settings.Settings;
23+
24+
public class IndexNameExpressionResolverExpressionsIterationTests extends IndexNameExpressionResolverTests {
25+
26+
protected IndexNameExpressionResolver getIndexNameExpressionResolver() {
27+
return new IndexNameExpressionResolver(Settings.EMPTY) {
28+
@Override
29+
boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) {
30+
return false;
31+
}
32+
};
33+
}
34+
35+
}

0 commit comments

Comments
 (0)