Skip to content

Commit

Permalink
Fix BanFailureLoggingTests (#75171)
Browse files Browse the repository at this point in the history
Today both `BanFailureLoggingTests` fail to remove the ban and report
this in the logs. `testLogsAtDebugOnDisconnectionDuringBan` does not
wait for this message to be logged, so the logging might happen
concurrently with the logger being stopped and removed, resulting in a
`Attempted to append to non-started appender mock` failure.

This commit ensures that both test cases wait for the expected removal
failure message to be logged.

Closes #75129
  • Loading branch information
DaveCTurner committed Jul 13, 2021
1 parent f3946a4 commit 1f92bf8
Showing 1 changed file with 28 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@

import java.io.Closeable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
Expand All @@ -52,11 +54,17 @@ public void testLogsAtDebugOnDisconnectionDuringBan() throws Exception {
}
connection.sendRequest(requestId, action, request, options);
},
childNode -> new MockLogAppender.SeenEventExpectation(
"cannot send message",
TaskCancellationService.class.getName(),
Level.DEBUG,
"*cannot send ban for tasks*" + childNode.getId() + "*"));
childNode -> Arrays.asList(
new MockLogAppender.SeenEventExpectation(
"cannot send ban",
TaskCancellationService.class.getName(),
Level.DEBUG,
"*cannot send ban for tasks*" + childNode.getId() + "*"),
new MockLogAppender.SeenEventExpectation(
"cannot remove ban",
TaskCancellationService.class.getName(),
Level.DEBUG,
"*failed to remove ban for tasks*" + childNode.getId() + "*")));
}

@TestLogging(reason = "testing logging at DEBUG", value = "org.elasticsearch.tasks.TaskCancellationService:DEBUG")
Expand All @@ -69,16 +77,22 @@ public void testLogsAtDebugOnDisconnectionDuringBanRemoval() throws Exception {
}
connection.sendRequest(requestId, action, request, options);
},
childNode -> new MockLogAppender.SeenEventExpectation(
"cannot send message",
TaskCancellationService.class.getName(),
Level.DEBUG,
"*failed to remove ban for tasks*" + childNode.getId() + "*"));
childNode -> Arrays.asList(
new MockLogAppender.UnseenEventExpectation(
"cannot send ban",
TaskCancellationService.class.getName(),
Level.DEBUG,
"*cannot send ban for tasks*" + childNode.getId() + "*"),
new MockLogAppender.SeenEventExpectation(
"cannot remove ban",
TaskCancellationService.class.getName(),
Level.DEBUG,
"*failed to remove ban for tasks*" + childNode.getId() + "*")));
}

private void runTest(
StubbableTransport.SendRequestBehavior sendRequestBehavior,
Function<DiscoveryNode, MockLogAppender.SeenEventExpectation> expectation) throws Exception {
Function<DiscoveryNode, List<MockLogAppender.LoggingExpectation>> expectations) throws Exception {

final ArrayList<Closeable> resources = new ArrayList<>(3);

Expand Down Expand Up @@ -140,7 +154,9 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
Loggers.addAppender(LogManager.getLogger(TaskCancellationService.class), appender);
resources.add(() -> Loggers.removeAppender(LogManager.getLogger(TaskCancellationService.class), appender));

appender.addExpectation(expectation.apply(childTransportService.getLocalDiscoNode()));
for (MockLogAppender.LoggingExpectation expectation : expectations.apply(childTransportService.getLocalDiscoNode())) {
appender.addExpectation(expectation);
}

final PlainActionFuture<Void> cancellationFuture = new PlainActionFuture<>();
parentTransportService.getTaskManager().cancelTaskAndDescendants(parentTask, "test", true, cancellationFuture);
Expand Down

0 comments on commit 1f92bf8

Please sign in to comment.