Skip to content

Commit

Permalink
Fix race condition in SimpleNioTransportTests.testTracerLog (opensear…
Browse files Browse the repository at this point in the history
…ch-project#12139)

The test currently blocks on receiving a response to the request for the
"internal:testNotSeen" action. However, that response is sent from
TransportService before the trace logger [writes its log message][1].
Since the test was not polling for this "sent response" log message to
appear that meant it was possible for the test to remove/stop the mock
log appender concurrently with the logging of that final message. The
fix is to include this final log message as an expectation, so the test
will poll until this message appears and the logger should be quiescent
when the appender is removed and stopped.

[1]: https://github.com/opensearch-project/OpenSearch/blob/71f1fabe149fd0777edf44502ace4a8f0911feeb/server/src/main/java/org/opensearch/transport/TransportService.java#L1273

Signed-off-by: Andrew Ross <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
  • Loading branch information
andrross authored and shiv0408 committed Apr 25, 2024
1 parent 7414043 commit b930590
Showing 1 changed file with 8 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1283,9 +1283,17 @@ public String executor() {
Level.TRACE,
notSeenReceived
);
final String notSeenResponseSent = ".*\\[internal:testNotSeen].*sent response.*";
final MockLogAppender.LoggingExpectation notSeenResponseSentExpectation = new MockLogAppender.PatternSeenEventExpectation(
"sent response",
"org.opensearch.transport.TransportService.tracer",
Level.TRACE,
notSeenResponseSent
);

appender.addExpectation(notSeenSentExpectation);
appender.addExpectation(notSeenReceivedExpectation);
appender.addExpectation(notSeenResponseSentExpectation);

PlainTransportFuture<StringMessageResponse> future = new PlainTransportFuture<>(noopResponseHandler);
serviceA.sendRequest(nodeB, "internal:testNotSeen", new StringMessageRequest(""), future);
Expand Down

0 comments on commit b930590

Please sign in to comment.