Skip to content

Commit 80e69c9

Browse files
committed
Harden QWP recovery and shutdown
Cursor engine closure now retains slot ownership until manager quiescence and transfers stalled cleanup to a dedicated daemon. Recovery validates persisted segments through bounded positional reads before mmap, preserves facade ownership, and retains corrupt data for diagnosis. Deterministic tests cover lifecycle races, compatibility, recovery faults, and slot reuse.
1 parent 358005c commit 80e69c9

17 files changed

Lines changed: 1160 additions & 443 deletions

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

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,9 @@ public class QwpWebSocketSender implements Sender {
219219
// into the engine's mmap'd ring; the cursorSendLoop is the I/O thread
220220
// that walks the ring and sends frames.
221221
private CursorSendEngine cursorEngine;
222+
// True after a failed I/O-thread stop handed engine ownership to that
223+
// thread's exit path. Repeated sender close calls must not race that owner.
224+
private boolean cursorEngineCloseDelegated;
222225
private CursorWebSocketSendLoop cursorSendLoop;
223226
private boolean deferCommit;
224227
// User-supplied observer for background orphan-slot drainer events.
@@ -1043,8 +1046,20 @@ public QwpWebSocketSender charColumn(CharSequence columnName, char value) {
10431046
*/
10441047
@Override
10451048
public void close() {
1046-
if (!closed) {
1047-
closed = true;
1049+
if (closed) {
1050+
// A prior close may have timed out waiting for manager quiescence.
1051+
// Retry only when this thread still owns cleanup; delegated engine
1052+
// closes belong exclusively to the I/O thread's exit path.
1053+
if (!cursorEngineCloseDelegated) {
1054+
try {
1055+
closeOwnedCursorEngine();
1056+
} catch (Throwable t) {
1057+
LOG.error("Error retrying owned CursorSendEngine close: {}", String.valueOf(t));
1058+
}
1059+
}
1060+
return;
1061+
}
1062+
closed = true;
10481063
boolean ioThreadStopped = true;
10491064
// Captures the first error from the flush/drain path AND any
10501065
// secondary errors from cleanup steps (added via addSuppressed).
@@ -1187,17 +1202,17 @@ public void close() {
11871202
// close actually runs, so the pool must not reuse the slot
11881203
// meanwhile. A false return means the thread exited between
11891204
// the failed close() and now — then closing here is safe.
1190-
if (ownsCursorEngine && cursorEngine != null && cursorSendLoop != null
1191-
&& !cursorSendLoop.delegateEngineClose()) {
1192-
try {
1193-
cursorEngine.close();
1194-
} catch (Throwable t) {
1195-
LOG.error("Error closing owned CursorSendEngine: {}", String.valueOf(t));
1196-
terminalError = captureCloseError(terminalError, t);
1205+
if (ownsCursorEngine && cursorEngine != null && cursorSendLoop != null) {
1206+
if (cursorSendLoop.delegateEngineClose()) {
1207+
cursorEngineCloseDelegated = true;
1208+
} else {
1209+
try {
1210+
closeOwnedCursorEngine();
1211+
} catch (Throwable t) {
1212+
LOG.error("Error closing owned CursorSendEngine: {}", String.valueOf(t));
1213+
terminalError = captureCloseError(terminalError, t);
1214+
}
11971215
}
1198-
cursorEngine = null;
1199-
ownsCursorEngine = false;
1200-
slotLockReleased = true;
12011216
}
12021217
rethrowTerminal(terminalError);
12031218
return;
@@ -1232,18 +1247,15 @@ public void close() {
12321247

12331248
if (ownsCursorEngine && cursorEngine != null) {
12341249
try {
1235-
cursorEngine.close();
1250+
closeOwnedCursorEngine();
12361251
} catch (Throwable t) {
12371252
LOG.error("Error closing owned CursorSendEngine: {}", String.valueOf(t));
12381253
terminalError = captureCloseError(terminalError, t);
12391254
}
1240-
cursorEngine = null;
1241-
ownsCursorEngine = false;
1255+
} else {
1256+
// This sender never owned an engine holding the pool's slot.
1257+
slotLockReleased = true;
12421258
}
1243-
// Past the ioThreadStopped guard => cursorEngine.close() ran and
1244-
// released the SF flock in its finally (or this sender owned no
1245-
// engine holding one). Signal the pool it may reuse the slot.
1246-
slotLockReleased = true;
12471259

12481260
// Shutdown order: dispatcher last, after the I/O loop has stopped
12491261
// producing into it. close() drains pending entries with a short
@@ -1283,7 +1295,6 @@ public void close() {
12831295
terminalError = null;
12841296
}
12851297
rethrowTerminal(terminalError);
1286-
}
12871298
}
12881299

12891300
/**
@@ -1293,7 +1304,8 @@ public void close() {
12931304
* slot index reserved instead of reusing the still-locked slot dir.
12941305
*/
12951306
public boolean isSlotLockReleased() {
1296-
return slotLockReleased;
1307+
CursorSendEngine engine = cursorEngine;
1308+
return slotLockReleased || (engine != null && engine.isCloseCompleted());
12971309
}
12981310

12991311
@Override
@@ -2527,6 +2539,24 @@ public boolean wasEverConnected() {
25272539
return l != null && l.hasEverConnected();
25282540
}
25292541

2542+
private void closeOwnedCursorEngine() {
2543+
if (!ownsCursorEngine || cursorEngine == null || cursorEngineCloseDelegated) {
2544+
return;
2545+
}
2546+
cursorEngine.close();
2547+
if (cursorEngine.isCloseCompleted()) {
2548+
cursorEngine = null;
2549+
ownsCursorEngine = false;
2550+
slotLockReleased = true;
2551+
} else {
2552+
// Preserve the sole owner reference for a later retry. The pool
2553+
// observes false and retires this index rather than reusing a
2554+
// directory whose flock is still held.
2555+
slotLockReleased = false;
2556+
cursorEngine.closeEventually();
2557+
}
2558+
}
2559+
25302560
private static Throwable captureCloseError(Throwable terminalError, Throwable t) {
25312561
if (terminalError == null) {
25322562
return t;

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,10 +704,15 @@ public void run() {
704704
// between the failed close() and now: then it is safe (and
705705
// necessary) to close the engine here.
706706
if (ioThreadStopped || !loop.delegateEngineClose()) {
707+
// Make one bounded attempt. If manager quiescence times
708+
// out, transfer ownership so this pool task can terminate.
707709
try {
708-
// engine.close() releases the slot lock too.
709710
engine.close();
710711
} catch (Throwable ignored) {
712+
// The deferred owner retries below when needed.
713+
}
714+
if (!engine.isCloseCompleted()) {
715+
engine.closeEventually();
711716
}
712717
} else {
713718
LOG.warn("drainer slot {}: engine close delegated to the I/O thread; "

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

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ public final class CursorSendEngine implements QuietCloseable {
6161
* Default deadline for {@link #appendBlocking}: 30 seconds.
6262
*/
6363
public static final long DEFAULT_APPEND_DEADLINE_NANOS = 30_000_000_000L;
64+
private static final java.util.concurrent.ExecutorService DEFERRED_CLOSE_EXECUTOR =
65+
java.util.concurrent.Executors.newCachedThreadPool(runnable -> {
66+
Thread thread = new Thread(runnable, "qdb-cursor-engine-close");
67+
thread.setDaemon(true);
68+
return thread;
69+
});
6470
private static final org.slf4j.Logger LOG =
6571
org.slf4j.LoggerFactory.getLogger(CursorSendEngine.class);
6672
private final long appendDeadlineNanos;
@@ -118,8 +124,11 @@ public final class CursorSendEngine implements QuietCloseable {
118124
// a close attempt could not confirm manager-worker quiescence and had to
119125
// leak the ring/watermark/slot lock — in that case a later close() call
120126
// retries the cleanup (the worker may have exited by then). Guarded by
121-
// the synchronized close() method; never read elsewhere.
127+
// the synchronized close() method and isCloseCompleted() accessor.
122128
private boolean closeCompleted;
129+
// True once a dedicated daemon has accepted ownership of incomplete close
130+
// retries. Guarded by synchronized closeEventually().
131+
private boolean deferredCloseScheduled;
123132
// Producer-thread-only: timestamp of the last "we're backpressured" log
124133
// line, used to throttle. Plain long is fine.
125134
private long lastBackpressureLogNs;
@@ -613,6 +622,25 @@ public synchronized void close() {
613622
}
614623
}
615624

625+
/**
626+
* Transfers an incomplete close to a dedicated daemon owner. This lets
627+
* I/O and drainer executors terminate even when a manager service pass is
628+
* permanently stalled. The deferred owner retains this engine, and thus
629+
* its flock and native mappings, until a retry completes.
630+
*/
631+
public synchronized void closeEventually() {
632+
if (closeCompleted || deferredCloseScheduled) {
633+
return;
634+
}
635+
deferredCloseScheduled = true;
636+
try {
637+
DEFERRED_CLOSE_EXECUTOR.execute(this::runDeferredClose);
638+
} catch (Throwable t) {
639+
deferredCloseScheduled = false;
640+
throw t;
641+
}
642+
}
643+
616644
/**
617645
* Pass-through to {@link SegmentRing#findSegmentContaining(long)}.
618646
*/
@@ -637,6 +665,16 @@ public long getTotalBackpressureStalls() {
637665
return backpressureStallCount.get();
638666
}
639667

668+
/**
669+
* True only after close released every engine-owned resource, including
670+
* the SF slot flock. A false result means close deliberately retained the
671+
* ring/watermark/lock because the manager worker did not quiesce; owners
672+
* must preserve this engine and retry later.
673+
*/
674+
public synchronized boolean isCloseCompleted() {
675+
return closeCompleted;
676+
}
677+
640678
/**
641679
* Pass-through to {@link SegmentRing#nextSealedAfter(MmapSegment)}.
642680
*/
@@ -718,6 +756,23 @@ public long recoveredOrphanTipFsn() {
718756
* Best-effort: logs and continues on failures, since we're already on
719757
* the close path.
720758
*/
759+
private void runDeferredClose() {
760+
while (!isCloseCompleted()) {
761+
try {
762+
close();
763+
} catch (Throwable ignored) {
764+
// Retain ownership and retry after a bounded pause.
765+
}
766+
if (!isCloseCompleted()) {
767+
LockSupport.parkNanos(10_000_000L);
768+
// The internal daemon owns cleanup independently of caller
769+
// cancellation. Clear interrupts so they cannot create a hot
770+
// retry loop if an executor implementation interrupts it.
771+
Thread.interrupted();
772+
}
773+
}
774+
}
775+
721776
private static void unlinkAllSegmentFiles(String dir) {
722777
if (!io.questdb.client.std.Files.exists(dir)) return;
723778
long find = io.questdb.client.std.Files.findFirst(dir);

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1679,10 +1679,16 @@ private void ioLoop() {
16791679
// either this branch or the owner's fallback runs (or both —
16801680
// engine.close() is idempotent).
16811681
if (engineCloseDelegated) {
1682+
// Make one bounded attempt on this exit path. If manager
1683+
// quiescence times out, transfer ownership to the dedicated
1684+
// deferred closer so this I/O daemon can terminate.
16821685
try {
16831686
engine.close();
16841687
} catch (Throwable ignored) {
1685-
// best-effort
1688+
// The deferred owner retries below when needed.
1689+
}
1690+
if (!engine.isCloseCompleted()) {
1691+
engine.closeEventually();
16861692
}
16871693
}
16881694
}

0 commit comments

Comments
 (0)