Skip to content

Commit 8302335

Browse files
committed
feat(qwp): NACK policy v2 -- no drop, no lists, no dead senders
Replaces the two-policy NACK model (DROP_AND_CONTINUE / HALT) with three policies that never silently lose data and never kill the sender on a transient condition: - RETRIABLE (WRITE_ERROR, INTERNAL_ERROR, UNKNOWN fail-open): recycle the connection and replay from ackedFsn+1 through the existing wire-failure reconnect machinery. No watermark advance, nothing dropped. - RETRIABLE_OTHER (NOT_WRITABLE, reserved wire byte 0x0C): same, biased to endpoint rotation. Read-only/demotion today arrives as a role-change NORMAL_CLOSURE close, so the byte is reserved for future servers. - TERMINAL (SCHEMA_MISMATCH, PARSE_ERROR, SECURITY_ERROR on a writable node, PROTOCOL_VIOLATION): deterministic under byte-identical replay; latch loudly, bytes preserved in the SF log. WS close codes carry no policy semantics anymore: every close is reconnect-eligible. A frame that deterministically kills the connection is caught behaviorally by the poison-frame detector (MAX_HEAD_FRAME_REJECTIONS consecutive rejections of the same head FSN with no ack progress escalate to a typed PROTOCOL_VIOLATION terminal) instead of a close-code list. Source-breaking: SenderError.Policy renames; ackedFsn now advances only on server OKs. Design doc: design/qwp-nack-policy-v2.md.
1 parent 9b33ae6 commit 8302335

27 files changed

Lines changed: 677 additions & 348 deletions

.claude/skills/review-pr/SKILL.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,20 @@ loses data and never hard-fails on a transient outage.
358358
transport error) must never increment it or burn its wall clock — a
359359
transient state consuming the terminal budget (shared attempt counter,
360360
entry-anchored deadline) IS a Critical violation of this checklist.
361+
- **Mid-stream server NACKs (no drop policy).** The NACK policy must mirror
362+
the connect-time tiering. A rejection category that a transient cluster
363+
state can produce (`WRITE_ERROR`, `INTERNAL_ERROR`, `UNKNOWN` — and any
364+
future status byte) is RETRIABLE: recycle the wire and replay from
365+
`ackedFsn+1`. It must NEVER drop the batch and NEVER latch a terminal /
366+
quarantine a slot on first sight. Only rejections deterministic under
367+
byte-identical replay (`SCHEMA_MISMATCH`, `PARSE_ERROR`, `SECURITY_ERROR`
368+
on a writable node) may go TERMINAL. A client that advances the ack
369+
watermark past a NACKed frame is silently losing data — Critical. A frame
370+
repeatedly rejected with no ack progress must escalate through the
371+
poison-frame detector (bounded consecutive strikes at the same head FSN),
372+
not through a WS close-code list — close codes carry no policy semantics.
373+
`UNKNOWN` must fail OPEN (retry), never closed (terminal): a status byte
374+
from a newer server must degrade to retry, not to a dead sender.
361375

362376
**Pool startup — two modes; the mode decides who sees connectivity errors.**
363377
- `lazy_connect=true`: `build()` MUST succeed with **no server present**. The

.pi/skills/review-pr/SKILL.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,20 @@ loses data and never hard-fails on a transient outage.
369369
transport error) must never increment it or burn its wall clock — a
370370
transient state consuming the terminal budget (shared attempt counter,
371371
entry-anchored deadline) IS a Critical violation of this checklist.
372+
- **Mid-stream server NACKs (no drop policy).** The NACK policy must mirror
373+
the connect-time tiering. A rejection category that a transient cluster
374+
state can produce (`WRITE_ERROR`, `INTERNAL_ERROR`, `UNKNOWN` — and any
375+
future status byte) is RETRIABLE: recycle the wire and replay from
376+
`ackedFsn+1`. It must NEVER drop the batch and NEVER latch a terminal /
377+
quarantine a slot on first sight. Only rejections deterministic under
378+
byte-identical replay (`SCHEMA_MISMATCH`, `PARSE_ERROR`, `SECURITY_ERROR`
379+
on a writable node) may go TERMINAL. A client that advances the ack
380+
watermark past a NACKed frame is silently losing data — Critical. A frame
381+
repeatedly rejected with no ack progress must escalate through the
382+
poison-frame detector (bounded consecutive strikes at the same head FSN),
383+
not through a WS close-code list — close codes carry no policy semantics.
384+
`UNKNOWN` must fail OPEN (retry), never closed (terminal): a status byte
385+
from a newer server must degrade to retry, not to a dead sender.
372386

373387
**Pool startup — two modes; the mode decides who sees connectivity errors.**
374388
- `lazy_connect=true`: `build()` MUST succeed with **no server present**. The

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
/**
3131
* Thrown from a producer-thread API call (typically {@link Sender#flush()}) when the
3232
* asynchronous SF send loop has latched a server-side rejection with policy
33-
* {@link SenderError.Policy#HALT}.
33+
* {@link SenderError.Policy#TERMINAL}.
3434
*
3535
* <p>The wrapped {@link SenderError} carries the rejection details — category, status byte,
3636
* server message, FSN span, and (best-effort) table name. Use {@link #getServerError()} to

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,8 +601,7 @@ default Sender geoHashColumn(CharSequence name, CharSequence value) {
601601
}
602602

603603
/**
604-
* Highest frame sequence number (FSN) the server has acknowledged, or that the sender
605-
* has skipped past on a {@link SenderError.Policy#DROP_AND_CONTINUE} rejection.
604+
* Highest frame sequence number (FSN) the server has acknowledged.
606605
* Returns {@code -1} when no batch has been published yet, and on transports that
607606
* do not track FSNs (HTTP, TCP, UDP).
608607
* <br>

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

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
* <ul>
3535
* <li>Asynchronously via {@link SenderErrorHandler} registered on the builder.</li>
3636
* <li>Synchronously as the payload of a {@link LineSenderServerException} thrown
37-
* from the next producer-thread API call after a {@link Policy#HALT} error has
37+
* from the next producer-thread API call after a {@link Policy#TERMINAL} error has
3838
* been latched.</li>
3939
* </ul>
4040
*
@@ -87,9 +87,10 @@ public SenderError(
8787
}
8888

8989
/**
90-
* @return the policy the I/O loop actually applied — DROP_AND_CONTINUE means the data
91-
* was dropped; HALT means a {@link LineSenderServerException} will be thrown on the next
92-
* producer-thread API call.
90+
* @return the policy the I/O loop actually applied — RETRIABLE / RETRIABLE_OTHER means
91+
* the batch stays in the store-and-forward log and is replayed after a reconnect (no data
92+
* loss, informational delivery); TERMINAL means a {@link LineSenderServerException} will be
93+
* thrown on the next producer-thread API call.
9394
*/
9495
public @NotNull Policy getAppliedPolicy() {
9596
return appliedPolicy;
@@ -195,7 +196,17 @@ public enum Category {
195196
*/
196197
WRITE_ERROR,
197198
/**
198-
* WebSocket-layer close frame with a terminal code (PROTOCOL_ERROR, UNSUPPORTED_DATA, MESSAGE_TOO_BIG).
199+
* Node cannot serve writes at all right now (read-only replica, primary demoting).
200+
* Wire {@code 0x0C} — reserved: current servers signal this state with a
201+
* reconnect-eligible close instead of a mid-stream NACK, so this category is
202+
* mapped for forward compatibility with servers that NACK it explicitly.
203+
*/
204+
NOT_WRITABLE,
205+
/**
206+
* A frame the server (or an intermediary) deterministically rejects: the
207+
* poison-frame detector observed the same head-of-line frame fail
208+
* {@link io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop#MAX_HEAD_FRAME_REJECTIONS}
209+
* consecutive times with no ack progress — replaying it cannot succeed.
199210
*/
200211
PROTOCOL_VIOLATION,
201212
/**
@@ -210,21 +221,37 @@ public enum Category {
210221
* connect-string per-category {@code on_*_error} → connect-string global {@code on_server_error}
211222
* → spec defaults.
212223
*
213-
* <p>{@link Category#PROTOCOL_VIOLATION} and {@link Category#UNKNOWN} are forced {@link #HALT};
214-
* user overrides for those categories are ignored.
224+
* <p>There is no drop policy by design: the client never silently discards data. A rejected
225+
* batch is either replayed ({@link #RETRIABLE} / {@link #RETRIABLE_OTHER}) or halts the
226+
* sender loudly with the bytes preserved on disk ({@link #TERMINAL}).
227+
*
228+
* <p>{@link Category#PROTOCOL_VIOLATION} is forced {@link #TERMINAL} and
229+
* {@link Category#UNKNOWN} is forced {@link #RETRIABLE} (fail open: a status byte from a
230+
* newer server must degrade to retry, not to a dead sender); user overrides for those
231+
* categories are ignored.
215232
*/
216233
public enum Policy {
217234
/**
218-
* Drop the rejected batch from the SF disk store (advance ackedFsn past it) and continue
219-
* draining subsequent batches. The data is lost from the sender's perspective; the user
220-
* must dead-letter via {@link SenderErrorHandler} if a record is needed.
235+
* Recycle the connection and replay from the store-and-forward log: reconnect with
236+
* capped exponential backoff and reposition at {@code ackedFsn + 1}. No data is
237+
* dropped and the producer keeps writing; delivery through {@link SenderErrorHandler}
238+
* is informational. A frame that keeps being rejected with no ack progress escalates
239+
* to {@link #TERMINAL} via the poison-frame detector.
240+
*/
241+
RETRIABLE,
242+
/**
243+
* Same replay semantics as {@link #RETRIABLE}, but the rejection says this node cannot
244+
* serve writes at all (read-only replica / demoting primary), so the reconnect rotates
245+
* to the next configured endpoint rather than waiting out a backoff against the same
246+
* node.
221247
*/
222-
DROP_AND_CONTINUE,
248+
RETRIABLE_OTHER,
223249
/**
224250
* Latch the error as terminal. The next producer-thread API call (e.g. {@link Sender#flush()})
225251
* throws {@link LineSenderServerException}. The sender does not drain further until the
226-
* caller closes and rebuilds it.
252+
* caller closes and rebuilds it. The rejected bytes remain in the store-and-forward log
253+
* on disk — nothing is silently discarded.
227254
*/
228-
HALT
255+
TERMINAL
229256
}
230257
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
* <h2>What this callback is for</h2>
4545
* Dead-lettering rejected data, alerting, metrics. Producer-thread retry/abort logic
4646
* should not live here — that belongs in the {@code catch (LineSenderServerException)}
47-
* block on the producer thread, which fires after a {@link SenderError.Policy#HALT}
47+
* block on the producer thread, which fires after a {@link SenderError.Policy#TERMINAL}
4848
* latch on the next API call.
4949
*
5050
* @see SenderError

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

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,15 @@
3030
* on {@code QwpWebSocketSender} via {@code setProgressHandler(...)} or on the
3131
* builder via {@code LineSenderBuilder.progressHandler(...)}.
3232
*
33-
* <h2>WARNING -- settled is not durable</h2>
34-
* {@code ackedFsn} is a <em>settled</em> watermark, not a <em>durable</em> one.
35-
* A {@code DROP_AND_CONTINUE} server rejection (the default policy for the
36-
* {@code SCHEMA_MISMATCH} and {@code WRITE_ERROR} categories) advances
37-
* {@code ackedFsn} past the dropped FSN range exactly as a successful OK
38-
* does -- the loop cannot leave a dropped FSN unsettled without leaking
39-
* storage and stalling the wire. This handler therefore CANNOT distinguish
40-
* a batch that the server committed to the WAL from one that the server
41-
* discarded.
33+
* <h2>Watermark advances only on server acceptance</h2>
34+
* {@code ackedFsn} advances exclusively on server OK frames. A server
35+
* rejection NEVER advances the watermark: retriable rejections recycle the
36+
* connection and replay the batch, terminal rejections halt the sender with
37+
* the bytes preserved in the store-and-forward log. There is no drop policy.
4238
*
43-
* <p>Code that gates a downstream side effect on {@code onAcked} without
44-
* also tracking {@link SenderErrorHandler} drops will treat dropped batches
45-
* as durable. The result is silent data loss: rows discarded by the server
46-
* are marked "saved" by the user's outbox, locks released, source records
47-
* deleted, downstream confirmations emitted -- for data that no longer
48-
* exists on the server.
49-
*
50-
* <p>Required pattern when durability matters: register a
51-
* {@link SenderErrorHandler}, record the {@code [fromFsn, toFsn]} range of
52-
* every error whose {@code getAppliedPolicy()} is
53-
* {@link SenderError.Policy#DROP_AND_CONTINUE}, and exclude those FSNs from
54-
* the "durable" set you derive from the watermark.
39+
* <p>Note that in non-durable-ack mode an OK acknowledges server-side commit,
40+
* not object-store durability; opt in to durable acks when replication-grade
41+
* durability gates downstream side effects.
5542
*
5643
* <h2>Watermark semantics</h2>
5744
* The handler fires only when the watermark <em>advances</em>:
@@ -93,10 +80,9 @@
9380
public interface SenderProgressHandler {
9481
/**
9582
* Called when the settled watermark advances. Strictly monotonic:
96-
* {@code ackedFsn} on call N+1 is greater than on call N. "Settled" covers
97-
* both successful OK frames and {@code DROP_AND_CONTINUE} rejections --
98-
* see the class javadoc WARNING before treating this as a durability
99-
* signal.
83+
* {@code ackedFsn} on call N+1 is greater than on call N. The watermark
84+
* advances only on server OK frames -- rejections never advance it (see
85+
* the class javadoc for durable-ack considerations).
10086
*/
10187
void onAcked(long ackedFsn);
10288
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,9 +1036,9 @@ public void close() {
10361036
// user-installed custom handler
10371037
// (hasDeliveredTerminalToCustomHandler, checked here).
10381038
// The test is terminal-specific on purpose: an earlier
1039-
// routine DROP_AND_CONTINUE rejection delivered to the
1040-
// handler must NOT suppress a later genuine HALT
1041-
// terminal (the "any error ever" flag did, silently
1039+
// routine RETRIABLE rejection delivered to the
1040+
// handler must NOT suppress a later genuine TERMINAL
1041+
// error (the "any error ever" flag did, silently
10421042
// losing it). It also stays false when the terminal
10431043
// reached only the default handler after a
10441044
// setErrorHandler(null) revert, or is still
@@ -1585,8 +1585,8 @@ public QwpWebSocketSender geoHashColumn(CharSequence columnName, CharSequence va
15851585
}
15861586

15871587
/**
1588-
* Highest FSN that has been server-acknowledged (or skipped past on a
1589-
* {@link SenderError.Policy#DROP_AND_CONTINUE} rejection). {@code -1} if
1588+
* Highest FSN that has been server-acknowledged. Rejections never advance
1589+
* the watermark. {@code -1} if
15901590
* the I/O loop has not yet started or no batch has been published.
15911591
* <p>
15921592
* Snapshot accessor — for a bounded wait, use
@@ -1816,7 +1816,7 @@ public long getTotalReconnectsSucceeded() {
18161816
}
18171817

18181818
/**
1819-
* Total errors observed by the I/O loop (DROP and HALT combined).
1819+
* Total errors observed by the I/O loop (retriable and terminal combined).
18201820
*/
18211821
public long getTotalServerErrors() {
18221822
CursorWebSocketSendLoop l = cursorSendLoop;
@@ -2649,7 +2649,7 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
26492649
// fabricate lifecycle transitions the foreground never had, steal the
26502650
// once-per-lifetime CONNECTED classification, and re-size the
26512651
// producer's batch guard for a connection the producer is not on
2652-
// (oversize batch -> ws-close[1009] -> producer-terminal HALT caused
2652+
// (oversize batch -> ws-close[1009] -> poison-frame escalation caused
26532653
// by background activity).
26542654
final boolean background = ctx.isBackground();
26552655
// Private full-sweep cursor for background walks: claim-at-pick over

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ public class WebSocketResponse {
7575
*/
7676
public static final byte STATUS_DURABLE_ACK = 0x02;
7777
public static final byte STATUS_INTERNAL_ERROR = 0x06;
78+
/**
79+
* Node cannot serve writes (read-only replica / demoting primary). Reserved:
80+
* current servers signal this with a reconnect-eligible close instead of a
81+
* NACK; mapped so a future server that NACKs it mid-stream gets
82+
* retriable-with-rotation treatment instead of falling into UNKNOWN.
83+
*/
84+
public static final byte STATUS_NOT_WRITABLE = 0x0C;
7885
// Status codes (must match https://questdb.com/docs/connect/wire-protocols/qwp-ingress-websocket/)
7986
public static final byte STATUS_OK = 0x00;
8087
public static final byte STATUS_PARSE_ERROR = 0x05;
@@ -215,6 +222,8 @@ public String getStatusName() {
215222
return "SECURITY_ERROR";
216223
case STATUS_INTERNAL_ERROR:
217224
return "INTERNAL_ERROR";
225+
case STATUS_NOT_WRITABLE:
226+
return "NOT_WRITABLE";
218227
default:
219228
return "UNKNOWN(" + (status & 0xFF) + ")";
220229
}

0 commit comments

Comments
 (0)