Skip to content

Commit e9a1e7d

Browse files
committed
fix(qwp): async/mid-stream never surface a connection-error terminal; drop dead RECONNECT_BUDGET_EXHAUSTED
Under Invariant B all senders are SF senders, so a connection error is never terminal: SYNC initial connect (connectWithRetry) still surfaces it (fail loud), but ASYNC initial connect and mid-stream reconnect (connectLoop) retry forever and surface only GENUINE terminals (auth / non-421 upgrade / durable-ack capability gap) or SF exhaustion. The wall-clock budget give-up was removed from connectLoop earlier; this removes the now-dead RECONNECT_BUDGET_EXHAUSTED connection-event kind and fixes the 7 tests that asserted the old behavior. Main: - Remove SenderConnectionEvent.Kind.RECONNECT_BUDGET_EXHAUSTED (never fired now) and its references in DefaultSenderConnectionListener (switch case + javadoc), SenderConnectionListener/SenderConnectionEvent javadoc, and CursorWebSocketSendLoop comments. Tests (all ASYNC or mid-stream -- verified none were SYNC): - Group 1 (asserted a connection/budget terminal -> now wrong) converted to Invariant-B "retries forever, no terminal" guards: InitialConnectAsyncTest.testAsyncBudgetExhaustionDeliversToErrorInbox -> testAsyncNoServerRetriesForeverNoTerminal InitialConnectAsyncTest.testConnectionLostBudgetExhaustionTagsDifferently -> testConnectionLostRetriesForeverNoTerminal ReconnectTest.testReconnectGivesUpAfterCap -> testReconnectNeverGivesUpInvariantB - Group 2 (used a terminal to exercise close/facade/ownership) re-pointed from a dead-port connection error to a genuine 401 auth terminal (still surfaces in async): CloseSafetyNetTest (x2, via TestWebSocketServer.setRejectWithStatus), QuestDBFacadeCallbacksTest.testFacadeErrorHandlerReceivesAsyncIngestError, CloseOwnershipRaceTest (factory throws QwpAuthFailedException). 67 affected tests pass. SYNC surfacing and its tests are untouched.
1 parent f9ece1a commit e9a1e7d

9 files changed

Lines changed: 164 additions & 159 deletions

File tree

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ public long getAttemptNumber() {
9696
/**
9797
* The classified cause of the event, or {@code null} for success/info
9898
* events ({@link Kind#CONNECTED}, {@link Kind#FAILED_OVER},
99-
* {@link Kind#RECONNECTED}). For terminal kinds
100-
* ({@link Kind#AUTH_FAILED}, {@link Kind#RECONNECT_BUDGET_EXHAUSTED}) this
99+
* {@link Kind#RECONNECTED}). For the terminal kind
100+
* ({@link Kind#AUTH_FAILED}) this
101101
* carries the typed exception that caused the sender to halt.
102102
*/
103103
@Nullable
@@ -234,14 +234,6 @@ public enum Kind {
234234
* producer-thread API call surfaces a {@code LineSenderException}.
235235
* {@link #getCause()} carries the {@code QwpAuthFailedException}.
236236
*/
237-
AUTH_FAILED,
238-
239-
/**
240-
* Terminal: the configured reconnect time budget was exhausted without
241-
* a successful reconnect. The sender will halt; the next producer-thread
242-
* API call surfaces a {@code LineSenderException}. {@link #getCause()}
243-
* carries the last observed reconnect error.
244-
*/
245-
RECONNECT_BUDGET_EXHAUSTED
237+
AUTH_FAILED
246238
}
247239
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@
5151
* {@link SenderConnectionEvent.Kind#RECONNECTED}) are guaranteed to fire on
5252
* each transition. Failure events ({@code ENDPOINT_ATTEMPT_FAILED},
5353
* {@code ALL_ENDPOINTS_UNREACHABLE}) may be coalesced under inbox pressure.
54-
* Terminal events ({@code AUTH_FAILED}, {@code RECONNECT_BUDGET_EXHAUSTED})
55-
* fire before the producer-thread {@code LineSenderException} is observable on
54+
* The terminal event {@code AUTH_FAILED}
55+
* fires before the producer-thread {@code LineSenderException} is observable on
5656
* the next API call -- so a listener can react sooner than the producer learns
5757
* via exception, but should not assume the listener fires first under heavy
5858
* notification load.

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,11 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
177177
// alike) is offered to the dispatcher for async delivery to the user's
178178
// handler. Null disables async delivery entirely; the producer-side
179179
// typed-throw path is unaffected.
180-
// Optional: when non-null, RECONNECT_BUDGET_EXHAUSTED is offered to the
181-
// dispatcher for async delivery to the user's listener at the moment
182-
// connectLoop gives up. Sender-side fire points (CONNECTED, FAILED_OVER,
183-
// ENDPOINT_ATTEMPT_FAILED, AUTH_FAILED, ALL_ENDPOINTS_UNREACHABLE) write
184-
// directly to the same dispatcher from QwpWebSocketSender.
180+
// Optional: when non-null, sender-side connection events (CONNECTED,
181+
// FAILED_OVER, ENDPOINT_ATTEMPT_FAILED, AUTH_FAILED, ALL_ENDPOINTS_UNREACHABLE)
182+
// are written to this dispatcher from QwpWebSocketSender. connectLoop itself
183+
// no longer emits a terminal budget-exhaustion event (Invariant B: it retries
184+
// indefinitely and never gives up on a wall-clock budget).
185185
private volatile SenderConnectionDispatcher connectionDispatcher;
186186
private volatile SenderErrorDispatcher errorDispatcher;
187187
// The send cursor has two coordinate systems:
@@ -667,10 +667,10 @@ public boolean isRunning() {
667667

668668
/**
669669
* Plug an async-delivery sink for {@link SenderConnectionEvent}
670-
* notifications. The loop fires {@code RECONNECT_BUDGET_EXHAUSTED}
671-
* through this sink when {@code connectLoop} gives up; other connection
672-
* events fire from {@code QwpWebSocketSender.buildAndConnect} directly
673-
* into the same dispatcher. Same lifecycle contract as
670+
* notifications. Connection events fire from
671+
* {@code QwpWebSocketSender.buildAndConnect} directly into this dispatcher;
672+
* {@code connectLoop} no longer emits a terminal budget-exhaustion event
673+
* (Invariant B: it retries indefinitely). Same lifecycle contract as
674674
* {@link #setErrorDispatcher}.
675675
*/
676676
public void setConnectionDispatcher(SenderConnectionDispatcher dispatcher) {

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@
3636
* transition so silence is never the default -- connect-string-only users
3737
* still see failover and outage signals in their logs.
3838
*
39-
* <p>Terminal kinds ({@code AUTH_FAILED}, {@code RECONNECT_BUDGET_EXHAUSTED})
40-
* and {@code ALL_ENDPOINTS_UNREACHABLE} fire at WARN level; everything else
41-
* fires at INFO.
39+
* <p>Terminal kind {@code AUTH_FAILED} and {@code ALL_ENDPOINTS_UNREACHABLE}
40+
* fire at WARN level; everything else fires at INFO.
4241
*/
4342
public final class DefaultSenderConnectionListener implements SenderConnectionListener {
4443

@@ -52,7 +51,6 @@ private DefaultSenderConnectionListener() {
5251
public void onEvent(@NotNull SenderConnectionEvent e) {
5352
switch (e.getKind()) {
5453
case AUTH_FAILED:
55-
case RECONNECT_BUDGET_EXHAUSTED:
5654
case ALL_ENDPOINTS_UNREACHABLE:
5755
LOG.warn("connection event {}", e);
5856
break;

core/src/test/java/io/questdb/client/test/QuestDBFacadeCallbacksTest.java

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import io.questdb.client.SenderError;
3131
import io.questdb.client.SenderErrorHandler;
3232
import io.questdb.client.test.cutlass.qwp.client.TestPorts;
33+
import io.questdb.client.test.cutlass.qwp.websocket.TestWebSocketServer;
3334
import org.jetbrains.annotations.NotNull;
3435
import org.junit.Assert;
3536
import org.junit.Test;
@@ -52,6 +53,13 @@
5253
*/
5354
public class QuestDBFacadeCallbacksTest {
5455

56+
private static final TestWebSocketServer.WebSocketServerHandler NOOP_HANDLER =
57+
new TestWebSocketServer.WebSocketServerHandler() {
58+
@Override
59+
public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
60+
}
61+
};
62+
5563
@Test
5664
public void testFacadeConnectionListenerReceivesEvents() throws Exception {
5765
int port = TestPorts.findUnusedPort();
@@ -74,16 +82,24 @@ public void onEvent(@NotNull SenderConnectionEvent event) {
7482

7583
@Test
7684
public void testFacadeErrorHandlerReceivesAsyncIngestError() throws Exception {
77-
int port = TestPorts.findUnusedPort();
78-
ErrorInbox inbox = new ErrorInbox();
79-
try (QuestDB ignored = QuestDB.builder()
80-
.fromConfig(config(port))
81-
.errorHandler(inbox)
82-
.build()) {
83-
Assert.assertTrue(
84-
"facade-wired errorHandler must receive the async budget-exhaustion SenderError",
85-
inbox.await(5, TimeUnit.SECONDS));
86-
Assert.assertNotNull("a SenderError must be delivered", inbox.get());
85+
// A 401 server produces a genuine auth terminal that surfaces even in
86+
// async mode; the facade-wired errorHandler must receive it. (Under
87+
// Invariant B a mere connection error would retry forever and never
88+
// surface -- only a genuine terminal like auth does.)
89+
try (TestWebSocketServer server = new TestWebSocketServer(NOOP_HANDLER)) {
90+
server.setRejectWithStatus(401, "Unauthorized");
91+
server.start();
92+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
93+
ErrorInbox inbox = new ErrorInbox();
94+
try (QuestDB ignored = QuestDB.builder()
95+
.fromConfig(config(server.getPort()))
96+
.errorHandler(inbox)
97+
.build()) {
98+
Assert.assertTrue(
99+
"facade-wired errorHandler must receive the async auth-terminal SenderError",
100+
inbox.await(5, TimeUnit.SECONDS));
101+
Assert.assertNotNull("a SenderError must be delivered", inbox.get());
102+
}
87103
}
88104
}
89105

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

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import io.questdb.client.SenderErrorHandler;
3131
import io.questdb.client.cutlass.line.LineSenderException;
3232
import io.questdb.client.cutlass.qwp.client.QwpWebSocketSender;
33+
import io.questdb.client.test.cutlass.qwp.websocket.TestWebSocketServer;
3334
import org.jetbrains.annotations.NotNull;
3435
import org.junit.Assert;
3536
import org.junit.Rule;
@@ -62,47 +63,59 @@ public class CloseSafetyNetTest {
6263
public final TemporaryFolder sfDir = TemporaryFolder.builder().assureDeletion().build();
6364

6465
@Test(timeout = 30_000)
65-
public void testCloseRethrowsUnsurfacedTerminalWithoutCustomHandler() {
66-
// No server, no handler, tight reconnect budget: the I/O thread
67-
// latches a never-connected budget-exhaustion terminal that nothing
68-
// has surfaced to the user. close() must throw it.
69-
Sender sender = Sender.fromConfig(cfg());
70-
boolean closed = false;
71-
try {
72-
awaitLatchedTerminal((QwpWebSocketSender) sender);
66+
public void testCloseRethrowsUnsurfacedTerminalWithoutCustomHandler() throws Exception {
67+
// A 401 server, no handler: the I/O thread latches a genuine auth
68+
// terminal (ws-upgrade-failed / SECURITY_ERROR) that nothing has
69+
// surfaced to the user. close() must throw it. (Under Invariant B a
70+
// mere connection error would retry forever and never latch -- only a
71+
// genuine terminal like auth does.)
72+
try (TestWebSocketServer server = new TestWebSocketServer(NOOP_HANDLER)) {
73+
server.setRejectWithStatus(401, "Unauthorized");
74+
server.start();
75+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
76+
Sender sender = Sender.fromConfig(cfg(server.getPort()));
77+
boolean closed = false;
7378
try {
74-
closed = true;
75-
sender.close();
76-
Assert.fail("close() must rethrow a terminal error that no synchronous "
77-
+ "caller and no custom handler has seen");
78-
} catch (LineSenderException e) {
79-
String msg = e.getMessage() == null ? "" : e.getMessage();
80-
Assert.assertTrue("close() must rethrow the latched terminal: " + msg,
81-
msg.contains("never-connected-budget-exhausted"));
82-
Assert.assertTrue("the latched instance is the typed server exception",
83-
e instanceof LineSenderServerException);
84-
}
85-
} finally {
86-
if (!closed) {
87-
sender.close();
79+
awaitLatchedTerminal((QwpWebSocketSender) sender);
80+
try {
81+
closed = true;
82+
sender.close();
83+
Assert.fail("close() must rethrow a terminal error that no synchronous "
84+
+ "caller and no custom handler has seen");
85+
} catch (LineSenderException e) {
86+
String msg = e.getMessage() == null ? "" : e.getMessage();
87+
Assert.assertTrue("close() must rethrow the latched terminal: " + msg,
88+
msg.contains("ws-upgrade-failed") || msg.contains("401"));
89+
Assert.assertTrue("the latched instance is the typed server exception",
90+
e instanceof LineSenderServerException);
91+
}
92+
} finally {
93+
if (!closed) {
94+
sender.close();
95+
}
8896
}
8997
}
9098
}
9199

92100
@Test(timeout = 30_000)
93101
public void testCloseStaysSilentWhenCustomHandlerAlreadyDelivered() throws Exception {
94-
// Same terminal, but the user installed a custom error handler and
102+
// Same auth terminal, but the user installed a custom error handler and
95103
// the dispatcher delivered the error to it. close() must NOT
96104
// double-signal.
97-
ErrorInbox inbox = new ErrorInbox();
98-
Sender sender = Sender.builder(cfg())
99-
.errorHandler(inbox)
100-
.build();
101-
Assert.assertTrue("terminal must reach the custom handler within 10s",
102-
inbox.await(10, TimeUnit.SECONDS));
103-
Assert.assertNotNull(inbox.get());
104-
// The handler owns the error now; a rethrow here would double-signal.
105-
sender.close();
105+
try (TestWebSocketServer server = new TestWebSocketServer(NOOP_HANDLER)) {
106+
server.setRejectWithStatus(401, "Unauthorized");
107+
server.start();
108+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
109+
ErrorInbox inbox = new ErrorInbox();
110+
Sender sender = Sender.builder(cfg(server.getPort()))
111+
.errorHandler(inbox)
112+
.build();
113+
Assert.assertTrue("terminal must reach the custom handler within 10s",
114+
inbox.await(10, TimeUnit.SECONDS));
115+
Assert.assertNotNull(inbox.get());
116+
// The handler owns the error now; a rethrow here would double-signal.
117+
sender.close();
118+
}
106119
}
107120

108121
/**
@@ -120,8 +133,8 @@ private static void awaitLatchedTerminal(QwpWebSocketSender sender) {
120133
}
121134
}
122135

123-
private String cfg() {
124-
return "ws::addr=localhost:" + TestPorts.findUnusedPort()
136+
private String cfg(int port) {
137+
return "ws::addr=localhost:" + port
125138
+ ";sf_dir=" + sfDir.getRoot().getAbsolutePath()
126139
+ ";initial_connect_retry=async"
127140
+ ";reconnect_max_duration_millis=400"
@@ -130,6 +143,13 @@ private String cfg() {
130143
+ ";close_flush_timeout_millis=0;";
131144
}
132145

146+
private static final TestWebSocketServer.WebSocketServerHandler NOOP_HANDLER =
147+
new TestWebSocketServer.WebSocketServerHandler() {
148+
@Override
149+
public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
150+
}
151+
};
152+
133153
private static class ErrorInbox implements SenderErrorHandler {
134154
private final CountDownLatch latch = new CountDownLatch(1);
135155
private final AtomicReference<SenderError> ref = new AtomicReference<>();

0 commit comments

Comments
 (0)