Skip to content

Commit 0955443

Browse files
committed
fix(qwp): pace zero-progress strike-exempt recycles
Three recycle paths are strike-exempt by design -- orderly closes (NORMAL_CLOSURE/GOING_AWAY), non-orderly closes before any send on the connection, and pre-send RETRIABLE_OTHER (NOT_WRITABLE) rejections: none of them are a verdict on the bytes. But strike-exempt also meant pace-exempt: failPaced keys its dose on poisonStrikes, and connectLoop's backoff engages only on FAILED connect attempts, while a peer that completes TCP+TLS+upgrade before closing makes every attempt "succeed". With Invariant B having (correctly) removed the wall-clock reconnect budget, nothing bounded these recycles at all: an LB drain window, a GOING_AWAY rolling restart, or a health-checking middlebox in front of a dead backend drove full recycles -- fresh WebSocketClient + SSLContext + trust-store read -- at handshake RTT rate, forever. Measured red: ~100k reconnects in 1.2s. Fix: failExemptPaced, a zero-progress recycle pacer. It counts consecutive exempt recycles with no acceptance progress in between (max(ackedFsn, highestOkFsn) -- monotonic, so replay re-OKs cannot launder it) and parks with the same doubling, capped dose failPaced uses. The FIRST recycle after any progress stays immediate, preserving failover latency for genuine role-change/drain handoffs. Pacing only: the counter never escalates to a terminal (Invariant B). Tests: orderly-close churn and pre-send close churn (both red at ~85k recycles/sec unpaced, now bounded by the backoff), plus a recycle-level contract test pinning first-immediate, consecutive-paced, and reset-on-progress.
1 parent d52a9da commit 0955443

2 files changed

Lines changed: 365 additions & 4 deletions

File tree

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

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,24 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
319319
// STATUS_DURABLE_ACK coverage. I/O thread only; survives reconnects. Used
320320
// by the poison-frame detector to measure genuine acceptance progress.
321321
private long highestOkFsn = -1L;
322+
// Zero-progress recycle pacer (I/O thread only; survives reconnects).
323+
// Counts consecutive strike-EXEMPT recycles -- orderly closes
324+
// (NORMAL_CLOSURE/GOING_AWAY), non-orderly closes before any send on the
325+
// connection, and pre-send RETRIABLE_OTHER rejections -- with no
326+
// acceptance progress in between. These paths deliberately carry no
327+
// poison strike (they are not a verdict on the bytes), which also exempts
328+
// them from failPaced's strike-keyed dose; and a peer that completes
329+
// TCP+TLS+upgrade before closing makes every connect attempt "succeed",
330+
// so connectLoop's failed-connect backoff never engages either. Without
331+
// this counter such recycles churn at handshake RTT rate with no bound
332+
// (Invariant B removed the wall-clock cap), each cycle burning a fresh
333+
// WebSocketClient + SSLContext/trust-store read. Progress is measured as
334+
// max(ackedFsn, highestOkFsn): both watermarks are monotonic and advance
335+
// only on genuine new acceptance (replayed re-OKs of already-OK'd frames
336+
// advance neither), so replay cannot launder the counter. Pacing only --
337+
// this counter NEVER escalates to a terminal (Invariant B).
338+
private int zeroProgressRecycles;
339+
private long progressAtLastExemptRecycle = Long.MIN_VALUE;
322340
// Poison-frame detector threshold for this loop. Constructor-configured
323341
// (connect-string key max_frame_rejections); defaults to
324342
// DEFAULT_MAX_HEAD_FRAME_REJECTIONS.
@@ -1507,6 +1525,50 @@ private void failPaced(Throwable initial) {
15071525
connectLoop(initial, "reconnect", dose);
15081526
}
15091527

1528+
/**
1529+
* Recycle path for strike-exempt wire events: orderly closes
1530+
* (NORMAL_CLOSURE / GOING_AWAY), non-orderly closes before any send on
1531+
* the connection, and pre-send RETRIABLE_OTHER rejections. None of these
1532+
* implicate the head frame, so they carry no poison strike -- but that
1533+
* also means neither existing pacer can bound them: {@link #failPaced}
1534+
* keys its dose on poisonStrikes, and connectLoop's backoff engages only
1535+
* on FAILED connect attempts, while a peer that completes the upgrade
1536+
* before closing makes every attempt succeed. A load balancer draining
1537+
* with GOING_AWAY, a health-checking middlebox that upgrades then drops,
1538+
* or an all-replica window answering NOT_WRITABLE pre-send would
1539+
* otherwise drive full recycles -- fresh WebSocketClient, SSLContext,
1540+
* trust-store read -- at handshake RTT rate, forever (Invariant B
1541+
* deliberately removed the wall-clock cap).
1542+
* <p>
1543+
* The first recycle after any acceptance progress stays IMMEDIATE: a
1544+
* one-off orderly handoff must rotate endpoints without delay (failover
1545+
* latency). Consecutive recycles with no OK-level progress in between
1546+
* park for the same doubling, capped dose failPaced applies. Pacing
1547+
* only, never a terminal: unlike the poison detector these events are
1548+
* not a verdict on the bytes, so under Invariant B they retry forever --
1549+
* just not at wire speed.
1550+
*/
1551+
private void failExemptPaced(Throwable cause) {
1552+
long progress = Math.max(engine.ackedFsn(), highestOkFsn);
1553+
if (progress > progressAtLastExemptRecycle) {
1554+
zeroProgressRecycles = 0;
1555+
}
1556+
progressAtLastExemptRecycle = progress;
1557+
int level = zeroProgressRecycles++;
1558+
if (level == 0) {
1559+
fail(cause);
1560+
return;
1561+
}
1562+
long dose = reconnectInitialBackoffMillis;
1563+
if (dose > 0) {
1564+
dose <<= Math.min(level - 1, 6);
1565+
if (reconnectMaxBackoffMillis > 0 && dose > reconnectMaxBackoffMillis) {
1566+
dose = reconnectMaxBackoffMillis;
1567+
}
1568+
}
1569+
connectLoop(cause, "reconnect", dose);
1570+
}
1571+
15101572
/**
15111573
* True when {@link #terminalError} is set AND no synchronous user-thread
15121574
* caller has yet seen that same instance via {@link #checkError()}.
@@ -2134,9 +2196,17 @@ public void onClose(int code, String reason) {
21342196
return;
21352197
}
21362198
// No strike (orderly close, or a close before any send on this
2137-
// connection): a genuine transport event -- reconnect immediately,
2138-
// with backoff only on failed connect attempts.
2139-
fail(cause);
2199+
// connection): not a verdict on the bytes. But strike-exempt must
2200+
// not mean pace-exempt: a peer that completes the upgrade then
2201+
// closes (LB drain window, GOING_AWAY rolling restart, health-
2202+
// checking middlebox in front of a dead backend) succeeds at
2203+
// connect every cycle, so connectLoop's failed-connect backoff
2204+
// never engages, and with no strikes there is no poison-terminal
2205+
// bound either -- an unpaced fail() here churns full recycles at
2206+
// handshake RTT rate, forever. failExemptPaced keeps the first
2207+
// recycle immediate (failover latency) and paces consecutive
2208+
// zero-progress repeats with the escalating reconnect backoff.
2209+
failExemptPaced(cause);
21402210
}
21412211

21422212
private void handlePreSendRejection(long wireSeq, byte status,
@@ -2188,7 +2258,15 @@ private void handlePreSendRejection(long wireSeq, byte status,
21882258
if (policy == SenderError.Policy.RETRIABLE) {
21892259
failPaced(recycleCause);
21902260
} else {
2191-
fail(recycleCause);
2261+
// RETRIABLE_OTHER pre-send (NOT_WRITABLE before we sent
2262+
// anything): rotate endpoints -- but through the zero-
2263+
// progress pacer, not raw fail(). An all-replica window
2264+
// answers NOT_WRITABLE on every fresh connection, each
2265+
// connect attempt "succeeds", and pre-send rejections record
2266+
// no strike, so nothing else bounds the churn. The first
2267+
// recycle stays immediate so a genuine failover rotates
2268+
// without delay.
2269+
failExemptPaced(recycleCause);
21922270
}
21932271
}
21942272

0 commit comments

Comments
 (0)