Skip to content

Commit 47b3c9e

Browse files
glasstigerclaude
andcommitted
Fix inverted async initial-connect retry docs
Commit 994554e rewrote the initial_connect_retry=async docs to claim that transport, auth, upgrade and capability failures are all retried indefinitely and never surfaced. That over-generalized the transport retry-forever rule (Invariant B) onto endpoint-policy failures and inverted a deliberately engineered, test-proven contract. endpointPolicyFailureIsTerminal() returns ORPHAN || !hasEverConnected, so on a foreground sender's initial connect an auth, upgrade or durable-ack capability rejection is terminal: connectLoop latches it via recordFatal and dispatches it to the async SenderErrorHandler (a close() rethrow in ASYNC, a throw from build() in SYNC/OFF). Only transport failures, and endpoint-policy failures once the wire has been up at least once, retry forever. InitialConnectAsyncTest proves both halves: testAsyncAuthFailureSurfacesTerminal the terminal path, testAsyncNoServerRetriesForeverNoTerminal the transport path. The wrong wording risked an operator not wiring an error_handler and then silently buffering into store-and-forward on an auth misconfiguration. Restore the carve-out in all four docstrings the commit inverted: the Sender.InitialConnectMode.ASYNC javadoc, the CursorWebSocketSendLoop class-doc and attemptInitialConnect() javadoc, the QwpWebSocketSender ASYNC-case comment, and the InitialConnectAsyncTest class javadoc. Documentation only; no runtime behavior changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c1587c9 commit 47b3c9e

4 files changed

Lines changed: 35 additions & 20 deletions

File tree

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -795,9 +795,16 @@ default Sender uuidColumn(CharSequence name, long lo, long hi) {
795795
* unconnected sender; the I/O thread runs the same retry loop in
796796
* the background. The user thread can call {@code at()} /
797797
* {@code flush()} immediately; rows accumulate in the cursor SF
798-
* engine until the wire is up. Transport, auth, upgrade and
799-
* capability failures are retried indefinitely in the background;
800-
* none is surfaced to producer calls or the async error inbox.</li>
798+
* engine until the wire is up. Transport failures (unreachable or
799+
* dropped server) retry indefinitely and are never surfaced -- the
800+
* buffered rows are safe in SF and the server may still appear. A
801+
* terminal auth, upgrade or capability rejection on the initial
802+
* connect (before the wire has ever come up) has no caller left to
803+
* throw at, so it is delivered to the async error inbox as a
804+
* {@link io.questdb.client.SenderError}; wire {@code error_handler=...}
805+
* to observe it. Once the wire has come up even once, SF owns the
806+
* buffered data and the same rejections (e.g. a credential rotation)
807+
* become transients retried indefinitely.</li>
801808
* </ul>
802809
* <p>
803810
* Default resolution when the caller does not pick a value:

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3398,10 +3398,12 @@ private void ensureConnected() {
33983398
// Encoder stays at its default (V1 -- the only supported wire
33993399
// version today). Frames written before the first successful
34003400
// connect commit to V1 because cursor segments are immutable;
3401-
// a future version bump must account for that. Auth, upgrade,
3402-
// capability and transport failures all retry indefinitely on
3403-
// the I/O thread (Invariant B); none is surfaced here or later
3404-
// to the producer.
3401+
// a future version bump must account for that. Transport
3402+
// failures retry indefinitely on the I/O thread (Invariant B).
3403+
// But a terminal auth, upgrade or capability rejection on this
3404+
// initial connect -- before the wire is ever up -- is surfaced
3405+
// to the async SenderErrorHandler and latched for a close()
3406+
// rethrow, not retried.
34053407
client = null;
34063408
break;
34073409
case OFF:

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,13 @@
6868
* can trim fully-acked segments.</li>
6969
* <li>On wire failure, runs the configured reconnect policy: capped
7070
* exponential backoff with jitter, retried indefinitely (Invariant B --
71-
* a store-and-forward drainer never gives up on a wall-clock budget),
72-
* with endpoint-policy failures terminal only for an orphan drainer. A
73-
* foreground sender keeps retrying from asynchronous startup onward so
74-
* credential and rolling-capability changes remain contained by
75-
* store-and-forward. On reconnect success, repositions the cursor at
71+
* a store-and-forward drainer never gives up on a wall-clock budget).
72+
* Once a foreground sender has connected even once, endpoint-policy
73+
* failures (auth, upgrade, durable-ack capability) become transients it
74+
* keeps retrying, so credential and rolling-capability changes stay
75+
* contained by store-and-forward; they are terminal only for an orphan
76+
* drainer or on a foreground sender's initial connect (before the wire
77+
* has ever come up). On reconnect success, repositions the cursor at
7678
* {@code ackedFsn+1} and replays.</li>
7779
* </ol>
7880
* No locks on the steady-state path. The producer thread (user) writes
@@ -1505,9 +1507,11 @@ private void applyDurableAck() {
15051507
* Drives the very first connect attempt on the I/O thread, used in the
15061508
* async-initial-connect mode (constructed with {@code client == null}).
15071509
* Reuses the same retry+backoff machinery as {@link #fail(Throwable)}.
1508-
* Transport, authentication, upgrade and capability failures are all
1509-
* retried indefinitely here (Invariant B); none is surfaced to the
1510-
* foreground producer.
1510+
* Transport failures retry indefinitely here (Invariant B). But this path
1511+
* runs with {@code !hasEverConnected}, so an auth, upgrade or capability
1512+
* rejection is terminal (see {@link #endpointPolicyFailureIsTerminal()}):
1513+
* it is dispatched to the async {@code SenderErrorHandler} and latched for
1514+
* a {@code close()} rethrow, rather than retried.
15111515
*/
15121516
private void attemptInitialConnect() {
15131517
connectLoop(new LineSenderException(

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,13 @@
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. Plain
53-
* failures are retried indefinitely (Invariant B: no wall-clock budget
54-
* give-up), including authentication, upgrade and durable-ack capability
55-
* failures. Those endpoint states stay inside the I/O thread and never stop
56-
* the producer.
52+
* is reachable; the I/O thread retries connect in the background. Transport
53+
* failures (unreachable or dropped server) are retried indefinitely
54+
* (Invariant B: no wall-clock budget give-up) and never stop the producer.
55+
* But the initial connect has never reached the server, so an endpoint-policy
56+
* rejection there -- authentication, upgrade or durable-ack capability -- is
57+
* terminal: with no caller thread left to throw at, it is delivered to the
58+
* {@code SenderErrorHandler} instead of buffering forever.
5759
*/
5860
public class InitialConnectAsyncTest {
5961

0 commit comments

Comments
 (0)