Skip to content

Commit 64dd95b

Browse files
committed
fix(qwp): reject reconnect_max_duration_millis = 0 -- a zero budget can never work
The builder accepted >= 0, but 0 is a value with no working semantics: - SYNC initial connect: connectWithRetry's deadline (startNanos + 0) expires before the first attempt, so build() throws "failed after 0ms / 0 attempts" without ever dialing -- even against a healthy server. Worse, explicitly setting any reconnect_* knob implicitly promotes the initial-connect mode to SYNC, so writing =0 made the instant failure the guaranteed outcome, not an edge case. - Background drainer: capabilityGapBudgetNanos = 0 makes the durable-ack settle budget exhaust on the first gap sweep (0 >= 0), quarantining the slot instantly instead of after the documented 16-sweep budget. No consumer treats 0 as a disabled/infinite sentinel (the background reconnect loop ignores this knob entirely under Invariant B), so nothing loses expressible behavior. Validation sits in the builder setter -- the single choke point for the builder, config-string, and view paths -- matching the > 0 contract of the sibling backoff knobs. Tests cover zero and negative through both the config string and the builder.
1 parent 21fe8cb commit 64dd95b

2 files changed

Lines changed: 41 additions & 3 deletions

File tree

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2469,14 +2469,18 @@ public LineSenderBuilder reconnectMaxBackoffMillis(long millis) {
24692469
* auth/upgrade error or {@code close()}.
24702470
* <p>
24712471
* Default {@code 300_000} (5 minutes). Lower for fail-fast startup;
2472-
* higher for tolerating a slow server boot. WebSocket only.
2472+
* higher for tolerating a slow server boot. Must be positive: a zero
2473+
* budget would make the SYNC initial connect throw without a single
2474+
* attempt (even against a healthy server) and would collapse the
2475+
* background drainer's durable-ack settle budget to its first sweep.
2476+
* WebSocket only.
24732477
*/
24742478
public LineSenderBuilder reconnectMaxDurationMillis(long millis) {
24752479
if (protocol != PARAMETER_NOT_SET_EXPLICITLY && protocol != PROTOCOL_WEBSOCKET) {
24762480
throw new LineSenderException("reconnect_max_duration_millis is only supported for WebSocket transport");
24772481
}
2478-
if (millis < 0) {
2479-
throw new LineSenderException("reconnect_max_duration_millis must be >= 0: ").put(millis);
2482+
if (millis <= 0) {
2483+
throw new LineSenderException("reconnect_max_duration_millis must be > 0: ").put(millis);
24802484
}
24812485
this.reconnectMaxDurationMillis = millis;
24822486
return this;

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,40 @@ public void testWsConfigString_withRetryTimeout_fails() {
924924
"unknown configuration key: retry_timeout (use reconnect_max_duration_millis on ws/wss)");
925925
}
926926

927+
@Test
928+
public void testWsConfigString_reconnectMaxDurationZero_fails() {
929+
// A zero budget must be rejected up front: connectWithRetry's deadline
930+
// (startNanos + 0) makes the SYNC initial connect throw "after 0ms /
931+
// 0 attempts" without ever dialing a healthy server, and the drainer's
932+
// capabilityGapBudgetNanos=0 exhausts the durable-ack settle budget
933+
// (0 >= 0) on the first gap sweep, quarantining the slot instantly
934+
// instead of after the documented 16-sweep budget.
935+
assertBadConfig("ws::addr=localhost:9000;reconnect_max_duration_millis=0;",
936+
"reconnect_max_duration_millis must be > 0");
937+
}
938+
939+
@Test
940+
public void testWsConfigString_reconnectMaxDurationNegative_fails() {
941+
assertBadConfig("ws::addr=localhost:9000;reconnect_max_duration_millis=-1;",
942+
"reconnect_max_duration_millis must be > 0");
943+
}
944+
945+
@Test
946+
public void testReconnectMaxDurationBuilder_zeroRejected() {
947+
assertThrows("reconnect_max_duration_millis must be > 0",
948+
() -> Sender.builder(Sender.Transport.WEBSOCKET)
949+
.address(LOCALHOST)
950+
.reconnectMaxDurationMillis(0));
951+
}
952+
953+
@Test
954+
public void testReconnectMaxDurationBuilder_negativeRejected() {
955+
assertThrows("reconnect_max_duration_millis must be > 0",
956+
() -> Sender.builder(Sender.Transport.WEBSOCKET)
957+
.address(LOCALHOST)
958+
.reconnectMaxDurationMillis(-1));
959+
}
960+
927961
@Test
928962
public void testWsConfigString_usernameWithoutPassword_fails() {
929963
// The ingress ws path rejects a username with no password up front in

0 commit comments

Comments
 (0)