Skip to content

Commit 3caa2d3

Browse files
bluestreak01claude
andcommitted
feat(ilp): close() drain — bounded ACK wait via close_flush_timeout_millis
QwpWebSocketSender.close() previously stopped the cursor I/O loop the moment it was called: any frames already published into the engine but not yet sent (or sent but not yet ACK'd) were silently dropped on JVM exit. In memory mode that means data loss; in SF mode the next sender recovers from disk, but the durability claim of close() was weaker than the spec promised. Closes the gap with one knob from the durability spec (design/qwp-cursor-durability.md, decision #3): close_flush_timeout_millis (default 5000) > 0: close() blocks until ackedFsn >= publishedFsn or timeout 0/-1: fast close — no drain, opt-in to legacy fast-exit behavior On timeout, log WARN and proceed with shutdown. SF-mode pending data is recoverable; memory-mode pending data is not. Wired through: - LineSenderBuilder.closeFlushTimeoutMillis(long) - connect-string key close_flush_timeout_millis - new QwpWebSocketSender.connect overload that takes the timeout Tests cover all three regimes: - delayed-ACK server: close blocks ~ack delay - timeout=0: close returns immediately - silent server: close times out at the configured cap, logs WARN This is decision #3 of the spec; subsequent commits add the connectionGeneration foundation, reconnect/replay, slot dirs, and background drainers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4f4e1e5 commit 3caa2d3

3 files changed

Lines changed: 281 additions & 6 deletions

File tree

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,10 @@ public int getTimeout() {
658658
// never approaches it. SF mode = 10 GiB (2560 segments at default
659659
// size). Users can lower this on space-constrained hosts.
660660
private static final long DEFAULT_MAX_BYTES_SF = 10L * 1024 * 1024 * 1024;
661+
// Default close() drain timeout: block up to 5s waiting for the
662+
// server to ACK everything published into the engine before
663+
// shutting down the I/O loop.
664+
private static final long DEFAULT_CLOSE_FLUSH_TIMEOUT_MILLIS = 5_000L;
661665
// Store-and-forward (WebSocket only). SF is enabled iff sfDir is non-null —
662666
// there is no separate on/off flag (presence of the directory is the switch).
663667
// null sfDir → memory-only async ingest (same lock-free architecture, no disk).
@@ -668,6 +672,10 @@ public int getTimeout() {
668672
// implemented; FLUSH and APPEND are deferred follow-ups (cursor needs
669673
// to learn fsync first).
670674
private SfDurability sfDurability = SfDurability.MEMORY;
675+
// close() drain timeout. Default applied at build() time. 0 or -1
676+
// means "fast close" (skip the drain entirely); any positive value
677+
// bounds the wait for ackedFsn to catch up to publishedFsn.
678+
private long closeFlushTimeoutMillis = PARAMETER_NOT_SET_EXPLICITLY;
671679
private boolean tlsEnabled;
672680
private TlsValidationMode tlsValidationMode;
673681
private char[] trustStorePassword;
@@ -996,6 +1004,9 @@ public Sender build() {
9961004
long actualSfMaxTotalBytes = sfMaxTotalBytes == PARAMETER_NOT_SET_EXPLICITLY
9971005
? Math.max(defaultMaxTotal, actualSfMaxBytes * 2)
9981006
: sfMaxTotalBytes;
1007+
long actualCloseFlushTimeoutMillis = closeFlushTimeoutMillis == PARAMETER_NOT_SET_EXPLICITLY
1008+
? DEFAULT_CLOSE_FLUSH_TIMEOUT_MILLIS
1009+
: closeFlushTimeoutMillis;
9991010

10001011
CursorSendEngine cursorEngine = new CursorSendEngine(
10011012
sfDir, actualSfMaxBytes,
@@ -1012,7 +1023,8 @@ public Sender build() {
10121023
wsAuthHeader,
10131024
actualMaxSchemasPerConnection,
10141025
requestDurableAck,
1015-
cursorEngine
1026+
cursorEngine,
1027+
actualCloseFlushTimeoutMillis
10161028
);
10171029
} catch (Throwable t) {
10181030
try {
@@ -1662,6 +1674,26 @@ public LineSenderBuilder storeAndForwardMaxTotalBytes(long maxTotalBytes) {
16621674
return this;
16631675
}
16641676

1677+
/**
1678+
* close() drain timeout in milliseconds. The sender's {@code close()}
1679+
* method blocks up to this many millis waiting for the server to ACK
1680+
* every batch already published into the engine before shutting down
1681+
* the I/O loop. Default {@code 5000}.
1682+
* <p>
1683+
* Set to {@code 0} or {@code -1} to opt out — close() will not wait
1684+
* at all (fast close). Pending data is then lost in memory mode and
1685+
* recovered by the next sender in SF mode.
1686+
* <p>
1687+
* WebSocket transport only.
1688+
*/
1689+
public LineSenderBuilder closeFlushTimeoutMillis(long timeoutMillis) {
1690+
if (protocol != PARAMETER_NOT_SET_EXPLICITLY && protocol != PROTOCOL_WEBSOCKET) {
1691+
throw new LineSenderException("close_flush_timeout_millis is only supported for WebSocket transport");
1692+
}
1693+
this.closeFlushTimeoutMillis = timeoutMillis;
1694+
return this;
1695+
}
1696+
16651697
/**
16661698
* Selects the durability contract for SF appends and flushes. See
16671699
* {@link SfDurability} for the value semantics.
@@ -2190,6 +2222,12 @@ private LineSenderBuilder fromConfig(CharSequence configurationString) {
21902222
}
21912223
pos = getValue(configurationString, pos, sink, "sf_durability");
21922224
storeAndForwardDurability(parseDurabilityValue(sink));
2225+
} else if (Chars.equals("close_flush_timeout_millis", sink)) {
2226+
if (protocol != PROTOCOL_WEBSOCKET) {
2227+
throw new LineSenderException("close_flush_timeout_millis is only supported for WebSocket transport");
2228+
}
2229+
pos = getValue(configurationString, pos, sink, "close_flush_timeout_millis");
2230+
closeFlushTimeoutMillis(parseLongValue(sink, "close_flush_timeout_millis"));
21932231
} else if (Chars.equals("max_datagram_size", sink)) {
21942232
pos = getValue(configurationString, pos, sink, "max_datagram_size");
21952233
int mds = parseIntValue(sink, "max_datagram_size");

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

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ public class QwpWebSocketSender implements Sender {
160160
private CursorSendEngine cursorEngine;
161161
private boolean ownsCursorEngine;
162162
private CursorWebSocketSendLoop cursorSendLoop;
163+
// close() drain timeout in millis. Default applied at construction.
164+
// 0 or -1 means "fast close" (skip the drain); otherwise close blocks
165+
// up to this many millis for ackedFsn to catch up to publishedFsn.
166+
private long closeFlushTimeoutMillis = 5_000L;
163167

164168
private QwpWebSocketSender(
165169
String host,
@@ -267,6 +271,31 @@ public static QwpWebSocketSender connect(
267271
int maxSchemasPerConnection,
268272
boolean requestDurableAck,
269273
CursorSendEngine cursorEngine
274+
) {
275+
return connect(host, port, tlsConfig, autoFlushRows, autoFlushBytes, autoFlushIntervalNanos,
276+
inFlightWindowSize, authorizationHeader, maxSchemasPerConnection,
277+
requestDurableAck, cursorEngine, 5_000L);
278+
}
279+
280+
/**
281+
* Connect overload that also configures the {@code close()} drain
282+
* timeout. {@code 0} or {@code -1} disables the drain (fast close);
283+
* any positive value bounds the wait for {@code ackedFsn} to catch
284+
* up to {@code publishedFsn} during {@code close()}.
285+
*/
286+
public static QwpWebSocketSender connect(
287+
String host,
288+
int port,
289+
ClientTlsConfiguration tlsConfig,
290+
int autoFlushRows,
291+
int autoFlushBytes,
292+
long autoFlushIntervalNanos,
293+
int inFlightWindowSize,
294+
String authorizationHeader,
295+
int maxSchemasPerConnection,
296+
boolean requestDurableAck,
297+
CursorSendEngine cursorEngine,
298+
long closeFlushTimeoutMillis
270299
) {
271300
QwpWebSocketSender sender = new QwpWebSocketSender(
272301
host, port, tlsConfig,
@@ -275,6 +304,7 @@ public static QwpWebSocketSender connect(
275304
);
276305
try {
277306
sender.requestDurableAck = requestDurableAck;
307+
sender.closeFlushTimeoutMillis = closeFlushTimeoutMillis;
278308
if (cursorEngine != null) {
279309
sender.setCursorEngine(cursorEngine, true);
280310
}
@@ -485,16 +515,21 @@ public void close() {
485515
// up — close() is also called from createForTesting() teardown
486516
// and from connect() rollback paths where one or both may be null.
487517
if (connectionError.get() == null && cursorEngine != null && cursorSendLoop != null) {
488-
// Flush user-thread state (accumulated rows -> microbatch ->
489-
// engine append). The cursor engine append is durable
490-
// (on-disk in SF mode, in-RAM in memory mode); we don't
491-
// wait for the cursor I/O loop to acknowledge — the data
492-
// is already past the in-flight handoff.
518+
// 1) Flush user-thread state into the engine (encoded
519+
// rows → mmap'd / malloc'd ring). After this, the
520+
// cursor engine's publishedFsn reflects the final
521+
// target the I/O loop must drive ackedFsn up to.
493522
flushPendingRows();
494523
if (activeBuffer != null && activeBuffer.hasData()) {
495524
sealAndSwapBuffer();
496525
}
497526
cursorSendLoop.checkError();
527+
// 2) Bounded drain: block until the server has ACK'd
528+
// everything we just published, or until the
529+
// configured timeout elapses. closeFlushTimeoutMillis
530+
// <= 0 opts out (fast close, may lose memory-mode
531+
// data on JVM exit).
532+
drainOnClose();
498533
}
499534
} catch (Exception e) {
500535
LOG.error("Error during close: {}", String.valueOf(e));
@@ -1417,6 +1452,33 @@ private void resetSchemaStateForNewConnection() {
14171452
}
14181453
}
14191454

1455+
/**
1456+
* Bounded drain on close: block until {@code ackedFsn >= publishedFsn}
1457+
* or until {@code closeFlushTimeoutMillis} elapses. {@code <= 0} skips
1458+
* the drain (fast close). On timeout, log a WARN and proceed with
1459+
* shutdown — pending data is lost in memory mode and recovered by
1460+
* the next sender in SF mode.
1461+
*/
1462+
private void drainOnClose() {
1463+
if (closeFlushTimeoutMillis <= 0L) {
1464+
return;
1465+
}
1466+
long target = cursorEngine.publishedFsn();
1467+
if (cursorEngine.ackedFsn() >= target) {
1468+
return;
1469+
}
1470+
long deadlineNanos = System.nanoTime() + closeFlushTimeoutMillis * 1_000_000L;
1471+
while (cursorEngine.ackedFsn() < target) {
1472+
cursorSendLoop.checkError();
1473+
if (System.nanoTime() >= deadlineNanos) {
1474+
LOG.warn("close() drain timed out after {}ms [target={} acked={}] — pending data may be lost",
1475+
closeFlushTimeoutMillis, target, cursorEngine.ackedFsn());
1476+
return;
1477+
}
1478+
java.util.concurrent.locks.LockSupport.parkNanos(50_000L);
1479+
}
1480+
}
1481+
14201482
private void rollbackRow() {
14211483
if (currentTableBuffer != null) {
14221484
currentTableBuffer.cancelCurrentRow();
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
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;
26+
27+
import io.questdb.client.Sender;
28+
import io.questdb.client.test.cutlass.qwp.websocket.TestWebSocketServer;
29+
import org.junit.Assert;
30+
import org.junit.Test;
31+
32+
import java.io.IOException;
33+
import java.nio.ByteBuffer;
34+
import java.nio.ByteOrder;
35+
import java.util.concurrent.TimeUnit;
36+
import java.util.concurrent.atomic.AtomicLong;
37+
38+
/**
39+
* Regression tests for the close() drain semantics specified in
40+
* design/qwp-cursor-durability.md.
41+
* <p>
42+
* Without {@code close_flush_timeout_millis}, close() returned as soon as
43+
* the cursor I/O loop's {@code running} flag flipped — meaning frames
44+
* still queued in the engine could be dropped when the JVM exited
45+
* immediately after close(). The drain timeout makes close() wait for
46+
* the server to ACK everything published before shutting the loop down.
47+
*/
48+
public class CloseDrainTest {
49+
50+
private static final int TEST_PORT = 19_700 + (int) (System.nanoTime() % 100);
51+
52+
@Test
53+
public void testCloseBlocksUntilAckArrives() throws Exception {
54+
// Server delays every ACK by 800ms. With the default
55+
// close_flush_timeout_millis=5000, close() must wait for that ACK
56+
// before returning. Pre-fix close() returned within milliseconds.
57+
int port = TEST_PORT + 1;
58+
long ackDelayMs = 800;
59+
DelayingAckHandler handler = new DelayingAckHandler(ackDelayMs);
60+
try (TestWebSocketServer server = new TestWebSocketServer(port, handler)) {
61+
server.start();
62+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
63+
64+
String cfg = "ws::addr=localhost:" + port + ";"; // memory mode
65+
long elapsedMs;
66+
try (Sender sender = Sender.fromConfig(cfg)) {
67+
sender.table("foo").longColumn("v", 1L).atNow();
68+
sender.flush();
69+
long t0 = System.nanoTime();
70+
sender.close();
71+
elapsedMs = (System.nanoTime() - t0) / 1_000_000;
72+
}
73+
Assert.assertTrue(
74+
"close() took only " + elapsedMs + "ms — did not wait for ACK; "
75+
+ "drain timeout is broken or never enabled",
76+
elapsedMs >= ackDelayMs / 2);
77+
}
78+
}
79+
80+
@Test
81+
public void testCloseFastWhenTimeoutIsZero() throws Exception {
82+
// Same delayed-ACK server, but with close_flush_timeout_millis=0
83+
// (fast close). close() must return immediately, well before the
84+
// ACK delay would have elapsed.
85+
int port = TEST_PORT + 2;
86+
long ackDelayMs = 1500;
87+
DelayingAckHandler handler = new DelayingAckHandler(ackDelayMs);
88+
try (TestWebSocketServer server = new TestWebSocketServer(port, handler)) {
89+
server.start();
90+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
91+
92+
String cfg = "ws::addr=localhost:" + port
93+
+ ";close_flush_timeout_millis=0;";
94+
long elapsedMs;
95+
try (Sender sender = Sender.fromConfig(cfg)) {
96+
sender.table("foo").longColumn("v", 1L).atNow();
97+
sender.flush();
98+
long t0 = System.nanoTime();
99+
sender.close();
100+
elapsedMs = (System.nanoTime() - t0) / 1_000_000;
101+
}
102+
Assert.assertTrue(
103+
"close() with timeout=0 took " + elapsedMs + "ms — fast close is broken",
104+
elapsedMs < ackDelayMs / 2);
105+
}
106+
}
107+
108+
@Test
109+
public void testCloseDrainTimesOutWhenAcksNeverArrive() throws Exception {
110+
// Server that buffers frames silently and never ACKs. close() must
111+
// return after roughly the configured timeout — not hang forever
112+
// and not return immediately.
113+
int port = TEST_PORT + 3;
114+
long timeoutMs = 500;
115+
SilentHandler handler = new SilentHandler();
116+
try (TestWebSocketServer server = new TestWebSocketServer(port, handler)) {
117+
server.start();
118+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
119+
120+
String cfg = "ws::addr=localhost:" + port
121+
+ ";close_flush_timeout_millis=" + timeoutMs + ";";
122+
long elapsedMs;
123+
try (Sender sender = Sender.fromConfig(cfg)) {
124+
sender.table("foo").longColumn("v", 1L).atNow();
125+
sender.flush();
126+
long t0 = System.nanoTime();
127+
sender.close();
128+
elapsedMs = (System.nanoTime() - t0) / 1_000_000;
129+
}
130+
Assert.assertTrue("close() returned too early: " + elapsedMs + "ms",
131+
elapsedMs >= timeoutMs);
132+
Assert.assertTrue("close() exceeded the bounded timeout by too much: " + elapsedMs + "ms",
133+
elapsedMs < timeoutMs * 4);
134+
}
135+
}
136+
137+
/** Acks every binary frame after a fixed delay, so we can observe close() blocking. */
138+
private static class DelayingAckHandler implements TestWebSocketServer.WebSocketServerHandler {
139+
private final long delayMs;
140+
private final AtomicLong nextSeq = new AtomicLong(0);
141+
142+
DelayingAckHandler(long delayMs) {
143+
this.delayMs = delayMs;
144+
}
145+
146+
@Override
147+
public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
148+
try {
149+
Thread.sleep(delayMs);
150+
client.sendBinary(buildAck(nextSeq.getAndIncrement()));
151+
} catch (IOException | InterruptedException e) {
152+
Thread.currentThread().interrupt();
153+
throw new RuntimeException(e);
154+
}
155+
}
156+
}
157+
158+
/** Receives but never ACKs — used to verify close() honors its timeout cap. */
159+
private static class SilentHandler implements TestWebSocketServer.WebSocketServerHandler {
160+
@Override
161+
public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
162+
// intentionally drop the frame on the floor
163+
}
164+
}
165+
166+
// Mirrors WebSocketResponse STATUS_OK layout: status u8 | sequence u64 | table_count u16
167+
static byte[] buildAck(long seq) {
168+
byte[] buf = new byte[1 + 8 + 2];
169+
ByteBuffer bb = ByteBuffer.wrap(buf).order(ByteOrder.LITTLE_ENDIAN);
170+
bb.put((byte) 0x00); // STATUS_OK
171+
bb.putLong(seq);
172+
bb.putShort((short) 0);
173+
return buf;
174+
}
175+
}

0 commit comments

Comments
 (0)