Skip to content

Commit

Permalink
ClientEffectiveStrategyTest: Attempt at fixing flakyness (#2804)
Browse files Browse the repository at this point in the history
Based on initial analysis the ClientInvokingThreadRecorder's state
gets written and read from multiple threads, but not all of it is
properly thread-safe. The invokingThreads and errors use thread-safe
datastructures, but the offloadPoints do not.

Since the offloadPoints are on the critical path for the flaky
test failures, let's start by making them thread safe and check
if the errors still show up.

If they still persist, there might be more coordination needed
between the recording and checking threads as well.
  • Loading branch information
daschl authored Jan 16, 2024
1 parent b6b98cc commit b2fc0bc
Showing 1 changed file with 6 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,12 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.LinkedBlockingQueue;
Expand Down Expand Up @@ -478,9 +480,10 @@ private static Buffer content(HttpExecutionContext ctx) {

private static final class ClientInvokingThreadRecorder implements StreamingHttpClientFilterFactory {

private Thread applicationThread = Thread.currentThread();
private HttpExecutionStrategy expectedStrategy;
private final EnumSet<ClientOffloadPoint> offloadPoints = EnumSet.noneOf(ClientOffloadPoint.class);
private volatile Thread applicationThread = Thread.currentThread();
private volatile HttpExecutionStrategy expectedStrategy;
private final Set<ClientOffloadPoint> offloadPoints =
Collections.synchronizedSet(EnumSet.noneOf(ClientOffloadPoint.class));
private final ConcurrentMap<ClientOffloadPoint, String> invokingThreads = new ConcurrentHashMap<>();
private final Queue<Throwable> errors = new LinkedBlockingQueue<>();

Expand Down

0 comments on commit b2fc0bc

Please sign in to comment.