Skip to content

Commit

Permalink
Fix BanFailureLoggingTests some more (#76668)
Browse files Browse the repository at this point in the history
We spawn a background task to mark a task as cancelled when its inbound
connection closes. In these tests this might not happen until we've shut
the threadpool down, in which case it never happens. This change ensures
that we're not waiting for the cancellation to happen when the test
exits.

Closes #76558
  • Loading branch information
DaveCTurner committed Aug 23, 2021
1 parent 343df12 commit 3c69796
Showing 1 changed file with 15 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Function;

import static org.hamcrest.Matchers.anyOf;
Expand Down Expand Up @@ -98,6 +100,9 @@ private void runTest(

try {

// the child task might not run, but if it does we must wait for it to be cancelled before shutting everything down
final ReentrantLock childTaskLock = new ReentrantLock();

final MockTransportService parentTransportService = MockTransportService.createNewService(
Settings.EMPTY,
Version.CURRENT,
Expand All @@ -124,7 +129,13 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
},
(request, channel, task) -> {
final CancellableTask cancellableTask = (CancellableTask) task;
assertBusy(() -> assertTrue(cancellableTask.isCancelled()));
if (childTaskLock.tryLock()) {
try {
assertBusy(() -> assertTrue("task " + task.getId() + " should be cancelled", cancellableTask.isCancelled()));
} finally {
childTaskLock.unlock();
}
}
channel.sendResponse(new TaskCancelledException("task cancelled"));
});

Expand Down Expand Up @@ -161,13 +172,15 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
final PlainActionFuture<Void> cancellationFuture = new PlainActionFuture<>();
parentTransportService.getTaskManager().cancelTaskAndDescendants(parentTask, "test", true, cancellationFuture);
try {
cancellationFuture.actionGet(TimeValue.timeValueSeconds(5));
cancellationFuture.actionGet(TimeValue.timeValueSeconds(10));
} catch (NodeDisconnectedException e) {
// acceptable; we mostly ignore the result of cancellation anyway
}

// assert busy since failure to remove a ban may be logged after cancellation completed
assertBusy(appender::assertAllExpectationsMatched);

assertTrue("child tasks did not finish in time", childTaskLock.tryLock(15, TimeUnit.SECONDS));
} finally {
Collections.reverse(resources);
IOUtils.close(resources);
Expand Down

0 comments on commit 3c69796

Please sign in to comment.