Skip to content

Commit 9f3cb32

Browse files
committed
Add ROLE_CHANGE (4001) close code for the role-change handoff
The server's role-change CLOSE now carries the private-use code 4001 (RFC 6455 s7.4.2) instead of NORMAL_CLOSURE, so the client's verbatim CLOSE echo proves it actually read the server's CLOSE frame. Classify ROLE_CHANGE as orderly in the send loop (strike-exempt, paced recycle, reconnect-eligible), matching the treatment the handoff got as 1000. Pin the classification with a poison-frame test: repeated demote closes at an unadvanced head FSN must never latch a PROTOCOL_VIOLATION terminal.
1 parent 80e69c9 commit 9f3cb32

4 files changed

Lines changed: 75 additions & 10 deletions

File tree

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
321321
private long highestOkFsn = -1L;
322322
// Zero-progress recycle pacer (I/O thread only; survives reconnects).
323323
// Counts consecutive strike-EXEMPT recycles -- orderly closes
324-
// (NORMAL_CLOSURE/GOING_AWAY), non-orderly closes before any send on the
324+
// (NORMAL_CLOSURE/GOING_AWAY/ROLE_CHANGE), non-orderly closes before any send on the
325325
// connection, and pre-send RETRIABLE_OTHER rejections -- with no
326326
// acceptance progress in between. These paths deliberately carry no
327327
// poison strike (they are not a verdict on the bytes), which also exempts
@@ -1527,7 +1527,7 @@ private void failPaced(Throwable initial) {
15271527

15281528
/**
15291529
* Recycle path for strike-exempt wire events: orderly closes
1530-
* (NORMAL_CLOSURE / GOING_AWAY), non-orderly closes before any send on
1530+
* (NORMAL_CLOSURE / GOING_AWAY / ROLE_CHANGE), non-orderly closes before any send on
15311531
* the connection, and RETRIABLE_OTHER rejections (pre- and post-send:
15321532
* NOT_WRITABLE is a node-state verdict, not a frame verdict). None of
15331533
* these implicate the head frame, so they carry no poison strike -- but that
@@ -2172,11 +2172,12 @@ public void onClose(int code, String reason) {
21722172
// after this connection already sent the head frame counts a poison
21732173
// strike; maxHeadFrameRejections consecutive strikes at the same
21742174
// head FSN with no ack progress escalate to a typed terminal. Orderly
2175-
// closes (NORMAL_CLOSURE role-change handoff, GOING_AWAY restart
2176-
// drain) never count strikes — they are the server asking us to go
2177-
// elsewhere, not a verdict on the bytes.
2175+
// closes (ROLE_CHANGE role-change handoff, NORMAL_CLOSURE, GOING_AWAY
2176+
// restart drain) never count strikes — they are the server asking us
2177+
// to go elsewhere, not a verdict on the bytes.
21782178
boolean orderly = code == WebSocketCloseCode.NORMAL_CLOSURE
2179-
|| code == WebSocketCloseCode.GOING_AWAY;
2179+
|| code == WebSocketCloseCode.GOING_AWAY
2180+
|| code == WebSocketCloseCode.ROLE_CHANGE;
21802181
LineSenderException cause = new LineSenderException(
21812182
"WebSocket closed by server: code=" + code + " reason=" + reason);
21822183
if (!orderly && nextWireSeq > 0) {

core/src/main/java/io/questdb/client/cutlass/qwp/websocket/WebSocketCloseCode.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,16 @@ public final class WebSocketCloseCode {
8484
* Reserved for future use.
8585
*/
8686
public static final int RESERVED = 1004;
87+
/**
88+
* Role-change close (4001). QWP application-defined code in the RFC 6455
89+
* Section 7.4.2 private-use range: the server closed because its role
90+
* changed (primary demoted); reconnect-eligible, not a verdict on the
91+
* bytes. Deliberately distinct from {@link #NORMAL_CLOSURE} so the
92+
* client's verbatim CLOSE echo proves to the server that the client
93+
* received the server's CLOSE (and, by TCP ordering, everything before
94+
* it -- the final durable ack included).
95+
*/
96+
public static final int ROLE_CHANGE = 4001;
8797
/**
8898
* TLS handshake (1015).
8999
* Reserved value. MUST NOT be sent in a Close frame.
@@ -134,6 +144,8 @@ public static String describe(int code) {
134144
return "Internal Error";
135145
case TLS_HANDSHAKE:
136146
return "TLS Handshake";
147+
case ROLE_CHANGE:
148+
return "Role Change (QWP)";
137149
default:
138150
if (code >= 3000 && code < 4000) {
139151
return "Library/Framework Code (" + code + ")";

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoopPoisonFrameTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import io.questdb.client.cutlass.qwp.client.WebSocketResponse;
3333
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
3434
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop;
35+
import io.questdb.client.cutlass.qwp.websocket.WebSocketCloseCode;
3536
import io.questdb.client.network.PlainSocketFactory;
3637
import io.questdb.client.std.Files;
3738
import io.questdb.client.std.MemoryTag;
@@ -244,6 +245,43 @@ public void testNonOrderlyClosePoisonKeysOnOkLevelHeadOfLine() throws Exception
244245
});
245246
}
246247

248+
@Test
249+
public void testRoleChangeCloseIsOrderlyNeverCountsStrikes() throws Exception {
250+
// The server's role-change close (ROLE_CHANGE, 4001) is the demote
251+
// handoff: "go elsewhere", not a verdict on the bytes. Like
252+
// NORMAL_CLOSURE and GOING_AWAY it must never count a poison strike.
253+
// Dropping ROLE_CHANGE from the orderly set would pass the rest of
254+
// this suite while latching a PROTOCOL_VIOLATION terminal after
255+
// maxHeadFrameRejections demote closes at an unadvanced head FSN --
256+
// turning a routine failover into a false data-poison verdict.
257+
// Mirrors testNonOrderlyClosePoisonKeysOnOkLevelHeadOfLine with the
258+
// opposite expectation.
259+
TestUtils.assertMemoryLeak(() -> {
260+
List<WebSocketClient> clients = new ArrayList<>();
261+
try (CursorSendEngine engine = newEngine()) {
262+
appendFrames(engine, 2);
263+
CursorWebSocketSendLoop loop = newDurableLoop(engine, clients);
264+
setSentCount(loop, 2);
265+
266+
deliverOk(loop, 0, names("trades"), txns(7L));
267+
for (int i = 0; i < MAX_REJECTIONS + 1; i++) {
268+
// Role-change close after at least one send on this
269+
// connection: strike-exempt, so no number of repeats may
270+
// escalate. Restore the sent count after each recycle,
271+
// exactly like the non-orderly variant.
272+
setSentCount(loop, 2);
273+
deliverRoleChangeClose(loop);
274+
}
275+
276+
// Must NOT throw: orderly closes never escalate to a typed
277+
// terminal, however many accumulate at the same head FSN.
278+
loop.checkError();
279+
} finally {
280+
closeAll(clients);
281+
}
282+
});
283+
}
284+
247285
@Test
248286
public void testNackRecycleIsPacedAgainstHealthyServer() throws Exception {
249287
// A reachable, healthy server that NACKs the head frame (RETRIABLE)
@@ -907,6 +945,16 @@ private static void deliverOrderlyClose(CursorWebSocketSendLoop loop) throws Exc
907945
m.invoke(handler, 1001, "server draining"); // GOING_AWAY: orderly, strike-exempt
908946
}
909947

948+
private static void deliverRoleChangeClose(CursorWebSocketSendLoop loop) throws Exception {
949+
Field f = CursorWebSocketSendLoop.class.getDeclaredField("responseHandler");
950+
f.setAccessible(true);
951+
Object handler = f.get(loop);
952+
Method m = handler.getClass().getDeclaredMethod("onClose", int.class, String.class);
953+
m.setAccessible(true);
954+
// ROLE_CHANGE (4001): the demote handoff, orderly, strike-exempt
955+
m.invoke(handler, WebSocketCloseCode.ROLE_CHANGE, "role change: primary demoted");
956+
}
957+
910958
private static void deliverNonOrderlyClose(CursorWebSocketSendLoop loop) throws Exception {
911959
Field f = CursorWebSocketSendLoop.class.getDeclaredField("responseHandler");
912960
f.setAccessible(true);

design/qwp-nack-policy-v2.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ is caught *behaviorally*:
6666
> in durable-ack mode the trim watermark advances only on durable coverage,
6767
> so every post-NACK recycle replays from the durable watermark and re-OKs
6868
> frames *behind* the suspect — those re-OKs say nothing about the poisoned
69-
> bytes and must not launder the count. Orderly closes (`NORMAL_CLOSURE`
70-
> role-change handoff, `GOING_AWAY` restart drain) never count strikes.
69+
> bytes and must not launder the count. Orderly closes (`ROLE_CHANGE` (4001)
70+
> role-change handoff, `NORMAL_CLOSURE`, `GOING_AWAY` restart drain) never
71+
> count strikes.
7172
7273
Below the escalation threshold, a RETRIABLE NACK's recycle is **paced**: the
7374
server is reachable (it just answered), so the reconnect succeeds immediately
@@ -99,8 +100,11 @@ diagnostics only.
99100

100101
The server already handles it at the right layer (Invariant B work): the
101102
read-only gate and the commit-path authorization refusal both set
102-
`roleChangeClosePending` and close with a reconnect-eligible `NORMAL_CLOSURE`
103-
instead of NACKing `SECURITY_ERROR` (`QwpIngressProcessorState`). The client
103+
`roleChangeClosePending` and close with a reconnect-eligible `ROLE_CHANGE`
104+
(4001, private-use range; distinct from `NORMAL_CLOSURE` so the client's
105+
verbatim CLOSE echo is distinguishable from a voluntary client CLOSE that
106+
crossed the server's CLOSE on the wire) instead of NACKing `SECURITY_ERROR`
107+
(`QwpIngressProcessorState`). The client
104108
reconnects, hits the 421 role reject on the now-replica, and retries from SF
105109
until a primary is reachable. Consequently:
106110

0 commit comments

Comments
 (0)