Avoid concurrent modification in mock log appender (#41424)
It can be the case that while we are setting up expectations that also a log message is appended. For example, if we are setting up these expectations after a cluster has formed and messages start being sent around the cluster. In this case, we would hit a concurrent modification exception while we are mutating the expectations, and also while the expectations are being iterated over as a message is appended. This commit avoids this by using a copy-on-write array list which is safe for concurrent modification and iteration. Note that another possible approach here is to use synchronized, but that seems unnecessary since we don't appear to rely on messages that are sent while we are setting up expectations. Rather, we are setting up some expectations and some situation that we think will cause those expectations to be met. Using copy-on-write array list here is nice since we avoid bottlenecking these tests on synchronizing these methods.
This commit is contained in:
parent
e6c24e3d2d
commit
4a288af85f
|
@ -24,8 +24,8 @@ import org.apache.logging.log4j.core.appender.AbstractAppender;
|
|||
import org.apache.logging.log4j.core.filter.RegexFilter;
|
||||
import org.elasticsearch.common.regex.Regex;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.CopyOnWriteArrayList;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
|
@ -42,7 +42,12 @@ public class MockLogAppender extends AbstractAppender {
|
|||
|
||||
public MockLogAppender() throws IllegalAccessException {
|
||||
super("mock", RegexFilter.createFilter(".*(\n.*)*", new String[0], false, null, null), null);
|
||||
expectations = new ArrayList<>();
|
||||
/*
|
||||
* We use a copy-on-write array list since log messages could be appended while we are setting up expectations. When that occurs,
|
||||
* we would run into a concurrent modification exception from the iteration over the expectations in #append, concurrent with a
|
||||
* modification from #addExpectation.
|
||||
*/
|
||||
expectations = new CopyOnWriteArrayList<>();
|
||||
}
|
||||
|
||||
public void addExpectation(LoggingExpectation expectation) {
|
||||
|
|
|
@ -993,7 +993,6 @@ public abstract class AbstractSimpleTransportTestCase extends ESTestCase {
|
|||
}
|
||||
|
||||
@TestLogging(value = "org.elasticsearch.transport.TransportService.tracer:trace")
|
||||
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/40586")
|
||||
public void testTracerLog() throws Exception {
|
||||
TransportRequestHandler<TransportRequest> handler = (request, channel, task) -> channel.sendResponse(new StringMessageResponse(""));
|
||||
TransportRequestHandler<StringMessageRequest> handlerWithError = (request, channel, task) -> {
|
||||
|
|
Loading…
Reference in New Issue