Skip to content

Commit 0623f9f

Browse files
committed
docs(qwp): fix stale comments left behind by the Invariant-B budget-terminal removal (M4 residual)
Comment-only; no code changes. 6be20d6/e9a1e7d removed connectLoop's wall-clock budget give-up and the never-connected/connection-lost terminal, but four comments still described the old world: - InitialConnectRetryTest.makeSfDir / InitialConnectAsyncTest.sfDirOpt claimed 'initial_connect_retry on/sync/async requires sf_dir per spec s3.5; without it the builder rejects construction' -- no such validation exists (Sender.build silently falls back to a memory-mode CursorSendEngine, and QuestDBBuilder.resolveLazyConnect injects initial_connect_retry=async with no sf_dir check). Now documents that the tests set sf_dir deliberately to exercise the durable SF path. - InitialConnectAsyncTest class javadoc listed 'budget exhaustion' as a terminal delivered to the error inbox -- under Invariant B plain connect failures retry indefinitely; only auth / upgrade reject / durable-ack capability gap terminals reach the inbox. - testWasEverConnectedTrueImmediatelyInSyncMode referenced the deleted never-connected-vs-connection-lost misclassification rationale. - CursorWebSocketSendLoop.connectWithRetry javadoc said it is the 'same retry loop the I/O thread uses' -- the loops diverged: connectLoop retries forever (Invariant B), connectWithRetry (SYNC) remains bounded by maxDurationMillis and still throws on budget exhaustion. PrReviewRedTestsE2e's budget-exhausted reference is left as-is: it is a dated historical record of the PR-17 review findings.
1 parent 844454c commit 0623f9f

3 files changed

Lines changed: 24 additions & 16 deletions

File tree

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -374,11 +374,13 @@ public static SenderError.Category classify(byte status) {
374374
}
375375

376376
/**
377-
* Same retry-with-exponential-backoff-and-jitter loop the I/O thread
378-
* uses on a wire failure, but reusable from {@code ensureConnected} to
379-
* implement {@code initial_connect_retry=true}. Returns the connected
380-
* client on success; throws on terminal upgrade error (won't retry) or
381-
* budget exhaustion.
377+
* Same exponential-backoff-with-jitter machinery as the I/O thread's
378+
* {@code connectLoop}, but reusable from {@code ensureConnected} to
379+
* implement {@code initial_connect_retry=true}. Unlike {@code connectLoop}
380+
* (which retries indefinitely under Invariant B), this blocking variant
381+
* IS bounded by {@code maxDurationMillis}: it returns the connected
382+
* client on success and throws on terminal upgrade error (won't retry)
383+
* or budget exhaustion.
382384
* <p>
383385
* Caller-supplied {@code factory} is invoked once per attempt and
384386
* should produce a fresh, connected, upgraded client (or throw). The

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,11 @@
4949
/**
5050
* Behavior of {@code initial_connect_retry=async}: the producer-thread
5151
* {@code Sender.fromConfig} must return immediately even when no server
52-
* is reachable; the I/O thread retries connect in the background, and
53-
* terminal failures (auth/upgrade reject, budget exhaustion) are
54-
* delivered through the async error inbox rather than thrown at the
55-
* call site.
52+
* is reachable; the I/O thread retries connect in the background. Plain
53+
* connect failures are retried indefinitely (Invariant B: no wall-clock
54+
* budget give-up); only genuine terminals (auth/upgrade reject,
55+
* durable-ack capability gap) are delivered through the async error
56+
* inbox rather than thrown at the call site.
5657
*/
5758
public class InitialConnectAsyncTest {
5859

@@ -283,8 +284,7 @@ public void testWasEverConnectedTrueImmediatelyInSyncMode() throws Exception {
283284
// before fromConfig returns. wasEverConnected() must therefore
284285
// already be true the instant the sender becomes visible to the
285286
// caller — there is no observable "never connected" window in
286-
// those modes, so misclassifying a budget exhaustion as
287-
// never-connected is impossible.
287+
// those modes.
288288
try (TestWebSocketServer server = new TestWebSocketServer(new AckHandler())) {
289289
int port = server.getPort();
290290
server.start();
@@ -362,8 +362,11 @@ private static void assertCloseRethrowsTerminal(Sender sender, String expectedSu
362362

363363
/**
364364
* Returns a unique temp sf_dir snippet for embedding in a config
365-
* string. initial_connect_retry on/sync/async requires sf_dir per
366-
* spec §3.5; without it the builder rejects construction.
365+
* string. The builder does NOT require sf_dir for any
366+
* initial_connect_retry mode — without it the sender builds in
367+
* memory mode and buffers rows in the in-RAM cursor ring. These
368+
* tests set an sf_dir so the rows accumulated before the first
369+
* successful connect are disk-backed (the durable SF path).
367370
*/
368371
private static String sfDirOpt() {
369372
String dir = java.nio.file.Paths.get(

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,12 @@
4242
public class InitialConnectRetryTest {
4343

4444
/**
45-
* Temp sf_dir for retry-mode tests. Per spec §3.5,
46-
* initial_connect_retry on/sync/async requires sf_dir — memory-mode
47-
* senders cannot durably retry across reconnects.
45+
* Temp sf_dir for retry-mode tests. The builder does NOT require
46+
* sf_dir for any initial_connect_retry mode — memory-mode senders
47+
* share the same retry machinery, buffering rows in the in-RAM
48+
* cursor ring instead of on disk. These tests use an sf_dir so the
49+
* retried rows are disk-backed and the tests exercise the durable
50+
* SF path.
4851
*/
4952
private static String makeSfDir() {
5053
return java.nio.file.Paths.get(

0 commit comments

Comments
 (0)