From d8c934a7fa78d93a40a5cf9fdb95de4e74e0924b Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Thu, 24 Nov 2016 09:43:42 +0100 Subject: [PATCH] Use index uuid as key in the alias filter map rather than the index name (#21749) The index uuid is unique across multiple clusters, while the index name is not. Using the index uuid to look up filters in the alias filters map is better and will be needed for multi cluster search. --- .../search/AbstractSearchAsyncAction.java | 5 ++--- .../action/search/TransportSearchAction.java | 22 +++++++++++-------- .../metadata/IndexNameExpressionResolver.java | 4 ++-- .../action/search/SearchAsyncActionTests.java | 5 ++++- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java b/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java index bd19e924a37..c634381d851 100644 --- a/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java @@ -93,8 +93,6 @@ abstract class AbstractSearchAsyncAction this.aliasFilter = aliasFilter; } - - public void start() { if (expectedSuccessfulOps == 0) { //no search shards to search on, bail with empty response @@ -125,7 +123,8 @@ abstract class AbstractSearchAsyncAction if (node == null) { onFirstPhaseResult(shardIndex, shard, null, shardIt, new NoShardAvailableActionException(shardIt.shardId())); } else { - AliasFilter filter = this.aliasFilter.get(shard.index().getName()); + AliasFilter filter = this.aliasFilter.get(shard.index().getUUID()); + assert filter != null; ShardSearchTransportRequest transportRequest = new ShardSearchTransportRequest(request, shardIt.shardId(), shardsIts.size(), filter, startTime()); sendExecuteFirstPhase(node, transportRequest , new ActionListener() { diff --git a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index 5091dca26a9..b94f4b35096 100644 --- a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.Index; import org.elasticsearch.search.SearchService; import org.elasticsearch.search.internal.AliasFilter; import org.elasticsearch.tasks.Task; @@ -72,14 +73,13 @@ public class TransportSearchAction extends HandledTransportAction buildPerIndexAliasFilter(SearchRequest request, ClusterState clusterState, String...concreteIndices) { + private Map buildPerIndexAliasFilter(SearchRequest request, ClusterState clusterState, Index[] concreteIndices) { final Map aliasFilterMap = new HashMap<>(); - for (String index : concreteIndices) { - clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index); - AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index, request.indices()); - if (aliasFilter != null) { - aliasFilterMap.put(index, aliasFilter); - } + for (Index index : concreteIndices) { + clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index.getName()); + AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, index.getName(), request.indices()); + assert aliasFilter != null; + aliasFilterMap.put(index.getUUID(), aliasFilter); } return aliasFilterMap; } @@ -94,11 +94,15 @@ public class TransportSearchAction extends HandledTransportAction aliasFilter = buildPerIndexAliasFilter(searchRequest, clusterState, concreteIndices); + Map aliasFilter = buildPerIndexAliasFilter(searchRequest, clusterState, indices); Map> routingMap = indexNameExpressionResolver.resolveSearchRouting(clusterState, searchRequest.routing(), searchRequest.indices()); + String[] concreteIndices = new String[indices.length]; + for (int i = 0; i < indices.length; i++) { + concreteIndices[i] = indices[i].getName(); + } GroupShardsIterator shardIterators = clusterService.operationRouting().searchShards(clusterState, concreteIndices, routingMap, searchRequest.preference()); failIfOverShardCountLimit(clusterService, shardIterators.size()); diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 377409dd86b..0c7a2db68dc 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -130,9 +130,9 @@ public class IndexNameExpressionResolver extends AbstractComponent { * @throws IllegalArgumentException if one of the aliases resolve to multiple indices and the provided * indices options in the context don't allow such a case. */ - public String[] concreteIndexNames(ClusterState state, IndicesOptions options, long startTime, String... indexExpressions) { + public Index[] concreteIndices(ClusterState state, IndicesOptions options, long startTime, String... indexExpressions) { Context context = new Context(state, options, startTime); - return concreteIndexNames(context, indexExpressions); + return concreteIndices(context, indexExpressions); } String[] concreteIndexNames(Context context, String... indexExpressions) { diff --git a/core/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java b/core/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java index 5ab35085e40..c4645e20a5e 100644 --- a/core/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java +++ b/core/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java @@ -27,6 +27,7 @@ import org.elasticsearch.cluster.routing.RecoverySource; import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.UnassignedInfo; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; @@ -34,6 +35,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchPhaseResult; import org.elasticsearch.search.SearchShardTarget; +import org.elasticsearch.search.internal.AliasFilter; import org.elasticsearch.search.internal.ShardSearchTransportRequest; import org.elasticsearch.test.ESTestCase; @@ -84,8 +86,9 @@ public class SearchAsyncActionTests extends ESTestCase { }; Map lookup = new HashMap<>(); lookup.put(primaryNode.getId(), primaryNode); + Map aliasFilters = Collections.singletonMap("_na_", new AliasFilter(null, Strings.EMPTY_ARRAY)); AbstractSearchAsyncAction asyncAction = new AbstractSearchAsyncAction(logger, transportService, lookup::get, - Collections.emptyMap(), null, request, responseListener, shardsIter, 0, 0, null) { + aliasFilters, null, request, responseListener, shardsIter, 0, 0, null) { TestSearchResponse response = new TestSearchResponse(); @Override