From a0324f3f7e79a5dc5c36a6b38a74bdf7c8a78b39 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 20 Mar 2019 15:39:49 +0100 Subject: [PATCH 1/7] Fix alias resolution runtime complexity. 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 --- .../TransportClusterSearchShardsAction.java | 9 ++-- .../query/TransportValidateQueryAction.java | 4 +- .../explain/TransportExplainAction.java | 3 +- .../action/search/TransportSearchAction.java | 3 +- .../metadata/IndexNameExpressionResolver.java | 51 +++++++++---------- .../elasticsearch/indices/IndicesService.java | 11 ++-- .../elasticsearch/search/SearchService.java | 5 +- .../IndexNameExpressionResolverTests.java | 37 ++++++++++---- 8 files changed, 73 insertions(+), 50 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java index 41dce3148c1df..eaa387be34de4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java @@ -44,6 +44,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.function.Function; public class TransportClusterSearchShardsAction extends TransportMasterNodeReadAction { @@ -88,10 +89,12 @@ protected void masterOperation(final ClusterSearchShardsRequest request, final C String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request); Map> routingMap = indexNameExpressionResolver.resolveSearchRouting(state, request.routing(), request.indices()); Map indicesAndFilters = new HashMap<>(); - for (String index : concreteIndices) { - final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, request.indices()); - final String[] aliases = indexNameExpressionResolver.indexAliases(clusterState, index, aliasMetadata -> true, true, + Function indexToAliasFilter = indicesService.buildAliasFilter(clusterState, request.indices()); + Function indexToAliases = indexNameExpressionResolver.indexAliases(clusterState, aliasMetadata -> true, true, request.indices()); + for (String index : concreteIndices) { + final AliasFilter aliasFilter = indexToAliasFilter.apply(index); + final String[] aliases = indexToAliases.apply(index); indicesAndFilters.put(index, new AliasFilter(aliasFilter.getQueryBuilder(), aliases)); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java index 7016d1b42894f..614647494dc0a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java @@ -115,8 +115,8 @@ protected void doExecute(Task task, ValidateQueryRequest request, ActionListener @Override protected ShardValidateQueryRequest newShardRequest(int numShards, ShardRouting shard, ValidateQueryRequest request) { - final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterService.state(), shard.getIndexName(), - request.indices()); + final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterService.state(), request.indices()) + .apply(shard.getIndexName()); return new ShardValidateQueryRequest(shard.shardId(), aliasFilter, request); } diff --git a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java index cc1e842a1ee5e..f819e6deb2043 100644 --- a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java +++ b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java @@ -83,8 +83,7 @@ protected boolean resolveIndex(ExplainRequest request) { @Override protected void resolveRequest(ClusterState state, InternalRequest request) { - final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(), - request.request().index()); + final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.request().index()).apply(request.concreteIndex()); request.request().filteringAlias(aliasFilter); // Fail fast on the node that received the request. if (request.request().routing() == null && state.getMetaData().routingRequired(request.concreteIndex())) { diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index 50f96a3703379..3b6b088234dfd 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -116,9 +116,10 @@ public TransportSearchAction(ThreadPool threadPool, TransportService transportSe private Map buildPerIndexAliasFilter(SearchRequest request, ClusterState clusterState, Index[] concreteIndices, Map remoteAliasMap) { final Map aliasFilterMap = new HashMap<>(); + final Function indexToAliasFilter = searchService.buildAliasFilter(clusterState, request.indices());; for (Index index : concreteIndices) { clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index.getName()); - AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), request.indices()); + AliasFilter aliasFilter = indexToAliasFilter.apply(index.getName()); assert aliasFilter != null; aliasFilterMap.put(index.getUUID(), aliasFilter); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 050d97ba54cf0..57c02412d3b48 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -19,6 +19,8 @@ package org.elasticsearch.cluster.metadata; +import com.carrotsearch.hppc.cursors.ObjectCursor; + import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.support.IndicesOptions; @@ -49,6 +51,7 @@ import java.util.Map; import java.util.Set; import java.util.SortedMap; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -314,8 +317,8 @@ public String resolveDateMathExpression(String dateExpression) { *

Only aliases with filters are returned. If the indices list contains a non-filtering reference to * the index itself - null is returned. Returns {@code null} if no filtering is required. */ - public String[] filteringAliases(ClusterState state, String index, String... expressions) { - return indexAliases(state, index, AliasMetaData::filteringRequired, false, expressions); + public Function filteringAliases(ClusterState state, String... expressions) { + return indexAliases(state, AliasMetaData::filteringRequired, false, expressions); } /** @@ -323,8 +326,8 @@ public String[] filteringAliases(ClusterState state, String index, String... exp *

Only aliases where the given predicate tests successfully are returned. If the indices list contains a non-required reference to * the index itself - null is returned. Returns {@code null} if no filtering is required. */ - public String[] indexAliases(ClusterState state, String index, Predicate requiredAlias, boolean skipIdentity, - String... expressions) { + public Function indexAliases(ClusterState state, Predicate requiredAlias, boolean skipIdentity, + String... expressions) { // expand the aliases wildcard List resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList(); Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false); @@ -333,48 +336,42 @@ public String[] indexAliases(ClusterState state, String index, Predicate null; } + + final Set resolvedExpressionsSet = new HashSet<>(resolvedExpressions); + return index -> indexAliases(state, index, requiredAlias, skipIdentity, resolvedExpressionsSet); + } + + private String[] indexAliases(ClusterState state, String index, Predicate requiredAlias, boolean skipIdentity, + Set resolvedExpressions) { final IndexMetaData indexMetaData = state.metaData().getIndices().get(index); if (indexMetaData == null) { // Shouldn't happen throw new IndexNotFoundException(index); } - // optimize for the most common single index/alias scenario - if (resolvedExpressions.size() == 1) { - String alias = resolvedExpressions.get(0); - AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias); - if (aliasMetaData == null || requiredAlias.test(aliasMetaData) == false) { - return null; - } - return new String[]{alias}; + if (skipIdentity == false && resolvedExpressions.contains(index)) { + return null; } + List aliases = null; - for (String alias : resolvedExpressions) { - if (alias.equals(index)) { - if (skipIdentity) { - continue; - } else { - return null; - } - } - AliasMetaData aliasMetaData = indexMetaData.getAliases().get(alias); - // Check that this is an alias for the current index - // Otherwise - skip it - if (aliasMetaData != null) { + for (ObjectCursor aliasMetaDataCursor : indexMetaData.getAliases().values()) { + AliasMetaData aliasMetaData = aliasMetaDataCursor.value; + if (resolvedExpressions.contains(aliasMetaData.alias())) { if (requiredAlias.test(aliasMetaData)) { // If required - add it to the list of aliases if (aliases == null) { aliases = new ArrayList<>(); } - aliases.add(alias); + aliases.add(aliasMetaData.alias()); } else { // If not, we have a non required alias for this index - no further checking needed return null; } } } + if (aliases == null) { return null; } @@ -584,7 +581,7 @@ public long getStartTime() { /** * This is used to prevent resolving aliases to concrete indices but this also means * that we might return aliases that point to a closed index. This is currently only used - * by {@link #filteringAliases(ClusterState, String, String...)} since it's the only one that needs aliases + * by {@link #filteringAliases(ClusterState, String...)} since it's the only one that needs aliases */ boolean isPreserveAliases() { return preserveAliases; diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 4a12bdae6b9ea..c9be1dd01b825 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1376,7 +1376,7 @@ interface IndexDeletionAllowedPredicate { (Index index, IndexSettings indexSettings) -> canDeleteIndexContents(index, indexSettings); private final IndexDeletionAllowedPredicate ALWAYS_TRUE = (Index index, IndexSettings indexSettings) -> true; - public AliasFilter buildAliasFilter(ClusterState state, String index, String... expressions) { + public Function buildAliasFilter(ClusterState state, String... expressions) { /* Being static, parseAliasFilter doesn't have access to whatever guts it needs to parse a query. Instead of passing in a bunch * of dependencies we pass in a function that can perform the parsing. */ CheckedFunction filterParser = bytes -> { @@ -1385,9 +1385,12 @@ public AliasFilter buildAliasFilter(ClusterState state, String index, String... return parseInnerQueryBuilder(parser); } }; - String[] aliases = indexNameExpressionResolver.filteringAliases(state, index, expressions); - IndexMetaData indexMetaData = state.metaData().index(index); - return new AliasFilter(ShardSearchRequest.parseAliasFilter(filterParser, indexMetaData, aliases), aliases); + Function indexToAliases = indexNameExpressionResolver.filteringAliases(state, expressions); + return index -> { + IndexMetaData indexMetaData = state.metaData().index(index); + String[] aliases = indexToAliases.apply(index); + return new AliasFilter(ShardSearchRequest.parseAliasFilter(filterParser, indexMetaData, aliases), aliases); + }; } /** diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 5ad15fa626822..20fc2faebbe08 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -113,6 +113,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -1004,8 +1005,8 @@ public void run() { } } - public AliasFilter buildAliasFilter(ClusterState state, String index, String... expressions) { - return indicesService.buildAliasFilter(state, index, expressions); + public Function buildAliasFilter(ClusterState state, String... expressions) { + return indicesService.buildAliasFilter(state, expressions); } /** diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java index 571843126f98c..6428dacb46e1b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -1007,14 +1007,14 @@ public void testFilteringAliases() { .put(indexBuilder("test-1").state(State.OPEN).putAlias(AliasMetaData.builder("alias-1"))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); - String[] strings = indexNameExpressionResolver.filteringAliases(state, "test-0", "alias-*"); + String[] strings = indexNameExpressionResolver.filteringAliases(state, "alias-*").apply("test-0"); assertArrayEquals(new String[] {"alias-0"}, strings); // concrete index supersedes filtering alias - strings = indexNameExpressionResolver.filteringAliases(state, "test-0", "test-0,alias-*"); + strings = indexNameExpressionResolver.filteringAliases(state, "test-0,alias-*").apply("test-0"); assertNull(strings); - strings = indexNameExpressionResolver.filteringAliases(state, "test-0", "test-*,alias-*"); + strings = indexNameExpressionResolver.filteringAliases(state, "test-*,alias-*").apply("test-0"); assertNull(strings); } @@ -1026,11 +1026,30 @@ public void testIndexAliases() { .putAlias(AliasMetaData.builder("test-alias-non-filtering")) ); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); - String[] strings = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, true, "test-*"); + String[] strings = indexNameExpressionResolver.indexAliases(state, x -> true, true, "test-*").apply("test-0"); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias-0", "test-alias-1", "test-alias-non-filtering"}, strings); } + public void testIndexAliasesSkipIdentity() { + MetaData.Builder mdBuilder = MetaData.builder() + .put(indexBuilder("test-0").state(State.OPEN) + .putAlias(AliasMetaData.builder("test-alias")) + .putAlias(AliasMetaData.builder("other-alias")) + ); + ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); + + String[] aliases = indexNameExpressionResolver.indexAliases(state, x -> true, false, "test-*").apply("test-0"); + assertNull(aliases); + aliases = indexNameExpressionResolver.indexAliases(state, x -> true, true, "test-*").apply("test-0"); + assertArrayEquals(new String[] {"test-alias"}, aliases); + + aliases = indexNameExpressionResolver.indexAliases(state, x -> true, false, "other-*").apply("test-0"); + assertArrayEquals(new String[] {"other-alias"}, aliases); + aliases = indexNameExpressionResolver.indexAliases(state, x -> true, true, "other-*").apply("test-0"); + assertArrayEquals(new String[] {"other-alias"}, aliases); + } + public void testConcreteWriteIndexSuccessful() { boolean testZeroWriteIndex = randomBoolean(); MetaData.Builder mdBuilder = MetaData.builder() @@ -1038,7 +1057,7 @@ public void testConcreteWriteIndexSuccessful() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(testZeroWriteIndex ? true : null))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, "test-0", x -> true, true, "test-*"); + .indexAliases(state, x -> true, true, "test-*").apply("test-0"); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); IndicesRequest request = new IndicesRequest() { @@ -1099,7 +1118,7 @@ public void testConcreteWriteIndexWithWildcardExpansion() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(testZeroWriteIndex ? randomFrom(false, null) : true))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, "test-0", x -> true, true, "test-*"); + .indexAliases(state, x -> true, true, "test-*").apply("test-0"); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); IndicesRequest request = new IndicesRequest() { @@ -1127,7 +1146,7 @@ public void testConcreteWriteIndexWithNoWriteIndexWithSingleIndex() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(false))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, "test-0", x -> true, true, "test-*"); + .indexAliases(state, x -> true, true, "test-*").apply("test-0"); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); DocWriteRequest request = randomFrom(new IndexRequest("test-alias"), @@ -1147,7 +1166,7 @@ public void testConcreteWriteIndexWithNoWriteIndexWithMultipleIndices() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(randomFrom(false, null)))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, "test-0", x -> true, true, "test-*"); + .indexAliases(state, x -> true, true, "test-*").apply("test-0"); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); DocWriteRequest request = randomFrom(new IndexRequest("test-alias"), @@ -1168,7 +1187,7 @@ public void testAliasResolutionNotAllowingMultipleIndices() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(randomFrom(!test0WriteIndex, null)))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, "test-0", x -> true, true, "test-*"); + .indexAliases(state, x -> true, true, "test-*").apply("test-0"); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, From b31c693d1d395a3cf530cc1603edc1b66cf71ea5 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 22 Mar 2019 08:58:52 +0100 Subject: [PATCH 2/7] Refactor to avoid using a Function. --- .../TransportClusterSearchShardsAction.java | 10 ++-- .../query/TransportValidateQueryAction.java | 5 +- .../explain/TransportExplainAction.java | 4 +- .../action/search/TransportSearchAction.java | 4 +- .../metadata/IndexNameExpressionResolver.java | 47 +++++++++--------- .../elasticsearch/indices/IndicesService.java | 11 ++--- .../elasticsearch/search/SearchService.java | 6 +-- .../IndexNameExpressionResolverTests.java | 49 ++++++++++++++----- 8 files changed, 79 insertions(+), 57 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java index eaa387be34de4..39006cd1e8407 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java @@ -44,7 +44,6 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.function.Function; public class TransportClusterSearchShardsAction extends TransportMasterNodeReadAction { @@ -89,12 +88,11 @@ protected void masterOperation(final ClusterSearchShardsRequest request, final C String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request); Map> routingMap = indexNameExpressionResolver.resolveSearchRouting(state, request.routing(), request.indices()); Map indicesAndFilters = new HashMap<>(); - Function indexToAliasFilter = indicesService.buildAliasFilter(clusterState, request.indices()); - Function indexToAliases = indexNameExpressionResolver.indexAliases(clusterState, aliasMetadata -> true, true, - request.indices()); + Set indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices()); for (String index : concreteIndices) { - final AliasFilter aliasFilter = indexToAliasFilter.apply(index); - final String[] aliases = indexToAliases.apply(index); + final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, indicesAndAliases); + final String[] aliases = indexNameExpressionResolver.indexAliases(clusterState, index, aliasMetadata -> true, true, + indicesAndAliases); indicesAndFilters.put(index, new AliasFilter(aliasFilter.getQueryBuilder(), aliases)); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java index 614647494dc0a..6e10d3d42187f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java @@ -115,8 +115,9 @@ protected void doExecute(Task task, ValidateQueryRequest request, ActionListener @Override protected ShardValidateQueryRequest newShardRequest(int numShards, ShardRouting shard, ValidateQueryRequest request) { - final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterService.state(), request.indices()) - .apply(shard.getIndexName()); + final ClusterState clusterState = clusterService.state(); + final Set indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices()); + final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, shard.getIndexName(), indicesAndAliases); return new ShardValidateQueryRequest(shard.shardId(), aliasFilter, request); } diff --git a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java index f819e6deb2043..fe8475322592f 100644 --- a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java +++ b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java @@ -52,6 +52,7 @@ import org.elasticsearch.transport.TransportService; import java.io.IOException; +import java.util.Set; /** * Explain transport action. Computes the explain on the targeted shard. @@ -83,7 +84,8 @@ protected boolean resolveIndex(ExplainRequest request) { @Override protected void resolveRequest(ClusterState state, InternalRequest request) { - final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.request().index()).apply(request.concreteIndex()); + final Set indicesAndAliases = indexNameExpressionResolver.resolveExpressions(state, request.request().index()); + final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(), indicesAndAliases); request.request().filteringAlias(aliasFilter); // Fail fast on the node that received the request. if (request.request().routing() == null && state.getMetaData().routingRequired(request.concreteIndex())) { diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index 3b6b088234dfd..6eaf53b87c34c 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -116,10 +116,10 @@ public TransportSearchAction(ThreadPool threadPool, TransportService transportSe private Map buildPerIndexAliasFilter(SearchRequest request, ClusterState clusterState, Index[] concreteIndices, Map remoteAliasMap) { final Map aliasFilterMap = new HashMap<>(); - final Function indexToAliasFilter = searchService.buildAliasFilter(clusterState, request.indices());; + final Set indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices()); for (Index index : concreteIndices) { clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index.getName()); - AliasFilter aliasFilter = indexToAliasFilter.apply(index.getName()); + AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), indicesAndAliases); assert aliasFilter != null; aliasFilterMap.put(index.getUUID(), aliasFilter); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 57c02412d3b48..ab0d1ae3e0229 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -44,6 +44,7 @@ import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -51,7 +52,6 @@ import java.util.Map; import java.util.Set; import java.util.SortedMap; -import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -311,40 +311,41 @@ public String resolveDateMathExpression(String dateExpression) { return dateMathExpressionResolver.resolveExpression(dateExpression, new Context(null, null)); } + /** + * Resolve an array of expressions to the set of indices and aliases that these expressions match. + */ + public Set resolveExpressions(ClusterState state, String... expressions) { + Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false); + List resolvedExpressions = Arrays.asList(expressions); + for (ExpressionResolver expressionResolver : expressionResolvers) { + resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions); + } + return Collections.unmodifiableSet(new HashSet<>(resolvedExpressions)); + } + /** * Iterates through the list of indices and selects the effective list of filtering aliases for the * given index. *

Only aliases with filters are returned. If the indices list contains a non-filtering reference to * the index itself - null is returned. Returns {@code null} if no filtering is required. + * NOTE: The provided expressions must have been resolved already via {@link #resolveExpressions}. */ - public Function filteringAliases(ClusterState state, String... expressions) { - return indexAliases(state, AliasMetaData::filteringRequired, false, expressions); + public String[] filteringAliases(ClusterState state, String index, Set resolvedExpressions) { + return indexAliases(state, index, AliasMetaData::filteringRequired, false, resolvedExpressions); } /** * Iterates through the list of indices and selects the effective list of required aliases for the given index. *

Only aliases where the given predicate tests successfully are returned. If the indices list contains a non-required reference to * the index itself - null is returned. Returns {@code null} if no filtering is required. + *

NOTE: the provided expressions must have been resolved already via {@link #resolveExpressions}. */ - public Function indexAliases(ClusterState state, Predicate requiredAlias, boolean skipIdentity, - String... expressions) { - // expand the aliases wildcard - List resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList(); - Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false); - for (ExpressionResolver expressionResolver : expressionResolvers) { - resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions); - } - + public String[] indexAliases(ClusterState state, String index, Predicate requiredAlias, boolean skipIdentity, + Set resolvedExpressions) { if (isAllIndices(resolvedExpressions)) { - return index -> null; + return null; } - final Set resolvedExpressionsSet = new HashSet<>(resolvedExpressions); - return index -> indexAliases(state, index, requiredAlias, skipIdentity, resolvedExpressionsSet); - } - - private String[] indexAliases(ClusterState state, String index, Predicate requiredAlias, boolean skipIdentity, - Set resolvedExpressions) { final IndexMetaData indexMetaData = state.metaData().getIndices().get(index); if (indexMetaData == null) { // Shouldn't happen @@ -496,7 +497,7 @@ public Map> resolveSearchRoutingAllIndices(MetaData metaData * @param aliasesOrIndices the array containing index names * @return true if the provided array maps to all indices, false otherwise */ - public static boolean isAllIndices(List aliasesOrIndices) { + public static boolean isAllIndices(Collection aliasesOrIndices) { return aliasesOrIndices == null || aliasesOrIndices.isEmpty() || isExplicitAllPattern(aliasesOrIndices); } @@ -507,8 +508,8 @@ public static boolean isAllIndices(List aliasesOrIndices) { * @param aliasesOrIndices the array containing index names * @return true if the provided array explicitly maps to all indices, false otherwise */ - static boolean isExplicitAllPattern(List aliasesOrIndices) { - return aliasesOrIndices != null && aliasesOrIndices.size() == 1 && MetaData.ALL.equals(aliasesOrIndices.get(0)); + static boolean isExplicitAllPattern(Collection aliasesOrIndices) { + return aliasesOrIndices != null && aliasesOrIndices.size() == 1 && MetaData.ALL.equals(aliasesOrIndices.iterator().next()); } /** @@ -581,7 +582,7 @@ public long getStartTime() { /** * This is used to prevent resolving aliases to concrete indices but this also means * that we might return aliases that point to a closed index. This is currently only used - * by {@link #filteringAliases(ClusterState, String...)} since it's the only one that needs aliases + * by {@link #filteringAliases(ClusterState, String, Set)} since it's the only one that needs aliases */ boolean isPreserveAliases() { return preserveAliases; diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index c9be1dd01b825..210f173ecbb56 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1376,7 +1376,7 @@ interface IndexDeletionAllowedPredicate { (Index index, IndexSettings indexSettings) -> canDeleteIndexContents(index, indexSettings); private final IndexDeletionAllowedPredicate ALWAYS_TRUE = (Index index, IndexSettings indexSettings) -> true; - public Function buildAliasFilter(ClusterState state, String... expressions) { + public AliasFilter buildAliasFilter(ClusterState state, String index, Set resolvedExpressions) { /* Being static, parseAliasFilter doesn't have access to whatever guts it needs to parse a query. Instead of passing in a bunch * of dependencies we pass in a function that can perform the parsing. */ CheckedFunction filterParser = bytes -> { @@ -1385,12 +1385,9 @@ public Function buildAliasFilter(ClusterState state, String return parseInnerQueryBuilder(parser); } }; - Function indexToAliases = indexNameExpressionResolver.filteringAliases(state, expressions); - return index -> { - IndexMetaData indexMetaData = state.metaData().index(index); - String[] aliases = indexToAliases.apply(index); - return new AliasFilter(ShardSearchRequest.parseAliasFilter(filterParser, indexMetaData, aliases), aliases); - }; + IndexMetaData indexMetaData = state.metaData().index(index); + String[] aliases = indexNameExpressionResolver.filteringAliases(state, index, resolvedExpressions); + return new AliasFilter(ShardSearchRequest.parseAliasFilter(filterParser, indexMetaData, aliases), aliases); } /** diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 20fc2faebbe08..0b22f5d660655 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -109,11 +109,11 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -1005,8 +1005,8 @@ public void run() { } } - public Function buildAliasFilter(ClusterState state, String... expressions) { - return indicesService.buildAliasFilter(state, expressions); + public AliasFilter buildAliasFilter(ClusterState state, String index, Set resolvedExpressions) { + return indicesService.buildAliasFilter(state, index, resolvedExpressions); } /** diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java index 6428dacb46e1b..7154d324064e4 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -46,6 +46,7 @@ import java.util.EnumSet; import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.Function; import static org.elasticsearch.common.util.set.Sets.newHashSet; @@ -1001,20 +1002,39 @@ public void testFilterClosedIndicesOnAliases() { assertArrayEquals(new String[] {"test-0"}, strings); } + public void testResolveExpressions() { + MetaData.Builder mdBuilder = MetaData.builder() + .put(indexBuilder("test-0").state(State.OPEN).putAlias(AliasMetaData.builder("alias-0").filter("{ \"term\": \"foo\"}"))) + .put(indexBuilder("test-1").state(State.OPEN).putAlias(AliasMetaData.builder("alias-1"))); + ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); + + assertEquals(new HashSet<>(Arrays.asList("alias-0", "alias-1")), + indexNameExpressionResolver.resolveExpressions(state, "alias-*")); + assertEquals(new HashSet<>(Arrays.asList("test-0", "alias-0", "alias-1")), + indexNameExpressionResolver.resolveExpressions(state, "test-0", "alias-*")); + assertEquals(new HashSet<>(Arrays.asList("test-0", "test-1", "alias-0", "alias-1")), + indexNameExpressionResolver.resolveExpressions(state, "test-*", "alias-*")); + assertEquals(new HashSet<>(Arrays.asList("test-1", "alias-1")), + indexNameExpressionResolver.resolveExpressions(state, "*-1")); + } + public void testFilteringAliases() { MetaData.Builder mdBuilder = MetaData.builder() .put(indexBuilder("test-0").state(State.OPEN).putAlias(AliasMetaData.builder("alias-0").filter("{ \"term\": \"foo\"}"))) .put(indexBuilder("test-1").state(State.OPEN).putAlias(AliasMetaData.builder("alias-1"))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); - String[] strings = indexNameExpressionResolver.filteringAliases(state, "alias-*").apply("test-0"); + Set resolvedExpressions = new HashSet<>(Arrays.asList("alias-0", "alias-1")); + String[] strings = indexNameExpressionResolver.filteringAliases(state, "test-0", resolvedExpressions); assertArrayEquals(new String[] {"alias-0"}, strings); // concrete index supersedes filtering alias - strings = indexNameExpressionResolver.filteringAliases(state, "test-0,alias-*").apply("test-0"); + resolvedExpressions = new HashSet<>(Arrays.asList("test-0", "alias-0", "alias-1")); + strings = indexNameExpressionResolver.filteringAliases(state, "test-0", resolvedExpressions); assertNull(strings); - strings = indexNameExpressionResolver.filteringAliases(state, "test-*,alias-*").apply("test-0"); + resolvedExpressions = new HashSet<>(Arrays.asList("test-0", "test-1", "alias-0", "alias-1")); + strings = indexNameExpressionResolver.filteringAliases(state, "test-0", resolvedExpressions); assertNull(strings); } @@ -1026,7 +1046,8 @@ public void testIndexAliases() { .putAlias(AliasMetaData.builder("test-alias-non-filtering")) ); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); - String[] strings = indexNameExpressionResolver.indexAliases(state, x -> true, true, "test-*").apply("test-0"); + Set resolvedExpressions = indexNameExpressionResolver.resolveExpressions(state, "test-*"); + String[] strings = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, true, resolvedExpressions); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias-0", "test-alias-1", "test-alias-non-filtering"}, strings); } @@ -1039,14 +1060,16 @@ public void testIndexAliasesSkipIdentity() { ); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); - String[] aliases = indexNameExpressionResolver.indexAliases(state, x -> true, false, "test-*").apply("test-0"); + Set resolvedExpressions = new HashSet<>(Arrays.asList("test-0", "test-alias")); + String[] aliases = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, false, resolvedExpressions); assertNull(aliases); - aliases = indexNameExpressionResolver.indexAliases(state, x -> true, true, "test-*").apply("test-0"); + aliases = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, true, resolvedExpressions); assertArrayEquals(new String[] {"test-alias"}, aliases); - aliases = indexNameExpressionResolver.indexAliases(state, x -> true, false, "other-*").apply("test-0"); + resolvedExpressions = Collections.singleton("other-alias"); + aliases = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, false, resolvedExpressions); assertArrayEquals(new String[] {"other-alias"}, aliases); - aliases = indexNameExpressionResolver.indexAliases(state, x -> true, true, "other-*").apply("test-0"); + aliases = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, true, resolvedExpressions); assertArrayEquals(new String[] {"other-alias"}, aliases); } @@ -1057,7 +1080,7 @@ public void testConcreteWriteIndexSuccessful() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(testZeroWriteIndex ? true : null))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, x -> true, true, "test-*").apply("test-0"); + .indexAliases(state, "test-0", x -> true, true, new HashSet<>(Arrays.asList("test-0", "test-alias"))); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); IndicesRequest request = new IndicesRequest() { @@ -1118,7 +1141,7 @@ public void testConcreteWriteIndexWithWildcardExpansion() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(testZeroWriteIndex ? randomFrom(false, null) : true))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, x -> true, true, "test-*").apply("test-0"); + .indexAliases(state, "test-0", x -> true, true, new HashSet<>(Arrays.asList("test-0", "test-1", "test-alias"))); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); IndicesRequest request = new IndicesRequest() { @@ -1146,7 +1169,7 @@ public void testConcreteWriteIndexWithNoWriteIndexWithSingleIndex() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(false))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, x -> true, true, "test-*").apply("test-0"); + .indexAliases(state, "test-0", x -> true, true, new HashSet<>(Arrays.asList("test-0", "test-alias"))); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); DocWriteRequest request = randomFrom(new IndexRequest("test-alias"), @@ -1166,7 +1189,7 @@ public void testConcreteWriteIndexWithNoWriteIndexWithMultipleIndices() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(randomFrom(false, null)))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, x -> true, true, "test-*").apply("test-0"); + .indexAliases(state, "test-0", x -> true, true, new HashSet<>(Arrays.asList("test-0", "test-1", "test-alias"))); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); DocWriteRequest request = randomFrom(new IndexRequest("test-alias"), @@ -1187,7 +1210,7 @@ public void testAliasResolutionNotAllowingMultipleIndices() { .putAlias(AliasMetaData.builder("test-alias").writeIndex(randomFrom(!test0WriteIndex, null)))); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); String[] strings = indexNameExpressionResolver - .indexAliases(state, x -> true, true, "test-*").apply("test-0"); + .indexAliases(state, "test-0", x -> true, true, new HashSet<>(Arrays.asList("test-0", "test-1", "test-alias"))); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias"}, strings); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, From dcff4b22ab996afd41fb15a5a89ddcce2f194d28 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 22 Mar 2019 16:26:04 +0100 Subject: [PATCH 3/7] iter --- .../cluster/metadata/IndexNameExpressionResolver.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index ab0d1ae3e0229..878171d32cd78 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -376,7 +376,10 @@ public String[] indexAliases(ClusterState state, String index, Predicate Date: Tue, 26 Mar 2019 08:53:26 +0100 Subject: [PATCH 4/7] iter --- .../metadata/IndexNameExpressionResolver.java | 67 ++++++++++++------- ...ExpressionResolverAliasIterationTests.java | 30 +++++++++ ...sionResolverExpressionsIterationTests.java | 30 +++++++++ .../IndexNameExpressionResolverTests.java | 12 +++- 4 files changed, 114 insertions(+), 25 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java create mode 100644 server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 878171d32cd78..099579f181e65 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -19,14 +19,13 @@ package org.elasticsearch.cluster.metadata; -import com.carrotsearch.hppc.cursors.ObjectCursor; - import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.time.DateFormatter; @@ -50,10 +49,14 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.SortedMap; +import java.util.Spliterators; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import static java.util.Collections.unmodifiableList; @@ -334,6 +337,9 @@ public String[] filteringAliases(ClusterState state, String index, Set r return indexAliases(state, index, AliasMetaData::filteringRequired, false, resolvedExpressions); } + // pkg-private for testing + Boolean forceIterateIndexAliases; + /** * Iterates through the list of indices and selects the effective list of required aliases for the given index. *

Only aliases where the given predicate tests successfully are returned. If the indices list contains a non-required reference to @@ -356,30 +362,41 @@ public String[] indexAliases(ClusterState state, String index, Predicate aliases = null; - for (ObjectCursor aliasMetaDataCursor : indexMetaData.getAliases().values()) { - AliasMetaData aliasMetaData = aliasMetaDataCursor.value; - if (resolvedExpressions.contains(aliasMetaData.alias())) { - if (requiredAlias.test(aliasMetaData)) { - // If required - add it to the list of aliases - if (aliases == null) { - aliases = new ArrayList<>(); - } - aliases.add(aliasMetaData.alias()); - } else { - // If not, we have a non required alias for this index - no further checking needed - return null; - } - } - } - - if (aliases == null) { + final ImmutableOpenMap indexAliases = indexMetaData.getAliases(); + final Stream aliasCandidates; + final boolean iterateIndexAliases; + if (forceIterateIndexAliases != null) { + iterateIndexAliases = forceIterateIndexAliases; + } else { + // We need the intersection of indexAliases and resolvedExpressions, so we + // iterate the smaller set and filter based on the other set. + iterateIndexAliases = indexAliases.size() <= resolvedExpressions.size(); + } + if (iterateIndexAliases) { + // faster to iterate indexAliases + aliasCandidates = StreamSupport.stream(Spliterators.spliteratorUnknownSize(indexAliases.values().iterator(), 0), false) + .map(cursor -> cursor.value) + .filter(aliasMetaData -> resolvedExpressions.contains(aliasMetaData.alias())); + } else { + // faster to iterate resolvedExpressions + aliasCandidates = resolvedExpressions.stream() + .map(indexAliases::get) + .filter(Objects::nonNull); + } + + String[] aliases = aliasCandidates + .filter(requiredAlias) + .map(AliasMetaData::alias) + .toArray(String[]::new); + if (aliases.length == 0) { return null; + } else { + // Make order predictable as we have some tests that rely on it. + // TODO: Fix tests that rely on the order, this is conceptually a set + // (and maybe this method should return a set rather than an array). + Arrays.sort(aliases); + return aliases; } - String[] aliasesArray = aliases.toArray(new String[aliases.size()]); - // Make order predictable - Arrays.sort(aliasesArray); - return aliasesArray; } /** @@ -629,6 +646,8 @@ public List resolve(Context context, List expressions) { return resolveEmptyOrTrivialWildcard(options, metaData); } + // TODO: Fix API to work with sets rather than lists since we need to convert to sets + // internally anyway. Set result = innerResolve(context, expressions, options, metaData); if (result == null) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java new file mode 100644 index 0000000000000..f2242703c6afc --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cluster.metadata; + +public class IndexNameExpressionResolverAliasIterationTests extends IndexNameExpressionResolverTests { + + protected IndexNameExpressionResolver getIndexNameExpressionResolver() { + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(); + resolver.forceIterateIndexAliases = true; + return resolver; + } + +} diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java new file mode 100644 index 0000000000000..a6431de511d4d --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cluster.metadata; + +public class IndexNameExpressionResolverExpressionsIterationTests extends IndexNameExpressionResolverTests { + + protected IndexNameExpressionResolver getIndexNameExpressionResolver() { + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(); + resolver.forceIterateIndexAliases = false; + return resolver; + } + +} diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java index 7154d324064e4..1f9ddaf1d5b9c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -61,7 +61,17 @@ import static org.hamcrest.Matchers.notNullValue; public class IndexNameExpressionResolverTests extends ESTestCase { - private final IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(); + private IndexNameExpressionResolver indexNameExpressionResolver; + + protected IndexNameExpressionResolver getIndexNameExpressionResolver() { + return new IndexNameExpressionResolver(); + } + + @Override + public void setUp() throws Exception { + super.setUp(); + indexNameExpressionResolver = getIndexNameExpressionResolver(); + } public void testIndexOptionsStrict() { MetaData.Builder mdBuilder = MetaData.builder() From 775a8e3bb44e3c7acde39172a306080ce9a0d975 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 27 Mar 2019 13:58:58 +0100 Subject: [PATCH 5/7] Remove test dependency on order. --- .../test/search_shards/10_basic.yml | 3 +- .../metadata/IndexNameExpressionResolver.java | 36 +++++++++++-------- .../elasticsearch/aliases/IndexAliasesIT.java | 6 ++-- .../IndexNameExpressionResolverTests.java | 5 +++ 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search_shards/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search_shards/10_basic.yml index 03f218b140b8f..653979073b707 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search_shards/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search_shards/10_basic.yml @@ -64,10 +64,9 @@ - length: { shards: 1 } - match: { shards.0.0.index: test_index } - match: { indices.test_index.aliases: [test_alias_filter_1, test_alias_filter_2]} - - match: { indices.test_index.filter.bool.should.0.term.field.value: value1 } + - length: { indices.test_index.filter.bool.should: 2 } - lte: { indices.test_index.filter.bool.should.0.term.field.boost: 1.0 } - gte: { indices.test_index.filter.bool.should.0.term.field.boost: 1.0 } - - match: { indices.test_index.filter.bool.should.1.term.field.value: value2} - lte: { indices.test_index.filter.bool.should.1.term.field.boost: 1.0 } - gte: { indices.test_index.filter.bool.should.1.term.field.boost: 1.0 } - match: { indices.test_index.filter.bool.adjust_pure_negative: true} diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 099579f181e65..678cc0474cabd 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -55,7 +55,6 @@ import java.util.Spliterators; import java.util.function.Predicate; import java.util.stream.Collectors; -import java.util.stream.Stream; import java.util.stream.StreamSupport; import static java.util.Collections.unmodifiableList; @@ -363,7 +362,6 @@ public String[] indexAliases(ClusterState state, String index, Predicate indexAliases = indexMetaData.getAliases(); - final Stream aliasCandidates; final boolean iterateIndexAliases; if (forceIterateIndexAliases != null) { iterateIndexAliases = forceIterateIndexAliases; @@ -372,31 +370,39 @@ public String[] indexAliases(ClusterState state, String index, Predicate cursor.value) - .filter(aliasMetaData -> resolvedExpressions.contains(aliasMetaData.alias())); + .filter(aliasMetaData -> resolvedExpressions.contains(aliasMetaData.alias())) + .toArray(AliasMetaData[]::new); } else { // faster to iterate resolvedExpressions aliasCandidates = resolvedExpressions.stream() .map(indexAliases::get) - .filter(Objects::nonNull); + .filter(Objects::nonNull) + .toArray(AliasMetaData[]::new); } - String[] aliases = aliasCandidates - .filter(requiredAlias) - .map(AliasMetaData::alias) - .toArray(String[]::new); - if (aliases.length == 0) { + List aliases = null; + for (AliasMetaData aliasMetaData : aliasCandidates) { + if (requiredAlias.test(aliasMetaData)) { + // If required - add it to the list of aliases + if (aliases == null) { + aliases = new ArrayList<>(); + } + aliases.add(aliasMetaData.alias()); + } else { + // If not, we have a non required alias for this index - no further checking needed + return null; + } + } + if (aliases == null) { return null; - } else { - // Make order predictable as we have some tests that rely on it. - // TODO: Fix tests that rely on the order, this is conceptually a set - // (and maybe this method should return a set rather than an array). - Arrays.sort(aliases); - return aliases; } + return aliases.toArray(new String[aliases.size()]); } /** diff --git a/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java b/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java index a1058b03dacb0..a275dd0142663 100644 --- a/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java +++ b/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java @@ -319,7 +319,7 @@ public void testSearchingFilteringAliasesTwoIndices() throws Exception { refresh(); - logger.info("--> checking filtering alias for two indices"); + /*logger.info("--> checking filtering alias for two indices"); SearchResponse searchResponse = client().prepareSearch("foos").setQuery(QueryBuilders.matchAllQuery()).get(); assertHits(searchResponse.getHits(), "1", "5"); assertThat(client().prepareSearch("foos") @@ -335,10 +335,10 @@ public void testSearchingFilteringAliasesTwoIndices() throws Exception { searchResponse = client().prepareSearch("foos", "test1").setQuery(QueryBuilders.matchAllQuery()).get(); assertHits(searchResponse.getHits(), "1", "2", "3", "4", "5"); assertThat(client().prepareSearch("foos", "test1") - .setSize(0).setQuery(QueryBuilders.matchAllQuery()).get().getHits().getTotalHits().value, equalTo(5L)); + .setSize(0).setQuery(QueryBuilders.matchAllQuery()).get().getHits().getTotalHits().value, equalTo(5L));*/ logger.info("--> checking filtering alias for two indices and non-filtering alias for one index"); - searchResponse = client().prepareSearch("foos", "aliasToTest1").setQuery(QueryBuilders.matchAllQuery()).get(); + SearchResponse searchResponse = client().prepareSearch("foos", "aliasToTest1").setQuery(QueryBuilders.matchAllQuery()).get(); assertHits(searchResponse.getHits(), "1", "2", "3", "4", "5"); assertThat(client().prepareSearch("foos", "aliasToTest1") .setSize(0).setQuery(QueryBuilders.matchAllQuery()).get().getHits().getTotalHits().value, equalTo(5L)); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java index 1f9ddaf1d5b9c..14f75ae769dbd 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -1057,9 +1057,14 @@ public void testIndexAliases() { ); ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); Set resolvedExpressions = indexNameExpressionResolver.resolveExpressions(state, "test-*"); + String[] strings = indexNameExpressionResolver.indexAliases(state, "test-0", x -> true, true, resolvedExpressions); Arrays.sort(strings); assertArrayEquals(new String[] {"test-alias-0", "test-alias-1", "test-alias-non-filtering"}, strings); + + strings = indexNameExpressionResolver.indexAliases(state, "test-0", x -> x.alias().equals("test-alias-1"), true, + resolvedExpressions); + assertArrayEquals(null, strings); } public void testIndexAliasesSkipIdentity() { From 3f8d3e423d405a4609db0e7dbed5136dcd9e206d Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 2 Apr 2019 11:46:02 +0200 Subject: [PATCH 6/7] iter --- .../test/java/org/elasticsearch/aliases/IndexAliasesIT.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java b/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java index a275dd0142663..a1058b03dacb0 100644 --- a/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java +++ b/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java @@ -319,7 +319,7 @@ public void testSearchingFilteringAliasesTwoIndices() throws Exception { refresh(); - /*logger.info("--> checking filtering alias for two indices"); + logger.info("--> checking filtering alias for two indices"); SearchResponse searchResponse = client().prepareSearch("foos").setQuery(QueryBuilders.matchAllQuery()).get(); assertHits(searchResponse.getHits(), "1", "5"); assertThat(client().prepareSearch("foos") @@ -335,10 +335,10 @@ public void testSearchingFilteringAliasesTwoIndices() throws Exception { searchResponse = client().prepareSearch("foos", "test1").setQuery(QueryBuilders.matchAllQuery()).get(); assertHits(searchResponse.getHits(), "1", "2", "3", "4", "5"); assertThat(client().prepareSearch("foos", "test1") - .setSize(0).setQuery(QueryBuilders.matchAllQuery()).get().getHits().getTotalHits().value, equalTo(5L));*/ + .setSize(0).setQuery(QueryBuilders.matchAllQuery()).get().getHits().getTotalHits().value, equalTo(5L)); logger.info("--> checking filtering alias for two indices and non-filtering alias for one index"); - SearchResponse searchResponse = client().prepareSearch("foos", "aliasToTest1").setQuery(QueryBuilders.matchAllQuery()).get(); + searchResponse = client().prepareSearch("foos", "aliasToTest1").setQuery(QueryBuilders.matchAllQuery()).get(); assertHits(searchResponse.getHits(), "1", "2", "3", "4", "5"); assertThat(client().prepareSearch("foos", "aliasToTest1") .setSize(0).setQuery(QueryBuilders.matchAllQuery()).get().getHits().getTotalHits().value, equalTo(5L)); From 0d83ee42f8b96ba25aaa385d8819c61a24d9b149 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 3 Apr 2019 14:57:33 +0200 Subject: [PATCH 7/7] Use pkg-private method instead of member. --- .../metadata/IndexNameExpressionResolver.java | 20 +++++++++---------- ...ExpressionResolverAliasIterationTests.java | 9 ++++++--- ...sionResolverExpressionsIterationTests.java | 9 ++++++--- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 72b327ddeb84e..19c6d31ccc82a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -350,8 +350,15 @@ public String[] filteringAliases(ClusterState state, String index, Set r return indexAliases(state, index, AliasMetaData::filteringRequired, false, resolvedExpressions); } + /** + * Whether to generate the candidate set from index aliases, or from the set of resolved expressions. + * @param indexAliasesSize the number of aliases of the index + * @param resolvedExpressionsSize the number of resolved expressions + */ // pkg-private for testing - Boolean forceIterateIndexAliases; + boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) { + return indexAliasesSize <= resolvedExpressionsSize; + } /** * Iterates through the list of indices and selects the effective list of required aliases for the given index. @@ -376,17 +383,8 @@ public String[] indexAliases(ClusterState state, String index, Predicate indexAliases = indexMetaData.getAliases(); - final boolean iterateIndexAliases; - if (forceIterateIndexAliases != null) { - iterateIndexAliases = forceIterateIndexAliases; - } else { - // We need the intersection of indexAliases and resolvedExpressions, so we - // iterate the smaller set and filter based on the other set. - iterateIndexAliases = indexAliases.size() <= resolvedExpressions.size(); - } - final AliasMetaData[] aliasCandidates; - if (iterateIndexAliases) { + if (iterateIndexAliases(indexAliases.size(), resolvedExpressions.size())) { // faster to iterate indexAliases aliasCandidates = StreamSupport.stream(Spliterators.spliteratorUnknownSize(indexAliases.values().iterator(), 0), false) .map(cursor -> cursor.value) diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java index f2242703c6afc..13d3cfd6cea95 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverAliasIterationTests.java @@ -22,9 +22,12 @@ public class IndexNameExpressionResolverAliasIterationTests extends IndexNameExpressionResolverTests { protected IndexNameExpressionResolver getIndexNameExpressionResolver() { - IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(); - resolver.forceIterateIndexAliases = true; - return resolver; + return new IndexNameExpressionResolver() { + @Override + boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) { + return true; + } + }; } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java index a6431de511d4d..00d46aad0e8cd 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverExpressionsIterationTests.java @@ -22,9 +22,12 @@ public class IndexNameExpressionResolverExpressionsIterationTests extends IndexNameExpressionResolverTests { protected IndexNameExpressionResolver getIndexNameExpressionResolver() { - IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(); - resolver.forceIterateIndexAliases = false; - return resolver; + return new IndexNameExpressionResolver() { + @Override + boolean iterateIndexAliases(int indexAliasesSize, int resolvedExpressionsSize) { + return false; + } + }; } }