Skip to content

Commit b990535

Browse files
committed
fix(qwp): background drainer connects no longer commit foreground sender state or fire connection events
Orphan drainers share buildAndConnect with the foreground sender. Every drainer (re)connect ran the same success path and the same event dispatch sites, so background activity corrupted foreground-observable state: - applyServerBatchSizeLimit committed the DRAINER endpoint's advertised cap into serverMaxBatchSize/effectiveAutoFlushBytes, re-sizing the producer's batch guard for a connection the producer is not on. A larger drainer-side cap lets an oversize batch escape the guard onto the foreground wire: ws-close[1009] -> PROTOCOL_VIOLATION HALT -- a producer-terminal caused by a background drainer, defeating Invariant B. A smaller cap produced spurious row-too-large errors. - the drainer's success stole the once-per-lifetime CONNECTED classification and overwrote currentEndpointIdx, fabricating RECONNECTED/FAILED_OVER lifecycle events (with fabricated previous-endpoint data) while the foreground never dropped. - a drainer's mid-drain wire drop fired a phantom DISCONNECTED against an endpoint the foreground was healthily using, and drainer walks leaked ENDPOINT_ATTEMPT_FAILED / AUTH_FAILED / ALL_ENDPOINTS_UNREACHABLE into the user's listener while inflating the attempt numbering foreground events carry. Split the shared path on the discriminator that already exists: a ReconnectSupplier with a non-null abortCheck serves a background drainer (ReconnectSupplier.isBackground()). Background walks keep what they genuinely share -- the endpoint list, hostTracker health and round state (recordSuccess / recordTransportError / recordRoleReject / recordMidStreamFailure: a real wire outcome is a real health signal, whichever loop observed it) -- and skip everything that exists for the foreground: currentEndpointIdx, hasEverConnected, the batch-cap commit, the attempt counter, and every dispatchConnectionEvent site including the mid-stream DISCONNECTED block. The drainer's lifecycle stays observable via BackgroundDrainerListener and the drainer counters. Flips DrainerForegroundEventIsolationTest (red-first, ad25cdf) green without touching the tests; drainer, send-loop, dispatcher, failover, pool-SF, and sender suites pass unchanged.
1 parent ad25cdf commit b990535

1 file changed

Lines changed: 99 additions & 41 deletions

File tree

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

Lines changed: 99 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ public class QwpWebSocketSender implements Sender {
211211
// advertised X-QWP-Max-Batch-Size at handshake so the wire payload stays
212212
// under the server's cap even with encoding overhead. Volatile because the
213213
// I/O thread writes this inside buildAndConnect on every successful
214-
// (re)connect while the producer thread reads it from sendRow without
214+
// FOREGROUND (re)connect -- background drainer connects never touch it --
215+
// while the producer thread reads it from sendRow without
215216
// holding the sender monitor.
216217
private volatile int effectiveAutoFlushBytes;
217218
private SenderErrorDispatcher errorDispatcher;
@@ -222,7 +223,8 @@ public class QwpWebSocketSender implements Sender {
222223
private int errorInboxCapacity = SenderErrorDispatcher.DEFAULT_CAPACITY;
223224
private long firstPendingRowTimeNanos;
224225
private boolean hasDeferredMessages;
225-
// Stickys true once any successful connect has happened. Drives the
226+
// Stickys true once any successful FOREGROUND connect has happened
227+
// (background drainer connects never set it). Drives the
226228
// CONNECTED-vs-RECONNECTED-vs-FAILED_OVER classification at the success
227229
// point in buildAndConnect.
228230
private boolean hasEverConnected;
@@ -258,8 +260,9 @@ public class QwpWebSocketSender implements Sender {
258260
CursorWebSocketSendLoop.DEFAULT_RECONNECT_MAX_DURATION_MILLIS;
259261
private boolean requestDurableAck;
260262
// Monotonic per-attempt counter snapshotted onto every connection event
261-
// fired from buildAndConnect. Counts every endpoint try -- successes and
262-
// failures alike -- across this sender's lifetime.
263+
// fired from buildAndConnect. Counts every FOREGROUND endpoint try --
264+
// successes and failures alike -- across this sender's lifetime.
265+
// Background (drainer) walks fire no events and do not advance it.
263266
private long roundConnectAttemptSeq;
264267
// Monotonic per-round counter incremented inside buildAndConnect on each
265268
// beginRound(true) call. roundSeq=1 is the first round; CONNECTED in the
@@ -270,7 +273,8 @@ public class QwpWebSocketSender implements Sender {
270273
// arbitrarily large datasets that exceed the server's recv buffer.
271274
private boolean transactional;
272275
// Server-advertised hard cap on QWP ingest payload bytes, captured from
273-
// X-QWP-Max-Batch-Size on each successful handshake. 0 when the server
276+
// X-QWP-Max-Batch-Size on each successful FOREGROUND handshake (a
277+
// background drainer's endpoint cap is irrelevant to the producer's wire). 0 when the server
274278
// did not advertise the header (older builds); the sender then falls back
275279
// to its locally configured budget. Volatile because buildAndConnect can
276280
// refresh this from the cursor I/O thread on a mid-stream reconnect while
@@ -1020,8 +1024,9 @@ public void close() {
10201024
}
10211025
}
10221026
// Drainer pool closes after the foreground I/O loop is wound
1023-
// down. Drainers share buildAndConnect (endpoints, hostTracker,
1024-
// connection-event dispatcher) with the foreground, but their
1027+
// down. Drainers share buildAndConnect's endpoint walk and
1028+
// hostTracker state with the foreground (never its observable
1029+
// connection state or event stream), but their
10251030
// connect gate is their own stop flag — NOT the foreground
10261031
// loop's liveness — so the pool's graceful-drain window below
10271032
// still lets in-flight drainers finish (including reconnects)
@@ -2419,20 +2424,37 @@ private void atNanos(long timestampNanos) {
24192424
}
24202425

24212426
private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
2427+
// Background (drainer) factories share this connect walk -- endpoint
2428+
// list, hostTracker health and round state -- but must stay INVISIBLE
2429+
// in the foreground sender's observable state. SenderConnectionEvents
2430+
// describe the FOREGROUND connection's lifecycle, and the cap-derived
2431+
// sizing (serverMaxBatchSize / effectiveAutoFlushBytes) guards the
2432+
// FOREGROUND wire: a drainer connect that committed either would
2433+
// fabricate lifecycle transitions the foreground never had, steal the
2434+
// once-per-lifetime CONNECTED classification, and re-size the
2435+
// producer's batch guard for a connection the producer is not on
2436+
// (oversize batch -> ws-close[1009] -> producer-terminal HALT caused
2437+
// by background activity).
2438+
final boolean background = ctx.isBackground();
24222439
int previousIdx = ctx.previousIdx;
24232440
if (previousIdx >= 0) {
24242441
// Mid-stream wire failure -- the I/O loop just observed the active
2425-
// connection drop and called us via the reconnect factory. Surface
2426-
// a DISCONNECTED event identifying which endpoint just went away
2427-
// before we start the per-endpoint walk for a replacement.
2428-
Endpoint priorEp = endpoints.get(previousIdx);
2429-
dispatchConnectionEvent(
2430-
SenderConnectionEvent.Kind.DISCONNECTED,
2431-
priorEp.host, priorEp.port,
2432-
null, SenderConnectionEvent.NO_PORT,
2433-
SenderConnectionEvent.NO_ATTEMPT_NUMBER,
2434-
roundSeq,
2435-
null);
2442+
// connection drop and called us via the reconnect factory. Only a
2443+
// FOREGROUND drop surfaces DISCONNECTED: a drainer's wire drop is
2444+
// not a foreground outage, and reporting it would claim an outage
2445+
// against an endpoint the foreground may be healthily using. The
2446+
// hostTracker health penalty is recorded either way -- the drop
2447+
// was real, whichever loop observed it.
2448+
if (!background) {
2449+
Endpoint priorEp = endpoints.get(previousIdx);
2450+
dispatchConnectionEvent(
2451+
SenderConnectionEvent.Kind.DISCONNECTED,
2452+
priorEp.host, priorEp.port,
2453+
null, SenderConnectionEvent.NO_PORT,
2454+
SenderConnectionEvent.NO_ATTEMPT_NUMBER,
2455+
roundSeq,
2456+
null);
2457+
}
24362458
hostTracker.recordMidStreamFailure(previousIdx);
24372459
ctx.previousIdx = -1;
24382460
}
@@ -2460,7 +2482,12 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
24602482
if (idx < 0) break;
24612483
Endpoint ep = endpoints.get(idx);
24622484
lastEndpoint = ep;
2463-
long attemptNumber = ++roundConnectAttemptSeq;
2485+
// Attempt numbers exist for foreground observability only. A
2486+
// background walk fires no events and must not skew the numbering
2487+
// the user sees on subsequent foreground events.
2488+
long attemptNumber = background
2489+
? SenderConnectionEvent.NO_ATTEMPT_NUMBER
2490+
: ++roundConnectAttemptSeq;
24642491
WebSocketClient newClient = tlsConfig != null
24652492
? WebSocketClientFactory.newTlsInstance(tlsConfig)
24662493
: WebSocketClientFactory.newPlainTextInstance();
@@ -2480,10 +2507,12 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
24802507
hostTracker.recordRoleReject(idx, re.isTransient());
24812508
lastError = re;
24822509
lastRoleReject = re;
2483-
dispatchConnectionEvent(
2484-
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2485-
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2486-
attemptNumber, roundSeq, re);
2510+
if (!background) {
2511+
dispatchConnectionEvent(
2512+
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2513+
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2514+
attemptNumber, roundSeq, re);
2515+
}
24872516
continue;
24882517
}
24892518
if (classified instanceof QwpAuthFailedException) {
@@ -2493,10 +2522,12 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
24932522
// moment the I/O thread gives up, ahead of the producer
24942523
// thread learning via LineSenderException on the next
24952524
// API call.
2496-
dispatchConnectionEvent(
2497-
SenderConnectionEvent.Kind.AUTH_FAILED,
2498-
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2499-
attemptNumber, roundSeq, classified);
2525+
if (!background) {
2526+
dispatchConnectionEvent(
2527+
SenderConnectionEvent.Kind.AUTH_FAILED,
2528+
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2529+
attemptNumber, roundSeq, classified);
2530+
}
25002531
throw classified;
25012532
}
25022533
if (terminalUpgradeError == null && (
@@ -2507,19 +2538,23 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
25072538
}
25082539
hostTracker.recordTransportError(idx);
25092540
lastError = classified;
2510-
dispatchConnectionEvent(
2511-
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2512-
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2513-
attemptNumber, roundSeq, classified);
2541+
if (!background) {
2542+
dispatchConnectionEvent(
2543+
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2544+
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2545+
attemptNumber, roundSeq, classified);
2546+
}
25142547
continue;
25152548
} catch (Exception e) {
25162549
newClient.close();
25172550
hostTracker.recordTransportError(idx);
25182551
lastError = e;
2519-
dispatchConnectionEvent(
2520-
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2521-
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2522-
attemptNumber, roundSeq, e);
2552+
if (!background) {
2553+
dispatchConnectionEvent(
2554+
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2555+
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2556+
attemptNumber, roundSeq, e);
2557+
}
25232558
continue;
25242559
}
25252560
if (requestDurableAck && !newClient.isServerDurableAckEnabled()) {
@@ -2531,15 +2566,28 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
25312566
terminalUpgradeError = ackErr;
25322567
}
25332568
lastError = ackErr;
2534-
dispatchConnectionEvent(
2535-
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2536-
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2537-
attemptNumber, roundSeq, ackErr);
2569+
if (!background) {
2570+
dispatchConnectionEvent(
2571+
SenderConnectionEvent.Kind.ENDPOINT_ATTEMPT_FAILED,
2572+
ep.host, ep.port, null, SenderConnectionEvent.NO_PORT,
2573+
attemptNumber, roundSeq, ackErr);
2574+
}
25382575
continue;
25392576
}
2540-
int previousLiveIdx = currentEndpointIdx;
25412577
hostTracker.recordSuccess(idx);
25422578
ctx.previousIdx = idx;
2579+
if (background) {
2580+
// Walk bookkeeping only: recordSuccess feeds the shared health
2581+
// tracker and ctx.previousIdx arms this factory's own
2582+
// mid-stream-failure handling on its next reconnect. No
2583+
// lifecycle event, no CONNECTED/RECONNECTED/FAILED_OVER
2584+
// classification state, no producer batch re-sizing -- the
2585+
// drainer's lifecycle is observable via
2586+
// BackgroundDrainerListener and the drainer counters, never
2587+
// the foreground connection-event stream.
2588+
return newClient;
2589+
}
2590+
int previousLiveIdx = currentEndpointIdx;
25432591
currentEndpointIdx = idx;
25442592
// Classify the success. CONNECTED only fires once per sender
25452593
// lifetime; subsequent successes are RECONNECTED (same endpoint
@@ -2580,7 +2628,7 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) {
25802628
// which terminal branch fires next. The connectLoop wrapper retries,
25812629
// and each retry that re-enters this method and fails again produces
25822630
// its own ALL_ENDPOINTS_UNREACHABLE event.
2583-
if (lastEndpoint != null) {
2631+
if (!background && lastEndpoint != null) {
25842632
dispatchConnectionEvent(
25852633
SenderConnectionEvent.Kind.ALL_ENDPOINTS_UNREACHABLE,
25862634
lastEndpoint.host, lastEndpoint.port,
@@ -3385,6 +3433,16 @@ String abortMessage() {
33853433
return abortCheck != null ? abortMessage : "sender closed during connect";
33863434
}
33873435

3436+
/**
3437+
* True when this factory serves a background drainer. Background
3438+
* connects share buildAndConnect's endpoint walk and hostTracker
3439+
* health state, but commit none of the foreground sender's
3440+
* observable connection state and fire no connection events.
3441+
*/
3442+
boolean isBackground() {
3443+
return abortCheck != null;
3444+
}
3445+
33883446
boolean isAborted() {
33893447
return abortCheck != null
33903448
? abortCheck.getAsBoolean()

0 commit comments

Comments
 (0)