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 5e67a1068a1be..f2f5b6df35711 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -387,7 +387,7 @@ public CanMatchNodeRequest.Shard buildShardLevelRequest(SearchShardIterator shar } public void start() { - if (shardsIts.size() == 0) { + if (shardsIts.isEmpty()) { finishPhase(); return; } diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchShardIterator.java b/server/src/main/java/org/elasticsearch/action/search/SearchShardIterator.java index bf020cbd309eb..00ff8f33f5659 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchShardIterator.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchShardIterator.java @@ -11,7 +11,6 @@ import org.elasticsearch.action.OriginalIndices; import org.elasticsearch.cluster.routing.ShardRouting; -import org.elasticsearch.cluster.routing.ShardsIterator; import org.elasticsearch.common.util.PlainIterator; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; @@ -113,15 +112,6 @@ SearchShardTarget nextOrNull() { return null; } - /** - * Return the number of shards remaining in this {@link ShardsIterator} - * - * @return number of shard remaining - */ - int remaining() { - return targetNodesIterator.remaining(); - } - /** * Returns a non-null value if this request should use a specific search context instead of the latest one. */ diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java index 83889b7cf752a..d76ba859fac37 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java @@ -22,10 +22,8 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.index.Index; import org.elasticsearch.index.query.Rewriteable; -import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.search.SearchService; -import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.internal.AliasFilter; import org.elasticsearch.tasks.Task; @@ -172,15 +170,7 @@ public void searchShards(Task task, SearchShardsRequest searchShardsRequest, Act private static List toGroups(List shardIts) { List groups = new ArrayList<>(shardIts.size()); for (SearchShardIterator shardIt : shardIts) { - boolean skip = shardIt.skip(); - shardIt.reset(); - List targetNodes = new ArrayList<>(); - SearchShardTarget target; - while ((target = shardIt.nextOrNull()) != null) { - targetNodes.add(target.getNodeId()); - } - ShardId shardId = shardIt.shardId(); - groups.add(new SearchShardsGroup(shardId, targetNodes, skip)); + groups.add(new SearchShardsGroup(shardIt.shardId(), shardIt.getTargetNodeIds(), shardIt.skip())); } return groups; } 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 11085558dbe16..4ec3e972ae61b 100644 --- a/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/AbstractSearchAsyncActionTests.java @@ -225,7 +225,6 @@ public void testShardNotAvailableWithDisallowPartialFailures() { // skip one to avoid the "all shards failed" failure. SearchShardIterator skipIterator = new SearchShardIterator(null, null, Collections.emptyList(), null); skipIterator.skip(true); - skipIterator.reset(); action.skipShard(skipIterator); assertThat(exception.get(), instanceOf(SearchPhaseExecutionException.class)); SearchPhaseExecutionException searchPhaseExecutionException = (SearchPhaseExecutionException) exception.get(); 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 647d16977181f..afd3bee4c4ab8 100644 --- a/server/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/SearchAsyncActionTests.java @@ -545,7 +545,7 @@ public void testAllowPartialResults() throws InterruptedException { SearchTransportService transportService = new SearchTransportService(null, null, null); Map lookup = new HashMap<>(); - Map seenShard = new ConcurrentHashMap<>(); + Map seenShard = new ConcurrentHashMap<>(); lookup.put(primaryNode.getId(), new MockConnection(primaryNode)); lookup.put(replicaNode.getId(), new MockConnection(replicaNode)); Map aliasFilters = Collections.singletonMap("_na_", AliasFilter.EMPTY); @@ -581,17 +581,18 @@ protected void executePhaseOnShard( Transport.Connection connection, SearchActionListener listener ) { - seenShard.computeIfAbsent(shardIt.shardId(), (i) -> { + AtomicInteger retries = seenShard.computeIfAbsent(shardIt.shardId(), (i) -> { numRequests.incrementAndGet(); // only count this once per shard copy - return Boolean.TRUE; + return new AtomicInteger(0); }); + int numRetries = retries.incrementAndGet(); new Thread(() -> { TestSearchPhaseResult testSearchPhaseResult = new TestSearchPhaseResult( new ShardSearchContextId(UUIDs.randomBase64UUID(), contextIdGenerator.incrementAndGet()), connection.getNode() ); try { - if (shardIt.remaining() > 0) { + if (numRetries < shardIt.size()) { numFailReplicas.incrementAndGet(); listener.onFailure(new RuntimeException()); } else { @@ -643,10 +644,8 @@ public void testSkipUnavailableSearchShards() throws InterruptedException { ); // Skip all the shards searchShardIterator.skip(true); - searchShardIterator.reset(); searchShardIterators.add(searchShardIterator); } - List shardsIter = searchShardIterators; Map lookup = Map.of(primaryNode.getId(), new MockConnection(primaryNode)); CountDownLatch latch = new CountDownLatch(1); @@ -665,11 +664,11 @@ public void testSkipUnavailableSearchShards() throws InterruptedException { null, request, responseListener, - shardsIter, + searchShardIterators, new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0), ClusterState.EMPTY_STATE, null, - new ArraySearchPhaseResults<>(shardsIter.size()), + new ArraySearchPhaseResults<>(searchShardIterators.size()), request.getMaxConcurrentShardRequests(), SearchResponse.Clusters.EMPTY ) { @@ -702,7 +701,7 @@ protected void run() { assertNotNull(searchResponse.get()); assertThat(searchResponse.get().getSkippedShards(), equalTo(numUnavailableSkippedShards)); assertThat(searchResponse.get().getFailedShards(), equalTo(0)); - assertThat(searchResponse.get().getSuccessfulShards(), equalTo(shardsIter.size())); + assertThat(searchResponse.get().getSuccessfulShards(), equalTo(searchShardIterators.size())); } static List getShardsIter( @@ -728,7 +727,6 @@ static List getShardsIter( for (int i = 0; i < numShards; i++) { ArrayList started = new ArrayList<>(); ArrayList initializing = new ArrayList<>(); - ArrayList unassigned = new ArrayList<>(); ShardRouting routing = ShardRouting.newUnassigned( new ShardId(index, i), @@ -758,8 +756,6 @@ static List getShardsIter( } else { initializing.add(routing); } - } else { - unassigned.add(routing); // unused yet } } Collections.shuffle(started, random());