From b45f64eaf05d232ee23be74253a34ed6735b4c28 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 16 Apr 2018 15:53:00 +0200 Subject: [PATCH 1/8] Add additional shards routing info in ShardSearchRequest This commit adds two new methods to ShardSearchRequest: * #numberOfShardsIndex() that returns the number of shards of this index that participates in the request. * #remapShardId() that returns the remapped shard id of this shard for this request. The remapped shard id is the id of the requested shard among all shards of this index that are part of the request. Note that the remapped shard id is equal to the original shard id if all shards of this index are part of the request. These informations are useful when the _search is executed with a preference (or a routing) that restricts the number of shards requested for an index. This change allows to fix a bug in sliced scrolls executed with a preference (or a routing). Instead of computing the slice query from the total number of shards in the index, this change allows to compute this number from the number of shards per index that participates in the request. Fixes #27550 --- .../search/AbstractSearchAsyncAction.java | 24 +++-- .../action/search/InitialSearchPhase.java | 21 ++++- .../search/DefaultSearchContext.java | 16 +++- .../elasticsearch/search/SearchService.java | 2 +- .../internal/ShardSearchLocalRequest.java | 44 +++++++-- .../search/internal/ShardSearchRequest.java | 20 +++- .../internal/ShardSearchTransportRequest.java | 22 ++++- .../search/slice/SliceBuilder.java | 27 +++++- .../AbstractSearchAsyncActionTests.java | 85 +++++++++++++++-- .../index/SearchSlowLogTests.java | 10 ++ .../search/DefaultSearchContextTests.java | 6 +- .../search/SearchServiceTests.java | 28 +++--- .../ShardSearchTransportRequestTests.java | 6 +- .../search/slice/SearchSliceIT.java | 52 ++++++++--- .../search/slice/SliceBuilderTests.java | 93 ++++++++++++++++--- 15 files changed, 373 insertions(+), 83 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java index aad2638bd9de3..020f3e0d3bedb 100644 --- a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java @@ -37,6 +37,7 @@ import org.elasticsearch.search.internal.ShardSearchTransportRequest; import org.elasticsearch.transport.Transport; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.concurrent.Executor; @@ -128,17 +129,17 @@ public final void executeNextPhase(SearchPhase currentPhase, SearchPhase nextPha onPhaseFailure(currentPhase, "all shards failed", cause); } else { Boolean allowPartialResults = request.allowPartialSearchResults(); - assert allowPartialResults != null : "SearchRequest missing setting for allowPartialSearchResults"; + assert allowPartialResults != null : "SearchRequest missing setting for allowPartialSearchResults"; if (allowPartialResults == false && shardFailures.get() != null ){ if (logger.isDebugEnabled()) { final ShardOperationFailedException[] shardSearchFailures = ExceptionsHelper.groupBy(buildShardFailures()); Throwable cause = shardSearchFailures.length == 0 ? null : ElasticsearchException.guessRootCauses(shardSearchFailures[0].getCause())[0]; - logger.debug(() -> new ParameterizedMessage("{} shards failed for phase: [{}]", + logger.debug(() -> new ParameterizedMessage("{} shards failed for phase: [{}]", shardSearchFailures.length, getName()), cause); } - onPhaseFailure(currentPhase, "Partial shards failure", null); - } else { + onPhaseFailure(currentPhase, "Partial shards failure", null); + } else { if (logger.isTraceEnabled()) { final String resultsFrom = results.getSuccessfulResults() .map(r -> r.getSearchShardTarget().toString()).collect(Collectors.joining(",")); @@ -271,14 +272,14 @@ public final SearchRequest getRequest() { @Override public final SearchResponse buildSearchResponse(InternalSearchResponse internalSearchResponse, String scrollId) { - + ShardSearchFailure[] failures = buildShardFailures(); Boolean allowPartialResults = request.allowPartialSearchResults(); assert allowPartialResults != null : "SearchRequest missing setting for allowPartialSearchResults"; if (allowPartialResults == false && failures.length > 0){ - raisePhaseFailure(new SearchPhaseExecutionException("", "Shard failures", null, failures)); - } - + raisePhaseFailure(new SearchPhaseExecutionException("", "Shard failures", null, failures)); + } + return new SearchResponse(internalSearchResponse, scrollId, getNumShards(), successfulOps.get(), skippedOps.get(), buildTookInMillis(), failures, clusters); } @@ -318,8 +319,11 @@ public final ShardSearchTransportRequest buildShardSearchRequest(SearchShardIter AliasFilter filter = aliasFilter.get(shardIt.shardId().getIndex().getUUID()); assert filter != null; float indexBoost = concreteIndexBoosts.getOrDefault(shardIt.shardId().getIndex().getUUID(), DEFAULT_INDEX_BOOST); - return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), getNumShards(), - filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); + int[] indexShards = getIndexShards(shardIt.shardId().getIndex()); + int remapShardId = Arrays.binarySearch(indexShards, shardIt.shardId().getId()); + assert remapShardId >= 0; + return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), remapShardId, + indexShards.length, getNumShards(), filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); } /** diff --git a/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java index 559c7ca102e6b..8a0f5d5477421 100644 --- a/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java @@ -23,15 +23,18 @@ import org.elasticsearch.action.NoShardAvailableActionException; import org.elasticsearch.action.support.TransportActions; import org.elasticsearch.cluster.routing.GroupShardsIterator; +import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.AtomicArray; +import org.elasticsearch.index.Index; import org.elasticsearch.search.SearchPhaseResult; import org.elasticsearch.search.SearchShardTarget; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; @@ -131,7 +134,7 @@ public final void run() throws IOException { if (shardsIts.size() > 0) { int maxConcurrentShardRequests = Math.min(this.maxConcurrentShardRequests, shardsIts.size()); final boolean success = shardExecutionIndex.compareAndSet(0, maxConcurrentShardRequests); - assert success; + assert success; assert request.allowPartialSearchResults() != null : "SearchRequest missing setting for allowPartialSearchResults"; if (request.allowPartialSearchResults() == false) { final StringBuilder missingShards = new StringBuilder(); @@ -140,7 +143,7 @@ public final void run() throws IOException { final SearchShardIterator shardRoutings = shardsIts.get(index); if (shardRoutings.size() == 0) { if(missingShards.length() >0 ){ - missingShards.append(", "); + missingShards.append(", "); } missingShards.append(shardRoutings.shardId()); } @@ -377,4 +380,18 @@ protected void skipShard(SearchShardIterator iterator) { successfulShardExecution(iterator); } + /** + * Returns the list of shard ids in the request that match the provided {@link Index}. + */ + protected int[] getIndexShards(Index index) { + List shards = new ArrayList<>(); + for (ShardIterator it : shardsIts) { + if (index.equals(it.shardId().getIndex())) { + shards.add(it.shardId().getId()); + } + } + Collections.sort(shards); + return shards.stream().mapToInt((i) -> i).toArray(); + } + } diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index 1356a1458a2ed..e90ce523928e1 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -25,6 +25,7 @@ import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; import org.apache.lucene.util.Counter; +import org.elasticsearch.Version; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.Nullable; @@ -120,6 +121,7 @@ final class DefaultSearchContext extends SearchContext { // filter for sliced scroll private SliceBuilder sliceBuilder; private SearchTask task; + private final Version minNodeVersion; /** @@ -154,7 +156,7 @@ final class DefaultSearchContext extends SearchContext { DefaultSearchContext(long id, ShardSearchRequest request, SearchShardTarget shardTarget, Engine.Searcher engineSearcher, IndexService indexService, IndexShard indexShard, BigArrays bigArrays, Counter timeEstimateCounter, - TimeValue timeout, FetchPhase fetchPhase, String clusterAlias) { + TimeValue timeout, FetchPhase fetchPhase, String clusterAlias, Version minNodeVersion) { this.id = id; this.request = request; this.fetchPhase = fetchPhase; @@ -171,6 +173,7 @@ final class DefaultSearchContext extends SearchContext { this.searcher = new ContextIndexSearcher(engineSearcher, indexService.cache().query(), indexShard.getQueryCachingPolicy()); this.timeEstimateCounter = timeEstimateCounter; this.timeout = timeout; + this.minNodeVersion = minNodeVersion; queryShardContext = indexService.newQueryShardContext(request.shardId().id(), searcher.getIndexReader(), request::nowInMillis, clusterAlias); queryShardContext.setTypes(request.types()); @@ -278,8 +281,7 @@ && new NestedHelper(mapperService()).mightMatchNestedDocs(query) } if (sliceBuilder != null) { - filters.add(sliceBuilder.toFilter(queryShardContext, shardTarget().getShardId().getId(), - queryShardContext.getIndexSettings().getNumberOfShards())); + filters.add(sliceBuilder.toFilter(queryShardContext, remapShardId(), numberOfIndexShards(), minNodeVersion)); } if (filters.isEmpty()) { @@ -335,6 +337,14 @@ public int numberOfShards() { return request.numberOfShards(); } + public int numberOfIndexShards() { + return request.numberOfIndexShards(); + } + + public int remapShardId() { + return request.remapShardId(); + } + @Override public float queryBoost() { return queryBoost; diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index a742a3a06ae13..ff2fa9644cbd8 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -617,7 +617,7 @@ private DefaultSearchContext createSearchContext(ShardSearchRequest request, Tim final DefaultSearchContext searchContext = new DefaultSearchContext(idGenerator.incrementAndGet(), request, shardTarget, engineSearcher, indexService, indexShard, bigArrays, threadPool.estimatedTimeInMillisCounter(), timeout, fetchPhase, - request.getClusterAlias()); + request.getClusterAlias(), clusterService.state().nodes().getMinNodeVersion()); boolean success = false; try { // we clone the query shard context here just for rewriting otherwise we diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java index af52924a2de2c..b6474562f20ec 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java @@ -64,6 +64,8 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { private String clusterAlias; private ShardId shardId; + private int remapShardId; + private int numberOfIndexShards; private int numberOfShards; private SearchType searchType; private Scroll scroll; @@ -80,10 +82,10 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { ShardSearchLocalRequest() { } - ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int numberOfShards, + ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int remapShardId, int numberOfIndexShards, int numberOfShards, AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { - this(shardId, numberOfShards, searchRequest.searchType(), - searchRequest.source(), searchRequest.types(), searchRequest.requestCache(), aliasFilter, indexBoost, + this(shardId, remapShardId, numberOfIndexShards, numberOfShards, searchRequest.searchType(), + searchRequest.source(), searchRequest.types(), searchRequest.requestCache(), aliasFilter, indexBoost, searchRequest.allowPartialSearchResults()); // If allowPartialSearchResults is unset (ie null), the cluster-level default should have been substituted // at this stage. Any NPEs in the above are therefore an error in request preparation logic. @@ -101,9 +103,12 @@ public ShardSearchLocalRequest(ShardId shardId, String[] types, long nowInMillis indexBoost = 1.0f; } - public ShardSearchLocalRequest(ShardId shardId, int numberOfShards, SearchType searchType, SearchSourceBuilder source, String[] types, - Boolean requestCache, AliasFilter aliasFilter, float indexBoost, boolean allowPartialSearchResults) { + public ShardSearchLocalRequest(ShardId shardId, int remapShardId, int numberOfIndexShards, int numberOfShards, + SearchType searchType, SearchSourceBuilder source, String[] types, + Boolean requestCache, AliasFilter aliasFilter, float indexBoost, boolean allowPartialSearchResults) { this.shardId = shardId; + this.remapShardId = remapShardId; + this.numberOfIndexShards = numberOfIndexShards; this.numberOfShards = numberOfShards; this.searchType = searchType; this.source = source; @@ -114,7 +119,6 @@ public ShardSearchLocalRequest(ShardId shardId, int numberOfShards, SearchType s this.allowPartialSearchResults = allowPartialSearchResults; } - @Override public ShardId shardId() { return shardId; @@ -150,6 +154,16 @@ public int numberOfShards() { return numberOfShards; } + @Override + public int numberOfIndexShards() { + return numberOfIndexShards; + } + + @Override + public int remapShardId() { + return remapShardId; + } + @Override public SearchType searchType() { return searchType; @@ -169,12 +183,12 @@ public long nowInMillis() { public Boolean requestCache() { return requestCache; } - + @Override public Boolean allowPartialSearchResults() { return allowPartialSearchResults; } - + @Override public Scroll scroll() { @@ -199,6 +213,14 @@ protected void innerReadFrom(StreamInput in) throws IOException { shardId = ShardId.readShardId(in); searchType = SearchType.fromId(in.readByte()); numberOfShards = in.readVInt(); + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + remapShardId = in.readVInt(); + numberOfIndexShards = in.readVInt(); + assert remapShardId != -1 && numberOfIndexShards != -1; + } else { + remapShardId = -1; + numberOfIndexShards = -1; + } scroll = in.readOptionalWriteable(Scroll::new); source = in.readOptionalWriteable(SearchSourceBuilder::new); types = in.readStringArray(); @@ -232,6 +254,10 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException out.writeByte(searchType.id()); if (!asKey) { out.writeVInt(numberOfShards); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeVInt(remapShardId); + out.writeVInt(numberOfIndexShards); + } } out.writeOptionalWriteable(scroll); out.writeOptionalWriteable(source); @@ -250,7 +276,7 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException if (out.getVersion().onOrAfter(Version.V_6_3_0)) { out.writeOptionalBoolean(allowPartialSearchResults); } - + } @Override diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java index 19eb0f17ccc84..4d88e80231ec5 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java @@ -49,6 +49,15 @@ public interface ShardSearchRequest { ShardId shardId(); + /** + * Returns the remapped shard id of the requested shard for this request + * or -1 if this information is not available. + * The remapped shard id is the id of the requested shard among all shards + * of this index that are part of the request. Note that the remapped shard id + * is equal to the original shard id if all shards of this index are part of the request. + */ + int remapShardId(); + String[] types(); SearchSourceBuilder source(); @@ -59,6 +68,15 @@ public interface ShardSearchRequest { void source(SearchSourceBuilder source); + /** + * Returns the number of shards of this index ({@link ShardId#getIndex()}) that participates in the request + * or -1 if this information is not available. + */ + int numberOfIndexShards(); + + /** + * Returns the number of shards that participates in the request. + */ int numberOfShards(); SearchType searchType(); @@ -68,7 +86,7 @@ public interface ShardSearchRequest { long nowInMillis(); Boolean requestCache(); - + Boolean allowPartialSearchResults(); Scroll scroll(); diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java index ac86d24ed000d..9aa86f0f98db3 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java @@ -56,9 +56,11 @@ public class ShardSearchTransportRequest extends TransportRequest implements Sha public ShardSearchTransportRequest(){ } - public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int numberOfShards, - AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { - this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, numberOfShards, aliasFilter, indexBoost, + public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int remapShardId, + int numberOfIndexShards, int numberOfShards, AliasFilter aliasFilter, float indexBoost, + long nowInMillis, String clusterAlias) { + this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, remapShardId, + numberOfIndexShards, numberOfShards, aliasFilter, indexBoost, nowInMillis, clusterAlias); this.originalIndices = originalIndices; } @@ -102,6 +104,11 @@ public ShardId shardId() { return shardSearchLocalRequest.shardId(); } + @Override + public int remapShardId() { + return shardSearchLocalRequest.remapShardId(); + } + @Override public String[] types() { return shardSearchLocalRequest.types(); @@ -132,6 +139,11 @@ public int numberOfShards() { return shardSearchLocalRequest.numberOfShards(); } + @Override + public int numberOfIndexShards() { + return shardSearchLocalRequest.numberOfIndexShards(); + } + @Override public SearchType searchType() { return shardSearchLocalRequest.searchType(); @@ -151,11 +163,11 @@ public long nowInMillis() { public Boolean requestCache() { return shardSearchLocalRequest.requestCache(); } - + @Override public Boolean allowPartialSearchResults() { return shardSearchLocalRequest.allowPartialSearchResults(); - } + } @Override public Scroll scroll() { diff --git a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java index aabf0c3fd0c69..79a15eb5b4890 100644 --- a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java @@ -203,7 +203,32 @@ public int hashCode() { return Objects.hash(this.field, this.id, this.max); } - public Query toFilter(QueryShardContext context, int shardId, int numShards) { + /** + * Converts this QueryBuilder to a lucene {@link Query}. + * + * @param context Additional information needed to build the query + * @param remapShardId The shardId of this shard for this request + * or -1 if this information is not available. + * @param numIndexShards The number of shards of this index that participates in the request + * or -1 if this information is not available. + * @param minNodeVersion The version of the node with the youngest version in the cluster. + */ + public Query toFilter(QueryShardContext context, int remapShardId, int numIndexShards, Version minNodeVersion) { + final int numShards; + final int shardId; + if (remapShardId != -1 && minNodeVersion.onOrAfter(Version.V_7_0_0_alpha1)) { + /** + * We use the remapped shard id (added in {@link Version#V_7_0_0_alpha1} only if all nodes + * are able to pass this information otherwise another slice might use the original shard + * id and that would lead to duplicated results. + */ + assert numIndexShards != -1; + shardId = remapShardId; + numShards = numIndexShards; + } else { + shardId = context.getShardId(); + numShards = context.getIndexSettings().getNumberOfShards(); + } final MappedFieldType type = context.fieldMapper(field); if (type == null) { throw new IllegalArgumentException("field " + field + " not found"); diff --git a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java index 6ade2b8781ecf..d90518f30f33b 100644 --- a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java @@ -31,7 +31,11 @@ import org.elasticsearch.search.internal.ShardSearchTransportRequest; import org.elasticsearch.test.ESTestCase; +import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -41,6 +45,7 @@ public class AbstractSearchAsyncActionTests extends ESTestCase { private AbstractSearchAsyncAction createAction( + final GroupShardsIterator shardsIt, final boolean controlled, final AtomicLong expected) { @@ -61,12 +66,17 @@ private AbstractSearchAsyncAction createAction( } final SearchRequest request = new SearchRequest(); + Map aliasFilterMap = new HashMap<>(); + Map concreteIndexBoosts = new HashMap<>(); + for (SearchShardIterator it : shardsIt) { + aliasFilterMap.put(it.shardId().getIndex().getUUID(), new AliasFilter(new MatchAllQueryBuilder())); + concreteIndexBoosts.put(it.shardId().getIndex().getUUID(), 2.0f); + } request.allowPartialSearchResults(true); return new AbstractSearchAsyncAction("test", null, null, null, - Collections.singletonMap("foo", new AliasFilter(new MatchAllQueryBuilder())), Collections.singletonMap("foo", 2.0f), null, - request, null, new GroupShardsIterator<>(Collections.singletonList( - new SearchShardIterator(null, null, Collections.emptyList(), null))), timeProvider, 0, null, - new InitialSearchPhase.ArraySearchPhaseResults<>(10), request.getMaxConcurrentShardRequests(), + aliasFilterMap, concreteIndexBoosts, null, + request, null, shardsIt, timeProvider, 0, null, + new InitialSearchPhase.ArraySearchPhaseResults<>(shardsIt.size()), request.getMaxConcurrentShardRequests(), SearchResponse.Clusters.EMPTY) { @Override protected SearchPhase getNextPhase(final SearchPhaseResults results, final SearchPhaseContext context) { @@ -96,7 +106,13 @@ public void testTookWithRealClock() { private void runTestTook(final boolean controlled) { final AtomicLong expected = new AtomicLong(); - AbstractSearchAsyncAction action = createAction(controlled, expected); + GroupShardsIterator shardsIt = new GroupShardsIterator<>( + Collections.singletonList( + new SearchShardIterator(null, new ShardId(new Index("name", "foo"), 0), + Collections.emptyList(), null) + ) + ); + AbstractSearchAsyncAction action = createAction(shardsIt, controlled, expected); final long actual = action.buildTookInMillis(); if (controlled) { // with a controlled clock, we can assert the exact took time @@ -109,7 +125,13 @@ private void runTestTook(final boolean controlled) { public void testBuildShardSearchTransportRequest() { final AtomicLong expected = new AtomicLong(); - AbstractSearchAsyncAction action = createAction(false, expected); + GroupShardsIterator shardsIt = new GroupShardsIterator<>( + Collections.singletonList( + new SearchShardIterator(null, new ShardId(new Index("name", "foo"), 1), + Collections.emptyList(), null) + ) + ); + AbstractSearchAsyncAction action = createAction(shardsIt, false, expected); SearchShardIterator iterator = new SearchShardIterator("test-cluster", new ShardId(new Index("name", "foo"), 1), Collections.emptyList(), new OriginalIndices(new String[] {"name", "name1"}, IndicesOptions.strictExpand())); ShardSearchTransportRequest shardSearchTransportRequest = action.buildShardSearchRequest(iterator); @@ -117,5 +139,56 @@ public void testBuildShardSearchTransportRequest() { assertArrayEquals(new String[] {"name", "name1"}, shardSearchTransportRequest.indices()); assertEquals(new MatchAllQueryBuilder(), shardSearchTransportRequest.getAliasFilter().getQueryBuilder()); assertEquals(2.0f, shardSearchTransportRequest.indexBoost(), 0.0f); + assertEquals(0, shardSearchTransportRequest.remapShardId()); + assertEquals(1, shardSearchTransportRequest.numberOfIndexShards()); + assertEquals(1, shardSearchTransportRequest.numberOfShards()); + } + + public void testBuildFilteredShardSearchTransportRequest() { + final AtomicLong expected = new AtomicLong(); + int numIndex = randomIntBetween(1, 5); + List shards = new ArrayList<>(); + int[] numIndexShards = new int[numIndex]; + Map> remapShards = new HashMap<>(); + int totalShards = 0; + for (int i = 0; i < numIndex; i++) { + int numShards = randomIntBetween(1, 10); + numIndexShards[i] = numShards; + String indexName = Integer.toString(i); + int shardIndex = 0; + Map shardMap = new HashMap<>(); + for (int j = 0; j < numShards; j++) { + if (randomBoolean()) { + shards.add(new SearchShardIterator(null, new ShardId(new Index(indexName, indexName), j), + Collections.emptyList(), null)); + shardMap.put(j, shardIndex++); + totalShards++; + } + } + remapShards.put(i, shardMap); + } + Collections.shuffle(shards, random()); + GroupShardsIterator shardsIt = new GroupShardsIterator<>(shards); + AbstractSearchAsyncAction action = createAction(shardsIt,false, expected); + + for (int i = 0; i < numIndex; i++) { + int numShards = numIndexShards[i]; + String indexName = Integer.toString(i); + Map shardMap = remapShards.get(i); + if (shardMap.size() == 0) { + continue; + } + for (int j = 0; j < numShards; j++) { + if (shardMap.containsKey(j) == false) { + continue; + } + SearchShardIterator iterator = new SearchShardIterator("test-cluster", new ShardId(new Index(indexName, indexName), j), + Collections.emptyList(), new OriginalIndices(new String[]{"name", "name1"}, IndicesOptions.strictExpand())); + ShardSearchTransportRequest shardSearchTransportRequest = action.buildShardSearchRequest(iterator); + assertThat(shardSearchTransportRequest.numberOfIndexShards(), equalTo(shardMap.size())); + assertThat(shardSearchTransportRequest.remapShardId(), equalTo(shardMap.get(j))); + assertThat(shardSearchTransportRequest.numberOfShards(), equalTo(totalShards)); + } + } } } diff --git a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java index 6e8e679188c66..03a55e815056b 100644 --- a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java +++ b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java @@ -62,6 +62,11 @@ public ShardId shardId() { return new ShardId(indexService.index(), 0); } + @Override + public int remapShardId() { + return 0; + } + @Override public String[] types() { return new String[0]; @@ -92,6 +97,11 @@ public int numberOfShards() { return 0; } + @Override + public int numberOfIndexShards() { + return 0; + } + @Override public SearchType searchType() { return null; diff --git a/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java b/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java index ec422435e4e07..bbe4e4505ef87 100644 --- a/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java +++ b/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java @@ -113,7 +113,7 @@ public void testPreProcess() throws Exception { Engine.Searcher searcher = new Engine.Searcher("test", new IndexSearcher(reader))) { DefaultSearchContext context1 = new DefaultSearchContext(1L, shardSearchRequest, null, searcher, indexService, - indexShard, bigArrays, null, timeout, null, null); + indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); context1.from(300); // resultWindow greater than maxResultWindow and scrollContext is null @@ -154,7 +154,7 @@ public void testPreProcess() throws Exception { // rescore is null but sliceBuilder is not null DefaultSearchContext context2 = new DefaultSearchContext(2L, shardSearchRequest, null, searcher, indexService, - indexShard, bigArrays, null, timeout, null, null); + indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); SliceBuilder sliceBuilder = mock(SliceBuilder.class); int numSlices = maxSlicesPerScroll + randomIntBetween(1, 100); @@ -171,7 +171,7 @@ public void testPreProcess() throws Exception { when(shardSearchRequest.indexBoost()).thenReturn(AbstractQueryBuilder.DEFAULT_BOOST); DefaultSearchContext context3 = new DefaultSearchContext(3L, shardSearchRequest, null, searcher, indexService, - indexShard, bigArrays, null, timeout, null, null); + indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); ParsedQuery parsedQuery = ParsedQuery.parsedMatchAllQuery(); context3.sliceBuilder(null).parsedQuery(parsedQuery).preProcess(false); assertEquals(context3.query(), context3.buildFilteredQuery(parsedQuery.query())); diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index f5552ee0d2e46..dae22aa68e125 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -211,7 +211,7 @@ public void onFailure(Exception e) { for (int i = 0; i < rounds; i++) { try { SearchPhaseResult searchPhaseResult = service.executeQueryPhase( - new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, new SearchSourceBuilder(), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true), new SearchTask(123L, "", "", "", null, Collections.emptyMap())); @@ -243,6 +243,8 @@ public void testTimeout() throws IOException { final SearchContext contextWithDefaultTimeout = service.createContext( new ShardSearchLocalRequest( indexShard.shardId(), + 0, + 1, 1, SearchType.DEFAULT, new SearchSourceBuilder(), @@ -263,6 +265,8 @@ public void testTimeout() throws IOException { final SearchContext context = service.createContext( new ShardSearchLocalRequest( indexShard.shardId(), + 0, + 1, 1, SearchType.DEFAULT, new SearchSourceBuilder().timeout(TimeValue.timeValueSeconds(seconds)), @@ -296,12 +300,12 @@ public void testMaxDocvalueFieldsSearch() throws IOException { for (int i = 0; i < indexService.getIndexSettings().getMaxDocvalueFields(); i++) { searchSourceBuilder.docValueField("field" + i); } - try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0,1, 1, SearchType.DEFAULT, searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))) { assertNotNull(context); searchSourceBuilder.docValueField("one_field_too_much"); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))); assertEquals( "Trying to retrieve too many docvalue_fields. Must be less than or equal to: [100] but was [101]. " @@ -327,13 +331,13 @@ public void testMaxScriptFieldsSearch() throws IOException { searchSourceBuilder.scriptField("field" + i, new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); } - try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))) { assertNotNull(context); searchSourceBuilder.scriptField("anotherScriptField", new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))); assertEquals( "Trying to retrieve too many script_fields. Must be less than or equal to: [" + maxScriptFields + "] but was [" @@ -405,27 +409,27 @@ public void testCanMatch() throws IOException { final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index")); final IndexShard indexShard = indexService.getShard(0); final boolean allowPartialSearchResults = true; - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, null, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, null, Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, - new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, + new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchAllQueryBuilder()), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()) .aggregation(new TermsAggregationBuilder("test", ValueType.STRING).minDocCount(0)), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()) .aggregation(new GlobalAggregationBuilder("test")), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); - assertFalse(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, + assertFalse(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); diff --git a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java index c2016ceb02ce7..2f66f8a2266a8 100644 --- a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java @@ -74,6 +74,7 @@ public void testSerialization() throws Exception { assertEquals(deserializedRequest.searchType(), shardSearchTransportRequest.searchType()); assertEquals(deserializedRequest.shardId(), shardSearchTransportRequest.shardId()); assertEquals(deserializedRequest.numberOfShards(), shardSearchTransportRequest.numberOfShards()); + assertEquals(deserializedRequest.numberOfIndexShards(), shardSearchTransportRequest.numberOfIndexShards()); assertEquals(deserializedRequest.cacheKey(), shardSearchTransportRequest.cacheKey()); assertNotSame(deserializedRequest, shardSearchTransportRequest); assertEquals(deserializedRequest.getAliasFilter(), shardSearchTransportRequest.getAliasFilter()); @@ -92,8 +93,9 @@ private ShardSearchTransportRequest createShardSearchTransportRequest() throws I } else { filteringAliases = new AliasFilter(null, Strings.EMPTY_ARRAY); } - return new ShardSearchTransportRequest(new OriginalIndices(searchRequest), searchRequest, shardId, - randomIntBetween(1, 100), filteringAliases, randomBoolean() ? 1.0f : randomFloat(), Math.abs(randomLong()), null); + return new ShardSearchTransportRequest(new OriginalIndices(searchRequest), searchRequest, shardId, shardId.getId(), + randomIntBetween(1, 100), randomIntBetween(1, 100), filteringAliases, randomBoolean() ? 1.0f : randomFloat(), + Math.abs(randomLong()), null); } public void testFilteringAliases() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java b/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java index 2227cbb806b3f..e8a046a8a7ae8 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java @@ -50,7 +50,7 @@ public class SearchSliceIT extends ESIntegTestCase { private static final int NUM_DOCS = 1000; - private int setupIndex(boolean withDocs) throws IOException, ExecutionException, InterruptedException { + private void setupIndex(int numberOfShards, boolean withDocs) throws IOException, ExecutionException, InterruptedException { String mapping = Strings.toString(XContentFactory.jsonBuilder(). startObject() .startObject("type") @@ -70,14 +70,13 @@ private int setupIndex(boolean withDocs) throws IOException, ExecutionException, .endObject() .endObject() .endObject()); - int numberOfShards = randomIntBetween(1, 7); assertAcked(client().admin().indices().prepareCreate("test") .setSettings(Settings.builder().put("number_of_shards", numberOfShards).put("index.max_slices_per_scroll", 10000)) .addMapping("type", mapping, XContentType.JSON)); ensureGreen(); if (withDocs == false) { - return numberOfShards; + return; } List requests = new ArrayList<>(); @@ -92,11 +91,11 @@ private int setupIndex(boolean withDocs) throws IOException, ExecutionException, requests.add(client().prepareIndex("test", "type").setSource(builder)); } indexRandom(true, requests); - return numberOfShards; } public void testDocIdSort() throws Exception { - int numShards = setupIndex(true); + int numShards = randomIntBetween(1, 7); + setupIndex(numShards, true); SearchResponse sr = client().prepareSearch("test") .setQuery(matchAllQuery()) .setSize(0) @@ -111,12 +110,35 @@ public void testDocIdSort() throws Exception { .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) .setSize(fetchSize) .addSort(SortBuilders.fieldSort("_doc")); - assertSearchSlicesWithScroll(request, field, max); + assertSearchSlicesWithScroll(request, field, max, numDocs); + } + } + + public void testWithRouting() throws Exception { + int numShards = 10; + setupIndex(numShards, true); + SearchResponse sr = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setPreference("_shards:1,4") + .setSize(0) + .get(); + int numDocs = (int) sr.getHits().getTotalHits(); + int max = randomIntBetween(2, numShards*3); + for (String field : new String[]{"_id", "random_int", "static_int"}) { + int fetchSize = randomIntBetween(10, 100); + SearchRequestBuilder request = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) + .setSize(fetchSize) + .setPreference("_shards:1,4") + .addSort(SortBuilders.fieldSort("_doc")); + assertSearchSlicesWithScroll(request, field, max, numDocs); } } public void testNumericSort() throws Exception { - int numShards = setupIndex(true); + int numShards = randomIntBetween(1, 7); + setupIndex(numShards, true); SearchResponse sr = client().prepareSearch("test") .setQuery(matchAllQuery()) .setSize(0) @@ -132,12 +154,13 @@ public void testNumericSort() throws Exception { .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) .addSort(SortBuilders.fieldSort("random_int")) .setSize(fetchSize); - assertSearchSlicesWithScroll(request, field, max); + assertSearchSlicesWithScroll(request, field, max, numDocs); } } public void testInvalidFields() throws Exception { - setupIndex(false); + int numShards = randomIntBetween(1, 7); + setupIndex(numShards, true); SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch("test") .setQuery(matchAllQuery()) @@ -161,7 +184,8 @@ public void testInvalidFields() throws Exception { } public void testInvalidQuery() throws Exception { - setupIndex(false); + int numShards = randomIntBetween(1, 7); + setupIndex(numShards, true); SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch() .setQuery(matchAllQuery()) @@ -173,7 +197,7 @@ public void testInvalidQuery() throws Exception { equalTo("`slice` cannot be used outside of a scroll context")); } - private void assertSearchSlicesWithScroll(SearchRequestBuilder request, String field, int numSlice) { + private void assertSearchSlicesWithScroll(SearchRequestBuilder request, String field, int numSlice, int numDocs) { int totalResults = 0; List keys = new ArrayList<>(); for (int id = 0; id < numSlice; id++) { @@ -201,9 +225,9 @@ private void assertSearchSlicesWithScroll(SearchRequestBuilder request, String f assertThat(numSliceResults, equalTo(expectedSliceResults)); clearScroll(scrollId); } - assertThat(totalResults, equalTo(NUM_DOCS)); - assertThat(keys.size(), equalTo(NUM_DOCS)); - assertThat(new HashSet(keys).size(), equalTo(NUM_DOCS)); + assertThat(totalResults, equalTo(numDocs)); + assertThat(keys.size(), equalTo(numDocs)); + assertThat(new HashSet(keys).size(), equalTo(numDocs)); } private Throwable findRootCause(Exception e) { diff --git a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java index 75802e92ee176..f6da7c438fb4e 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java @@ -171,13 +171,13 @@ public Query existsQuery(QueryShardContext context) { IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY); when(context.getIndexSettings()).thenReturn(indexSettings); SliceBuilder builder = new SliceBuilder(5, 10); - Query query = builder.toFilter(context, 0, 1); + Query query = builder.toFilter(context, 0, 1, Version.CURRENT); assertThat(query, instanceOf(TermsSliceQuery.class)); - assertThat(builder.toFilter(context, 0, 1), equalTo(query)); + assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); try (IndexReader newReader = DirectoryReader.open(dir)) { when(context.getIndexReader()).thenReturn(newReader); - assertThat(builder.toFilter(context, 0, 1), equalTo(query)); + assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); } } @@ -210,13 +210,13 @@ public Query existsQuery(QueryShardContext context) { IndexNumericFieldData fd = mock(IndexNumericFieldData.class); when(context.getForField(fieldType)).thenReturn(fd); SliceBuilder builder = new SliceBuilder("field_doc_values", 5, 10); - Query query = builder.toFilter(context, 0, 1); + when(context.getShardId()).thenReturn(0); + Query query = builder.toFilter(context, 0, 1, Version.CURRENT); assertThat(query, instanceOf(DocValuesSliceQuery.class)); - - assertThat(builder.toFilter(context, 0, 1), equalTo(query)); + assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); try (IndexReader newReader = DirectoryReader.open(dir)) { when(context.getIndexReader()).thenReturn(newReader); - assertThat(builder.toFilter(context, 0, 1), equalTo(query)); + assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); } // numSlices > numShards @@ -226,7 +226,7 @@ public Query existsQuery(QueryShardContext context) { for (int i = 0; i < numSlices; i++) { for (int j = 0; j < numShards; j++) { SliceBuilder slice = new SliceBuilder("_id", i, numSlices); - Query q = slice.toFilter(context, j, numShards); + Query q = slice.toFilter(context, j, numShards, Version.CURRENT); if (q instanceof TermsSliceQuery || q instanceof MatchAllDocsQuery) { AtomicInteger count = numSliceMap.get(j); if (count == null) { @@ -255,7 +255,7 @@ public Query existsQuery(QueryShardContext context) { for (int i = 0; i < numSlices; i++) { for (int j = 0; j < numShards; j++) { SliceBuilder slice = new SliceBuilder("_id", i, numSlices); - Query q = slice.toFilter(context, j, numShards); + Query q = slice.toFilter(context, j, numShards, Version.CURRENT); if (q instanceof MatchNoDocsQuery == false) { assertThat(q, instanceOf(MatchAllDocsQuery.class)); targetShards.add(j); @@ -271,7 +271,7 @@ public Query existsQuery(QueryShardContext context) { for (int i = 0; i < numSlices; i++) { for (int j = 0; j < numShards; j++) { SliceBuilder slice = new SliceBuilder("_id", i, numSlices); - Query q = slice.toFilter(context, j, numShards); + Query q = slice.toFilter(context, j, numShards, Version.CURRENT); if (i == j) { assertThat(q, instanceOf(MatchAllDocsQuery.class)); } else { @@ -306,8 +306,9 @@ public Query existsQuery(QueryShardContext context) { when(context.fieldMapper("field_without_doc_values")).thenReturn(fieldType); when(context.getIndexReader()).thenReturn(reader); SliceBuilder builder = new SliceBuilder("field_without_doc_values", 5, 10); + when(context.getShardId()).thenReturn(0); IllegalArgumentException exc = - expectThrows(IllegalArgumentException.class, () -> builder.toFilter(context, 0, 1)); + expectThrows(IllegalArgumentException.class, () -> builder.toFilter(context, 0, 1, Version.CURRENT)); assertThat(exc.getMessage(), containsString("cannot load numeric doc values")); } } @@ -353,12 +354,12 @@ public Query existsQuery(QueryShardContext context) { IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY); when(context.getIndexSettings()).thenReturn(indexSettings); SliceBuilder builder = new SliceBuilder("_uid", 5, 10); - Query query = builder.toFilter(context, 0, 1); + when(context.getShardId()).thenReturn(0); + Query query = builder.toFilter(context, 0, 1, Version.CURRENT); assertThat(query, instanceOf(TermsSliceQuery.class)); - assertThat(builder.toFilter(context, 0, 1), equalTo(query)); + assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); assertWarnings("Computing slices on the [_uid] field is deprecated for 6.x indices, use [_id] instead"); } - } public void testSerializationBackcompat() throws IOException { @@ -375,4 +376,68 @@ public void testSerializationBackcompat() throws IOException { SliceBuilder::new, Version.V_6_3_0); assertEquals(sliceBuilder, copy63); } + + public void testRemapShards() { + QueryShardContext context = mock(QueryShardContext.class); + Settings settings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 10) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .build(); + IndexMetaData indexState = IndexMetaData.builder("index").settings(settings).build(); + IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY); + when(context.getIndexSettings()).thenReturn(indexSettings); + MappedFieldType fieldType = new MappedFieldType() { + @Override + public MappedFieldType clone() { + return null; + } + + @Override + public String typeName() { + return null; + } + + @Override + public Query termQuery(Object value, @Nullable QueryShardContext context) { + return null; + } + + public Query existsQuery(QueryShardContext context) { + return null; + } + }; + fieldType.setName(IdFieldMapper.NAME); + fieldType.setHasDocValues(false); + when(context.fieldMapper(IdFieldMapper.NAME)).thenReturn(fieldType); + + for (int id = 0; id < 10; id++) { + SliceBuilder builder = new SliceBuilder("_id", id, 10); + for (int shard = 0; shard < 10; shard++) { + when(context.getShardId()).thenReturn(shard); + Query query = builder.toFilter(context, -1, -1, Version.CURRENT); + if (id == shard) { + assertThat(query, equalTo(new MatchAllDocsQuery())); + } else { + assertThat(query, equalTo(new MatchNoDocsQuery())); + } + query = builder.toFilter(context, shard, 10, Version.CURRENT); + if (id == shard) { + assertThat(query, equalTo(new MatchAllDocsQuery())); + } else { + assertThat(query, equalTo(new MatchNoDocsQuery())); + } + query = builder.toFilter(context, shard, 5, Version.CURRENT); + if (shard >= 5) { + assertThat(query, equalTo(new MatchNoDocsQuery())); + } else if (shard == id) { + assertThat(query, equalTo(new TermsSliceQuery("_id", 0, 2))); + } else if (id % 5 == shard) { + assertThat(query, equalTo(new TermsSliceQuery("_id", 1, 2))); + } else { + assertThat(query, equalTo(new MatchNoDocsQuery())); + } + } + } + } } From b90716cfa7eb17b10de0d8398220431fbe856da5 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 19 Apr 2018 15:30:06 +0200 Subject: [PATCH 2/8] Address reviews Rename remapShardId to shardRequestOrdinal. Compute request ordinal per shard and number of request shard per index only once. --- .../search/AbstractSearchAsyncAction.java | 9 ++--- .../action/search/InitialSearchPhase.java | 36 ++++++++++++------- .../search/DefaultSearchContext.java | 6 ++-- .../internal/ShardSearchLocalRequest.java | 25 ++++++------- .../search/internal/ShardSearchRequest.java | 10 +++--- .../internal/ShardSearchTransportRequest.java | 11 +++--- .../search/slice/SliceBuilder.java | 11 +++--- .../AbstractSearchAsyncActionTests.java | 10 +++--- .../index/SearchSlowLogTests.java | 2 +- 9 files changed, 61 insertions(+), 59 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java index 020f3e0d3bedb..f693789a923cd 100644 --- a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java @@ -37,7 +37,6 @@ import org.elasticsearch.search.internal.ShardSearchTransportRequest; import org.elasticsearch.transport.Transport; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.concurrent.Executor; @@ -319,11 +318,9 @@ public final ShardSearchTransportRequest buildShardSearchRequest(SearchShardIter AliasFilter filter = aliasFilter.get(shardIt.shardId().getIndex().getUUID()); assert filter != null; float indexBoost = concreteIndexBoosts.getOrDefault(shardIt.shardId().getIndex().getUUID(), DEFAULT_INDEX_BOOST); - int[] indexShards = getIndexShards(shardIt.shardId().getIndex()); - int remapShardId = Arrays.binarySearch(indexShards, shardIt.shardId().getId()); - assert remapShardId >= 0; - return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), remapShardId, - indexShards.length, getNumShards(), filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); + return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), + getShardRequestOrdinal(shardIt.shardId()), getNumberOfRequestShards(shardIt.shardId().getIndex()), + getNumShards(), filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); } /** diff --git a/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java index 8a0f5d5477421..d355c60d15293 100644 --- a/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java @@ -23,19 +23,20 @@ import org.elasticsearch.action.NoShardAvailableActionException; import org.elasticsearch.action.support.TransportActions; import org.elasticsearch.cluster.routing.GroupShardsIterator; -import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.AtomicArray; import org.elasticsearch.index.Index; +import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchPhaseResult; import org.elasticsearch.search.SearchShardTarget; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; @@ -52,6 +53,8 @@ abstract class InitialSearchPhase extends private final SearchRequest request; private final GroupShardsIterator toSkipShardsIts; private final GroupShardsIterator shardsIts; + private final Map shardRequestOrdinals; + private final Map numberOfRequestShardsPerIndex; private final Logger logger; private final int expectedTotalOps; private final AtomicInteger totalOps = new AtomicInteger(); @@ -65,10 +68,18 @@ abstract class InitialSearchPhase extends this.request = request; final List toSkipIterators = new ArrayList<>(); final List iterators = new ArrayList<>(); + this.shardRequestOrdinals = new HashMap<>(iterators.size()); + this.numberOfRequestShardsPerIndex = new HashMap<>(); + Map shardOrdMap = new HashMap<>(); for (final SearchShardIterator iterator : shardsIts) { if (iterator.skip()) { toSkipIterators.add(iterator); } else { + ShardId shardId = iterator.shardId(); + Index index = shardId.getIndex(); + AtomicInteger shardOrd = shardOrdMap.computeIfAbsent(index.getUUID(), (a) -> new AtomicInteger(0)); + shardRequestOrdinals.put(iterator.shardId(), shardOrd.getAndIncrement()); + numberOfRequestShardsPerIndex.put(index.getUUID(), shardOrd.intValue()); iterators.add(iterator); } } @@ -381,17 +392,18 @@ protected void skipShard(SearchShardIterator iterator) { } /** - * Returns the list of shard ids in the request that match the provided {@link Index}. + * Returns the shard request ordinal for the provided shardId. */ - protected int[] getIndexShards(Index index) { - List shards = new ArrayList<>(); - for (ShardIterator it : shardsIts) { - if (index.equals(it.shardId().getIndex())) { - shards.add(it.shardId().getId()); - } - } - Collections.sort(shards); - return shards.stream().mapToInt((i) -> i).toArray(); + protected int getShardRequestOrdinal(ShardId shardId) { + return shardRequestOrdinals.get(shardId); + } + + /** + * Return the number of requested shards for the provided index. + */ + protected int getNumberOfRequestShards(Index index) { + return numberOfRequestShardsPerIndex.get(index.getUUID()); + } } diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index e90ce523928e1..e6f7cebdb5cf4 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -281,7 +281,7 @@ && new NestedHelper(mapperService()).mightMatchNestedDocs(query) } if (sliceBuilder != null) { - filters.add(sliceBuilder.toFilter(queryShardContext, remapShardId(), numberOfIndexShards(), minNodeVersion)); + filters.add(sliceBuilder.toFilter(queryShardContext, shardRequestOrdinal(), numberOfIndexShards(), minNodeVersion)); } if (filters.isEmpty()) { @@ -341,8 +341,8 @@ public int numberOfIndexShards() { return request.numberOfIndexShards(); } - public int remapShardId() { - return request.remapShardId(); + public int shardRequestOrdinal() { + return request.shardRequestOrdinal(); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java index b6474562f20ec..983894f1757ea 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java @@ -28,13 +28,10 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.Scroll; -import org.elasticsearch.search.SearchService; import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; @@ -64,7 +61,7 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { private String clusterAlias; private ShardId shardId; - private int remapShardId; + private int shardRequestOrdinal; private int numberOfIndexShards; private int numberOfShards; private SearchType searchType; @@ -82,9 +79,9 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { ShardSearchLocalRequest() { } - ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int remapShardId, int numberOfIndexShards, int numberOfShards, + ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int shardRequestOrdinal, int numberOfIndexShards, int numberOfShards, AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { - this(shardId, remapShardId, numberOfIndexShards, numberOfShards, searchRequest.searchType(), + this(shardId, shardRequestOrdinal, numberOfIndexShards, numberOfShards, searchRequest.searchType(), searchRequest.source(), searchRequest.types(), searchRequest.requestCache(), aliasFilter, indexBoost, searchRequest.allowPartialSearchResults()); // If allowPartialSearchResults is unset (ie null), the cluster-level default should have been substituted @@ -103,11 +100,11 @@ public ShardSearchLocalRequest(ShardId shardId, String[] types, long nowInMillis indexBoost = 1.0f; } - public ShardSearchLocalRequest(ShardId shardId, int remapShardId, int numberOfIndexShards, int numberOfShards, + public ShardSearchLocalRequest(ShardId shardId, int shardRequestOrdinal, int numberOfIndexShards, int numberOfShards, SearchType searchType, SearchSourceBuilder source, String[] types, Boolean requestCache, AliasFilter aliasFilter, float indexBoost, boolean allowPartialSearchResults) { this.shardId = shardId; - this.remapShardId = remapShardId; + this.shardRequestOrdinal = shardRequestOrdinal; this.numberOfIndexShards = numberOfIndexShards; this.numberOfShards = numberOfShards; this.searchType = searchType; @@ -160,8 +157,8 @@ public int numberOfIndexShards() { } @Override - public int remapShardId() { - return remapShardId; + public int shardRequestOrdinal() { + return shardRequestOrdinal; } @Override @@ -214,11 +211,11 @@ protected void innerReadFrom(StreamInput in) throws IOException { searchType = SearchType.fromId(in.readByte()); numberOfShards = in.readVInt(); if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - remapShardId = in.readVInt(); + shardRequestOrdinal = in.readVInt(); numberOfIndexShards = in.readVInt(); - assert remapShardId != -1 && numberOfIndexShards != -1; + assert shardRequestOrdinal != -1 && numberOfIndexShards != -1; } else { - remapShardId = -1; + shardRequestOrdinal = -1; numberOfIndexShards = -1; } scroll = in.readOptionalWriteable(Scroll::new); @@ -255,7 +252,7 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException if (!asKey) { out.writeVInt(numberOfShards); if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - out.writeVInt(remapShardId); + out.writeVInt(shardRequestOrdinal); out.writeVInt(numberOfIndexShards); } } diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java index 4d88e80231ec5..43ec542d589d5 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java @@ -50,13 +50,13 @@ public interface ShardSearchRequest { ShardId shardId(); /** - * Returns the remapped shard id of the requested shard for this request + * Returns the shard request ordinal of the shard for this request * or -1 if this information is not available. - * The remapped shard id is the id of the requested shard among all shards - * of this index that are part of the request. Note that the remapped shard id - * is equal to the original shard id if all shards of this index are part of the request. + * The request shard ordinal is the id of the requested shard among all shards + * of this index that are part of the request. Note that the request shard ordinal + * is equal to the shard id if all shards of the index are part of the request. */ - int remapShardId(); + int shardRequestOrdinal(); String[] types(); diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java index 9aa86f0f98db3..9066b43eb12f7 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java @@ -28,9 +28,6 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryRewriteContext; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.Scroll; @@ -56,10 +53,10 @@ public class ShardSearchTransportRequest extends TransportRequest implements Sha public ShardSearchTransportRequest(){ } - public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int remapShardId, + public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int shardRequestOrdinal, int numberOfIndexShards, int numberOfShards, AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { - this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, remapShardId, + this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, shardRequestOrdinal, numberOfIndexShards, numberOfShards, aliasFilter, indexBoost, nowInMillis, clusterAlias); this.originalIndices = originalIndices; @@ -105,8 +102,8 @@ public ShardId shardId() { } @Override - public int remapShardId() { - return shardSearchLocalRequest.remapShardId(); + public int shardRequestOrdinal() { + return shardSearchLocalRequest.shardRequestOrdinal(); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java index 79a15eb5b4890..24a828cdbd857 100644 --- a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java @@ -207,23 +207,22 @@ public int hashCode() { * Converts this QueryBuilder to a lucene {@link Query}. * * @param context Additional information needed to build the query - * @param remapShardId The shardId of this shard for this request - * or -1 if this information is not available. + * @param shardRequestOrdinal The shard ordinal of for this request or -1 if this information is not available. * @param numIndexShards The number of shards of this index that participates in the request * or -1 if this information is not available. * @param minNodeVersion The version of the node with the youngest version in the cluster. */ - public Query toFilter(QueryShardContext context, int remapShardId, int numIndexShards, Version minNodeVersion) { + public Query toFilter(QueryShardContext context, int shardRequestOrdinal, int numIndexShards, Version minNodeVersion) { final int numShards; final int shardId; - if (remapShardId != -1 && minNodeVersion.onOrAfter(Version.V_7_0_0_alpha1)) { + if (shardRequestOrdinal != -1 && minNodeVersion.onOrAfter(Version.V_7_0_0_alpha1)) { /** - * We use the remapped shard id (added in {@link Version#V_7_0_0_alpha1} only if all nodes + * We use the request shard ordinal (added in {@link Version#V_7_0_0_alpha1} only if all nodes * are able to pass this information otherwise another slice might use the original shard * id and that would lead to duplicated results. */ assert numIndexShards != -1; - shardId = remapShardId; + shardId = shardRequestOrdinal; numShards = numIndexShards; } else { shardId = context.getShardId(); diff --git a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java index d90518f30f33b..7dbd3e9579fd4 100644 --- a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java @@ -139,7 +139,7 @@ public void testBuildShardSearchTransportRequest() { assertArrayEquals(new String[] {"name", "name1"}, shardSearchTransportRequest.indices()); assertEquals(new MatchAllQueryBuilder(), shardSearchTransportRequest.getAliasFilter().getQueryBuilder()); assertEquals(2.0f, shardSearchTransportRequest.indexBoost(), 0.0f); - assertEquals(0, shardSearchTransportRequest.remapShardId()); + assertEquals(0, shardSearchTransportRequest.shardRequestOrdinal()); assertEquals(1, shardSearchTransportRequest.numberOfIndexShards()); assertEquals(1, shardSearchTransportRequest.numberOfShards()); } @@ -149,7 +149,7 @@ public void testBuildFilteredShardSearchTransportRequest() { int numIndex = randomIntBetween(1, 5); List shards = new ArrayList<>(); int[] numIndexShards = new int[numIndex]; - Map> remapShards = new HashMap<>(); + Map> shardOrdinals = new HashMap<>(); int totalShards = 0; for (int i = 0; i < numIndex; i++) { int numShards = randomIntBetween(1, 10); @@ -165,7 +165,7 @@ public void testBuildFilteredShardSearchTransportRequest() { totalShards++; } } - remapShards.put(i, shardMap); + shardOrdinals.put(i, shardMap); } Collections.shuffle(shards, random()); GroupShardsIterator shardsIt = new GroupShardsIterator<>(shards); @@ -174,7 +174,7 @@ public void testBuildFilteredShardSearchTransportRequest() { for (int i = 0; i < numIndex; i++) { int numShards = numIndexShards[i]; String indexName = Integer.toString(i); - Map shardMap = remapShards.get(i); + Map shardMap = shardOrdinals.get(i); if (shardMap.size() == 0) { continue; } @@ -186,7 +186,7 @@ public void testBuildFilteredShardSearchTransportRequest() { Collections.emptyList(), new OriginalIndices(new String[]{"name", "name1"}, IndicesOptions.strictExpand())); ShardSearchTransportRequest shardSearchTransportRequest = action.buildShardSearchRequest(iterator); assertThat(shardSearchTransportRequest.numberOfIndexShards(), equalTo(shardMap.size())); - assertThat(shardSearchTransportRequest.remapShardId(), equalTo(shardMap.get(j))); + assertThat(shardSearchTransportRequest.shardRequestOrdinal(), equalTo(shardMap.get(j))); assertThat(shardSearchTransportRequest.numberOfShards(), equalTo(totalShards)); } } diff --git a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java index 03a55e815056b..de20f288c9dd4 100644 --- a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java +++ b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java @@ -63,7 +63,7 @@ public ShardId shardId() { } @Override - public int remapShardId() { + public int shardRequestOrdinal() { return 0; } From 6fb78b3ed5d03a67c4525f84359f0fbb97231eca Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 20 Apr 2018 12:04:40 +0200 Subject: [PATCH 3/8] Address reviews Propagate preference and routing in the ShardSearchRequest in order to compute the shard ordinal and number of requested shard lazily on each node. The computation is done in the slice builder only when preference or routing are set on the original search request. --- .../search/AbstractSearchAsyncAction.java | 5 +- .../action/search/InitialSearchPhase.java | 29 -- .../search/DefaultSearchContext.java | 27 +- .../elasticsearch/search/SearchService.java | 4 +- .../internal/ShardSearchLocalRequest.java | 68 +-- .../search/internal/ShardSearchRequest.java | 31 +- .../internal/ShardSearchTransportRequest.java | 28 +- .../search/slice/SliceBuilder.java | 70 ++- .../AbstractSearchAsyncActionTests.java | 85 +--- .../index/SearchSlowLogTests.java | 20 +- .../search/DefaultSearchContextTests.java | 10 +- .../search/SearchServiceTests.java | 54 +- .../ShardSearchTransportRequestTests.java | 7 +- .../search/slice/SearchSliceIT.java | 24 +- .../search/slice/SliceBuilderTests.java | 466 ++++++++++-------- 15 files changed, 467 insertions(+), 461 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java index f693789a923cd..53f681a2243c5 100644 --- a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java @@ -318,9 +318,8 @@ public final ShardSearchTransportRequest buildShardSearchRequest(SearchShardIter AliasFilter filter = aliasFilter.get(shardIt.shardId().getIndex().getUUID()); assert filter != null; float indexBoost = concreteIndexBoosts.getOrDefault(shardIt.shardId().getIndex().getUUID(), DEFAULT_INDEX_BOOST); - return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), - getShardRequestOrdinal(shardIt.shardId()), getNumberOfRequestShards(shardIt.shardId().getIndex()), - getNumShards(), filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); + return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), getNumShards(), + filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); } /** diff --git a/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java index d355c60d15293..0d8c0c33cc998 100644 --- a/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/InitialSearchPhase.java @@ -27,16 +27,12 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.AtomicArray; -import org.elasticsearch.index.Index; -import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchPhaseResult; import org.elasticsearch.search.SearchShardTarget; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; @@ -53,8 +49,6 @@ abstract class InitialSearchPhase extends private final SearchRequest request; private final GroupShardsIterator toSkipShardsIts; private final GroupShardsIterator shardsIts; - private final Map shardRequestOrdinals; - private final Map numberOfRequestShardsPerIndex; private final Logger logger; private final int expectedTotalOps; private final AtomicInteger totalOps = new AtomicInteger(); @@ -68,18 +62,10 @@ abstract class InitialSearchPhase extends this.request = request; final List toSkipIterators = new ArrayList<>(); final List iterators = new ArrayList<>(); - this.shardRequestOrdinals = new HashMap<>(iterators.size()); - this.numberOfRequestShardsPerIndex = new HashMap<>(); - Map shardOrdMap = new HashMap<>(); for (final SearchShardIterator iterator : shardsIts) { if (iterator.skip()) { toSkipIterators.add(iterator); } else { - ShardId shardId = iterator.shardId(); - Index index = shardId.getIndex(); - AtomicInteger shardOrd = shardOrdMap.computeIfAbsent(index.getUUID(), (a) -> new AtomicInteger(0)); - shardRequestOrdinals.put(iterator.shardId(), shardOrd.getAndIncrement()); - numberOfRequestShardsPerIndex.put(index.getUUID(), shardOrd.intValue()); iterators.add(iterator); } } @@ -391,19 +377,4 @@ protected void skipShard(SearchShardIterator iterator) { successfulShardExecution(iterator); } - /** - * Returns the shard request ordinal for the provided shardId. - */ - protected int getShardRequestOrdinal(ShardId shardId) { - return shardRequestOrdinals.get(shardId); - } - - /** - * Return the number of requested shards for the provided index. - */ - protected int getNumberOfRequestShards(Index index) { - return numberOfRequestShardsPerIndex.get(index.getUUID()); - - } - } diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index e6f7cebdb5cf4..069c3e419db18 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -28,11 +28,17 @@ import org.elasticsearch.Version; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.routing.GroupShardsIterator; +import org.elasticsearch.cluster.routing.ShardIterator; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery; import org.elasticsearch.common.lucene.search.function.WeightFactorFunction; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.index.IndexService; @@ -51,6 +57,7 @@ import org.elasticsearch.index.search.NestedHelper; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; +import org.elasticsearch.node.ResponseCollectorService; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.collapse.CollapseContext; import org.elasticsearch.search.dfs.DfsSearchResult; @@ -81,6 +88,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; final class DefaultSearchContext extends SearchContext { @@ -92,6 +100,7 @@ final class DefaultSearchContext extends SearchContext { private final Engine.Searcher engineSearcher; private final BigArrays bigArrays; private final IndexShard indexShard; + private final ClusterService clusterService; private final IndexService indexService; private final ContextIndexSearcher searcher; private final DfsSearchResult dfsResult; @@ -154,9 +163,10 @@ final class DefaultSearchContext extends SearchContext { private final QueryShardContext queryShardContext; private FetchPhase fetchPhase; - DefaultSearchContext(long id, ShardSearchRequest request, SearchShardTarget shardTarget, Engine.Searcher engineSearcher, - IndexService indexService, IndexShard indexShard, BigArrays bigArrays, Counter timeEstimateCounter, - TimeValue timeout, FetchPhase fetchPhase, String clusterAlias, Version minNodeVersion) { + DefaultSearchContext(long id, ShardSearchRequest request, SearchShardTarget shardTarget, + Engine.Searcher engineSearcher, ClusterService clusterService, IndexService indexService, + IndexShard indexShard, BigArrays bigArrays, Counter timeEstimateCounter, TimeValue timeout, + FetchPhase fetchPhase, String clusterAlias, Version minNodeVersion) { this.id = id; this.request = request; this.fetchPhase = fetchPhase; @@ -170,6 +180,7 @@ final class DefaultSearchContext extends SearchContext { this.fetchResult = new FetchSearchResult(id, shardTarget); this.indexShard = indexShard; this.indexService = indexService; + this.clusterService = clusterService; this.searcher = new ContextIndexSearcher(engineSearcher, indexService.cache().query(), indexShard.getQueryCachingPolicy()); this.timeEstimateCounter = timeEstimateCounter; this.timeout = timeout; @@ -281,7 +292,7 @@ && new NestedHelper(mapperService()).mightMatchNestedDocs(query) } if (sliceBuilder != null) { - filters.add(sliceBuilder.toFilter(queryShardContext, shardRequestOrdinal(), numberOfIndexShards(), minNodeVersion)); + filters.add(sliceBuilder.toFilter(clusterService, request, queryShardContext, minNodeVersion)); } if (filters.isEmpty()) { @@ -337,14 +348,6 @@ public int numberOfShards() { return request.numberOfShards(); } - public int numberOfIndexShards() { - return request.numberOfIndexShards(); - } - - public int shardRequestOrdinal() { - return request.shardRequestOrdinal(); - } - @Override public float queryBoost() { return queryBoost; diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index ff2fa9644cbd8..ed7f98c3b0b12 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -616,8 +616,8 @@ private DefaultSearchContext createSearchContext(ShardSearchRequest request, Tim Engine.Searcher engineSearcher = indexShard.acquireSearcher("search"); final DefaultSearchContext searchContext = new DefaultSearchContext(idGenerator.incrementAndGet(), request, shardTarget, - engineSearcher, indexService, indexShard, bigArrays, threadPool.estimatedTimeInMillisCounter(), timeout, fetchPhase, - request.getClusterAlias(), clusterService.state().nodes().getMinNodeVersion()); + engineSearcher, clusterService, indexService, indexShard, bigArrays, threadPool.estimatedTimeInMillisCounter(), timeout, + fetchPhase, request.getClusterAlias(), clusterService.state().nodes().getMinNodeVersion()); boolean success = false; try { // we clone the query shard context here just for rewriting otherwise we diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java index 983894f1757ea..a8a0ad40eee37 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java @@ -61,8 +61,6 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { private String clusterAlias; private ShardId shardId; - private int shardRequestOrdinal; - private int numberOfIndexShards; private int numberOfShards; private SearchType searchType; private Scroll scroll; @@ -73,17 +71,18 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { private Boolean requestCache; private long nowInMillis; private boolean allowPartialSearchResults; - + private String routing; + private String preference; private boolean profile; ShardSearchLocalRequest() { } - ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int shardRequestOrdinal, int numberOfIndexShards, int numberOfShards, + ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int numberOfShards, AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { - this(shardId, shardRequestOrdinal, numberOfIndexShards, numberOfShards, searchRequest.searchType(), + this(shardId, numberOfShards, searchRequest.searchType(), searchRequest.source(), searchRequest.types(), searchRequest.requestCache(), aliasFilter, indexBoost, - searchRequest.allowPartialSearchResults()); + searchRequest.allowPartialSearchResults(), searchRequest.routing(), searchRequest.preference()); // If allowPartialSearchResults is unset (ie null), the cluster-level default should have been substituted // at this stage. Any NPEs in the above are therefore an error in request preparation logic. assert searchRequest.allowPartialSearchResults() != null; @@ -100,12 +99,10 @@ public ShardSearchLocalRequest(ShardId shardId, String[] types, long nowInMillis indexBoost = 1.0f; } - public ShardSearchLocalRequest(ShardId shardId, int shardRequestOrdinal, int numberOfIndexShards, int numberOfShards, - SearchType searchType, SearchSourceBuilder source, String[] types, - Boolean requestCache, AliasFilter aliasFilter, float indexBoost, boolean allowPartialSearchResults) { + public ShardSearchLocalRequest(ShardId shardId, int numberOfShards, SearchType searchType, SearchSourceBuilder source, String[] types, + Boolean requestCache, AliasFilter aliasFilter, float indexBoost, boolean allowPartialSearchResults, + String routing, String preference) { this.shardId = shardId; - this.shardRequestOrdinal = shardRequestOrdinal; - this.numberOfIndexShards = numberOfIndexShards; this.numberOfShards = numberOfShards; this.searchType = searchType; this.source = source; @@ -114,8 +111,11 @@ public ShardSearchLocalRequest(ShardId shardId, int shardRequestOrdinal, int num this.aliasFilter = aliasFilter; this.indexBoost = indexBoost; this.allowPartialSearchResults = allowPartialSearchResults; + this.routing = routing; + this.preference = preference; } + @Override public ShardId shardId() { return shardId; @@ -151,16 +151,6 @@ public int numberOfShards() { return numberOfShards; } - @Override - public int numberOfIndexShards() { - return numberOfIndexShards; - } - - @Override - public int shardRequestOrdinal() { - return shardRequestOrdinal; - } - @Override public SearchType searchType() { return searchType; @@ -192,6 +182,16 @@ public Scroll scroll() { return scroll; } + @Override + public String routing() { + return routing; + } + + @Override + public String preference() { + return preference; + } + @Override public void setProfile(boolean profile) { this.profile = profile; @@ -210,14 +210,6 @@ protected void innerReadFrom(StreamInput in) throws IOException { shardId = ShardId.readShardId(in); searchType = SearchType.fromId(in.readByte()); numberOfShards = in.readVInt(); - if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - shardRequestOrdinal = in.readVInt(); - numberOfIndexShards = in.readVInt(); - assert shardRequestOrdinal != -1 && numberOfIndexShards != -1; - } else { - shardRequestOrdinal = -1; - numberOfIndexShards = -1; - } scroll = in.readOptionalWriteable(Scroll::new); source = in.readOptionalWriteable(SearchSourceBuilder::new); types = in.readStringArray(); @@ -244,6 +236,13 @@ protected void innerReadFrom(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_6_3_0)) { allowPartialSearchResults = in.readOptionalBoolean(); } + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + routing = in.readOptionalString(); + preference = in.readOptionalString(); + } else { + routing = null; + preference = null; + } } protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException { @@ -251,10 +250,6 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException out.writeByte(searchType.id()); if (!asKey) { out.writeVInt(numberOfShards); - if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - out.writeVInt(shardRequestOrdinal); - out.writeVInt(numberOfIndexShards); - } } out.writeOptionalWriteable(scroll); out.writeOptionalWriteable(source); @@ -273,7 +268,12 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException if (out.getVersion().onOrAfter(Version.V_6_3_0)) { out.writeOptionalBoolean(allowPartialSearchResults); } - + if (!asKey) { + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeOptionalString(routing); + out.writeOptionalString(preference); + } + } } @Override diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java index 43ec542d589d5..3b07ef8979b56 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.internal; +import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -28,8 +29,6 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryRewriteContext; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.AliasFilterParsingException; @@ -49,15 +48,6 @@ public interface ShardSearchRequest { ShardId shardId(); - /** - * Returns the shard request ordinal of the shard for this request - * or -1 if this information is not available. - * The request shard ordinal is the id of the requested shard among all shards - * of this index that are part of the request. Note that the request shard ordinal - * is equal to the shard id if all shards of the index are part of the request. - */ - int shardRequestOrdinal(); - String[] types(); SearchSourceBuilder source(); @@ -68,15 +58,6 @@ public interface ShardSearchRequest { void source(SearchSourceBuilder source); - /** - * Returns the number of shards of this index ({@link ShardId#getIndex()}) that participates in the request - * or -1 if this information is not available. - */ - int numberOfIndexShards(); - - /** - * Returns the number of shards that participates in the request. - */ int numberOfShards(); SearchType searchType(); @@ -91,6 +72,16 @@ public interface ShardSearchRequest { Scroll scroll(); + /** + * Returns the routing of the original {@link SearchRequest}. + */ + String routing(); + + /** + * Returns the preference of the original {@link SearchRequest}. + */ + String preference(); + /** * Sets if this shard search needs to be profiled or not * @param profile True if the shard should be profiled diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java index 9066b43eb12f7..3d3a020447415 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java @@ -53,11 +53,9 @@ public class ShardSearchTransportRequest extends TransportRequest implements Sha public ShardSearchTransportRequest(){ } - public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int shardRequestOrdinal, - int numberOfIndexShards, int numberOfShards, AliasFilter aliasFilter, float indexBoost, - long nowInMillis, String clusterAlias) { - this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, shardRequestOrdinal, - numberOfIndexShards, numberOfShards, aliasFilter, indexBoost, + public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int numberOfShards, + AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { + this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, numberOfShards, aliasFilter, indexBoost, nowInMillis, clusterAlias); this.originalIndices = originalIndices; } @@ -101,11 +99,6 @@ public ShardId shardId() { return shardSearchLocalRequest.shardId(); } - @Override - public int shardRequestOrdinal() { - return shardSearchLocalRequest.shardRequestOrdinal(); - } - @Override public String[] types() { return shardSearchLocalRequest.types(); @@ -136,11 +129,6 @@ public int numberOfShards() { return shardSearchLocalRequest.numberOfShards(); } - @Override - public int numberOfIndexShards() { - return shardSearchLocalRequest.numberOfIndexShards(); - } - @Override public SearchType searchType() { return shardSearchLocalRequest.searchType(); @@ -171,6 +159,16 @@ public Scroll scroll() { return shardSearchLocalRequest.scroll(); } + @Override + public String routing() { + return shardSearchLocalRequest.routing(); + } + + @Override + public String preference() { + return shardSearchLocalRequest.preference(); + } + @Override public void readFrom(StreamInput in) throws IOException { throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); diff --git a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java index 24a828cdbd857..b6eb337d9c696 100644 --- a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java @@ -23,6 +23,12 @@ import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.Version; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.routing.GroupShardsIterator; +import org.elasticsearch.cluster.routing.ShardIterator; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -30,6 +36,7 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -39,9 +46,12 @@ import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.search.internal.ShardSearchRequest; import java.io.IOException; +import java.util.Map; import java.util.Objects; +import java.util.Set; /** * A slice builder allowing to split a scroll in multiple partitions. @@ -207,32 +217,40 @@ public int hashCode() { * Converts this QueryBuilder to a lucene {@link Query}. * * @param context Additional information needed to build the query - * @param shardRequestOrdinal The shard ordinal of for this request or -1 if this information is not available. - * @param numIndexShards The number of shards of this index that participates in the request - * or -1 if this information is not available. - * @param minNodeVersion The version of the node with the youngest version in the cluster. */ - public Query toFilter(QueryShardContext context, int shardRequestOrdinal, int numIndexShards, Version minNodeVersion) { - final int numShards; - final int shardId; - if (shardRequestOrdinal != -1 && minNodeVersion.onOrAfter(Version.V_7_0_0_alpha1)) { - /** - * We use the request shard ordinal (added in {@link Version#V_7_0_0_alpha1} only if all nodes - * are able to pass this information otherwise another slice might use the original shard - * id and that would lead to duplicated results. - */ - assert numIndexShards != -1; - shardId = shardRequestOrdinal; - numShards = numIndexShards; - } else { - shardId = context.getShardId(); - numShards = context.getIndexSettings().getNumberOfShards(); - } + public Query toFilter(ClusterService clusterService, ShardSearchRequest request, QueryShardContext context, Version minNodeVersion) { final MappedFieldType type = context.fieldMapper(field); if (type == null) { throw new IllegalArgumentException("field " + field + " not found"); } + int shardId = request.shardId().id(); + int numShards = context.getIndexSettings().getNumberOfShards(); + if (minNodeVersion.onOrAfter(Version.V_7_0_0_alpha1) && + (request.preference() != null || request.routing() != null)) { + /** + * Resolve the routings and preference to compute the request shard id and the number of shards + * since they can differ if some shards are filtered by the request. + * This filtering has been added in {@link Version#V_7_0_0_alpha1} so if there is another node in the cluster + * with a smaller version smaller we use the original shard id and number of shards in order to ensure that all + * slices use the same numbers. + */ + GroupShardsIterator group = buildShardIterator(clusterService, request); + assert group.size() <= numShards; + numShards = group.size(); + int ord = 0; + shardId = -1; + for (ShardIterator it : group) { + assert it.shardId().getIndex().equals(request.shardId().getIndex()); + if (request.shardId().equals(it.shardId())) { + shardId = ord; + break; + } + ++ ord; + } + assert shardId != -1; + } + String field = this.field; boolean useTermQuery = false; if ("_uid".equals(field)) { @@ -297,6 +315,18 @@ public Query toFilter(QueryShardContext context, int shardRequestOrdinal, int nu return new MatchAllDocsQuery(); } + /** + * Returns the {@link GroupShardsIterator} for the provided request. + */ + private GroupShardsIterator buildShardIterator(ClusterService clusterService, ShardSearchRequest request) { + final ClusterState state = clusterService.state(); + final Settings settings = clusterService.getSettings(); + final String[] indices = ((IndicesRequest) request).indices(); + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(settings); + Map> routings = resolver.resolveSearchRouting(state, request.routing(), indices); + return clusterService.operationRouting().searchShards(state, indices, routings, request.preference()); + } + @Override public String toString() { return Strings.toString(this, true, true); diff --git a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java index 7dbd3e9579fd4..6ade2b8781ecf 100644 --- a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java @@ -31,11 +31,7 @@ import org.elasticsearch.search.internal.ShardSearchTransportRequest; import org.elasticsearch.test.ESTestCase; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -45,7 +41,6 @@ public class AbstractSearchAsyncActionTests extends ESTestCase { private AbstractSearchAsyncAction createAction( - final GroupShardsIterator shardsIt, final boolean controlled, final AtomicLong expected) { @@ -66,17 +61,12 @@ private AbstractSearchAsyncAction createAction( } final SearchRequest request = new SearchRequest(); - Map aliasFilterMap = new HashMap<>(); - Map concreteIndexBoosts = new HashMap<>(); - for (SearchShardIterator it : shardsIt) { - aliasFilterMap.put(it.shardId().getIndex().getUUID(), new AliasFilter(new MatchAllQueryBuilder())); - concreteIndexBoosts.put(it.shardId().getIndex().getUUID(), 2.0f); - } request.allowPartialSearchResults(true); return new AbstractSearchAsyncAction("test", null, null, null, - aliasFilterMap, concreteIndexBoosts, null, - request, null, shardsIt, timeProvider, 0, null, - new InitialSearchPhase.ArraySearchPhaseResults<>(shardsIt.size()), request.getMaxConcurrentShardRequests(), + Collections.singletonMap("foo", new AliasFilter(new MatchAllQueryBuilder())), Collections.singletonMap("foo", 2.0f), null, + request, null, new GroupShardsIterator<>(Collections.singletonList( + new SearchShardIterator(null, null, Collections.emptyList(), null))), timeProvider, 0, null, + new InitialSearchPhase.ArraySearchPhaseResults<>(10), request.getMaxConcurrentShardRequests(), SearchResponse.Clusters.EMPTY) { @Override protected SearchPhase getNextPhase(final SearchPhaseResults results, final SearchPhaseContext context) { @@ -106,13 +96,7 @@ public void testTookWithRealClock() { private void runTestTook(final boolean controlled) { final AtomicLong expected = new AtomicLong(); - GroupShardsIterator shardsIt = new GroupShardsIterator<>( - Collections.singletonList( - new SearchShardIterator(null, new ShardId(new Index("name", "foo"), 0), - Collections.emptyList(), null) - ) - ); - AbstractSearchAsyncAction action = createAction(shardsIt, controlled, expected); + AbstractSearchAsyncAction action = createAction(controlled, expected); final long actual = action.buildTookInMillis(); if (controlled) { // with a controlled clock, we can assert the exact took time @@ -125,13 +109,7 @@ private void runTestTook(final boolean controlled) { public void testBuildShardSearchTransportRequest() { final AtomicLong expected = new AtomicLong(); - GroupShardsIterator shardsIt = new GroupShardsIterator<>( - Collections.singletonList( - new SearchShardIterator(null, new ShardId(new Index("name", "foo"), 1), - Collections.emptyList(), null) - ) - ); - AbstractSearchAsyncAction action = createAction(shardsIt, false, expected); + AbstractSearchAsyncAction action = createAction(false, expected); SearchShardIterator iterator = new SearchShardIterator("test-cluster", new ShardId(new Index("name", "foo"), 1), Collections.emptyList(), new OriginalIndices(new String[] {"name", "name1"}, IndicesOptions.strictExpand())); ShardSearchTransportRequest shardSearchTransportRequest = action.buildShardSearchRequest(iterator); @@ -139,56 +117,5 @@ public void testBuildShardSearchTransportRequest() { assertArrayEquals(new String[] {"name", "name1"}, shardSearchTransportRequest.indices()); assertEquals(new MatchAllQueryBuilder(), shardSearchTransportRequest.getAliasFilter().getQueryBuilder()); assertEquals(2.0f, shardSearchTransportRequest.indexBoost(), 0.0f); - assertEquals(0, shardSearchTransportRequest.shardRequestOrdinal()); - assertEquals(1, shardSearchTransportRequest.numberOfIndexShards()); - assertEquals(1, shardSearchTransportRequest.numberOfShards()); - } - - public void testBuildFilteredShardSearchTransportRequest() { - final AtomicLong expected = new AtomicLong(); - int numIndex = randomIntBetween(1, 5); - List shards = new ArrayList<>(); - int[] numIndexShards = new int[numIndex]; - Map> shardOrdinals = new HashMap<>(); - int totalShards = 0; - for (int i = 0; i < numIndex; i++) { - int numShards = randomIntBetween(1, 10); - numIndexShards[i] = numShards; - String indexName = Integer.toString(i); - int shardIndex = 0; - Map shardMap = new HashMap<>(); - for (int j = 0; j < numShards; j++) { - if (randomBoolean()) { - shards.add(new SearchShardIterator(null, new ShardId(new Index(indexName, indexName), j), - Collections.emptyList(), null)); - shardMap.put(j, shardIndex++); - totalShards++; - } - } - shardOrdinals.put(i, shardMap); - } - Collections.shuffle(shards, random()); - GroupShardsIterator shardsIt = new GroupShardsIterator<>(shards); - AbstractSearchAsyncAction action = createAction(shardsIt,false, expected); - - for (int i = 0; i < numIndex; i++) { - int numShards = numIndexShards[i]; - String indexName = Integer.toString(i); - Map shardMap = shardOrdinals.get(i); - if (shardMap.size() == 0) { - continue; - } - for (int j = 0; j < numShards; j++) { - if (shardMap.containsKey(j) == false) { - continue; - } - SearchShardIterator iterator = new SearchShardIterator("test-cluster", new ShardId(new Index(indexName, indexName), j), - Collections.emptyList(), new OriginalIndices(new String[]{"name", "name1"}, IndicesOptions.strictExpand())); - ShardSearchTransportRequest shardSearchTransportRequest = action.buildShardSearchRequest(iterator); - assertThat(shardSearchTransportRequest.numberOfIndexShards(), equalTo(shardMap.size())); - assertThat(shardSearchTransportRequest.shardRequestOrdinal(), equalTo(shardMap.get(j))); - assertThat(shardSearchTransportRequest.numberOfShards(), equalTo(totalShards)); - } - } } } diff --git a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java index de20f288c9dd4..171c0ca671dc7 100644 --- a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java +++ b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java @@ -62,11 +62,6 @@ public ShardId shardId() { return new ShardId(indexService.index(), 0); } - @Override - public int shardRequestOrdinal() { - return 0; - } - @Override public String[] types() { return new String[0]; @@ -97,11 +92,6 @@ public int numberOfShards() { return 0; } - @Override - public int numberOfIndexShards() { - return 0; - } - @Override public SearchType searchType() { return null; @@ -132,6 +122,16 @@ public Scroll scroll() { return null; } + @Override + public String routing() { + return null; + } + + @Override + public String preference() { + return null; + } + @Override public void setProfile(boolean profile) { diff --git a/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java b/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java index bbe4e4505ef87..f59cc85c09ccf 100644 --- a/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java +++ b/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java @@ -112,7 +112,7 @@ public void testPreProcess() throws Exception { IndexReader reader = w.getReader(); Engine.Searcher searcher = new Engine.Searcher("test", new IndexSearcher(reader))) { - DefaultSearchContext context1 = new DefaultSearchContext(1L, shardSearchRequest, null, searcher, indexService, + DefaultSearchContext context1 = new DefaultSearchContext(1L, shardSearchRequest, null, searcher, null, indexService, indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); context1.from(300); @@ -153,8 +153,8 @@ public void testPreProcess() throws Exception { + "] index level setting.")); // rescore is null but sliceBuilder is not null - DefaultSearchContext context2 = new DefaultSearchContext(2L, shardSearchRequest, null, searcher, indexService, - indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); + DefaultSearchContext context2 = new DefaultSearchContext(2L, shardSearchRequest, null, searcher, + null, indexService, indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); SliceBuilder sliceBuilder = mock(SliceBuilder.class); int numSlices = maxSlicesPerScroll + randomIntBetween(1, 100); @@ -170,8 +170,8 @@ public void testPreProcess() throws Exception { when(shardSearchRequest.getAliasFilter()).thenReturn(AliasFilter.EMPTY); when(shardSearchRequest.indexBoost()).thenReturn(AbstractQueryBuilder.DEFAULT_BOOST); - DefaultSearchContext context3 = new DefaultSearchContext(3L, shardSearchRequest, null, searcher, indexService, - indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); + DefaultSearchContext context3 = new DefaultSearchContext(3L, shardSearchRequest, null, searcher, null, + indexService, indexShard, bigArrays, null, timeout, null, null, Version.CURRENT); ParsedQuery parsedQuery = ParsedQuery.parsedMatchAllQuery(); context3.sliceBuilder(null).parsedQuery(parsedQuery).preProcess(false); assertEquals(context3.query(), context3.buildFilteredQuery(parsedQuery.query())); diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index dae22aa68e125..c58a158fc677d 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -211,9 +211,9 @@ public void onFailure(Exception e) { for (int i = 0; i < rounds; i++) { try { SearchPhaseResult searchPhaseResult = service.executeQueryPhase( - new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, + new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, new SearchSourceBuilder(), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, - true), + true, null, null), new SearchTask(123L, "", "", "", null, Collections.emptyMap())); IntArrayList intCursors = new IntArrayList(1); intCursors.add(0); @@ -243,15 +243,13 @@ public void testTimeout() throws IOException { final SearchContext contextWithDefaultTimeout = service.createContext( new ShardSearchLocalRequest( indexShard.shardId(), - 0, - 1, 1, SearchType.DEFAULT, new SearchSourceBuilder(), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), - 1.0f, true) + 1.0f, true, null, null) ); try { // the search context should inherit the default timeout @@ -265,15 +263,13 @@ public void testTimeout() throws IOException { final SearchContext context = service.createContext( new ShardSearchLocalRequest( indexShard.shardId(), - 0, - 1, 1, SearchType.DEFAULT, new SearchSourceBuilder().timeout(TimeValue.timeValueSeconds(seconds)), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), - 1.0f, true) + 1.0f, true, null, null) ); try { // the search context should inherit the query timeout @@ -300,13 +296,14 @@ public void testMaxDocvalueFieldsSearch() throws IOException { for (int i = 0; i < indexService.getIndexSettings().getMaxDocvalueFields(); i++) { searchSourceBuilder.docValueField("field" + i); } - try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0,1, 1, SearchType.DEFAULT, - searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))) { + try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true, null, null))) { assertNotNull(context); searchSourceBuilder.docValueField("one_field_too_much"); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, - searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))); + () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, + true, null, null))); assertEquals( "Trying to retrieve too many docvalue_fields. Must be less than or equal to: [100] but was [101]. " + "This limit can be set by changing the [index.max_docvalue_fields_search] index level setting.", @@ -331,14 +328,15 @@ public void testMaxScriptFieldsSearch() throws IOException { searchSourceBuilder.scriptField("field" + i, new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); } - try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, - searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))) { + try (SearchContext context = service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true, null, null))) { assertNotNull(context); searchSourceBuilder.scriptField("anotherScriptField", new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.DEFAULT, - searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))); + () -> service.createContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), + 1.0f, true, null, null))); assertEquals( "Trying to retrieve too many script_fields. Must be less than or equal to: [" + maxScriptFields + "] but was [" + (maxScriptFields + 1) @@ -409,29 +407,29 @@ public void testCanMatch() throws IOException { final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index")); final IndexShard indexShard = indexService.getShard(0); final boolean allowPartialSearchResults = true; - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, null, - Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, null, + Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults, null, null))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, - allowPartialSearchResults))); + allowPartialSearchResults, null, null))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchAllQueryBuilder()), Strings.EMPTY_ARRAY, false, - new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); + new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults, null, null))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()) .aggregation(new TermsAggregationBuilder("test", ValueType.STRING).minDocCount(0)), Strings.EMPTY_ARRAY, false, - new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); - assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, + new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults, null, null))); + assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()) .aggregation(new GlobalAggregationBuilder("test")), Strings.EMPTY_ARRAY, false, - new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); + new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults, null, null))); - assertFalse(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 0, 1, 1, SearchType.QUERY_THEN_FETCH, + assertFalse(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, new SearchSourceBuilder().query(new MatchNoneQueryBuilder()), Strings.EMPTY_ARRAY, false, - new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); + new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults, null, null))); } diff --git a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java index 2f66f8a2266a8..ba1dc565cd28e 100644 --- a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java @@ -74,7 +74,8 @@ public void testSerialization() throws Exception { assertEquals(deserializedRequest.searchType(), shardSearchTransportRequest.searchType()); assertEquals(deserializedRequest.shardId(), shardSearchTransportRequest.shardId()); assertEquals(deserializedRequest.numberOfShards(), shardSearchTransportRequest.numberOfShards()); - assertEquals(deserializedRequest.numberOfIndexShards(), shardSearchTransportRequest.numberOfIndexShards()); + assertEquals(deserializedRequest.routing(), shardSearchTransportRequest.routing()); + assertEquals(deserializedRequest.preference(), shardSearchTransportRequest.preference()); assertEquals(deserializedRequest.cacheKey(), shardSearchTransportRequest.cacheKey()); assertNotSame(deserializedRequest, shardSearchTransportRequest); assertEquals(deserializedRequest.getAliasFilter(), shardSearchTransportRequest.getAliasFilter()); @@ -93,8 +94,8 @@ private ShardSearchTransportRequest createShardSearchTransportRequest() throws I } else { filteringAliases = new AliasFilter(null, Strings.EMPTY_ARRAY); } - return new ShardSearchTransportRequest(new OriginalIndices(searchRequest), searchRequest, shardId, shardId.getId(), - randomIntBetween(1, 100), randomIntBetween(1, 100), filteringAliases, randomBoolean() ? 1.0f : randomFloat(), + return new ShardSearchTransportRequest(new OriginalIndices(searchRequest), searchRequest, shardId, + randomIntBetween(1, 100), filteringAliases, randomBoolean() ? 1.0f : randomFloat(), Math.abs(randomLong()), null); } diff --git a/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java b/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java index e8a046a8a7ae8..75ae9f62fa2bd 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java @@ -114,7 +114,7 @@ public void testDocIdSort() throws Exception { } } - public void testWithRouting() throws Exception { + public void testWithPreference() throws Exception { int numShards = 10; setupIndex(numShards, true); SearchResponse sr = client().prepareSearch("test") @@ -136,6 +136,28 @@ public void testWithRouting() throws Exception { } } + public void testWithRouting() throws Exception { + int numShards = 10; + setupIndex(numShards, true); + SearchResponse sr = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setRouting("foo", "bar") + .setSize(0) + .get(); + int numDocs = (int) sr.getHits().getTotalHits(); + int max = randomIntBetween(2, numShards*3); + for (String field : new String[]{"_id", "random_int", "static_int"}) { + int fetchSize = randomIntBetween(10, 100); + SearchRequestBuilder request = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) + .setSize(fetchSize) + .setRouting("foo", "bar") + .addSort(SortBuilders.fieldSort("_doc")); + assertSearchSlicesWithScroll(request, field, max, numDocs); + } + } + public void testNumericSort() throws Exception { int numShards = randomIntBetween(1, 7); setupIndex(numShards, true); diff --git a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java index f6da7c438fb4e..4a7ade0a9a521 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java @@ -30,19 +30,37 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.store.RAMDirectory; import org.elasticsearch.Version; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.search.SearchShardIterator; +import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.routing.GroupShardsIterator; +import org.elasticsearch.cluster.routing.OperationRouting; +import org.elasticsearch.cluster.routing.ShardIterator; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.index.query.Rewriteable; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.search.Scroll; +import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.internal.AliasFilter; +import org.elasticsearch.search.internal.ShardSearchRequest; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -58,13 +76,138 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class SliceBuilderTests extends ESTestCase { private static final int MAX_SLICE = 20; - private static SliceBuilder randomSliceBuilder() throws IOException { + static class ShardSearchRequestTest implements IndicesRequest, ShardSearchRequest { + private final String[] indices; + private final int shardId; + private final String routing; + private final String preference; + + ShardSearchRequestTest(String index, int shardId, String routing, String preference) { + this.indices = new String[] { index }; + this.shardId = shardId; + this.routing = routing; + this.preference = preference; + } + + @Override + public String[] indices() { + return indices; + } + + @Override + public IndicesOptions indicesOptions() { + return null; + } + + @Override + public ShardId shardId() { + return new ShardId(new Index(indices[0], indices[0]), shardId); + } + + @Override + public String[] types() { + return new String[0]; + } + + @Override + public SearchSourceBuilder source() { + return null; + } + + @Override + public AliasFilter getAliasFilter() { + return null; + } + + @Override + public void setAliasFilter(AliasFilter filter) { + + } + + @Override + public void source(SearchSourceBuilder source) { + + } + + @Override + public int numberOfShards() { + return 0; + } + + @Override + public SearchType searchType() { + return null; + } + + @Override + public float indexBoost() { + return 0; + } + + @Override + public long nowInMillis() { + return 0; + } + + @Override + public Boolean requestCache() { + return null; + } + + @Override + public Boolean allowPartialSearchResults() { + return null; + } + + @Override + public Scroll scroll() { + return null; + } + + @Override + public String routing() { + return routing; + } + + @Override + public String preference() { + return preference; + } + + @Override + public void setProfile(boolean profile) { + + } + + @Override + public boolean isProfile() { + return false; + } + + @Override + public BytesReference cacheKey() throws IOException { + return null; + } + + @Override + public String getClusterAlias() { + return null; + } + + @Override + public Rewriteable getRewriteable() { + return null; + } + } + + private static SliceBuilder randomSliceBuilder() { int max = randomIntBetween(2, MAX_SLICE); int id = randomIntBetween(1, max - 1); String field = randomAlphaOfLengthBetween(5, 20); @@ -75,7 +218,7 @@ private static SliceBuilder serializedCopy(SliceBuilder original) throws IOExcep return copyWriteable(original, new NamedWriteableRegistry(Collections.emptyList()), SliceBuilder::new); } - private static SliceBuilder mutate(SliceBuilder original) throws IOException { + private static SliceBuilder mutate(SliceBuilder original) { switch (randomIntBetween(0, 2)) { case 0: return new SliceBuilder(original.getField() + "_xyz", original.getId(), original.getMax()); case 1: return new SliceBuilder(original.getField(), original.getId() - 1, original.getMax()); @@ -84,6 +227,63 @@ private static SliceBuilder mutate(SliceBuilder original) throws IOException { } } + private IndexSettings createIndexSettings(Version indexVersionCreated, int numShards) { + Settings settings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, indexVersionCreated) + .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .build(); + IndexMetaData indexState = IndexMetaData.builder("index").settings(settings).build(); + return new IndexSettings(indexState, Settings.EMPTY); + } + + private ShardSearchRequest createRequest(int shardId) { + return createRequest(shardId,null, null); + } + + private ShardSearchRequest createRequest(int shardId, String routing, String preference) { + return new ShardSearchRequestTest("index", shardId, routing, preference); + } + + private QueryShardContext createShardContext(Version indexVersionCreated, IndexReader reader, + String fieldName, DocValuesType dvType, int numShards, int shardId) { + MappedFieldType fieldType = new MappedFieldType() { + @Override + public MappedFieldType clone() { + return null; + } + + @Override + public String typeName() { + return null; + } + + @Override + public Query termQuery(Object value, @Nullable QueryShardContext context) { + return null; + } + + public Query existsQuery(QueryShardContext context) { + return null; + } + }; + fieldType.setName(fieldName); + QueryShardContext context = mock(QueryShardContext.class); + when(context.fieldMapper(fieldName)).thenReturn(fieldType); + when(context.getIndexReader()).thenReturn(reader); + when(context.getShardId()).thenReturn(shardId); + IndexSettings indexSettings = createIndexSettings(indexVersionCreated, numShards); + when(context.getIndexSettings()).thenReturn(indexSettings); + if (dvType != null) { + fieldType.setHasDocValues(true); + fieldType.setDocValuesType(dvType); + IndexNumericFieldData fd = mock(IndexNumericFieldData.class); + when(context.getForField(fieldType)).thenReturn(fd); + } + return context; + + } + public void testSerialization() throws Exception { SliceBuilder original = randomSliceBuilder(); SliceBuilder deserialized = serializedCopy(original); @@ -131,92 +331,41 @@ public void testInvalidArguments() throws Exception { assertEquals("max must be greater than id", e.getMessage()); } - public void testToFilter() throws IOException { + public void testToFilterSimple() throws IOException { Directory dir = new RAMDirectory(); try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) { writer.commit(); } - QueryShardContext context = mock(QueryShardContext.class); try (IndexReader reader = DirectoryReader.open(dir)) { - MappedFieldType fieldType = new MappedFieldType() { - @Override - public MappedFieldType clone() { - return null; - } - - @Override - public String typeName() { - return null; - } - - @Override - public Query termQuery(Object value, @Nullable QueryShardContext context) { - return null; - } - - public Query existsQuery(QueryShardContext context) { - return null; - } - }; - fieldType.setName(IdFieldMapper.NAME); - fieldType.setHasDocValues(false); - when(context.fieldMapper(IdFieldMapper.NAME)).thenReturn(fieldType); - when(context.getIndexReader()).thenReturn(reader); - Settings settings = Settings.builder() - .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 2) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) - .build(); - IndexMetaData indexState = IndexMetaData.builder("index").settings(settings).build(); - IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY); - when(context.getIndexSettings()).thenReturn(indexSettings); + QueryShardContext context = + createShardContext(Version.CURRENT, reader, "_id", DocValuesType.SORTED_NUMERIC, 1,0); SliceBuilder builder = new SliceBuilder(5, 10); - Query query = builder.toFilter(context, 0, 1, Version.CURRENT); + Query query = builder.toFilter(null, createRequest(0), context, Version.CURRENT); assertThat(query, instanceOf(TermsSliceQuery.class)); - assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); + assertThat(builder.toFilter(null, createRequest(0), context, Version.CURRENT), equalTo(query)); try (IndexReader newReader = DirectoryReader.open(dir)) { when(context.getIndexReader()).thenReturn(newReader); - assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); + assertThat(builder.toFilter(null, createRequest(0), context, Version.CURRENT), equalTo(query)); } } + } + public void testToFilterRandom() throws IOException { + Directory dir = new RAMDirectory(); + try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) { + writer.commit(); + } try (IndexReader reader = DirectoryReader.open(dir)) { - MappedFieldType fieldType = new MappedFieldType() { - @Override - public MappedFieldType clone() { - return null; - } - - @Override - public String typeName() { - return null; - } - - @Override - public Query termQuery(Object value, @Nullable QueryShardContext context) { - return null; - } - - public Query existsQuery(QueryShardContext context) { - return null; - } - }; - fieldType.setName("field_doc_values"); - fieldType.setHasDocValues(true); - fieldType.setDocValuesType(DocValuesType.SORTED_NUMERIC); - when(context.fieldMapper("field_doc_values")).thenReturn(fieldType); - when(context.getIndexReader()).thenReturn(reader); - IndexNumericFieldData fd = mock(IndexNumericFieldData.class); - when(context.getForField(fieldType)).thenReturn(fd); - SliceBuilder builder = new SliceBuilder("field_doc_values", 5, 10); - when(context.getShardId()).thenReturn(0); - Query query = builder.toFilter(context, 0, 1, Version.CURRENT); + QueryShardContext context = + createShardContext(Version.CURRENT, reader, "field", DocValuesType.SORTED_NUMERIC, 1,0); + SliceBuilder builder = new SliceBuilder("field", 5, 10); + Query query = builder.toFilter(null, createRequest(0), context, Version.CURRENT); assertThat(query, instanceOf(DocValuesSliceQuery.class)); - assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); + assertThat(builder.toFilter(null, createRequest(0), context, Version.CURRENT), equalTo(query)); try (IndexReader newReader = DirectoryReader.open(dir)) { when(context.getIndexReader()).thenReturn(newReader); - assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); + assertThat(builder.toFilter(null, createRequest(0), context, Version.CURRENT), equalTo(query)); } // numSlices > numShards @@ -226,7 +375,8 @@ public Query existsQuery(QueryShardContext context) { for (int i = 0; i < numSlices; i++) { for (int j = 0; j < numShards; j++) { SliceBuilder slice = new SliceBuilder("_id", i, numSlices); - Query q = slice.toFilter(context, j, numShards, Version.CURRENT); + context = createShardContext(Version.CURRENT, reader, "_id", DocValuesType.SORTED, numShards, j); + Query q = slice.toFilter(null, createRequest(j), context, Version.CURRENT); if (q instanceof TermsSliceQuery || q instanceof MatchAllDocsQuery) { AtomicInteger count = numSliceMap.get(j); if (count == null) { @@ -250,12 +400,13 @@ public Query existsQuery(QueryShardContext context) { // numShards > numSlices numShards = randomIntBetween(4, 100); - numSlices = randomIntBetween(2, numShards-1); + numSlices = randomIntBetween(2, numShards - 1); List targetShards = new ArrayList<>(); for (int i = 0; i < numSlices; i++) { for (int j = 0; j < numShards; j++) { SliceBuilder slice = new SliceBuilder("_id", i, numSlices); - Query q = slice.toFilter(context, j, numShards, Version.CURRENT); + context = createShardContext(Version.CURRENT, reader, "_id", DocValuesType.SORTED, numShards, j); + Query q = slice.toFilter(null, createRequest(j), context, Version.CURRENT); if (q instanceof MatchNoDocsQuery == false) { assertThat(q, instanceOf(MatchAllDocsQuery.class)); targetShards.add(j); @@ -271,7 +422,7 @@ public Query existsQuery(QueryShardContext context) { for (int i = 0; i < numSlices; i++) { for (int j = 0; j < numShards; j++) { SliceBuilder slice = new SliceBuilder("_id", i, numSlices); - Query q = slice.toFilter(context, j, numShards, Version.CURRENT); + Query q = slice.toFilter(null, createRequest(j), context, Version.CURRENT); if (i == j) { assertThat(q, instanceOf(MatchAllDocsQuery.class)); } else { @@ -280,84 +431,33 @@ public Query existsQuery(QueryShardContext context) { } } } + } + public void testInvalidField() throws IOException { + Directory dir = new RAMDirectory(); + try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) { + writer.commit(); + } try (IndexReader reader = DirectoryReader.open(dir)) { - MappedFieldType fieldType = new MappedFieldType() { - @Override - public MappedFieldType clone() { - return null; - } - - @Override - public String typeName() { - return null; - } - - @Override - public Query termQuery(Object value, @Nullable QueryShardContext context) { - return null; - } - - public Query existsQuery(QueryShardContext context) { - return null; - } - }; - fieldType.setName("field_without_doc_values"); - when(context.fieldMapper("field_without_doc_values")).thenReturn(fieldType); - when(context.getIndexReader()).thenReturn(reader); - SliceBuilder builder = new SliceBuilder("field_without_doc_values", 5, 10); - when(context.getShardId()).thenReturn(0); - IllegalArgumentException exc = - expectThrows(IllegalArgumentException.class, () -> builder.toFilter(context, 0, 1, Version.CURRENT)); + QueryShardContext context = createShardContext(Version.CURRENT, reader, "field", null, 1,0); + SliceBuilder builder = new SliceBuilder("field", 5, 10); + IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, + () -> builder.toFilter(null, createRequest(0), context, Version.CURRENT)); assertThat(exc.getMessage(), containsString("cannot load numeric doc values")); } } - public void testToFilterDeprecationMessage() throws IOException { Directory dir = new RAMDirectory(); try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) { writer.commit(); } - QueryShardContext context = mock(QueryShardContext.class); try (IndexReader reader = DirectoryReader.open(dir)) { - MappedFieldType fieldType = new MappedFieldType() { - @Override - public MappedFieldType clone() { - return null; - } - - @Override - public String typeName() { - return null; - } - - @Override - public Query termQuery(Object value, @Nullable QueryShardContext context) { - return null; - } - - public Query existsQuery(QueryShardContext context) { - return null; - } - }; - fieldType.setName("_uid"); - fieldType.setHasDocValues(false); - when(context.fieldMapper("_uid")).thenReturn(fieldType); - when(context.getIndexReader()).thenReturn(reader); - Settings settings = Settings.builder() - .put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_6_3_0) - .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 2) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) - .build(); - IndexMetaData indexState = IndexMetaData.builder("index").settings(settings).build(); - IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY); - when(context.getIndexSettings()).thenReturn(indexSettings); + QueryShardContext context = createShardContext(Version.V_6_3_0, reader, "_uid", null, 1,0); SliceBuilder builder = new SliceBuilder("_uid", 5, 10); - when(context.getShardId()).thenReturn(0); - Query query = builder.toFilter(context, 0, 1, Version.CURRENT); + Query query = builder.toFilter(null, createRequest(0), context, Version.CURRENT); assertThat(query, instanceOf(TermsSliceQuery.class)); - assertThat(builder.toFilter(context, 0, 1, Version.CURRENT), equalTo(query)); + assertThat(builder.toFilter(null, createRequest(0), context, Version.CURRENT), equalTo(query)); assertWarnings("Computing slices on the [_uid] field is deprecated for 6.x indices, use [_id] instead"); } } @@ -377,67 +477,33 @@ public void testSerializationBackcompat() throws IOException { assertEquals(sliceBuilder, copy63); } - public void testRemapShards() { - QueryShardContext context = mock(QueryShardContext.class); - Settings settings = Settings.builder() - .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 10) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) - .build(); - IndexMetaData indexState = IndexMetaData.builder("index").settings(settings).build(); - IndexSettings indexSettings = new IndexSettings(indexState, Settings.EMPTY); - when(context.getIndexSettings()).thenReturn(indexSettings); - MappedFieldType fieldType = new MappedFieldType() { - @Override - public MappedFieldType clone() { - return null; - } - - @Override - public String typeName() { - return null; - } - - @Override - public Query termQuery(Object value, @Nullable QueryShardContext context) { - return null; - } - - public Query existsQuery(QueryShardContext context) { - return null; - } - }; - fieldType.setName(IdFieldMapper.NAME); - fieldType.setHasDocValues(false); - when(context.fieldMapper(IdFieldMapper.NAME)).thenReturn(fieldType); - - for (int id = 0; id < 10; id++) { - SliceBuilder builder = new SliceBuilder("_id", id, 10); - for (int shard = 0; shard < 10; shard++) { - when(context.getShardId()).thenReturn(shard); - Query query = builder.toFilter(context, -1, -1, Version.CURRENT); - if (id == shard) { - assertThat(query, equalTo(new MatchAllDocsQuery())); - } else { - assertThat(query, equalTo(new MatchNoDocsQuery())); - } - query = builder.toFilter(context, shard, 10, Version.CURRENT); - if (id == shard) { - assertThat(query, equalTo(new MatchAllDocsQuery())); - } else { - assertThat(query, equalTo(new MatchNoDocsQuery())); - } - query = builder.toFilter(context, shard, 5, Version.CURRENT); - if (shard >= 5) { - assertThat(query, equalTo(new MatchNoDocsQuery())); - } else if (shard == id) { - assertThat(query, equalTo(new TermsSliceQuery("_id", 0, 2))); - } else if (id % 5 == shard) { - assertThat(query, equalTo(new TermsSliceQuery("_id", 1, 2))); - } else { - assertThat(query, equalTo(new MatchNoDocsQuery())); - } - } + public void testToFilterWithRouting() throws IOException { + Directory dir = new RAMDirectory(); + try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) { + writer.commit(); + } + ClusterService clusterService = mock(ClusterService.class); + ClusterState state = mock(ClusterState.class); + when(state.metaData()).thenReturn(MetaData.EMPTY_META_DATA); + when(clusterService.state()).thenReturn(state); + OperationRouting routing = mock(OperationRouting.class); + GroupShardsIterator it = new GroupShardsIterator<>( + Collections.singletonList( + new SearchShardIterator(null, new ShardId("index", "index", 1), null, null) + ) + ); + when(routing.searchShards(any(), any(), any(), any())).thenReturn(it); + when(clusterService.operationRouting()).thenReturn(routing); + when(clusterService.getSettings()).thenReturn(Settings.EMPTY); + try (IndexReader reader = DirectoryReader.open(dir)) { + QueryShardContext context = createShardContext(Version.CURRENT, reader, "field", DocValuesType.SORTED, 5, 0); + SliceBuilder builder = new SliceBuilder("field", 6, 10); + Query query = builder.toFilter(clusterService, createRequest(1, "foo", null), context, Version.CURRENT); + assertEquals(new DocValuesSliceQuery("field", 6, 10), query); + query = builder.toFilter(clusterService, createRequest(1, null, "foo"), context, Version.CURRENT); + assertEquals(new DocValuesSliceQuery("field", 6, 10), query); + query = builder.toFilter(clusterService, createRequest(1, null, "foo"), context, Version.V_6_2_0); + assertEquals(new DocValuesSliceQuery("field", 1, 2), query); } } } From 9fedb3b0505721265eee6b246a68ef53c202d863 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 20 Apr 2018 12:57:45 +0200 Subject: [PATCH 4/8] cosmetics --- .../org/elasticsearch/search/DefaultSearchContext.java | 7 ------- .../java/org/elasticsearch/search/slice/SliceBuilder.java | 7 ++++++- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index 069c3e419db18..d681a186892db 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -28,17 +28,12 @@ import org.elasticsearch.Version; import org.elasticsearch.action.search.SearchTask; import org.elasticsearch.action.search.SearchType; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; -import org.elasticsearch.cluster.routing.GroupShardsIterator; -import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery; import org.elasticsearch.common.lucene.search.function.WeightFactorFunction; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.index.IndexService; @@ -57,7 +52,6 @@ import org.elasticsearch.index.search.NestedHelper; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; -import org.elasticsearch.node.ResponseCollectorService; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.collapse.CollapseContext; import org.elasticsearch.search.dfs.DfsSearchResult; @@ -88,7 +82,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; final class DefaultSearchContext extends SearchContext { diff --git a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java index b6eb337d9c696..677ea1dfb916a 100644 --- a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java @@ -321,7 +321,12 @@ public Query toFilter(ClusterService clusterService, ShardSearchRequest request, private GroupShardsIterator buildShardIterator(ClusterService clusterService, ShardSearchRequest request) { final ClusterState state = clusterService.state(); final Settings settings = clusterService.getSettings(); - final String[] indices = ((IndicesRequest) request).indices(); + final String[] indices; + if (request instanceof IndicesRequest) { + indices = ((IndicesRequest) request).indices(); + } else { + indices = new String[] { request.shardId().getIndex().getName() }; + } IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(settings); Map> routings = resolver.resolveSearchRouting(state, request.routing(), indices); return clusterService.operationRouting().searchShards(state, indices, routings, request.preference()); From 82899935af18700e12979bd2744c553a5f9d90ca Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 23 Apr 2018 13:38:39 +0200 Subject: [PATCH 5/8] send resolved routing values for the index in shard search request --- .../search/AbstractSearchAsyncAction.java | 10 +- .../search/CanMatchPreFilterSearchPhase.java | 8 +- .../SearchDfsQueryThenFetchAsyncAction.java | 7 +- .../SearchQueryThenFetchAsyncAction.java | 10 +- .../action/search/TransportSearchAction.java | 22 +-- .../cluster/routing/PlainShardsIterator.java | 2 +- .../cluster/routing/ShardRouting.java | 6 +- .../internal/ShardSearchLocalRequest.java | 25 ++-- .../search/internal/ShardSearchRequest.java | 7 +- .../internal/ShardSearchTransportRequest.java | 9 +- .../search/slice/SliceBuilder.java | 61 ++++---- .../AbstractSearchAsyncActionTests.java | 15 +- .../CanMatchPreFilterSearchPhaseTests.java | 9 +- .../action/search/SearchAsyncActionTests.java | 3 + .../cluster/routing/PrimaryTermsTests.java | 2 +- .../cluster/routing/RoutingTableTests.java | 2 +- .../index/SearchSlowLogTests.java | 2 +- .../elasticsearch/routing/AliasRoutingIT.java | 2 +- .../routing/SimpleRoutingIT.java | 4 +- .../ShardSearchTransportRequestTests.java | 10 +- .../search/slice/SearchSliceIT.java | 137 ++++++++---------- .../search/slice/SliceBuilderTests.java | 17 ++- 22 files changed, 199 insertions(+), 171 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java index 53f681a2243c5..91aec1171dcd6 100644 --- a/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java @@ -37,8 +37,10 @@ import org.elasticsearch.search.internal.ShardSearchTransportRequest; import org.elasticsearch.transport.Transport; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -62,6 +64,7 @@ abstract class AbstractSearchAsyncAction exten private final long clusterStateVersion; private final Map aliasFilter; private final Map concreteIndexBoosts; + private final Map> indexRoutings; private final SetOnce> shardFailures = new SetOnce<>(); private final Object shardFailuresMutex = new Object(); private final AtomicInteger successfulOps = new AtomicInteger(); @@ -72,6 +75,7 @@ abstract class AbstractSearchAsyncAction exten protected AbstractSearchAsyncAction(String name, Logger logger, SearchTransportService searchTransportService, BiFunction nodeIdToConnection, Map aliasFilter, Map concreteIndexBoosts, + Map> indexRoutings, Executor executor, SearchRequest request, ActionListener listener, GroupShardsIterator shardsIts, TransportSearchAction.SearchTimeProvider timeProvider, long clusterStateVersion, @@ -89,6 +93,7 @@ protected AbstractSearchAsyncAction(String name, Logger logger, SearchTransportS this.clusterStateVersion = clusterStateVersion; this.concreteIndexBoosts = concreteIndexBoosts; this.aliasFilter = aliasFilter; + this.indexRoutings = indexRoutings; this.results = resultConsumer; this.clusters = clusters; } @@ -318,8 +323,11 @@ public final ShardSearchTransportRequest buildShardSearchRequest(SearchShardIter AliasFilter filter = aliasFilter.get(shardIt.shardId().getIndex().getUUID()); assert filter != null; float indexBoost = concreteIndexBoosts.getOrDefault(shardIt.shardId().getIndex().getUUID(), DEFAULT_INDEX_BOOST); + String indexName = shardIt.shardId().getIndex().getName(); + final String[] routings = indexRoutings.getOrDefault(indexName, Collections.emptySet()) + .toArray(new String[0]); return new ShardSearchTransportRequest(shardIt.getOriginalIndices(), request, shardIt.shardId(), getNumShards(), - filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias); + filter, indexBoost, timeProvider.getAbsoluteStartMillis(), clusterAlias, routings); } /** diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index fe42d50393635..0873ff40f7500 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -27,6 +27,7 @@ import org.elasticsearch.transport.Transport; import java.util.Map; +import java.util.Set; import java.util.concurrent.Executor; import java.util.function.BiFunction; import java.util.function.Function; @@ -47,6 +48,7 @@ final class CanMatchPreFilterSearchPhase extends AbstractSearchAsyncAction nodeIdToConnection, Map aliasFilter, Map concreteIndexBoosts, + Map> indexRoutings, Executor executor, SearchRequest request, ActionListener listener, GroupShardsIterator shardsIts, TransportSearchAction.SearchTimeProvider timeProvider, long clusterStateVersion, @@ -56,9 +58,9 @@ final class CanMatchPreFilterSearchPhase extends AbstractSearchAsyncAction nodeIdToConnection, final Map aliasFilter, - final Map concreteIndexBoosts, final SearchPhaseController searchPhaseController, final Executor executor, + final Map concreteIndexBoosts, final Map> indexRoutings, + final SearchPhaseController searchPhaseController, final Executor executor, final SearchRequest request, final ActionListener listener, final GroupShardsIterator shardsIts, final TransportSearchAction.SearchTimeProvider timeProvider, final long clusterStateVersion, final SearchTask task, SearchResponse.Clusters clusters) { - super("dfs", logger, searchTransportService, nodeIdToConnection, aliasFilter, concreteIndexBoosts, executor, request, listener, + super("dfs", logger, searchTransportService, nodeIdToConnection, aliasFilter, concreteIndexBoosts, indexRoutings, + executor, request, listener, shardsIts, timeProvider, clusterStateVersion, task, new ArraySearchPhaseResults<>(shardsIts.size()), request.getMaxConcurrentShardRequests(), clusters); this.searchPhaseController = searchPhaseController; diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java b/server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java index b7669312b0088..bbd84011de00b 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java @@ -28,6 +28,7 @@ import org.elasticsearch.transport.Transport; import java.util.Map; +import java.util.Set; import java.util.concurrent.Executor; import java.util.function.BiFunction; @@ -37,13 +38,14 @@ final class SearchQueryThenFetchAsyncAction extends AbstractSearchAsyncAction nodeIdToConnection, final Map aliasFilter, - final Map concreteIndexBoosts, final SearchPhaseController searchPhaseController, final Executor executor, + final Map concreteIndexBoosts, final Map> indexRoutings, + final SearchPhaseController searchPhaseController, final Executor executor, final SearchRequest request, final ActionListener listener, final GroupShardsIterator shardsIts, final TransportSearchAction.SearchTimeProvider timeProvider, long clusterStateVersion, SearchTask task, SearchResponse.Clusters clusters) { - super("query", logger, searchTransportService, nodeIdToConnection, aliasFilter, concreteIndexBoosts, executor, request, listener, - shardsIts, timeProvider, clusterStateVersion, task, searchPhaseController.newSearchPhaseResults(request, shardsIts.size()), - request.getMaxConcurrentShardRequests(), clusters); + super("query", logger, searchTransportService, nodeIdToConnection, aliasFilter, concreteIndexBoosts, indexRoutings, + executor, request, listener, shardsIts, timeProvider, clusterStateVersion, task, + searchPhaseController.newSearchPhaseResults(request, shardsIts.size()), request.getMaxConcurrentShardRequests(), clusters); this.searchPhaseController = searchPhaseController; } 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 bd533ce7b097a..6b39af478f432 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -297,6 +297,7 @@ private void executeSearch(SearchTask task, SearchTimeProvider timeProvider, Sea Map aliasFilter = buildPerIndexAliasFilter(searchRequest, clusterState, indices, remoteAliasMap); Map> routingMap = indexNameExpressionResolver.resolveSearchRouting(clusterState, searchRequest.routing(), searchRequest.indices()); + routingMap = routingMap == null ? Collections.emptyMap() : Collections.unmodifiableMap(routingMap); String[] concreteIndices = new String[indices.length]; for (int i = 0; i < indices.length; i++) { concreteIndices[i] = indices[i].getName(); @@ -350,7 +351,7 @@ private void executeSearch(SearchTask task, SearchTimeProvider timeProvider, Sea } boolean preFilterSearchShards = shouldPreFilterSearchShards(searchRequest, shardIterators); searchAsyncAction(task, searchRequest, shardIterators, timeProvider, connectionLookup, clusterState.version(), - Collections.unmodifiableMap(aliasFilter), concreteIndexBoosts, listener, preFilterSearchShards, clusters).start(); + Collections.unmodifiableMap(aliasFilter), concreteIndexBoosts, routingMap, listener, preFilterSearchShards, clusters).start(); } private boolean shouldPreFilterSearchShards(SearchRequest searchRequest, GroupShardsIterator shardIterators) { @@ -380,17 +381,20 @@ private AbstractSearchAsyncAction searchAsyncAction(SearchTask task, SearchReque GroupShardsIterator shardIterators, SearchTimeProvider timeProvider, BiFunction connectionLookup, - long clusterStateVersion, Map aliasFilter, + long clusterStateVersion, + Map aliasFilter, Map concreteIndexBoosts, - ActionListener listener, boolean preFilter, + Map> indexRoutings, + ActionListener listener, + boolean preFilter, SearchResponse.Clusters clusters) { Executor executor = threadPool.executor(ThreadPool.Names.SEARCH); if (preFilter) { return new CanMatchPreFilterSearchPhase(logger, searchTransportService, connectionLookup, - aliasFilter, concreteIndexBoosts, executor, searchRequest, listener, shardIterators, + aliasFilter, concreteIndexBoosts, indexRoutings, executor, searchRequest, listener, shardIterators, timeProvider, clusterStateVersion, task, (iter) -> { AbstractSearchAsyncAction action = searchAsyncAction(task, searchRequest, iter, timeProvider, connectionLookup, - clusterStateVersion, aliasFilter, concreteIndexBoosts, listener, false, clusters); + clusterStateVersion, aliasFilter, concreteIndexBoosts, indexRoutings, listener, false, clusters); return new SearchPhase(action.getName()) { @Override public void run() throws IOException { @@ -403,14 +407,14 @@ public void run() throws IOException { switch (searchRequest.searchType()) { case DFS_QUERY_THEN_FETCH: searchAsyncAction = new SearchDfsQueryThenFetchAsyncAction(logger, searchTransportService, connectionLookup, - aliasFilter, concreteIndexBoosts, searchPhaseController, executor, searchRequest, listener, shardIterators, - timeProvider, clusterStateVersion, task, clusters); + aliasFilter, concreteIndexBoosts, indexRoutings, searchPhaseController, executor, searchRequest, listener, + shardIterators, timeProvider, clusterStateVersion, task, clusters); break; case QUERY_AND_FETCH: case QUERY_THEN_FETCH: searchAsyncAction = new SearchQueryThenFetchAsyncAction(logger, searchTransportService, connectionLookup, - aliasFilter, concreteIndexBoosts, searchPhaseController, executor, searchRequest, listener, shardIterators, - timeProvider, clusterStateVersion, task, clusters); + aliasFilter, concreteIndexBoosts, indexRoutings, searchPhaseController, executor, searchRequest, listener, + shardIterators, timeProvider, clusterStateVersion, task, clusters); break; default: throw new IllegalStateException("Unknown search type: [" + searchRequest.searchType() + "]"); diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java b/server/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java index 6cb1989a8dd02..e9a99b7b456c4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java @@ -24,7 +24,7 @@ /** * A simple {@link ShardsIterator} that iterates a list or sub-list of - * {@link ShardRouting shard routings}. + * {@link ShardRouting shard indexRoutings}. */ public class PlainShardsIterator implements ShardsIterator { diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java index be1213ad134f1..6a9a105b6c432 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java @@ -38,7 +38,7 @@ /** * {@link ShardRouting} immutably encapsulates information about shard - * routings like id, state, version, etc. + * indexRoutings like id, state, version, etc. */ public final class ShardRouting implements Writeable, ToXContentObject { @@ -477,7 +477,7 @@ public boolean isRelocationTargetOf(ShardRouting other) { "ShardRouting is a relocation target but current node id isn't equal to source relocating node. This [" + this + "], other [" + other + "]"; assert b == false || this.shardId.equals(other.shardId) : - "ShardRouting is a relocation target but both routings are not of the same shard id. This [" + this + "], other [" + other + "]"; + "ShardRouting is a relocation target but both indexRoutings are not of the same shard id. This [" + this + "], other [" + other + "]"; assert b == false || this.primary == other.primary : "ShardRouting is a relocation target but primary flag is different. This [" + this + "], target [" + other + "]"; @@ -504,7 +504,7 @@ public boolean isRelocationSourceOf(ShardRouting other) { "ShardRouting is a relocation source but relocating node isn't equal to other's current node. This [" + this + "], other [" + other + "]"; assert b == false || this.shardId.equals(other.shardId) : - "ShardRouting is a relocation source but both routings are not of the same shard. This [" + this + "], target [" + other + "]"; + "ShardRouting is a relocation source but both indexRoutings are not of the same shard. This [" + this + "], target [" + other + "]"; assert b == false || this.primary == other.primary : "ShardRouting is a relocation source but primary flag is different. This [" + this + "], target [" + other + "]"; diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java index a8a0ad40eee37..52892d4d52eb9 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java @@ -58,7 +58,6 @@ */ public class ShardSearchLocalRequest implements ShardSearchRequest { - private String clusterAlias; private ShardId shardId; private int numberOfShards; @@ -71,7 +70,7 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { private Boolean requestCache; private long nowInMillis; private boolean allowPartialSearchResults; - private String routing; + private String[] indexRoutings = Strings.EMPTY_ARRAY; private String preference; private boolean profile; @@ -79,10 +78,10 @@ public class ShardSearchLocalRequest implements ShardSearchRequest { } ShardSearchLocalRequest(SearchRequest searchRequest, ShardId shardId, int numberOfShards, - AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { + AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias, String[] indexRoutings) { this(shardId, numberOfShards, searchRequest.searchType(), searchRequest.source(), searchRequest.types(), searchRequest.requestCache(), aliasFilter, indexBoost, - searchRequest.allowPartialSearchResults(), searchRequest.routing(), searchRequest.preference()); + searchRequest.allowPartialSearchResults(), indexRoutings, searchRequest.preference()); // If allowPartialSearchResults is unset (ie null), the cluster-level default should have been substituted // at this stage. Any NPEs in the above are therefore an error in request preparation logic. assert searchRequest.allowPartialSearchResults() != null; @@ -101,7 +100,7 @@ public ShardSearchLocalRequest(ShardId shardId, String[] types, long nowInMillis public ShardSearchLocalRequest(ShardId shardId, int numberOfShards, SearchType searchType, SearchSourceBuilder source, String[] types, Boolean requestCache, AliasFilter aliasFilter, float indexBoost, boolean allowPartialSearchResults, - String routing, String preference) { + String[] indexRoutings, String preference) { this.shardId = shardId; this.numberOfShards = numberOfShards; this.searchType = searchType; @@ -111,7 +110,7 @@ public ShardSearchLocalRequest(ShardId shardId, int numberOfShards, SearchType s this.aliasFilter = aliasFilter; this.indexBoost = indexBoost; this.allowPartialSearchResults = allowPartialSearchResults; - this.routing = routing; + this.indexRoutings = indexRoutings; this.preference = preference; } @@ -183,8 +182,8 @@ public Scroll scroll() { } @Override - public String routing() { - return routing; + public String[] indexRoutings() { + return indexRoutings; } @Override @@ -237,10 +236,10 @@ protected void innerReadFrom(StreamInput in) throws IOException { allowPartialSearchResults = in.readOptionalBoolean(); } if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - routing = in.readOptionalString(); + indexRoutings = in.readStringArray(); preference = in.readOptionalString(); } else { - routing = null; + indexRoutings = Strings.EMPTY_ARRAY; preference = null; } } @@ -258,7 +257,7 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException if (out.getVersion().onOrAfter(Version.V_5_2_0)) { out.writeFloat(indexBoost); } - if (!asKey) { + if (asKey == false) { out.writeVLong(nowInMillis); } out.writeOptionalBoolean(requestCache); @@ -268,9 +267,9 @@ protected void innerWriteTo(StreamOutput out, boolean asKey) throws IOException if (out.getVersion().onOrAfter(Version.V_6_3_0)) { out.writeOptionalBoolean(allowPartialSearchResults); } - if (!asKey) { + if (asKey == false) { if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - out.writeOptionalString(routing); + out.writeStringArray(indexRoutings); out.writeOptionalString(preference); } } diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java index 3b07ef8979b56..0a1513e17d08e 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.CheckedFunction; @@ -73,12 +74,12 @@ public interface ShardSearchRequest { Scroll scroll(); /** - * Returns the routing of the original {@link SearchRequest}. + * Returns the routing values resolved by the coordinating node for the index pointed by {@link #shardId()}. */ - String routing(); + String[] indexRoutings(); /** - * Returns the preference of the original {@link SearchRequest}. + * Returns the preference of the original {@link SearchRequest#preference()}. */ String preference(); diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java index 3d3a020447415..08060a2b249b6 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchTransportRequest.java @@ -54,9 +54,10 @@ public ShardSearchTransportRequest(){ } public ShardSearchTransportRequest(OriginalIndices originalIndices, SearchRequest searchRequest, ShardId shardId, int numberOfShards, - AliasFilter aliasFilter, float indexBoost, long nowInMillis, String clusterAlias) { + AliasFilter aliasFilter, float indexBoost, long nowInMillis, + String clusterAlias, String[] indexRoutings) { this.shardSearchLocalRequest = new ShardSearchLocalRequest(searchRequest, shardId, numberOfShards, aliasFilter, indexBoost, - nowInMillis, clusterAlias); + nowInMillis, clusterAlias, indexRoutings); this.originalIndices = originalIndices; } @@ -160,8 +161,8 @@ public Scroll scroll() { } @Override - public String routing() { - return shardSearchLocalRequest.routing(); + public String[] indexRoutings() { + return shardSearchLocalRequest.indexRoutings(); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java index 677ea1dfb916a..b692f9d433829 100644 --- a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java @@ -23,9 +23,7 @@ import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.Version; -import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.routing.GroupShardsIterator; import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.service.ClusterService; @@ -36,7 +34,7 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.Loggers; -import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -49,6 +47,7 @@ import org.elasticsearch.search.internal.ShardSearchRequest; import java.io.IOException; +import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -227,28 +226,33 @@ public Query toFilter(ClusterService clusterService, ShardSearchRequest request, int shardId = request.shardId().id(); int numShards = context.getIndexSettings().getNumberOfShards(); if (minNodeVersion.onOrAfter(Version.V_7_0_0_alpha1) && - (request.preference() != null || request.routing() != null)) { - /** - * Resolve the routings and preference to compute the request shard id and the number of shards - * since they can differ if some shards are filtered by the request. - * This filtering has been added in {@link Version#V_7_0_0_alpha1} so if there is another node in the cluster - * with a smaller version smaller we use the original shard id and number of shards in order to ensure that all - * slices use the same numbers. - */ + (request.preference() != null || request.indexRoutings().length > 0)) { GroupShardsIterator group = buildShardIterator(clusterService, request); - assert group.size() <= numShards; - numShards = group.size(); - int ord = 0; - shardId = -1; - for (ShardIterator it : group) { - assert it.shardId().getIndex().equals(request.shardId().getIndex()); - if (request.shardId().equals(it.shardId())) { - shardId = ord; - break; + assert group.size() <= numShards : "index routing shards: " + group.size() + + " cannot be greater than total number of shards: " + numShards; + if (group.size() < numShards) { + /** + * The routing of this request targets a subset of the shards of this index so we need to we retrieve + * the original {@link GroupShardsIterator} and compute the request shard id and number of + * shards from it. + * This behavior has been added in {@link Version#V_7_0_0_alpha1} so if there is another node in the cluster + * with an older version we use the original shard id and number of shards in order to ensure that all + * slices use the same numbers. + */ + numShards = group.size(); + int ord = 0; + shardId = -1; + // remap the original shard id with its index (position) in the sorted shard iterator. + for (ShardIterator it : group) { + assert it.shardId().getIndex().equals(request.shardId().getIndex()); + if (request.shardId().equals(it.shardId())) { + shardId = ord; + break; + } + ++ord; } - ++ ord; + assert shardId != -1 : "shard id: " + request.shardId().getId() + " not found in index shard routing"; } - assert shardId != -1; } String field = this.field; @@ -320,16 +324,9 @@ public Query toFilter(ClusterService clusterService, ShardSearchRequest request, */ private GroupShardsIterator buildShardIterator(ClusterService clusterService, ShardSearchRequest request) { final ClusterState state = clusterService.state(); - final Settings settings = clusterService.getSettings(); - final String[] indices; - if (request instanceof IndicesRequest) { - indices = ((IndicesRequest) request).indices(); - } else { - indices = new String[] { request.shardId().getIndex().getName() }; - } - IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(settings); - Map> routings = resolver.resolveSearchRouting(state, request.routing(), indices); - return clusterService.operationRouting().searchShards(state, indices, routings, request.preference()); + String[] indices = new String[] { request.shardId().getIndex().getName() }; + Map> routingMap = Collections.singletonMap(indices[0], Sets.newHashSet(request.indexRoutings())); + return clusterService.operationRouting().searchShards(state, indices, routingMap, request.preference()); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java index 6ade2b8781ecf..193878e2f5e04 100644 --- a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.routing.GroupShardsIterator; import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.index.Index; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.shard.ShardId; @@ -62,10 +63,15 @@ private AbstractSearchAsyncAction createAction( final SearchRequest request = new SearchRequest(); request.allowPartialSearchResults(true); + request.preference("_shards:1,3"); return new AbstractSearchAsyncAction("test", null, null, null, - Collections.singletonMap("foo", new AliasFilter(new MatchAllQueryBuilder())), Collections.singletonMap("foo", 2.0f), null, - request, null, new GroupShardsIterator<>(Collections.singletonList( - new SearchShardIterator(null, null, Collections.emptyList(), null))), timeProvider, 0, null, + Collections.singletonMap("foo", new AliasFilter(new MatchAllQueryBuilder())), Collections.singletonMap("foo", 2.0f), + Collections.singletonMap("name", Sets.newHashSet("bar", "baz")),null, request, null, + new GroupShardsIterator<>( + Collections.singletonList( + new SearchShardIterator(null, null, Collections.emptyList(), null) + ) + ), timeProvider, 0, null, new InitialSearchPhase.ArraySearchPhaseResults<>(10), request.getMaxConcurrentShardRequests(), SearchResponse.Clusters.EMPTY) { @Override @@ -117,5 +123,8 @@ public void testBuildShardSearchTransportRequest() { assertArrayEquals(new String[] {"name", "name1"}, shardSearchTransportRequest.indices()); assertEquals(new MatchAllQueryBuilder(), shardSearchTransportRequest.getAliasFilter().getQueryBuilder()); assertEquals(2.0f, shardSearchTransportRequest.indexBoost(), 0.0f); + assertArrayEquals(new String[] {"name", "name1"}, shardSearchTransportRequest.indices()); + assertArrayEquals(new String[] {"bar", "baz"}, shardSearchTransportRequest.indexRoutings()); + assertEquals("_shards:1,3", shardSearchTransportRequest.preference()); } } diff --git a/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java index d60f29a5d5395..8b1741967734c 100644 --- a/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java @@ -78,12 +78,12 @@ public void sendCanMatch(Transport.Connection connection, ShardSearchTransportRe 2, randomBoolean(), primaryNode, replicaNode); final SearchRequest searchRequest = new SearchRequest(); searchRequest.allowPartialSearchResults(true); - + CanMatchPreFilterSearchPhase canMatchPhase = new CanMatchPreFilterSearchPhase(logger, searchTransportService, (clusterAlias, node) -> lookup.get(node), Collections.singletonMap("_na_", new AliasFilter(null, Strings.EMPTY_ARRAY)), - Collections.emptyMap(), EsExecutors.newDirectExecutorService(), + Collections.emptyMap(), Collections.emptyMap(), EsExecutors.newDirectExecutorService(), searchRequest, null, shardsIter, timeProvider, 0, null, (iter) -> new SearchPhase("test") { @Override @@ -159,12 +159,12 @@ public void sendCanMatch(Transport.Connection connection, ShardSearchTransportRe final SearchRequest searchRequest = new SearchRequest(); searchRequest.allowPartialSearchResults(true); - + CanMatchPreFilterSearchPhase canMatchPhase = new CanMatchPreFilterSearchPhase(logger, searchTransportService, (clusterAlias, node) -> lookup.get(node), Collections.singletonMap("_na_", new AliasFilter(null, Strings.EMPTY_ARRAY)), - Collections.emptyMap(), EsExecutors.newDirectExecutorService(), + Collections.emptyMap(), Collections.emptyMap(), EsExecutors.newDirectExecutorService(), searchRequest, null, shardsIter, timeProvider, 0, null, (iter) -> new SearchPhase("test") { @Override @@ -222,6 +222,7 @@ public void sendCanMatch( (clusterAlias, node) -> lookup.get(node), Collections.singletonMap("_na_", new AliasFilter(null, Strings.EMPTY_ARRAY)), Collections.emptyMap(), + Collections.emptyMap(), EsExecutors.newDirectExecutorService(), searchRequest, null, diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java index c731d1aaabed0..82e0fcaf5d667 100644 --- a/server/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java @@ -106,6 +106,7 @@ public void onFailure(Exception e) { return lookup.get(node); }, aliasFilters, Collections.emptyMap(), + Collections.emptyMap(), null, request, responseListener, @@ -198,6 +199,7 @@ public void onFailure(Exception e) { return lookup.get(node); }, aliasFilters, Collections.emptyMap(), + Collections.emptyMap(), null, request, responseListener, @@ -303,6 +305,7 @@ public void sendFreeContext(Transport.Connection connection, long contextId, Ori return lookup.get(node); }, aliasFilters, Collections.emptyMap(), + Collections.emptyMap(), executor, request, responseListener, diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/PrimaryTermsTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/PrimaryTermsTests.java index 65526896864d6..8a9b00a8d4ff7 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/PrimaryTermsTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/PrimaryTermsTests.java @@ -83,7 +83,7 @@ public void setUp() throws Exception { } /** - * puts primary shard routings into initializing state + * puts primary shard indexRoutings into initializing state */ private void initPrimaries() { logger.info("adding {} nodes and performing rerouting", this.numberOfReplicas + 1); diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/RoutingTableTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/RoutingTableTests.java index 055adbaebbce5..349997d7793eb 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/RoutingTableTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/RoutingTableTests.java @@ -83,7 +83,7 @@ public void setUp() throws Exception { } /** - * puts primary shard routings into initializing state + * puts primary shard indexRoutings into initializing state */ private void initPrimaries() { logger.info("adding {} nodes and performing rerouting", this.numberOfReplicas + 1); diff --git a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java index 171c0ca671dc7..4ef9c36d9306c 100644 --- a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java +++ b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java @@ -123,7 +123,7 @@ public Scroll scroll() { } @Override - public String routing() { + public String[] indexRoutings() { return null; } diff --git a/server/src/test/java/org/elasticsearch/routing/AliasRoutingIT.java b/server/src/test/java/org/elasticsearch/routing/AliasRoutingIT.java index 14ec800c3a65d..6b3b6a67a9783 100644 --- a/server/src/test/java/org/elasticsearch/routing/AliasRoutingIT.java +++ b/server/src/test/java/org/elasticsearch/routing/AliasRoutingIT.java @@ -170,7 +170,7 @@ public void testAliasSearchRouting() throws Exception { assertThat(client().prepareSearch("alias1").setSize(0).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(1L)); } - logger.info("--> search with 0,1 routings , should find two"); + logger.info("--> search with 0,1 indexRoutings , should find two"); for (int i = 0; i < 5; i++) { assertThat(client().prepareSearch().setRouting("0", "1").setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(2L)); assertThat(client().prepareSearch().setSize(0).setRouting("0", "1").setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(2L)); diff --git a/server/src/test/java/org/elasticsearch/routing/SimpleRoutingIT.java b/server/src/test/java/org/elasticsearch/routing/SimpleRoutingIT.java index 84caed948a2be..0a2a43f2f83d4 100644 --- a/server/src/test/java/org/elasticsearch/routing/SimpleRoutingIT.java +++ b/server/src/test/java/org/elasticsearch/routing/SimpleRoutingIT.java @@ -173,13 +173,13 @@ public void testSimpleSearchRouting() { assertThat(client().prepareSearch().setSize(0).setRouting(secondRoutingValue).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(1L)); } - logger.info("--> search with {},{} routings , should find two", routingValue, "1"); + logger.info("--> search with {},{} indexRoutings , should find two", routingValue, "1"); for (int i = 0; i < 5; i++) { assertThat(client().prepareSearch().setRouting(routingValue, secondRoutingValue).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(2L)); assertThat(client().prepareSearch().setSize(0).setRouting(routingValue, secondRoutingValue).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(2L)); } - logger.info("--> search with {},{},{} routings , should find two", routingValue, secondRoutingValue, routingValue); + logger.info("--> search with {},{},{} indexRoutings , should find two", routingValue, secondRoutingValue, routingValue); for (int i = 0; i < 5; i++) { assertThat(client().prepareSearch().setRouting(routingValue, secondRoutingValue, routingValue).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(2L)); assertThat(client().prepareSearch().setSize(0).setRouting(routingValue, secondRoutingValue,routingValue).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet().getHits().getTotalHits(), equalTo(2L)); diff --git a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java index ba1dc565cd28e..eed4898a258ba 100644 --- a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java @@ -74,7 +74,7 @@ public void testSerialization() throws Exception { assertEquals(deserializedRequest.searchType(), shardSearchTransportRequest.searchType()); assertEquals(deserializedRequest.shardId(), shardSearchTransportRequest.shardId()); assertEquals(deserializedRequest.numberOfShards(), shardSearchTransportRequest.numberOfShards()); - assertEquals(deserializedRequest.routing(), shardSearchTransportRequest.routing()); + assertEquals(deserializedRequest.indexRoutings(), shardSearchTransportRequest.indexRoutings()); assertEquals(deserializedRequest.preference(), shardSearchTransportRequest.preference()); assertEquals(deserializedRequest.cacheKey(), shardSearchTransportRequest.cacheKey()); assertNotSame(deserializedRequest, shardSearchTransportRequest); @@ -94,9 +94,15 @@ private ShardSearchTransportRequest createShardSearchTransportRequest() throws I } else { filteringAliases = new AliasFilter(null, Strings.EMPTY_ARRAY); } + final String[] routings; + if (randomBoolean()) { + routings = generateRandomStringArray(10, 10, false, false); + } else { + routings = null; + } return new ShardSearchTransportRequest(new OriginalIndices(searchRequest), searchRequest, shardId, randomIntBetween(1, 100), filteringAliases, randomBoolean() ? 1.0f : randomFloat(), - Math.abs(randomLong()), null); + Math.abs(randomLong()), null, routings); } public void testFilteringAliases() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java b/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java index 75ae9f62fa2bd..d609f84e4192e 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SearchSliceIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.slice; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchRequestBuilder; @@ -48,9 +49,7 @@ import static org.hamcrest.Matchers.startsWith; public class SearchSliceIT extends ESIntegTestCase { - private static final int NUM_DOCS = 1000; - - private void setupIndex(int numberOfShards, boolean withDocs) throws IOException, ExecutionException, InterruptedException { + private void setupIndex(int numDocs, int numberOfShards) throws IOException, ExecutionException, InterruptedException { String mapping = Strings.toString(XContentFactory.jsonBuilder(). startObject() .startObject("type") @@ -75,56 +74,57 @@ private void setupIndex(int numberOfShards, boolean withDocs) throws IOException .addMapping("type", mapping, XContentType.JSON)); ensureGreen(); - if (withDocs == false) { - return; - } - List requests = new ArrayList<>(); - for (int i = 0; i < NUM_DOCS; i++) { - XContentBuilder builder = jsonBuilder(); - builder.startObject(); - builder.field("invalid_random_kw", randomAlphaOfLengthBetween(5, 20)); - builder.field("random_int", randomInt()); - builder.field("static_int", 0); - builder.field("invalid_random_int", randomInt()); - builder.endObject(); + for (int i = 0; i < numDocs; i++) { + XContentBuilder builder = jsonBuilder() + .startObject() + .field("invalid_random_kw", randomAlphaOfLengthBetween(5, 20)) + .field("random_int", randomInt()) + .field("static_int", 0) + .field("invalid_random_int", randomInt()) + .endObject(); requests.add(client().prepareIndex("test", "type").setSource(builder)); } indexRandom(true, requests); } - public void testDocIdSort() throws Exception { + public void testSearchSort() throws Exception { int numShards = randomIntBetween(1, 7); - setupIndex(numShards, true); - SearchResponse sr = client().prepareSearch("test") - .setQuery(matchAllQuery()) - .setSize(0) - .get(); - int numDocs = (int) sr.getHits().getTotalHits(); - assertThat(numDocs, equalTo(NUM_DOCS)); - int max = randomIntBetween(2, numShards*3); + int numDocs = randomIntBetween(100, 1000); + setupIndex(numDocs, numShards); + int max = randomIntBetween(2, numShards * 3); for (String field : new String[]{"_id", "random_int", "static_int"}) { int fetchSize = randomIntBetween(10, 100); + // test _doc sort SearchRequestBuilder request = client().prepareSearch("test") .setQuery(matchAllQuery()) .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) .setSize(fetchSize) .addSort(SortBuilders.fieldSort("_doc")); assertSearchSlicesWithScroll(request, field, max, numDocs); + + // test numeric sort + request = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) + .addSort(SortBuilders.fieldSort("random_int")) + .setSize(fetchSize); + assertSearchSlicesWithScroll(request, field, max, numDocs); } } - public void testWithPreference() throws Exception { + public void testWithPreferenceAndRoutings() throws Exception { int numShards = 10; - setupIndex(numShards, true); - SearchResponse sr = client().prepareSearch("test") - .setQuery(matchAllQuery()) - .setPreference("_shards:1,4") - .setSize(0) - .get(); - int numDocs = (int) sr.getHits().getTotalHits(); - int max = randomIntBetween(2, numShards*3); - for (String field : new String[]{"_id", "random_int", "static_int"}) { + int totalDocs = randomIntBetween(100, 1000); + setupIndex(totalDocs, numShards); + { + SearchResponse sr = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setPreference("_shards:1,4") + .setSize(0) + .get(); + int numDocs = (int) sr.getHits().getTotalHits(); + int max = randomIntBetween(2, numShards * 3); int fetchSize = randomIntBetween(10, 100); SearchRequestBuilder request = client().prepareSearch("test") .setQuery(matchAllQuery()) @@ -132,21 +132,16 @@ public void testWithPreference() throws Exception { .setSize(fetchSize) .setPreference("_shards:1,4") .addSort(SortBuilders.fieldSort("_doc")); - assertSearchSlicesWithScroll(request, field, max, numDocs); + assertSearchSlicesWithScroll(request, "_id", max, numDocs); } - } - - public void testWithRouting() throws Exception { - int numShards = 10; - setupIndex(numShards, true); - SearchResponse sr = client().prepareSearch("test") - .setQuery(matchAllQuery()) - .setRouting("foo", "bar") - .setSize(0) - .get(); - int numDocs = (int) sr.getHits().getTotalHits(); - int max = randomIntBetween(2, numShards*3); - for (String field : new String[]{"_id", "random_int", "static_int"}) { + { + SearchResponse sr = client().prepareSearch("test") + .setQuery(matchAllQuery()) + .setRouting("foo", "bar") + .setSize(0) + .get(); + int numDocs = (int) sr.getHits().getTotalHits(); + int max = randomIntBetween(2, numShards * 3); int fetchSize = randomIntBetween(10, 100); SearchRequestBuilder request = client().prepareSearch("test") .setQuery(matchAllQuery()) @@ -154,35 +149,32 @@ public void testWithRouting() throws Exception { .setSize(fetchSize) .setRouting("foo", "bar") .addSort(SortBuilders.fieldSort("_doc")); - assertSearchSlicesWithScroll(request, field, max, numDocs); + assertSearchSlicesWithScroll(request, "_id", max, numDocs); } - } - - public void testNumericSort() throws Exception { - int numShards = randomIntBetween(1, 7); - setupIndex(numShards, true); - SearchResponse sr = client().prepareSearch("test") - .setQuery(matchAllQuery()) - .setSize(0) - .get(); - int numDocs = (int) sr.getHits().getTotalHits(); - assertThat(numDocs, equalTo(NUM_DOCS)); - - int max = randomIntBetween(2, numShards*3); - for (String field : new String[]{"_id", "random_int", "static_int"}) { + { + assertAcked(client().admin().indices().prepareAliases() + .addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test").alias("alias1").routing("foo")) + .addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test").alias("alias2").routing("bar")) + .addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test").alias("alias3").routing("baz")) + .get()); + SearchResponse sr = client().prepareSearch("alias1", "alias3") + .setQuery(matchAllQuery()) + .setSize(0) + .get(); + int numDocs = (int) sr.getHits().getTotalHits(); + int max = randomIntBetween(2, numShards * 3); int fetchSize = randomIntBetween(10, 100); - SearchRequestBuilder request = client().prepareSearch("test") + SearchRequestBuilder request = client().prepareSearch("alias1", "alias3") .setQuery(matchAllQuery()) .setScroll(new Scroll(TimeValue.timeValueSeconds(10))) - .addSort(SortBuilders.fieldSort("random_int")) - .setSize(fetchSize); - assertSearchSlicesWithScroll(request, field, max, numDocs); + .setSize(fetchSize) + .addSort(SortBuilders.fieldSort("_doc")); + assertSearchSlicesWithScroll(request, "_id", max, numDocs); } } public void testInvalidFields() throws Exception { - int numShards = randomIntBetween(1, 7); - setupIndex(numShards, true); + setupIndex(0, 1); SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch("test") .setQuery(matchAllQuery()) @@ -206,8 +198,7 @@ public void testInvalidFields() throws Exception { } public void testInvalidQuery() throws Exception { - int numShards = randomIntBetween(1, 7); - setupIndex(numShards, true); + setupIndex(0, 1); SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch() .setQuery(matchAllQuery()) @@ -230,7 +221,7 @@ private void assertSearchSlicesWithScroll(SearchRequestBuilder request, String f int numSliceResults = searchResponse.getHits().getHits().length; String scrollId = searchResponse.getScrollId(); for (SearchHit hit : searchResponse.getHits().getHits()) { - keys.add(hit.getId()); + assertTrue(keys.add(hit.getId())); } while (searchResponse.getHits().getHits().length > 0) { searchResponse = client().prepareSearchScroll("test") @@ -241,7 +232,7 @@ private void assertSearchSlicesWithScroll(SearchRequestBuilder request, String f totalResults += searchResponse.getHits().getHits().length; numSliceResults += searchResponse.getHits().getHits().length; for (SearchHit hit : searchResponse.getHits().getHits()) { - keys.add(hit.getId()); + assertTrue(keys.add(hit.getId())); } } assertThat(numSliceResults, equalTo(expectedSliceResults)); diff --git a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java index 4a7ade0a9a521..01bbf8c1d2b83 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java @@ -86,13 +86,13 @@ public class SliceBuilderTests extends ESTestCase { static class ShardSearchRequestTest implements IndicesRequest, ShardSearchRequest { private final String[] indices; private final int shardId; - private final String routing; + private final String[] indexRoutings; private final String preference; - ShardSearchRequestTest(String index, int shardId, String routing, String preference) { + ShardSearchRequestTest(String index, int shardId, String[] indexRoutings, String preference) { this.indices = new String[] { index }; this.shardId = shardId; - this.routing = routing; + this.indexRoutings = indexRoutings; this.preference = preference; } @@ -172,8 +172,8 @@ public Scroll scroll() { } @Override - public String routing() { - return routing; + public String[] indexRoutings() { + return indexRoutings; } @Override @@ -241,8 +241,8 @@ private ShardSearchRequest createRequest(int shardId) { return createRequest(shardId,null, null); } - private ShardSearchRequest createRequest(int shardId, String routing, String preference) { - return new ShardSearchRequestTest("index", shardId, routing, preference); + private ShardSearchRequest createRequest(int shardId, String[] routings, String preference) { + return new ShardSearchRequestTest("index", shardId, routings, preference); } private QueryShardContext createShardContext(Version indexVersionCreated, IndexReader reader, @@ -498,7 +498,8 @@ public void testToFilterWithRouting() throws IOException { try (IndexReader reader = DirectoryReader.open(dir)) { QueryShardContext context = createShardContext(Version.CURRENT, reader, "field", DocValuesType.SORTED, 5, 0); SliceBuilder builder = new SliceBuilder("field", 6, 10); - Query query = builder.toFilter(clusterService, createRequest(1, "foo", null), context, Version.CURRENT); + String[] routings = new String[] { "foo" }; + Query query = builder.toFilter(clusterService, createRequest(1, routings, null), context, Version.CURRENT); assertEquals(new DocValuesSliceQuery("field", 6, 10), query); query = builder.toFilter(clusterService, createRequest(1, null, "foo"), context, Version.CURRENT); assertEquals(new DocValuesSliceQuery("field", 6, 10), query); From ca87ce71cd6398f047ab467a1901b5ff5c005032 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 23 Apr 2018 16:18:29 +0200 Subject: [PATCH 6/8] fix ut --- .../org/elasticsearch/search/slice/SliceBuilderTests.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java index 01bbf8c1d2b83..d6bcbfa8e6b7d 100644 --- a/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/slice/SliceBuilderTests.java @@ -42,6 +42,7 @@ import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; @@ -238,7 +239,7 @@ private IndexSettings createIndexSettings(Version indexVersionCreated, int numSh } private ShardSearchRequest createRequest(int shardId) { - return createRequest(shardId,null, null); + return createRequest(shardId, Strings.EMPTY_ARRAY, null); } private ShardSearchRequest createRequest(int shardId, String[] routings, String preference) { @@ -501,9 +502,9 @@ public void testToFilterWithRouting() throws IOException { String[] routings = new String[] { "foo" }; Query query = builder.toFilter(clusterService, createRequest(1, routings, null), context, Version.CURRENT); assertEquals(new DocValuesSliceQuery("field", 6, 10), query); - query = builder.toFilter(clusterService, createRequest(1, null, "foo"), context, Version.CURRENT); + query = builder.toFilter(clusterService, createRequest(1, Strings.EMPTY_ARRAY, "foo"), context, Version.CURRENT); assertEquals(new DocValuesSliceQuery("field", 6, 10), query); - query = builder.toFilter(clusterService, createRequest(1, null, "foo"), context, Version.V_6_2_0); + query = builder.toFilter(clusterService, createRequest(1, Strings.EMPTY_ARRAY, "foo"), context, Version.V_6_2_0); assertEquals(new DocValuesSliceQuery("field", 1, 2), query); } } From 36ac0873f5dd4d820990f0ddcffe6460bba528d4 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 23 Apr 2018 18:21:55 +0200 Subject: [PATCH 7/8] fix random routings value (null is not allowed) --- .../search/internal/ShardSearchTransportRequestTests.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java index eed4898a258ba..21a4f099f5a32 100644 --- a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java @@ -94,12 +94,7 @@ private ShardSearchTransportRequest createShardSearchTransportRequest() throws I } else { filteringAliases = new AliasFilter(null, Strings.EMPTY_ARRAY); } - final String[] routings; - if (randomBoolean()) { - routings = generateRandomStringArray(10, 10, false, false); - } else { - routings = null; - } + final String[] routings = generateRandomStringArray(5, 10, false, true); return new ShardSearchTransportRequest(new OriginalIndices(searchRequest), searchRequest, shardId, randomIntBetween(1, 100), filteringAliases, randomBoolean() ? 1.0f : randomFloat(), Math.abs(randomLong()), null, routings); From 803c88a1cab4d531abf6db79d5d9b4eaab88c650 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 24 Apr 2018 09:37:41 +0200 Subject: [PATCH 8/8] handle empty routing --- .../main/java/org/elasticsearch/search/slice/SliceBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java index b692f9d433829..06eba08fd7f22 100644 --- a/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/slice/SliceBuilder.java @@ -325,7 +325,8 @@ public Query toFilter(ClusterService clusterService, ShardSearchRequest request, private GroupShardsIterator buildShardIterator(ClusterService clusterService, ShardSearchRequest request) { final ClusterState state = clusterService.state(); String[] indices = new String[] { request.shardId().getIndex().getName() }; - Map> routingMap = Collections.singletonMap(indices[0], Sets.newHashSet(request.indexRoutings())); + Map> routingMap = request.indexRoutings().length > 0 ? + Collections.singletonMap(indices[0], Sets.newHashSet(request.indexRoutings())) : null; return clusterService.operationRouting().searchShards(state, indices, routingMap, request.preference()); }