Skip to content

Commit 70c706a

Browse files
committed
fix(qwp): foreground role-reject retry backs off exponentially, not a fixed-interval TLS storm
The cursor I/O loop's role-reject branch reset backoff to reconnect_initial_backoff_millis, parked exactly that interval (no jitter, no growth), then `continue`d -- bypassing the shared capped exponential backoff-with-jitter block. Once an all-replica failover window (or a misconfigured replicas-only address list, now surfaced here as a retriable role reject rather than a terminal durable-ack mismatch) reaches this branch, every sweep pinned at ~100ms forever: a fixed ~10-20 handshakes/s storm of fresh WebSocketClient (2x64KB native) + fresh SSLContext (no session resumption, trust-store re-read) per endpoint. That broke the documented capped-exponential-backoff contract and was ~50x more aggressive than the orphan drainer, which already grows to reconnect_max_backoff_millis. Fall through to the shared backoff block so role-reject grows to reconnect_max_backoff_millis (5s default) like every other reconnect branch, and emit a throttled WARN naming the all-replica condition. close() still unparks the loop via LockSupport, so shutdown stays prompt under the longer park (QwpRoleRejectCloseRaceTest green). Adds QwpRoleRejectBackoffGrowthTest as a red-first guard: on the old behaviour the inter-attempt gaps are flat ([52,51,51,51,51,51]ms); after the fix they grow exponentially. Also lands accompanying connect-timeout / Invariant-B doc and comment refinements across Sender, QwpWebSocketSender, BackgroundDrainer, SenderConnectionEvent, QwpVersionMismatchException, and the design notes, plus .gitignore entries for root /target and .pi-subagents/.
1 parent b990535 commit 70c706a

16 files changed

Lines changed: 626 additions & 88 deletions

.gitignore

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,9 @@ core/CMakeCache.txt
2929
**/build
3030
**/CMakeFiles
3131
.envrc
32-
.vscode
32+
.vscode
33+
# Root-level Maven build output
34+
/target
35+
36+
# pi subagents runtime artifacts
37+
.pi-subagents/

core/src/main/java/io/questdb/client/QuestDBBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public QuestDBBuilder connectionListener(SenderConnectionListener listener) {
130130
/**
131131
* Sets the async error handler applied to every pooled ingest
132132
* {@link Sender}. The handler receives terminal/async ingest errors
133-
* (connect-budget exhaustion, terminal upgrade failures, write errors)
133+
* (terminal upgrade failures, write errors)
134134
* from across the whole sender pool; notifications are delivered on the
135135
* senders' I/O threads, so the handler must be thread-safe and must not
136136
* block. Pass {@code null} (the default) to keep each sender's

core/src/main/java/io/questdb/client/Sender.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -791,11 +791,12 @@ default Sender uuidColumn(CharSequence name, long lo, long hi) {
791791
* unconnected sender; the I/O thread runs the same retry loop in
792792
* the background. The user thread can call {@code at()} /
793793
* {@code flush()} immediately; rows accumulate in the cursor SF
794-
* engine until the wire is up. A connect-budget exhaustion or a
795-
* terminal upgrade failure is delivered to the async error inbox
796-
* as a {@link io.questdb.client.SenderError} (no synchronous
797-
* throw on the user call site). Wire {@code error_handler=...}
798-
* to observe these.</li>
794+
* engine until the wire is up. Connect failures are retried
795+
* indefinitely in the background; a terminal upgrade failure
796+
* (auth reject, capability mismatch) is delivered to the async
797+
* error inbox as a {@link io.questdb.client.SenderError} (no
798+
* synchronous throw on the user call site). Wire
799+
* {@code error_handler=...} to observe these.</li>
799800
* </ul>
800801
* <p>
801802
* Default resolution when the caller does not pick a value:
@@ -2388,15 +2389,16 @@ public LineSenderBuilder reconnectMaxBackoffMillis(long millis) {
23882389
}
23892390

23902391
/**
2391-
* Per-outage cap on the cursor I/O loop's reconnect retry budget.
2392-
* Once a wire failure occurs, the loop retries with exponential
2393-
* backoff until either reconnect succeeds (timer resets) or this
2394-
* many millis elapse since the first failure of this outage —
2395-
* whichever comes first. On budget exhaustion, the next user
2396-
* thread API call throws.
2392+
* Cap on the blocking initial-connect retry budget when
2393+
* {@code initial_connect_retry=sync}. {@code fromConfig} retries
2394+
* with exponential backoff until connect succeeds or this many
2395+
* millis elapse, then throws. The background reconnect loop
2396+
* (mid-stream outages and async initial connect) does NOT consult
2397+
* this value: it retries indefinitely and halts only on a terminal
2398+
* auth/upgrade error or {@code close()}.
23972399
* <p>
2398-
* Default {@code 300_000} (5 minutes). Lower for fail-fast services;
2399-
* higher for tolerating long maintenance windows. WebSocket only.
2400+
* Default {@code 300_000} (5 minutes). Lower for fail-fast startup;
2401+
* higher for tolerating a slow server boot. WebSocket only.
24002402
*/
24012403
public LineSenderBuilder reconnectMaxDurationMillis(long millis) {
24022404
if (protocol != PARAMETER_NOT_SET_EXPLICITLY && protocol != PROTOCOL_WEBSOCKET) {

core/src/main/java/io/questdb/client/SenderConnectionEvent.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,10 @@ public enum Kind {
223223
/**
224224
* Every endpoint in the configured address list was attempted and none
225225
* accepted the connection in this sweep. The client will back off and
226-
* retry the sweep until the reconnect budget is exhausted. Fired once
227-
* per failed sweep.
226+
* retry the sweep — bounded by {@code reconnect_max_duration_millis}
227+
* during a blocking (sync) initial connect, indefinitely otherwise
228+
* (Invariant B: the background loop never gives up on a wall-clock
229+
* budget). Fired once per failed sweep.
228230
*/
229231
ALL_ENDPOINTS_UNREACHABLE,
230232

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@
3131
* {@code X-QWP-Version} outside the client's supported range. Treated as
3232
* transient at every layer per sf-client.md section 13.3: the per-endpoint
3333
* round walks to the next host (rolling upgrade can leave one node ahead of
34-
* or behind its peers), and a full round of mismatches consumes the per-outage
35-
* reconnect budget. Only after the budget exhausts does the connect loop
36-
* surface a terminal error -- as {@code PROTOCOL_VIOLATION} via the natural
37-
* giveup path, not {@code SECURITY_ERROR}.
34+
* or behind its peers). The background reconnect loop retries a full round
35+
* of mismatches indefinitely (Invariant B: no wall-clock give-up); the
36+
* blocking (sync) initial connect consumes its retry budget and surfaces a
37+
* {@code LineSenderException} from {@code fromConfig} on exhaustion. Never
38+
* classified as {@code SECURITY_ERROR}.
3839
*/
3940
public final class QwpVersionMismatchException extends HttpClientException {
4041
public QwpVersionMismatchException(int serverVersion, int clientMaxVersion) {

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,14 @@ public class QwpWebSocketSender implements Sender {
229229
// point in buildAndConnect.
230230
private boolean hasEverConnected;
231231
// OFF → startup connect failure is immediately terminal (default).
232-
// SYNC → startup connect goes through the same retry-with-backoff
233-
// loop as in-flight reconnect; auth failures still terminal.
232+
// SYNC → startup connect retries with backoff on the user thread,
233+
// bounded by reconnect_max_duration_millis; auth failures
234+
// still terminal.
234235
// ASYNC → user thread does not connect at all. The I/O thread runs
235-
// the same retry loop in the background; terminal failures
236-
// (auth/upgrade reject, budget exhaustion) are delivered
237-
// to the SenderError dispatcher rather than thrown from the
238-
// constructor.
236+
// the reconnect loop in the background, indefinitely
237+
// (Invariant B); terminal failures (auth/upgrade reject)
238+
// are delivered to the SenderError dispatcher rather than
239+
// thrown from the constructor.
239240
private Sender.InitialConnectMode initialConnectMode = Sender.InitialConnectMode.OFF;
240241
private boolean ownsCursorEngine;
241242
private long pendingBytes;
@@ -2316,7 +2317,7 @@ public QwpWebSocketSender uuidColumn(CharSequence columnName, long lo, long hi)
23162317
* True iff this sender has at least once installed a live (connected
23172318
* + upgraded) WebSocket. Sticky — once true, stays true even after a
23182319
* subsequent disconnect. Lets a {@link SenderErrorHandler}
2319-
* disambiguate a "never reached the server" budget exhaustion (likely
2320+
* disambiguate a "never reached the server" terminal failure (likely
23202321
* a config typo or firewall block) from a "lost connection after we
23212322
* were up" failure (likely transient). Returns {@code false} if no
23222323
* I/O loop is running.
@@ -2891,8 +2892,9 @@ private void ensureConnected() {
28912892
// version today). Frames written before the first successful
28922893
// connect commit to V1 because cursor segments are immutable;
28932894
// a future version bump must account for that. Auth/upgrade
2894-
// rejects and budget exhaustion are surfaced via the error
2895-
// inbox by the I/O thread, not thrown here.
2895+
// rejects are surfaced via the error inbox by the I/O
2896+
// thread, not thrown here; plain connect failures retry
2897+
// indefinitely (Invariant B).
28962898
client = null;
28972899
break;
28982900
case OFF:

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import io.questdb.client.cutlass.qwp.client.QwpDurableAckMismatchException;
3131
import io.questdb.client.cutlass.qwp.client.QwpIngressRoleRejectedException;
3232
import io.questdb.client.cutlass.qwp.client.QwpRoleMismatchException;
33+
import io.questdb.client.cutlass.qwp.client.QwpVersionMismatchException;
3334
import org.jetbrains.annotations.TestOnly;
3435
import org.slf4j.Logger;
3536
import org.slf4j.LoggerFactory;
@@ -336,14 +337,29 @@ public WebSocketClient connectWithDurableAckRetry() {
336337
LOG.warn("drainer slot {} attempt {}: durable-ack unavailable, retrying after backoff",
337338
slotPath, capabilityGapAttempts);
338339
} catch (Throwable t) {
340+
if (t instanceof Error) {
341+
// java.lang.Error (OOM, LinkageError, StackOverflowError)
342+
// is a JVM/programming failure, not a transport outage:
343+
// retrying cannot clear it, and spinning here would pin
344+
// the slot .lock forever with no .failed sentinel and only
345+
// a throttled, possibly-null-message WARN as a trace.
346+
// Rethrow: run()'s outer catch quarantines the slot
347+
// (markFailed + FAILED) and its finally releases the lock
348+
// -- quarantine-and-exit, exactly as genuine terminals do.
349+
throw (Error) t;
350+
}
339351
// INVARIANT B: a transport failure -- the whole cluster is
340352
// unreachable right now (server down, network partition) -- is
341353
// TRANSIENT, exactly as the live sender's background loop treats
342354
// it. The server will come back; keep retrying (capped backoff)
343355
// until it does, stopRequested, or SF exhaustion. NEVER quarantine
344356
// the slot on a transport error. Genuine terminals (auth /
345357
// non-421 upgrade / durable-ack capability gap) are handled by the
346-
// catches above and still fail fast.
358+
// catches above and still fail fast. A QWP version mismatch also
359+
// reaches here (it extends HttpClientException, not
360+
// WebSocketUpgradeException) and is intentionally retried under
361+
// Invariant B -- but it is NOT a transport outage, so log it
362+
// truthfully below rather than mislabelling it "cluster unreachable".
347363
lastErrorMessage = t.getMessage();
348364
// Pause the episode wall clock: the gap-to-gap interval this
349365
// window interrupts is never charged. Attempts and elapsed
@@ -352,8 +368,24 @@ public WebSocketClient connectWithDurableAckRetry() {
352368
lastCapabilityGapNanos = 0L;
353369
long nowWarn = System.nanoTime();
354370
if (nowWarn - lastTransportWarnNanos >= 5_000_000_000L) {
355-
LOG.warn("drainer slot {}: cluster unreachable ({}), retrying after backoff",
356-
slotPath, t.getMessage());
371+
if (t instanceof QwpVersionMismatchException) {
372+
// The cluster IS reachable: every endpoint completed the
373+
// WebSocket upgrade but advertised a QWP protocol version
374+
// this client cannot speak. A rolling upgrade clears this
375+
// once peers converge, so Invariant B keeps retrying -- but
376+
// if it persists the client binary is version-incompatible
377+
// with the whole cluster and an operator must intervene
378+
// (upgrade the client or the servers). Name the real
379+
// condition so it is diagnosable, not hidden behind a
380+
// network-outage message.
381+
LOG.warn("drainer slot {}: every reachable endpoint advertises an unsupported "
382+
+ "QWP protocol version ({}); retrying (rolling-upgrade window) -- "
383+
+ "if this persists the client is version-incompatible with the cluster",
384+
slotPath, t.getMessage());
385+
} else {
386+
LOG.warn("drainer slot {}: cluster unreachable ({}), retrying after backoff",
387+
slotPath, t.getMessage());
388+
}
357389
lastTransportWarnNanos = nowWarn;
358390
}
359391
}

0 commit comments

Comments
 (0)