Skip to content

Commit 5e467f3

Browse files
committed
fix: harden reconnect and test cleanup handling
1 parent a341a9b commit 5e467f3

8 files changed

Lines changed: 551 additions & 120 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3430,7 +3430,7 @@ private void ensureConnected() {
34303430
maxFrameRejections,
34313431
poisonMinEscalationWindowMillis,
34323432
catchUpCapGapMinEscalationWindowMillis,
3433-
CursorWebSocketSendLoop.CatchUpCapGapPolicy.RETRY_FOREVER);
3433+
CursorWebSocketSendLoop.ReconnectPolicy.FOREGROUND);
34343434
// Plug the async-delivery sink before start() so the I/O thread
34353435
// never observes a null dispatcher between recordFatal and
34363436
// notification — the test for null in dispatchError handles

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,10 @@ public BackgroundDrainer() {
243243
* durable ack -- i.e. the symptom of a misconfigured cluster or a
244244
* rolling-upgrade transient.
245245
* <p>
246-
* For the foreground sender that condition is loud-fail: the producer
247-
* is actively pushing data. The drainer is asymmetric: source data is
246+
* For a foreground sender's initial connection that condition is loud-fail;
247+
* after the sender has been live, its reconnect loop keeps buffering and
248+
* retrying through a rolling capability change. The drainer is asymmetric:
249+
* source data is
248250
* pinned (durable-ack-mode trims only on STATUS_DURABLE_ACK frames,
249251
* which the offending endpoints by definition do not send), so we
250252
* give the cluster a budget to settle before quarantining the slot.
@@ -318,8 +320,7 @@ public WebSocketClient connectWithDurableAckRetry() {
318320
} catch (QwpAuthFailedException | WebSocketUpgradeException e) {
319321
// Genuinely non-retriable across the cluster (auth 401/403, or a
320322
// non-421 upgrade reject): waiting will not fix it, so quarantine
321-
// immediately -- exactly as the live sender's background loop
322-
// (CursorWebSocketSendLoop.connectLoop) halts on these errors.
323+
// immediately under the orphan reconnect policy.
323324
String msg = e.getMessage();
324325
LOG.error("drainer terminal upgrade/auth error for slot {}: {}", slotPath, msg);
325326
lastErrorMessage = msg;
@@ -639,7 +640,7 @@ public void run() {
639640
maxHeadFrameRejections,
640641
poisonMinEscalationWindowMillis,
641642
catchUpCapGapMinEscalationWindowMillis,
642-
CursorWebSocketSendLoop.CatchUpCapGapPolicy.TERMINAL_AFTER_SETTLE_BUDGET);
643+
CursorWebSocketSendLoop.ReconnectPolicy.ORPHAN);
643644
loop.start();
644645

645646
while (!stopRequestedOrInterrupted()) {

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

Lines changed: 143 additions & 77 deletions
Large diffs are not rendered by default.

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

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
package io.questdb.client.test.cutlass.qwp.client;
2626

2727
import io.questdb.client.Sender;
28-
import io.questdb.client.cutlass.line.LineSenderException;
2928
import io.questdb.client.test.cutlass.qwp.websocket.TestWebSocketServer;
3029
import org.junit.Assert;
3130
import org.junit.Test;
@@ -203,14 +202,13 @@ public void testReconnectNeverGivesUpInvariantB() throws Exception {
203202
}
204203

205204
@Test
206-
public void testTerminalUpgradeErrorAbortsReconnect() throws Exception {
205+
public void testPostStartAuthErrorRemainsBuffered() throws Exception {
207206
// Bespoke raw-socket fixture: first connection completes the
208207
// WebSocket upgrade and feeds back STATUS_OK ACKs; any subsequent
209208
// connection gets HTTP 401 Unauthorized — exercising the
210-
// auth-terminal path. With reconnect_max_duration_millis=10s and
211-
// a 401 happening on the very first reconnect, the cursor I/O
212-
// loop should surface the terminal error within hundreds of ms,
213-
// not after 10s.
209+
// post-start auth-rotation path. Once a foreground sender has been
210+
// live, store-and-forward owns its unacked data: repeated 401s must
211+
// remain contained and retried rather than stopping the producer.
214212
try (Auth401AfterFirstConnectionFixture fixture =
215213
new Auth401AfterFirstConnectionFixture()) {
216214
fixture.start();
@@ -224,9 +222,8 @@ public void testTerminalUpgradeErrorAbortsReconnect() throws Exception {
224222
// Wait for first connection to ACK + close
225223
waitFor(() -> fixture.acceptedConnections.get() >= 2, 5_000);
226224

227-
long t0 = System.nanoTime();
228225
Throwable observed = null;
229-
long deadline = System.currentTimeMillis() + 5_000;
226+
long deadline = System.currentTimeMillis() + 750;
230227
while (System.currentTimeMillis() < deadline) {
231228
try {
232229
sender.table("foo").longColumn("v", 2L).atNow();
@@ -237,23 +234,10 @@ public void testTerminalUpgradeErrorAbortsReconnect() throws Exception {
237234
}
238235
Thread.sleep(50);
239236
}
240-
long elapsedMs = (System.nanoTime() - t0) / 1_000_000L;
241-
Assert.assertNotNull("expected terminal error after auth rejection",
237+
Assert.assertNull("post-start auth rejection must stay buffered and retriable",
242238
observed);
243-
Assert.assertTrue(
244-
"terminal upgrade error must surface well inside the cap; took "
245-
+ elapsedMs + "ms (cap was 10000ms)",
246-
elapsedMs < 5_000);
247-
String msg = observed.getMessage() == null ? "" : observed.getMessage();
248-
Assert.assertTrue(
249-
"error must mention the terminal upgrade failure: " + msg,
250-
msg.contains("WebSocket upgrade failed")
251-
|| msg.contains("I/O thread failed")
252-
|| msg.contains("401"));
253-
} catch (LineSenderException ignored) {
239+
waitFor(() -> fixture.acceptedConnections.get() >= 3, 5_000);
254240
}
255-
// close() rethrows the latched terminal upgrade error
256-
// (commit 052f6ee). Already observed and asserted above.
257241
}
258242
}
259243

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/BackgroundDrainerDurableAckRetryTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ public void testTerminalUpgradeMarksFailedImmediately() throws Exception {
241241
CountingListener listener = new CountingListener();
242242
// A genuinely non-retriable upgrade error (non-421 5xx upgrade reject) is
243243
// terminal -- waiting will not fix it -- so the drainer quarantines on the
244-
// first attempt, exactly like the live sender's background loop halts on
245-
// auth/upgrade. A TRANSPORT error, by contrast, is transient and is
244+
// first attempt under the orphan reconnect policy. A TRANSPORT error,
245+
// by contrast, is transient and is
246246
// retried (see testTransportErrorNeverQuarantinesInvariantB).
247247
ScriptedFactory factory = ScriptedFactory.alwaysFailing(
248248
() -> new WebSocketUpgradeException(500, null, "server error during upgrade"));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
/*+*****************************************************************************
2+
* ___ _ ____ ____
3+
* / _ \ _ _ ___ ___| |_| _ \| __ )
4+
* | | | | | | |/ _ \/ __| __| | | | _ \
5+
* | |_| | |_| | __/\__ \ |_| |_| | |_) |
6+
* \__\_\\__,_|\___||___/\__|____/|____/
7+
*
8+
* Copyright (c) 2014-2019 Appsicle
9+
* Copyright (c) 2019-2026 QuestDB
10+
*
11+
* Licensed under the Apache License, Version 2.0 (the "License");
12+
* you may not use this file except in compliance with the License.
13+
* You may obtain a copy of the License at
14+
*
15+
* http://www.apache.org/licenses/LICENSE-2.0
16+
*
17+
* Unless required by applicable law or agreed to in writing, software
18+
* distributed under the License is distributed on an "AS IS" BASIS,
19+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
20+
* See the License for the specific language governing permissions and
21+
* limitations under the License.
22+
*
23+
******************************************************************************/
24+
25+
package io.questdb.client.test.cutlass.qwp.client.sf.cursor;
26+
27+
import io.questdb.client.cutlass.http.client.WebSocketClient;
28+
import io.questdb.client.cutlass.http.client.WebSocketClientFactory;
29+
import io.questdb.client.cutlass.qwp.client.QwpAuthFailedException;
30+
import io.questdb.client.cutlass.qwp.client.QwpDurableAckMismatchException;
31+
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
32+
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop;
33+
import io.questdb.client.std.MemoryTag;
34+
import io.questdb.client.std.Unsafe;
35+
import io.questdb.client.test.cutlass.qwp.websocket.TestWebSocketServer;
36+
import io.questdb.client.test.tools.TestUtils;
37+
import org.junit.Assert;
38+
import org.junit.Rule;
39+
import org.junit.Test;
40+
import org.junit.rules.TemporaryFolder;
41+
42+
import java.io.IOException;
43+
import java.nio.ByteBuffer;
44+
import java.nio.ByteOrder;
45+
import java.nio.charset.StandardCharsets;
46+
import java.util.IdentityHashMap;
47+
import java.util.Map;
48+
import java.util.concurrent.TimeUnit;
49+
import java.util.concurrent.atomic.AtomicInteger;
50+
51+
public class CursorWebSocketSendLoopForegroundReconnectPolicyTest {
52+
private static final long SEGMENT_SIZE_BYTES = 16_384L;
53+
54+
@Rule
55+
public final TemporaryFolder sfDir = TemporaryFolder.builder().assureDeletion().build();
56+
57+
@Test
58+
public void testPostStartAuthFailureRetriesUntilCredentialsRecover() throws Exception {
59+
assertForegroundRecovers(false,
60+
() -> new QwpAuthFailedException(401, "localhost", 1));
61+
}
62+
63+
@Test
64+
public void testPostStartDurableAckMismatchRetriesUntilCapabilityRecovers() throws Exception {
65+
assertForegroundRecovers(true,
66+
() -> new QwpDurableAckMismatchException("localhost", 1, "primary"));
67+
}
68+
69+
private void assertForegroundRecovers(boolean durableAck, FailureSupplier failureSupplier) throws Exception {
70+
TestUtils.assertMemoryLeak(() -> {
71+
DropFirstConnectionHandler handler = new DropFirstConnectionHandler(durableAck);
72+
try (TestWebSocketServer server = new TestWebSocketServer(handler, true);
73+
CursorSendEngine engine = new CursorSendEngine(
74+
sfDir.newFolder().getAbsolutePath(), SEGMENT_SIZE_BYTES)) {
75+
server.start();
76+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
77+
78+
WebSocketClient initialClient = connect(server.getPort(), durableAck);
79+
ScriptedFactory factory = new ScriptedFactory(
80+
server.getPort(), durableAck, failureSupplier);
81+
CursorWebSocketSendLoop loop = new CursorWebSocketSendLoop(
82+
initialClient,
83+
engine,
84+
0L,
85+
CursorWebSocketSendLoop.DEFAULT_PARK_NANOS,
86+
factory,
87+
100L,
88+
1L,
89+
4L,
90+
durableAck,
91+
durableAck ? 10L : 0L,
92+
CursorWebSocketSendLoop.DEFAULT_MAX_HEAD_FRAME_REJECTIONS,
93+
0L,
94+
0L,
95+
CursorWebSocketSendLoop.ReconnectPolicy.FOREGROUND);
96+
try {
97+
appendFrame(engine, (byte) 1);
98+
loop.start();
99+
100+
await(() -> loop.getTotalReconnects() >= 1L
101+
|| loop.getTerminalError() != null,
102+
"foreground reconnect did not complete");
103+
Assert.assertNull("post-start endpoint-policy failures must not stop the producer",
104+
loop.getTerminalError());
105+
Assert.assertTrue("the reconnect loop must retry both scripted failures",
106+
factory.attempts() >= 3);
107+
108+
long target = appendFrame(engine, (byte) 2);
109+
await(() -> engine.ackedFsn() >= target || loop.getTerminalError() != null,
110+
"foreground sender did not deliver after the policy failure cleared");
111+
Assert.assertNull(loop.getTerminalError());
112+
Assert.assertEquals(target, engine.ackedFsn());
113+
} finally {
114+
loop.close();
115+
initialClient.close();
116+
}
117+
}
118+
});
119+
}
120+
121+
private static long appendFrame(CursorSendEngine engine, byte marker) {
122+
long buffer = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT);
123+
try {
124+
for (int i = 0; i < 16; i++) {
125+
Unsafe.getUnsafe().putByte(buffer + i, marker);
126+
}
127+
return engine.appendBlocking(buffer, 16);
128+
} finally {
129+
Unsafe.free(buffer, 16, MemoryTag.NATIVE_DEFAULT);
130+
}
131+
}
132+
133+
private static void await(Condition condition, String failureMessage) throws Exception {
134+
long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
135+
while (!condition.isTrue()) {
136+
if (System.nanoTime() >= deadline) {
137+
Assert.fail(failureMessage);
138+
}
139+
Thread.sleep(1L);
140+
}
141+
}
142+
143+
private static WebSocketClient connect(int port, boolean durableAck) throws Exception {
144+
WebSocketClient client = WebSocketClientFactory.newPlainTextInstance();
145+
try {
146+
client.setQwpMaxVersion(1);
147+
client.setQwpRequestDurableAck(durableAck);
148+
client.setConnectTimeout(5_000);
149+
client.connect("localhost", port);
150+
client.upgrade("/write/v4", 5_000, null);
151+
return client;
152+
} catch (Throwable e) {
153+
client.close();
154+
throw e;
155+
}
156+
}
157+
158+
@FunctionalInterface
159+
private interface Condition {
160+
boolean isTrue() throws Exception;
161+
}
162+
163+
@FunctionalInterface
164+
private interface FailureSupplier {
165+
Exception get();
166+
}
167+
168+
private static final class ScriptedFactory implements CursorWebSocketSendLoop.ReconnectFactory {
169+
private final AtomicInteger attempts = new AtomicInteger();
170+
private final boolean durableAck;
171+
private final FailureSupplier failureSupplier;
172+
private final int port;
173+
174+
private ScriptedFactory(int port, boolean durableAck, FailureSupplier failureSupplier) {
175+
this.port = port;
176+
this.durableAck = durableAck;
177+
this.failureSupplier = failureSupplier;
178+
}
179+
180+
int attempts() {
181+
return attempts.get();
182+
}
183+
184+
@Override
185+
public WebSocketClient reconnect() throws Exception {
186+
if (attempts.incrementAndGet() <= 2) {
187+
throw failureSupplier.get();
188+
}
189+
return connect(port, durableAck);
190+
}
191+
}
192+
193+
private static final class DropFirstConnectionHandler
194+
implements TestWebSocketServer.WebSocketServerHandler {
195+
private static final String TABLE = "trades";
196+
private final boolean durableAck;
197+
private final Map<TestWebSocketServer.ClientHandler, long[]> wireSeqByConnection =
198+
new IdentityHashMap<>();
199+
private TestWebSocketServer.ClientHandler firstConnection;
200+
201+
private DropFirstConnectionHandler(boolean durableAck) {
202+
this.durableAck = durableAck;
203+
}
204+
205+
@Override
206+
public synchronized void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
207+
long[] sequence = wireSeqByConnection.get(client);
208+
if (sequence == null) {
209+
sequence = new long[1];
210+
wireSeqByConnection.put(client, sequence);
211+
if (firstConnection == null) {
212+
firstConnection = client;
213+
}
214+
}
215+
long wireSeq = sequence[0]++;
216+
try {
217+
client.sendBinary(okFrame(wireSeq));
218+
if (durableAck) {
219+
client.sendBinary(durableAckFrame(wireSeq));
220+
}
221+
if (client == firstConnection) {
222+
client.close();
223+
}
224+
} catch (IOException ignored) {
225+
// The deliberate close may race the acknowledgement write.
226+
}
227+
}
228+
229+
private static byte[] durableAckFrame(long seqTxn) {
230+
byte[] name = TABLE.getBytes(StandardCharsets.UTF_8);
231+
ByteBuffer bb = ByteBuffer.allocate(1 + 2 + 2 + name.length + 8)
232+
.order(ByteOrder.LITTLE_ENDIAN);
233+
bb.put((byte) 0x02);
234+
bb.putShort((short) 1);
235+
bb.putShort((short) name.length);
236+
bb.put(name);
237+
bb.putLong(seqTxn);
238+
return bb.array();
239+
}
240+
241+
private static byte[] okFrame(long wireSeq) {
242+
byte[] name = TABLE.getBytes(StandardCharsets.UTF_8);
243+
ByteBuffer bb = ByteBuffer.allocate(1 + 8 + 2 + 2 + name.length + 8)
244+
.order(ByteOrder.LITTLE_ENDIAN);
245+
bb.put((byte) 0x00);
246+
bb.putLong(wireSeq);
247+
bb.putShort((short) 1);
248+
bb.putShort((short) name.length);
249+
bb.put(name);
250+
bb.putLong(wireSeq);
251+
return bb.array();
252+
}
253+
}
254+
}

0 commit comments

Comments
 (0)