Skip to content

Commit 9b33ae6

Browse files
committed
refactor(qwp): lock-free background connect walks via private round cursors (M11 follow-up)
Completes the M11 concurrency work: no network I/O runs under a sender-wide lock for background work. QwpHostHealthTracker (additive): - newRoundCursor(): walker-private full-sweep cursor -- claim-at-pick over cursor-local attempted bits, ordered by the live shared (state, zone_tier) priority. Makes the pick->record pair safe without external serialization and guarantees every sweep tries every endpoint exactly once regardless of concurrent walkers. - Health-only record overloads (markRoundAttempted=false) for recordSuccess/recordTransportError/recordRoleReject: update the shared health ledger without touching the shared round's attempted bits. Existing signatures delegate with true; the query client is untouched. QwpWebSocketSender: - Background (drainer) walks take NO lock: private RoundCursor + health-only records. Concurrent drainer sweeps proceed in parallel with each other and with the foreground, share health observations, and can neither consume nor poison the foreground's round (roundSeq and the round lifecycle are now strictly foreground-owned). - The tryLock-yield path (and its busy exception) is gone: background sweeps are no longer deferred on contention, they just run. - Foreground walks keep connectWalkLock across the sweep -- they own the shared round state and cannot overlap by construction; the lock is insurance for that invariant. Tests: - QwpHostHealthTrackerTest: +6 tests -- cursor full-sweep priority order, live re-ranking mid-sweep, shared-round isolation, no endpoint stealing between cursors, health-only records vs round bits, health-only success feeding sticky-Healthy recency. - QwpConnectWalkLockPriorityTest replaced by QwpConnectWalkBackgroundIsolationTest: two full background sweeps run to completion while a foreground walk is parked inside a blocking connect holding the lock. Red-verified against the previous tryLock design: fails with the busy-exception artifact. - BackgroundDrainerPoolConnectPhaseCloseTest: comments updated (the busy exception no longer exists; the factory now throws a generic outage-shaped transport error -- same Invariant B contract).
1 parent 788b85a commit 9b33ae6

5 files changed

Lines changed: 330 additions & 130 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpHostHealthTracker.java

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,19 @@
3535
* so a known-good cross-zone host is picked before an untried local host.
3636
* <p>
3737
* Each method is internally synchronized, but pickNext + recordX is not atomic
38-
* across the pair. Callers must externally serialize a pick → record sequence
39-
* (the QWP clients do this via the sender's {@code synchronized buildAndConnect}
40-
* and the query client's documented one-execute-at-a-time contract).
38+
* across the pair. Callers of the SHARED-round API (pickNext / beginRound /
39+
* isRoundExhausted) must externally serialize a pick → record sequence (the
40+
* ingest sender does this by keeping its foreground connect walk single-file
41+
* behind a lock; the query client via its documented one-execute-at-a-time
42+
* contract).
43+
* <p>
44+
* Concurrent walkers that must not consume or poison the shared round --
45+
* the ingest sender's background orphan drainers -- use a private
46+
* {@link RoundCursor} ({@link #newRoundCursor()}) paired with the
47+
* health-only record overloads ({@code markRoundAttempted=false}): the
48+
* cursor's attempted set is walker-local (claim-at-pick, so concurrent
49+
* cursors never race on the pick → record pair), while state/zone updates
50+
* flow into the shared health ledger that orders everyone's picks.
4151
*/
4252
public final class QwpHostHealthTracker {
4353
public enum HostState {
@@ -250,24 +260,113 @@ public void recordMidStreamFailure(int idx) {
250260
}
251261

252262
public void recordRoleReject(int idx, boolean isTransient) {
263+
recordRoleReject(idx, isTransient, true);
264+
}
265+
266+
/**
267+
* Variant with an explicit round-bit policy. {@code markRoundAttempted =
268+
* false} updates only the shared health ledger (state), leaving the
269+
* shared round's attempted bit untouched — for walkers on a private
270+
* {@link RoundCursor} whose attempts must stay invisible to the shared
271+
* round (the ingest sender's background drainers).
272+
*/
273+
public void recordRoleReject(int idx, boolean isTransient, boolean markRoundAttempted) {
253274
synchronized (lock) {
254275
states[idx] = isTransient ? HostState.TRANSIENT_REJECT : HostState.TOPOLOGY_REJECT;
255-
attemptedThisRound[idx] = true;
276+
if (markRoundAttempted) {
277+
attemptedThisRound[idx] = true;
278+
}
256279
}
257280
}
258281

259282
public void recordSuccess(int idx) {
283+
recordSuccess(idx, true);
284+
}
285+
286+
/**
287+
* Variant with an explicit round-bit policy; see
288+
* {@link #recordRoleReject(int, boolean, boolean)}. The success epoch
289+
* (sticky-Healthy recency) is recorded either way — a background
290+
* walker's success is real health data.
291+
*/
292+
public void recordSuccess(int idx, boolean markRoundAttempted) {
260293
synchronized (lock) {
261294
states[idx] = HostState.HEALTHY;
262-
attemptedThisRound[idx] = true;
295+
if (markRoundAttempted) {
296+
attemptedThisRound[idx] = true;
297+
}
263298
lastSuccessEpoch[idx] = ++successEpoch;
264299
}
265300
}
266301

267302
public void recordTransportError(int idx) {
303+
recordTransportError(idx, true);
304+
}
305+
306+
/**
307+
* Variant with an explicit round-bit policy; see
308+
* {@link #recordRoleReject(int, boolean, boolean)}.
309+
*/
310+
public void recordTransportError(int idx, boolean markRoundAttempted) {
268311
synchronized (lock) {
269312
states[idx] = HostState.TRANSPORT_ERROR;
270-
attemptedThisRound[idx] = true;
313+
if (markRoundAttempted) {
314+
attemptedThisRound[idx] = true;
315+
}
316+
}
317+
}
318+
319+
/**
320+
* Creates a walker-private full-sweep cursor over the host list. Each
321+
* {@link RoundCursor#next()} returns the highest-priority host this
322+
* cursor has not yet returned — priority is the same live
323+
* {@code (state, zone_tier)} tuple {@link #pickNext()} uses — and
324+
* claims it at pick time in the cursor's OWN attempted set, so:
325+
* <ul>
326+
* <li>every cursor sweeps every host exactly once regardless of what
327+
* other walkers do concurrently (no endpoint stealing);</li>
328+
* <li>the pick → record pair needs no external serialization — the
329+
* claim is cursor-local, and the health records are atomic;</li>
330+
* <li>the shared round (attempted bits, {@link #beginRound},
331+
* {@link #isRoundExhausted}) is never consulted nor mutated.</li>
332+
* </ul>
333+
* Pair with the {@code markRoundAttempted=false} record overloads so the
334+
* walker's results update shared health without touching the shared
335+
* round.
336+
*/
337+
public RoundCursor newRoundCursor() {
338+
return new RoundCursor();
339+
}
340+
341+
/** See {@link #newRoundCursor()}. Not thread-safe for sharing a single
342+
* instance across walkers; create one per walk. */
343+
public final class RoundCursor {
344+
private final boolean[] attempted = new boolean[hostCount];
345+
346+
private RoundCursor() {
347+
}
348+
349+
/**
350+
* Highest-priority host this cursor has not yet returned, claimed at
351+
* pick time; -1 once the cursor has swept every host. Ordering reads
352+
* the LIVE shared health state under the tracker lock, so a state
353+
* change recorded by any walker between two calls re-ranks the
354+
* remaining hosts.
355+
*/
356+
public int next() {
357+
synchronized (lock) {
358+
for (HostState p : PRIORITY_ORDER) {
359+
for (ZoneTier z : ZONE_PRIORITY_ORDER) {
360+
for (int i = 0; i < hostCount; i++) {
361+
if (!attempted[i] && states[i] == p && zoneTiers[i] == z) {
362+
attempted[i] = true;
363+
return i;
364+
}
365+
}
366+
}
367+
}
368+
return -1;
369+
}
271370
}
272371
}
273372

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

Lines changed: 59 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,19 @@ public class QwpWebSocketSender implements Sender {
153153
private final List<Endpoint> endpoints;
154154
// Global symbol dictionary for delta encoding
155155
private final GlobalSymbolDictionary globalSymbolDictionary;
156-
// Guards the connect walk (see buildAndConnect): the shared
157-
// QwpHostHealthTracker round/health state, roundSeq,
158-
// roundConnectAttemptSeq, and the foreground lifecycle commits
159-
// (currentEndpointIdx, hasEverConnected, cap-derived sizing). A
160-
// dedicated lock rather than the sender monitor so that (a) unrelated
161-
// synchronized sender methods (setDrainerListener, startOrphanDrainers)
162-
// never contend with a connect walk's network I/O, and (b) background
163-
// walks can use tryLock to yield to the foreground instead of queuing
164-
// on an unfair monitor.
156+
// Serializes FOREGROUND connect walks only (see buildAndConnect): the
157+
// shared-round state in hostTracker (pickNext/beginRound/attempted
158+
// bits), roundSeq, roundConnectAttemptSeq, and the foreground lifecycle
159+
// commits (currentEndpointIdx, hasEverConnected, cap-derived sizing)
160+
// all have exactly one writer -- the foreground walk -- and foreground
161+
// walks cannot overlap by construction (the I/O loop is single-threaded
162+
// and the user-thread initial connect completes before the loop
163+
// starts); the lock is cheap insurance for that invariant. Background
164+
// (drainer) walks take NO lock at all: they walk a private
165+
// QwpHostHealthTracker.RoundCursor and record health-only results, so
166+
// no network I/O ever runs under a sender-wide lock for background
167+
// work, and neither the foreground's reconnect nor close() can queue
168+
// behind a drainer's endpoint walk.
165169
private final ReentrantLock connectWalkLock = new ReentrantLock();
166170
private final QwpHostHealthTracker hostTracker;
167171
private final CharSequenceObjHashMap<QwpTableBuffer> tableBuffers;
@@ -968,8 +972,9 @@ public QwpWebSocketSender charColumn(CharSequence columnName, char value) {
968972
* native connect — bounded by {@code connect_timeout}, or by the
969973
* OS SYN-retry deadline (60-130s on Linux) when the default
970974
* {@code 0} is in effect. Background drainer walks never delay
971-
* this stop: they yield the connect-walk lock to the foreground
972-
* (see {@link #buildAndConnect});</li>
975+
* this stop: they run lock-free on private round cursors and
976+
* never hold anything the foreground waits on (see
977+
* {@link #buildAndConnect});</li>
973978
* <li>drainer pool: drainers still in their connect-retry phase are
974979
* stop-signaled immediately (exit within ~50ms); drainers actively
975980
* replaying frames get a 2.5s grace window plus a 0.5s stop window
@@ -2585,13 +2590,9 @@ private WebSocketClient newWebSocketClient() {
25852590

25862591
/**
25872592
* Multi-endpoint connect walk shared by the foreground sender and the
2588-
* background orphan drainers. One invocation walks every endpoint of the
2589-
* current round, performing a TCP/TLS connect plus a WebSocket upgrade
2590-
* per endpoint, all while holding {@link #connectWalkLock} (the lock
2591-
* protects the shared {@link QwpHostHealthTracker} round/health state,
2592-
* the round counters, and the foreground lifecycle commits).
2593-
* <p>
2594-
* Worst-case lock hold per walk is
2593+
* background orphan drainers. One invocation sweeps the endpoint list,
2594+
* performing a TCP/TLS connect plus a WebSocket upgrade per endpoint;
2595+
* worst-case sweep duration is
25952596
* {@code endpoints x (connect timeout + upgrade timeout)}:
25962597
* <ul>
25972598
* <li>foreground walk: {@code connect_timeout} verbatim -- the default
@@ -2604,33 +2605,30 @@ private WebSocketClient newWebSocketClient() {
26042605
* {@link #effectiveConnectTimeoutMs(boolean, int)}.</li>
26052606
* </ul>
26062607
* <p>
2607-
* Lock policy -- the foreground has absolute priority. Foreground walks
2608-
* (the producer's initial connect and the I/O loop's reconnects) block
2609-
* on {@code lock()} and can therefore only ever wait behind another
2610-
* foreground walk. Background walks use {@code tryLock()} and treat
2611-
* contention as a transport-shaped transient: the thrown
2612-
* {@link LineSenderException} lands in the drainer's indefinite
2613-
* capped-backoff retry (Invariant B) -- both in
2614-
* {@code BackgroundDrainer.connectWithDurableAckRetry()} and in
2615-
* {@code CursorWebSocketSendLoop.connectLoop}'s Throwable branch -- so a
2616-
* drainer sweep is deferred, never failed, and the foreground's
2617-
* reconnect and {@code close()} paths are never queued behind a
2618-
* drainer's endpoint walk.
2608+
* Concurrency policy -- no network I/O under a sender-wide lock for
2609+
* background work. FOREGROUND walks (the producer's initial connect and
2610+
* the I/O loop's reconnects) hold {@link #connectWalkLock} across the
2611+
* sweep: they own the shared round state and the lifecycle commits, and
2612+
* can only ever wait behind another foreground walk (which cannot
2613+
* happen by construction -- the lock is insurance). BACKGROUND (drainer)
2614+
* walks take NO lock: each sweeps a private
2615+
* {@link QwpHostHealthTracker.RoundCursor} -- full sweep, claim-at-pick,
2616+
* ordered by the live shared health state -- and records results with
2617+
* the health-only overloads ({@code markRoundAttempted=false}), so
2618+
* concurrent drainer sweeps proceed in parallel with each other and
2619+
* with the foreground, share health observations, and can neither
2620+
* consume nor poison the foreground's round. The foreground's
2621+
* reconnect and {@code close()} paths are therefore never queued
2622+
* behind a drainer's endpoint walk.
26192623
*/
26202624
private WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
26212625
if (ctx.isBackground()) {
2622-
if (!connectWalkLock.tryLock()) {
2623-
// Transport-shaped on purpose: every drainer retry loop
2624-
// treats an untyped connect failure as transient and backs
2625-
// off (Invariant B) -- the sweep is deferred, never failed,
2626-
// and no .failed sentinel can result from lock contention.
2627-
throw new LineSenderException(
2628-
"connect walk lock is busy (another connect walk is in progress); "
2629-
+ "background drainer will retry after backoff");
2630-
}
2631-
} else {
2632-
connectWalkLock.lock();
2626+
// Lock-free: the walk below touches only internally-synchronized
2627+
// hostTracker health state and walk-local/cursor-local state on
2628+
// the background path.
2629+
return connectWalk(ctx);
26332630
}
2631+
connectWalkLock.lock();
26342632
try {
26352633
return connectWalk(ctx);
26362634
} finally {
@@ -2640,7 +2638,10 @@ private WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
26402638

26412639
private WebSocketClient connectWalk(ReconnectSupplier ctx) {
26422640
// Background (drainer) factories share this connect walk -- endpoint
2643-
// list, hostTracker health and round state -- but must stay INVISIBLE
2641+
// list and hostTracker HEALTH state (never the shared round: a
2642+
// background sweep walks its own RoundCursor and records with
2643+
// markRoundAttempted=false, so it cannot consume the foreground's
2644+
// round or skew roundSeq) -- but must stay INVISIBLE
26442645
// in the foreground sender's observable state. SenderConnectionEvents
26452646
// describe the FOREGROUND connection's lifecycle, and the cap-derived
26462647
// sizing (serverMaxBatchSize / effectiveAutoFlushBytes) guards the
@@ -2651,6 +2652,12 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
26512652
// (oversize batch -> ws-close[1009] -> producer-terminal HALT caused
26522653
// by background activity).
26532654
final boolean background = ctx.isBackground();
2655+
// Private full-sweep cursor for background walks: claim-at-pick over
2656+
// cursor-local attempted bits makes the pick -> record pair safe
2657+
// without any walk-wide lock, and guarantees every sweep tries every
2658+
// endpoint exactly once regardless of concurrent walkers.
2659+
final QwpHostHealthTracker.RoundCursor cursor =
2660+
background ? hostTracker.newRoundCursor() : null;
26542661
int previousIdx = ctx.previousIdx;
26552662
if (previousIdx >= 0) {
26562663
// Mid-stream wire failure -- the I/O loop just observed the active
@@ -2673,7 +2680,10 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
26732680
hostTracker.recordMidStreamFailure(previousIdx);
26742681
ctx.previousIdx = -1;
26752682
}
2676-
if (hostTracker.isRoundExhausted()) {
2683+
// Shared-round lifecycle is foreground-only: a background walk must
2684+
// not advance the round (or roundSeq, which numbers foreground
2685+
// events) under the foreground's feet.
2686+
if (!background && hostTracker.isRoundExhausted()) {
26772687
roundSeq++;
26782688
hostTracker.beginRound(true);
26792689
}
@@ -2693,7 +2703,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
26932703
if (ctx.isAborted()) {
26942704
throw new LineSenderException(ctx.abortMessage());
26952705
}
2696-
int idx = hostTracker.pickNext();
2706+
int idx = background ? cursor.next() : hostTracker.pickNext();
26972707
if (idx < 0) break;
26982708
Endpoint ep = endpoints.get(idx);
26992709
lastEndpoint = ep;
@@ -2717,7 +2727,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
27172727
newClient.close();
27182728
if (classified instanceof QwpIngressRoleRejectedException) {
27192729
QwpIngressRoleRejectedException re = (QwpIngressRoleRejectedException) classified;
2720-
hostTracker.recordRoleReject(idx, re.isTransient());
2730+
hostTracker.recordRoleReject(idx, re.isTransient(), !background);
27212731
lastError = re;
27222732
lastRoleReject = re;
27232733
if (!background) {
@@ -2749,7 +2759,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
27492759
&& !((WebSocketUpgradeException) classified).isRoleMismatch()))) {
27502760
terminalUpgradeError = classified;
27512761
}
2752-
hostTracker.recordTransportError(idx);
2762+
hostTracker.recordTransportError(idx, !background);
27532763
lastError = classified;
27542764
if (!background) {
27552765
dispatchConnectionEvent(
@@ -2760,7 +2770,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
27602770
continue;
27612771
} catch (Exception e) {
27622772
newClient.close();
2763-
hostTracker.recordTransportError(idx);
2773+
hostTracker.recordTransportError(idx, !background);
27642774
lastError = e;
27652775
if (!background) {
27662776
dispatchConnectionEvent(
@@ -2790,7 +2800,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
27902800
}
27912801
if (requestDurableAck && !newClient.isServerDurableAckEnabled()) {
27922802
newClient.close();
2793-
hostTracker.recordRoleReject(idx, false);
2803+
hostTracker.recordRoleReject(idx, false, !background);
27942804
QwpDurableAckMismatchException ackErr = new QwpDurableAckMismatchException(
27952805
ep.host, ep.port, null);
27962806
if (terminalUpgradeError == null) {
@@ -2805,7 +2815,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
28052815
}
28062816
continue;
28072817
}
2808-
hostTracker.recordSuccess(idx);
2818+
hostTracker.recordSuccess(idx, !background);
28092819
ctx.previousIdx = idx;
28102820
if (background) {
28112821
// Walk bookkeeping only: recordSuccess feeds the shared health

0 commit comments

Comments
 (0)