Skip to content

Commit d46b2af

Browse files
committed
Fix spurious failures in AsyncSearchActionTests
AsyncSearchActionTests#testCleanupOnFailure fails sporadically in CI but not locally. This commit switches the tests into a SuiteScopeTestCase that creates internal states once on static members in order to make the tests more reproducible. Relates #49931
1 parent 5b15aa6 commit d46b2af

File tree

4 files changed

+95
-13
lines changed

4 files changed

+95
-13
lines changed

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,15 @@ private static List<SearchShardIterator> sortShards(GroupShardsIterator<SearchSh
128128
}
129129

130130
private static boolean shouldSortShards(MinAndMax<?>[] minAndMaxes) {
131-
return Arrays.stream(minAndMaxes).anyMatch(Objects::nonNull);
131+
Class<?> clazz = null;
132+
for (MinAndMax<?> minAndMax : minAndMaxes) {
133+
if (clazz == null && minAndMax != null) {
134+
clazz = minAndMax.getMin().getClass();
135+
} else if (minAndMax != null && clazz != minAndMax.getMax().getClass()) {
136+
return false;
137+
}
138+
}
139+
return clazz != null;
132140
}
133141

134142
private static Comparator<Integer> shardComparator(GroupShardsIterator<SearchShardIterator> shardsIts,

server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@ public void writeTo(StreamOutput out) throws IOException {
5555
/**
5656
* Return the minimum value.
5757
*/
58-
T getMin() {
58+
public T getMin() {
5959
return minValue;
6060
}
6161

6262
/**
6363
* Return the maximum value.
6464
*/
65-
T getMax() {
65+
public T getMax() {
6666
return maxValue;
6767
}
6868

server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java

+73
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.elasticsearch.action.search;
2020

21+
import org.apache.lucene.util.BytesRef;
2122
import org.elasticsearch.Version;
2223
import org.elasticsearch.action.ActionListener;
2324
import org.elasticsearch.action.OriginalIndices;
@@ -54,6 +55,8 @@
5455
import java.util.concurrent.atomic.AtomicReference;
5556
import java.util.stream.IntStream;
5657

58+
import static org.hamcrest.Matchers.equalTo;
59+
5760
public class CanMatchPreFilterSearchPhaseTests extends ESTestCase {
5861

5962
public void testFilterShards() throws InterruptedException {
@@ -350,4 +353,74 @@ public void run() {
350353
}
351354
}
352355
}
356+
357+
public void testSortShardsDisabled() throws InterruptedException {
358+
final TransportSearchAction.SearchTimeProvider timeProvider =
359+
new TransportSearchAction.SearchTimeProvider(0, System.nanoTime(), System::nanoTime);
360+
361+
Map<String, Transport.Connection> lookup = new ConcurrentHashMap<>();
362+
DiscoveryNode primaryNode = new DiscoveryNode("node_1", buildNewFakeTransportAddress(), Version.CURRENT);
363+
DiscoveryNode replicaNode = new DiscoveryNode("node_2", buildNewFakeTransportAddress(), Version.CURRENT);
364+
lookup.put("node1", new SearchAsyncActionTests.MockConnection(primaryNode));
365+
lookup.put("node2", new SearchAsyncActionTests.MockConnection(replicaNode));
366+
367+
for (SortOrder order : SortOrder.values()) {
368+
int numShards = randomIntBetween(2, 20);
369+
List<ShardId> shardIds = new ArrayList<>();
370+
Set<ShardId> shardToSkip = new HashSet<>();
371+
372+
SearchTransportService searchTransportService = new SearchTransportService(null, null) {
373+
@Override
374+
public void sendCanMatch(Transport.Connection connection, ShardSearchRequest request, SearchTask task,
375+
ActionListener<SearchService.CanMatchResponse> listener) {
376+
final MinAndMax<?> minMax;
377+
if (request.shardId().id() == numShards-1) {
378+
minMax = new MinAndMax<>(new BytesRef("bar"), new BytesRef("baz"));
379+
} else {
380+
Long min = rarely() ? null : randomLong();
381+
Long max = min == null ? null : randomLongBetween(min, Long.MAX_VALUE);
382+
minMax = min == null ? null : new MinAndMax<>(min, max);
383+
}
384+
boolean canMatch = frequently();
385+
synchronized (shardIds) {
386+
shardIds.add(request.shardId());
387+
if (canMatch == false) {
388+
shardToSkip.add(request.shardId());
389+
}
390+
}
391+
new Thread(() -> listener.onResponse(new SearchService.CanMatchResponse(canMatch, minMax))).start();
392+
}
393+
};
394+
395+
AtomicReference<GroupShardsIterator<SearchShardIterator>> result = new AtomicReference<>();
396+
CountDownLatch latch = new CountDownLatch(1);
397+
GroupShardsIterator<SearchShardIterator> shardsIter = SearchAsyncActionTests.getShardsIter("logs",
398+
new OriginalIndices(new String[]{"logs"}, SearchRequest.DEFAULT_INDICES_OPTIONS),
399+
numShards, randomBoolean(), primaryNode, replicaNode);
400+
final SearchRequest searchRequest = new SearchRequest();
401+
searchRequest.source(new SearchSourceBuilder().sort(SortBuilders.fieldSort("timestamp").order(order)));
402+
searchRequest.allowPartialSearchResults(true);
403+
404+
CanMatchPreFilterSearchPhase canMatchPhase = new CanMatchPreFilterSearchPhase(logger,
405+
searchTransportService,
406+
(clusterAlias, node) -> lookup.get(node),
407+
Collections.singletonMap("_na_", new AliasFilter(null, Strings.EMPTY_ARRAY)),
408+
Collections.emptyMap(), Collections.emptyMap(), EsExecutors.newDirectExecutorService(),
409+
searchRequest, null, shardsIter, timeProvider, ClusterState.EMPTY_STATE, null,
410+
(iter) -> new SearchPhase("test") {
411+
@Override
412+
public void run() {
413+
result.set(iter);
414+
latch.countDown();
415+
}
416+
}, SearchResponse.Clusters.EMPTY);
417+
418+
canMatchPhase.start();
419+
latch.await();
420+
for (SearchShardIterator i : result.get()) {
421+
assertEquals(shardToSkip.contains(i.shardId()), i.skip());
422+
}
423+
assertThat(result.get().size(), equalTo(numShards));
424+
}
425+
}
353426
}

x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionTests.java

+11-10
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
import org.elasticsearch.search.aggregations.metrics.InternalMax;
1717
import org.elasticsearch.search.aggregations.metrics.InternalMin;
1818
import org.elasticsearch.search.builder.SearchSourceBuilder;
19+
import org.elasticsearch.test.ESIntegTestCase;
1920
import org.elasticsearch.xpack.core.search.action.AsyncSearchResponse;
2021
import org.elasticsearch.xpack.core.search.action.SubmitAsyncSearchRequest;
21-
import org.junit.Before;
2222

2323
import java.util.ArrayList;
2424
import java.util.HashMap;
@@ -36,18 +36,19 @@
3636
import static org.hamcrest.Matchers.lessThanOrEqualTo;
3737

3838
// TODO: add tests for keepAlive and expiration
39+
@ESIntegTestCase.SuiteScopeTestCase
3940
public class AsyncSearchActionTests extends AsyncSearchIntegTestCase {
40-
private String indexName;
41-
private int numShards;
42-
private int numDocs;
41+
private static String indexName;
42+
private static int numShards;
43+
private static int numDocs;
4344

44-
private int numKeywords;
45-
private Map<String, AtomicInteger> keywordFreqs;
46-
private float maxMetric = Float.NEGATIVE_INFINITY;
47-
private float minMetric = Float.POSITIVE_INFINITY;
45+
private static int numKeywords;
46+
private static Map<String, AtomicInteger> keywordFreqs;
47+
private static float maxMetric = Float.NEGATIVE_INFINITY;
48+
private static float minMetric = Float.POSITIVE_INFINITY;
4849

49-
@Before
50-
public void indexDocuments() throws InterruptedException {
50+
@Override
51+
public void setupSuiteScopeCluster() throws Exception {
5152
indexName = "test-async";
5253
numShards = randomIntBetween(internalCluster().numDataNodes(), internalCluster().numDataNodes()*10);
5354
numDocs = randomIntBetween(numShards, numShards*10);

0 commit comments

Comments
 (0)