Skip to content

Commit e2fc980

Browse files
committed
Bound the deferred-close daemon and abandon permanently stuck engines
CursorSendEngine.closeEventually() used to submit a forever-retrying task to an unbounded Executors.newCachedThreadPool(): each stuck engine pinned one daemon thread in a while-loop whose every close() attempt can block ~10s (5s awaitRingQuiescence + 5s owned-manager join), so E stuck engines retained E threads plus E rings, mmaps, watermarks and flocks with no global bound and no give-up. Replace it with a single shared single-thread scheduled daemon. Each task performs exactly ONE close() attempt and reschedules itself with exponential backoff (10ms doubling to a 10s cap), so E stuck engines cost E queued tasks, not E threads. After 40 attempts the engine is abandoned: terminal for the daemon, one ERROR logged, resources deliberately stay leaked until process exit (the kernel releases the flock; releasing them early would hand the slot to a replacement engine the stale worker could still corrupt), and the process-wide count is observable via getAbandonedDeferredCloseCount(). close() may still be invoked directly after abandonment. The per-engine deferredCloseScheduled flag keeps scheduling deduplicated. Add a many-stuck-engines test pinning the single-thread bound plus eventual completion, and a permanently-stuck-manager test pinning the retry budget, the retained slot lock after abandonment, and the direct close() recovery path. Also reattach the unlinkAllSegmentFiles javadoc that had drifted onto the old retry-loop method.
1 parent c1ad81a commit e2fc980

2 files changed

Lines changed: 312 additions & 26 deletions

File tree

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

Lines changed: 117 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.questdb.client.std.Files;
2929
import io.questdb.client.std.ObjList;
3030
import io.questdb.client.std.QuietCloseable;
31+
import org.jetbrains.annotations.TestOnly;
3132

3233
import java.util.concurrent.locks.LockSupport;
3334

@@ -61,14 +62,41 @@ public final class CursorSendEngine implements QuietCloseable {
6162
* Default deadline for {@link #appendBlocking}: 30 seconds.
6263
*/
6364
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 -> {
65+
/**
66+
* Default retry budget for {@link #closeEventually()}: initial backoff,
67+
* attempt cap and backoff cap. With these defaults a stuck engine is
68+
* retried for roughly five minutes of scheduled delay (plus whatever
69+
* time each {@link #close()} attempt spends blocked on the quiescence
70+
* barrier) before it is abandoned.
71+
*/
72+
public static final long DEFERRED_CLOSE_DEFAULT_INITIAL_BACKOFF_NANOS = 10_000_000L; // 10 ms
73+
public static final int DEFERRED_CLOSE_DEFAULT_MAX_ATTEMPTS = 40;
74+
public static final long DEFERRED_CLOSE_DEFAULT_MAX_BACKOFF_NANOS = 10_000_000_000L; // 10 s
75+
// Number of engines (process-wide) whose deferred close exhausted its
76+
// retry budget and was abandoned: their ring, watermark and slot lock
77+
// leak until process exit. Observability + test hook; never reset.
78+
private static final java.util.concurrent.atomic.AtomicLong DEFERRED_CLOSE_ABANDONED_COUNT =
79+
new java.util.concurrent.atomic.AtomicLong();
80+
// Single shared daemon that owns every deferred close retry. One thread
81+
// bounds the process footprint no matter how many engines are stuck:
82+
// each task performs ONE close() attempt and reschedules itself with
83+
// exponential backoff — it never loops or parks in-thread — so E stuck
84+
// engines cost E queued tasks, not E threads. Attempts serialize, which
85+
// is acceptable for a degraded-mode path whose per-attempt blocking is
86+
// bounded by the manager's worker-join timeout.
87+
private static final java.util.concurrent.ScheduledExecutorService DEFERRED_CLOSE_EXECUTOR =
88+
java.util.concurrent.Executors.newSingleThreadScheduledExecutor(runnable -> {
6689
Thread thread = new Thread(runnable, "qdb-cursor-engine-close");
6790
thread.setDaemon(true);
6891
return thread;
6992
});
7093
private static final org.slf4j.Logger LOG =
7194
org.slf4j.LoggerFactory.getLogger(CursorSendEngine.class);
95+
// Deferred-close retry tuning. volatile: the @TestOnly setter may run on
96+
// a different thread than the scheduler that reads them.
97+
private static volatile long deferredCloseInitialBackoffNanos = DEFERRED_CLOSE_DEFAULT_INITIAL_BACKOFF_NANOS;
98+
private static volatile int deferredCloseMaxAttempts = DEFERRED_CLOSE_DEFAULT_MAX_ATTEMPTS;
99+
private static volatile long deferredCloseMaxBackoffNanos = DEFERRED_CLOSE_DEFAULT_MAX_BACKOFF_NANOS;
72100
private final long appendDeadlineNanos;
73101
// Number of times appendBlocking observed BACKPRESSURE_NO_SPARE on its first
74102
// ring.appendOrFsn attempt. One increment per blocking-call that had to wait
@@ -126,8 +154,16 @@ public final class CursorSendEngine implements QuietCloseable {
126154
// retries the cleanup (the worker may have exited by then). Guarded by
127155
// the synchronized close() method and isCloseCompleted() accessor.
128156
private boolean closeCompleted;
129-
// True once a dedicated daemon has accepted ownership of incomplete close
130-
// retries. Guarded by synchronized closeEventually().
157+
// Deferred-close retry state. Initialized under closeEventually()'s
158+
// monitor before the first task is submitted (task submission
159+
// happens-before publishes them to the scheduler thread); thereafter
160+
// touched only by the single deferred-close scheduler thread.
161+
private int deferredCloseAttempts;
162+
private long deferredCloseBackoffNanos;
163+
// True once the shared daemon has accepted ownership of incomplete close
164+
// retries. Guarded by synchronized closeEventually(). Stays true after
165+
// the retry budget is exhausted — abandonment is terminal for the daemon,
166+
// though close() may still be invoked directly.
131167
private boolean deferredCloseScheduled;
132168
// Producer-thread-only: timestamp of the last "we're backpressured" log
133169
// line, used to throttle. Plain long is fine.
@@ -638,18 +674,27 @@ public synchronized void close() {
638674
}
639675

640676
/**
641-
* Transfers an incomplete close to a dedicated daemon owner. This lets
642-
* I/O and drainer executors terminate even when a manager service pass is
643-
* permanently stalled. The deferred owner retains this engine, and thus
644-
* its flock and native mappings, until a retry completes.
677+
* Transfers an incomplete close to a shared single-thread daemon owner.
678+
* This lets I/O and drainer executors terminate even when a manager
679+
* service pass is permanently stalled. The deferred owner retains this
680+
* engine, and thus its flock and native mappings, until a retry
681+
* completes — or until the retry budget
682+
* ({@link #DEFERRED_CLOSE_DEFAULT_MAX_ATTEMPTS} attempts with
683+
* exponential backoff) is exhausted, at which point the engine is
684+
* abandoned: its resources stay leaked until process exit (the kernel
685+
* releases the flock), {@link #getAbandonedDeferredCloseCount()} is
686+
* incremented, and one ERROR is logged. {@link #close()} may still be
687+
* invoked directly after abandonment.
645688
*/
646689
public synchronized void closeEventually() {
647690
if (closeCompleted || deferredCloseScheduled) {
648691
return;
649692
}
650693
deferredCloseScheduled = true;
694+
deferredCloseAttempts = 0;
695+
deferredCloseBackoffNanos = deferredCloseInitialBackoffNanos;
651696
try {
652-
DEFERRED_CLOSE_EXECUTOR.execute(this::runDeferredClose);
697+
DEFERRED_CLOSE_EXECUTOR.execute(this::deferredCloseAttempt);
653698
} catch (Throwable t) {
654699
deferredCloseScheduled = false;
655700
throw t;
@@ -670,6 +715,27 @@ public MmapSegment firstSealed() {
670715
return ring.firstSealed();
671716
}
672717

718+
/**
719+
* Number of engines (process-wide) whose deferred close exhausted its
720+
* retry budget and was abandoned, leaking their ring, watermark and
721+
* slot lock until process exit. Cumulative; never reset.
722+
*/
723+
public static long getAbandonedDeferredCloseCount() {
724+
return DEFERRED_CLOSE_ABANDONED_COUNT.get();
725+
}
726+
727+
/**
728+
* Tunes the deferred-close retry budget. Affects engines armed by
729+
* {@link #closeEventually()} after this call. Tests must restore the
730+
* {@code DEFERRED_CLOSE_DEFAULT_*} values when done.
731+
*/
732+
@TestOnly
733+
public static void setDeferredCloseTuning(long initialBackoffNanos, long maxBackoffNanos, int maxAttempts) {
734+
deferredCloseInitialBackoffNanos = initialBackoffNanos;
735+
deferredCloseMaxBackoffNanos = maxBackoffNanos;
736+
deferredCloseMaxAttempts = maxAttempts;
737+
}
738+
673739
/**
674740
* Number of times {@link #appendBlocking} hit
675741
* {@link SegmentRing#BACKPRESSURE_NO_SPARE} on its first attempt and
@@ -763,6 +829,48 @@ public long recoveredOrphanTipFsn() {
763829
return recoveredOrphanTipFsn;
764830
}
765831

832+
/**
833+
* One deferred-close retry. Performs a single {@link #close()} attempt;
834+
* on failure reschedules itself with exponential backoff until the
835+
* retry budget is exhausted, then abandons the engine (terminal for the
836+
* daemon: resources leak until process exit). Runs on the shared
837+
* single-thread deferred-close daemon and never loops or parks, so one
838+
* stuck engine cannot pin a thread and E stuck engines cannot pin E
839+
* threads.
840+
*/
841+
private void deferredCloseAttempt() {
842+
try {
843+
close();
844+
} catch (Throwable ignored) {
845+
// Retain ownership; the retry budget below decides what's next.
846+
}
847+
// The daemon owns cleanup independently of caller cancellation. Clear
848+
// a stray interrupt so it cannot poison the joins/waits of the next
849+
// attempt on this shared thread.
850+
Thread.interrupted();
851+
if (isCloseCompleted()) {
852+
return;
853+
}
854+
if (++deferredCloseAttempts >= deferredCloseMaxAttempts) {
855+
DEFERRED_CLOSE_ABANDONED_COUNT.incrementAndGet();
856+
LOG.error("abandoning deferred close after {} attempts: the SF manager worker never "
857+
+ "quiesced; ring, watermark and slot lock for {} stay leaked until process "
858+
+ "exit (the kernel releases the flock). close() may still be invoked "
859+
+ "directly to retry cleanup.", deferredCloseAttempts, sfDir == null ? "<memory>" : sfDir);
860+
return;
861+
}
862+
long delayNanos = deferredCloseBackoffNanos;
863+
deferredCloseBackoffNanos = Math.min(delayNanos * 2, deferredCloseMaxBackoffNanos);
864+
try {
865+
DEFERRED_CLOSE_EXECUTOR.schedule(
866+
this::deferredCloseAttempt, delayNanos, java.util.concurrent.TimeUnit.NANOSECONDS);
867+
} catch (Throwable t) {
868+
DEFERRED_CLOSE_ABANDONED_COUNT.incrementAndGet();
869+
LOG.error("could not reschedule deferred close for {}; ring, watermark and slot lock "
870+
+ "stay leaked until process exit", sfDir == null ? "<memory>" : sfDir, t);
871+
}
872+
}
873+
766874
/**
767875
* Unlinks every {@code .sfa} file under {@code dir}. Called only on
768876
* clean shutdown when the ring confirms every published FSN has been
@@ -771,23 +879,6 @@ public long recoveredOrphanTipFsn() {
771879
* Best-effort: logs and continues on failures, since we're already on
772880
* the close path.
773881
*/
774-
private void runDeferredClose() {
775-
while (!isCloseCompleted()) {
776-
try {
777-
close();
778-
} catch (Throwable ignored) {
779-
// Retain ownership and retry after a bounded pause.
780-
}
781-
if (!isCloseCompleted()) {
782-
LockSupport.parkNanos(10_000_000L);
783-
// The internal daemon owns cleanup independently of caller
784-
// cancellation. Clear interrupts so they cannot create a hot
785-
// retry loop if an executor implementation interrupts it.
786-
Thread.interrupted();
787-
}
788-
}
789-
}
790-
791882
private static void unlinkAllSegmentFiles(String dir) {
792883
if (!io.questdb.client.std.Files.exists(dir)) return;
793884
long find = io.questdb.client.std.Files.findFirst(dir);

0 commit comments

Comments
 (0)