Skip to content

Commit

Permalink
Fix flaky TransportMultiSearchActionTests.testCancellation (opensearc…
Browse files Browse the repository at this point in the history
…h-project#17193)

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.

Signed-off-by: Michael Froh <froh@amazon.com>
  • Loading branch information
msfroh authored Jan 29, 2025
1 parent fd37ac8 commit 6f644e1
Showing 1 changed file with 9 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -321,26 +321,24 @@ public TaskManager getTaskManager() {
// and if there are more searches than is allowed create an error and remember that.
int maxAllowedConcurrentSearches = 1; // Allow 1 search at a time.
AtomicInteger counter = new AtomicInteger();
AtomicReference<AssertionError> errorHolder = new AtomicReference<>();
// randomize whether or not requests are executed asynchronously
ExecutorService executorService = threadPool.executor(ThreadPool.Names.GENERIC);
final Set<SearchRequest> requests = Collections.newSetFromMap(Collections.synchronizedMap(new IdentityHashMap<>()));
CountDownLatch countDownLatch = new CountDownLatch(1);
CountDownLatch canceledLatch = new CountDownLatch(1);
CancellableTask[] parentTask = new CancellableTask[1];
NodeClient client = new NodeClient(settings, threadPool) {
@Override
public void search(final SearchRequest request, final ActionListener<SearchResponse> listener) {
if (parentTask[0] != null && parentTask[0].isCancelled()) {
fail("Should not execute search after parent task is cancelled");
}
try {
countDownLatch.await(10, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}

requests.add(request);
executorService.execute(() -> {
try {
if (!canceledLatch.await(1, TimeUnit.SECONDS)) {
fail("Latch should have counted down");
}
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
counter.decrementAndGet();
listener.onResponse(
new SearchResponse(
Expand Down Expand Up @@ -399,7 +397,7 @@ public void onFailure(Task task, Exception e) {
}
});
parentTask[0].cancel("Giving up");
countDownLatch.countDown();
canceledLatch.countDown();

assertNull(responses[0]);
assertNull(exceptions[0]);
Expand Down

0 comments on commit 6f644e1

Please sign in to comment.