Skip to content

Commit ffed717

Browse files
Fix flaky TransportMultiSearchActionTests.testCancellation (#17193) (#17196)
I recently added this test, but incorrectly placed a CountDownLatch#await call on the test thread. With this change, we actually kick off the request, return control to the testy thread, cancel the request, then continue executing. (cherry picked from commit 6f644e1) Signed-off-by: Michael Froh <froh@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 56b5726 commit ffed717

File tree

1 file changed

+9
-11
lines changed

1 file changed

+9
-11
lines changed

server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java

+9-11
Original file line numberDiff line numberDiff line change
@@ -321,26 +321,24 @@ public TaskManager getTaskManager() {
321321
// and if there are more searches than is allowed create an error and remember that.
322322
int maxAllowedConcurrentSearches = 1; // Allow 1 search at a time.
323323
AtomicInteger counter = new AtomicInteger();
324-
AtomicReference<AssertionError> errorHolder = new AtomicReference<>();
325-
// randomize whether or not requests are executed asynchronously
326324
ExecutorService executorService = threadPool.executor(ThreadPool.Names.GENERIC);
327-
final Set<SearchRequest> requests = Collections.newSetFromMap(Collections.synchronizedMap(new IdentityHashMap<>()));
328-
CountDownLatch countDownLatch = new CountDownLatch(1);
325+
CountDownLatch canceledLatch = new CountDownLatch(1);
329326
CancellableTask[] parentTask = new CancellableTask[1];
330327
NodeClient client = new NodeClient(settings, threadPool) {
331328
@Override
332329
public void search(final SearchRequest request, final ActionListener<SearchResponse> listener) {
333330
if (parentTask[0] != null && parentTask[0].isCancelled()) {
334331
fail("Should not execute search after parent task is cancelled");
335332
}
336-
try {
337-
countDownLatch.await(10, TimeUnit.MILLISECONDS);
338-
} catch (InterruptedException e) {
339-
throw new RuntimeException(e);
340-
}
341333

342-
requests.add(request);
343334
executorService.execute(() -> {
335+
try {
336+
if (!canceledLatch.await(1, TimeUnit.SECONDS)) {
337+
fail("Latch should have counted down");
338+
}
339+
} catch (InterruptedException e) {
340+
throw new RuntimeException(e);
341+
}
344342
counter.decrementAndGet();
345343
listener.onResponse(
346344
new SearchResponse(
@@ -399,7 +397,7 @@ public void onFailure(Task task, Exception e) {
399397
}
400398
});
401399
parentTask[0].cancel("Giving up");
402-
countDownLatch.countDown();
400+
canceledLatch.countDown();
403401

404402
assertNull(responses[0]);
405403
assertNull(exceptions[0]);

0 commit comments

Comments
 (0)