Skip to content

Commit b89384f

Browse files
committed
fix(qwp): recover SF slots after deferred cleanup
1 parent 79e0847 commit b89384f

16 files changed

Lines changed: 1387 additions & 234 deletions

core/src/main/c/share/files.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,18 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_lock
236236
return flock((int) fd, LOCK_EX | LOCK_NB);
237237
}
238238

239+
JNIEXPORT jint JNICALL Java_io_questdb_client_cutlass_qwp_client_sf_cursor_SlotLock_release0
240+
(JNIEnv *e, jclass cl, jint fd) {
241+
if (flock((int) fd, LOCK_UN) != 0) {
242+
return -1;
243+
}
244+
/* Unlock success confirms that the slot is reusable. close() is one-shot:
245+
* POSIX leaves descriptor state unspecified on EINTR, so retrying its
246+
* numeric value could close an unrelated descriptor after reuse. */
247+
(void) close((int) fd);
248+
return 0;
249+
}
250+
239251
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_mkdir0
240252
(JNIEnv *e, jclass cl, jlong lpszPath, jint mode) {
241253
return mkdir((const char *) (uintptr_t) lpszPath, (mode_t) mode);

core/src/main/c/windows/files.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,20 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_lock
331331
return 0;
332332
}
333333

334+
JNIEXPORT jint JNICALL Java_io_questdb_client_cutlass_qwp_client_sf_cursor_SlotLock_release0
335+
(JNIEnv *e, jclass cl, jint fd) {
336+
OVERLAPPED ov;
337+
memset(&ov, 0, sizeof(ov));
338+
if (!UnlockFileEx(FD_TO_HANDLE(fd), 0, MAXDWORD, MAXDWORD, &ov)) {
339+
SaveLastError();
340+
return -1;
341+
}
342+
/* Unlock success confirms that the slot is reusable. Match POSIX by
343+
* closing once without making handle cleanup part of that signal. */
344+
(void) CloseHandle(FD_TO_HANDLE(fd));
345+
return 0;
346+
}
347+
334348
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_mkdir0
335349
(JNIEnv *e, jclass cl, jlong lpszPath, jint mode) {
336350
(void) mode;

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

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ public class QwpWebSocketSender implements Sender {
194194
// up to this many millis for ackedFsn to catch up to publishedFsn.
195195
private long closeFlushTimeoutMillis = 5_000L;
196196
private volatile boolean closed;
197+
// Test-only lifecycle witness. close() invokes and clears it strictly after
198+
// publishing closed=true and before starting any drain or teardown work.
199+
private volatile Runnable closeStartedHook;
197200
private boolean connected;
198201
private SenderConnectionDispatcher connectionDispatcher;
199202
// Async-delivery sink for SenderConnectionEvent notifications. Default
@@ -1057,6 +1060,16 @@ public QwpWebSocketSender charColumn(CharSequence columnName, char value) {
10571060
public void close() {
10581061
if (!closed) {
10591062
closed = true;
1063+
Runnable hook = closeStartedHook;
1064+
closeStartedHook = null;
1065+
if (hook != null) {
1066+
try {
1067+
hook.run();
1068+
} catch (Throwable t) {
1069+
// A test witness must never prevent production resource cleanup.
1070+
LOG.error("Error in close-started test hook: {}", String.valueOf(t));
1071+
}
1072+
}
10601073
boolean ioThreadStopped = true;
10611074
// Captures the first error from the flush/drain path AND any
10621075
// secondary errors from cleanup steps (added via addSuppressed).
@@ -1275,10 +1288,10 @@ public void close() {
12751288
// The manager worker did not quiesce. Preserve ownership
12761289
// and report the retained flock so pools retire this slot.
12771290
// Repeated Sender.close() calls remain no-ops by contract.
1278-
// Engine cleanup was handed to the worker's exit path
1279-
// (owned manager); the getter re-probes the retained
1280-
// engine so the pool can reclaim the slot once cleanup
1281-
// actually completes.
1291+
// Engine cleanup was handed to a safe manager-worker path:
1292+
// owned-manager exit or shared-manager ring-pass completion.
1293+
// The getter re-probes the retained engine so the pool can
1294+
// reclaim the slot once cleanup actually completes.
12821295
slotLockReleased = false;
12831296
retainedEngine = engine;
12841297
}
@@ -1335,8 +1348,8 @@ public void close() {
13351348
* index reserved instead of reusing the still-locked dir.
13361349
* <p>
13371350
* Not a one-shot snapshot: when close() left engine cleanup pending on a
1338-
* worker/I/O-thread exit path, this re-probes the retained engine and
1339-
* latches true the moment that cleanup completes — pools re-probe retired
1351+
* manager-worker quiescence or I/O-thread exit path, this re-probes the
1352+
* retained engine and latches true the moment that cleanup completes — pools re-probe retired
13401353
* slots through this getter to recover their capacity. Monotonic:
13411354
* false→true only, never back. Lock-free (volatile reads) so pools may
13421355
* call it under their capacity lock.
@@ -2194,6 +2207,16 @@ public void setClientFactoryOverride(java.util.function.Supplier<WebSocketClient
21942207
this.clientFactoryOverride = factory;
21952208
}
21962209

2210+
/**
2211+
* Installs a one-shot test witness that {@link #close()} invokes after it
2212+
* publishes the closed-state transition and before it starts drain or
2213+
* teardown work. Production code never sets it.
2214+
*/
2215+
@TestOnly
2216+
public void setCloseStartedHook(Runnable hook) {
2217+
this.closeStartedHook = hook;
2218+
}
2219+
21972220
@Override
21982221
public void reset() {
21992222
checkNotClosed();
@@ -2262,7 +2285,9 @@ public void setConnectionListenerInboxCapacity(int capacity) {
22622285

22632286
/**
22642287
* Attach a {@link CursorSendEngine} for store-and-forward. Must be called
2265-
* before the first send.
2288+
* before the first send. Once a non-null engine has been attached, it
2289+
* cannot be replaced or detached. Ownership of a rejected engine remains
2290+
* with the caller.
22662291
*/
22672292
public void setCursorEngine(CursorSendEngine engine, boolean takeOwnership) {
22682293
if (closed) {
@@ -2272,6 +2297,9 @@ public void setCursorEngine(CursorSendEngine engine, boolean takeOwnership) {
22722297
throw new LineSenderException(
22732298
"setCursorEngine must be called before the first send");
22742299
}
2300+
if (cursorEngine != null) {
2301+
throw new LineSenderException("CursorSendEngine is already attached");
2302+
}
22752303
this.cursorEngine = engine;
22762304
this.ownsCursorEngine = takeOwnership && engine != null;
22772305
}

0 commit comments

Comments
 (0)