Skip to content

Commit 51bc8af

Browse files
committed
fix(client): role-reject evidence outranks latched non-421 upgrade error in mixed connect sweep
In a mixed sweep -- e.g. [replica(421+role), replica(421+role), node(503)] -- the exhausted-round epilogue threw the latched non-421 WebSocketUpgradeException before checking lastRoleReject, misclassifying a transient failover/promotion window as terminal: dead foreground sender, or a drainer slot quarantine via BackgroundDrainer markFailed. Demote a plain non-421 WebSocketUpgradeException below role-reject evidence: when any endpoint answered 421+role in the same sweep, surface the retriable QwpRoleMismatchException so the connect/reconnect loops keep rows in store-and-forward and retry through the promotion window. The demoted upgrade error rides along as a suppressed diagnostic. Typed capability gaps are NOT demoted: QwpVersionMismatchException and QwpDurableAckMismatchException extend HttpClientException directly, so they fall through the instanceof check and stay terminal even when replicas role-rejected in the same sweep -- preserving the documented durable-ack contract. Covered by WriteFailoverTest#testMixedSweepRoleRejectOutranksLatchedTerminalUpgradeError, which also asserts the suppressed 503 diagnostic.
1 parent 70adbf1 commit 51bc8af

2 files changed

Lines changed: 113 additions & 8 deletions

File tree

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

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,8 +2761,13 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
27612761
// non-421 WebSocketUpgradeException (4xx/5xx). The catch block walks
27622762
// remaining endpoints in case the failure is per-endpoint, then surfaces
27632763
// this latched typed exception when the round ends without a successful
2764-
// connect. Auth failures are NOT latched here -- they throw immediately
2765-
// because a rejected credential is uniformly rejected across the cluster.
2764+
// connect -- except that a plain non-421 WebSocketUpgradeException is
2765+
// demoted below role-reject evidence in the epilogue: when any endpoint
2766+
// answered 421+role in the same sweep, the window is transient and the
2767+
// retriable role-mismatch classification wins (the demoted error rides
2768+
// along as a suppressed diagnostic). Auth failures are NOT latched here
2769+
// -- they throw immediately because a rejected credential is uniformly
2770+
// rejected across the cluster.
27662771
HttpClientException terminalUpgradeError = null;
27672772
QwpIngressRoleRejectedException lastRoleReject = null;
27682773
Endpoint lastEndpoint = null;
@@ -2963,15 +2968,32 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
29632968
null, SenderConnectionEvent.NO_PORT,
29642969
SenderConnectionEvent.NO_ATTEMPT_NUMBER, roundSeq, lastError);
29652970
}
2966-
if (terminalUpgradeError != null) {
2971+
// Role-reject evidence outranks a latched plain non-421 upgrade
2972+
// error. In a mixed sweep -- e.g. [replica(421+role),
2973+
// replica(421+role), node(503)] -- the co-occurring 421+role
2974+
// responses are positive evidence of a transient failover/promotion
2975+
// window; throwing the latched 5xx/4xx here would misclassify that
2976+
// window as terminal (dead foreground sender, or a drainer slot
2977+
// quarantine via BackgroundDrainer markFailed). Only plain
2978+
// WebSocketUpgradeException is demoted: the typed capability gaps
2979+
// (QwpVersionMismatchException, QwpDurableAckMismatchException)
2980+
// extend HttpClientException directly, so they fall through this
2981+
// check and stay terminal even when replicas role-rejected in the
2982+
// same sweep -- the contract the durable-ack paragraph below
2983+
// documents and relies on.
2984+
if (terminalUpgradeError != null
2985+
&& !(lastRoleReject != null
2986+
&& terminalUpgradeError instanceof WebSocketUpgradeException)) {
29672987
throw terminalUpgradeError;
29682988
}
29692989
if (lastRoleReject != null) {
2970-
// Every endpoint role-rejected the /write/v4 upgrade: right now the
2971-
// reachable nodes are all replicas (or primary-catchup). That is a
2972-
// TRANSIENT failover window, not a terminal condition -- a replica
2973-
// can be promoted and a primary will reappear. Surface it as a
2974-
// retriable QwpRoleMismatchException so the SYNC/ASYNC connect and
2990+
// Every endpoint either role-rejected the /write/v4 upgrade or
2991+
// failed with a demoted non-421 upgrade error: right now the
2992+
// reachable, role-classified nodes are all replicas (or
2993+
// primary-catchup). That is a TRANSIENT failover window, not a
2994+
// terminal condition -- a replica can be promoted and a primary
2995+
// will reappear. Surface it as a retriable
2996+
// QwpRoleMismatchException so the SYNC/ASYNC connect and
29752997
// reconnect loops keep the rows in store-and-forward and retry
29762998
// within reconnect_max_duration_millis (for an SF sender the only
29772999
// terminal condition is SF exhaustion).
@@ -2993,6 +3015,11 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
29933015
+ "; last observed role=" + lastRoleReject.getRole()
29943016
+ " at " + lastRoleReject.getHost() + ':' + lastRoleReject.getPort());
29953017
ex.initCause(lastRoleReject);
3018+
if (terminalUpgradeError != null) {
3019+
// Keep the demoted non-421 upgrade error observable for
3020+
// diagnostics without changing the surfaced classification.
3021+
ex.addSuppressed(terminalUpgradeError);
3022+
}
29963023
throw ex;
29973024
}
29983025
LineSenderException ex = new LineSenderException(lastError);

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/WriteFailoverTest.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,84 @@ public void testRoleMismatchExceptionWhenAllReplicas() throws Exception {
306306
}
307307
}
308308

309+
@Test
310+
public void testMixedSweepRoleRejectOutranksLatchedTerminalUpgradeError() throws Exception {
311+
// Mixed connect sweep: [replica(421+role), replica(421+role),
312+
// node(503)]. The co-occurring 421 role rejects are positive
313+
// evidence of a transient failover/promotion window -- a replica
314+
// can be promoted and a primary will reappear -- so the round
315+
// epilogue must surface the retriable QwpRoleMismatchException,
316+
// NOT the latched non-421 terminal upgrade error. Preferring the
317+
// 503 turns a transient window into a dead sender (or a drainer
318+
// slot quarantine on the background path).
319+
AckHandler ack = new AckHandler();
320+
TestWebSocketServer r1 = new TestWebSocketServer(ack);
321+
int port1 = r1.getPort();
322+
r1.setRejectWithRole("REPLICA");
323+
TestWebSocketServer r2 = new TestWebSocketServer(ack);
324+
int port2 = r2.getPort();
325+
r2.setRejectWithRole("REPLICA");
326+
TestWebSocketServer sick = new TestWebSocketServer(ack);
327+
int port3 = sick.getPort();
328+
sick.setRejectWithStatus(503, "Service Unavailable");
329+
try {
330+
r1.start();
331+
r2.start();
332+
sick.start();
333+
Assert.assertTrue(r1.awaitStart(5, TimeUnit.SECONDS));
334+
Assert.assertTrue(r2.awaitStart(5, TimeUnit.SECONDS));
335+
Assert.assertTrue(sick.awaitStart(5, TimeUnit.SECONDS));
336+
337+
QwpRoleMismatchException observed = null;
338+
// Off-mode walk sweeps every endpoint once and classifies the
339+
// exhausted round at build(). Endpoint pick order does not
340+
// matter: 421 role rejects and the 503 both `continue` the walk,
341+
// so all three endpoints are always visited before the epilogue.
342+
try (Sender ignored = Sender.builder(Sender.Transport.WEBSOCKET)
343+
.address("localhost:" + port1)
344+
.address("localhost:" + port2)
345+
.address("localhost:" + port3)
346+
.build()) {
347+
Assert.fail("expected the mixed-sweep connect to fail");
348+
} catch (QwpRoleMismatchException e) {
349+
observed = e;
350+
} catch (WebSocketUpgradeException e) {
351+
throw new AssertionError(
352+
"mixed sweep misclassified as terminal: latched non-421 upgrade error "
353+
+ "(status=" + e.getStatusCode()
354+
+ ") outranked role-reject evidence: " + e.getMessage(), e);
355+
} catch (LineSenderException e) {
356+
throw new AssertionError(
357+
"expected QwpRoleMismatchException but got LineSenderException: "
358+
+ e.getMessage(), e);
359+
}
360+
Assert.assertNotNull("expected a role-mismatch surface", observed);
361+
Assert.assertEquals("PRIMARY", observed.getTargetRole());
362+
String msg = observed.getMessage();
363+
Assert.assertNotNull(msg);
364+
Assert.assertTrue("error must mention the observed replica role: " + msg,
365+
msg.contains("REPLICA"));
366+
// The demoted non-421 upgrade error must stay observable for
367+
// diagnostics: it rides along as a suppressed exception on the
368+
// surfaced role-mismatch classification.
369+
WebSocketUpgradeException demoted = null;
370+
for (Throwable s : observed.getSuppressed()) {
371+
if (s instanceof WebSocketUpgradeException) {
372+
demoted = (WebSocketUpgradeException) s;
373+
break;
374+
}
375+
}
376+
Assert.assertNotNull(
377+
"expected the demoted 503 upgrade error as a suppressed diagnostic",
378+
demoted);
379+
Assert.assertEquals(503, demoted.getStatusCode());
380+
} finally {
381+
r1.close();
382+
r2.close();
383+
sick.close();
384+
}
385+
}
386+
309387
@Test
310388
public void testStandaloneIsTreatedAsWritable() throws Exception {
311389
// OSS / single-node deployments advertise STANDALONE. The client

0 commit comments

Comments
 (0)