Skip to content

Commit cce3091

Browse files
committed
fix(qwp): make SF barrier retries genuine re-persists (C-1 unsound latch clear)
After a failed writeback the kernel marks the dirtied pages clean and reports the error once per open file description (errseq_t; fsyncgate), so re-running msync+fsync over the same range returns a vacuous 0: durableCursor advanced, the PERIODIC rotation gate opened, and the round-3 unlatch (958a36d) resumed the producer -- all without the data being durable. A clean non-durable page is also evictable, so reclaim could silently replace the only good copy with stale disk content. - MmapSegment.syncPublished now brackets the barrier: best-effort mlock pin over the page-aligned [durableCursor, published) range, then msync+fsync; the failure arm re-dirties the pinned range (one same-value store per page -- kernel dirty tracking is value-blind) before the pin is released. The pin closes the clean-page reclaim race by construction; the re-dirty guarantees the next barrier performs real writeback and reports real errors, keeping the manager's clearDurabilityFailure() honest. - mlock/munlock natives added (POSIX mlock, Windows VirtualLock) and exposed through Files with an UnsatisfiedLinkError guard plus FilesFacade defaults. Refusal (RLIMIT_MEMLOCK, missing capability) is a soft downgrade: one deduped WARN, barrier outcome and latch semantics unaffected. - 4 regression tests: bracket ordering on success, re-dirty-before-unpin on failure, an fsyncgate facade model (first fsync fails, later msync/fsync return vacuous 0 without delegating) proving the latch only clears over re-dirtied pages, and mlock-refusal degrade. The drop-the-redirty mutant was applied and killed (2 failures) before restoring. Native libraries are built from source, so the new symbols ship with every build; the UnsatisfiedLinkError guard remains as defense-in-depth for environments running a stale prebuilt library.
1 parent 7a716ef commit cce3091

7 files changed

Lines changed: 393 additions & 11 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,3 +388,15 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_msync
388388
(JNIEnv *e, jclass cl, jlong addr, jlong len, jboolean async) {
389389
return msync((void *) (uintptr_t) addr, (size_t) len, async ? MS_ASYNC : MS_SYNC);
390390
}
391+
392+
/* Best-effort page pin. Callers treat a refusal (RLIMIT_MEMLOCK, missing
393+
* privilege) as a soft downgrade, so no errno capture is required here. */
394+
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_mlock0
395+
(JNIEnv *e, jclass cl, jlong addr, jlong len) {
396+
return mlock((void *) (uintptr_t) addr, (size_t) len);
397+
}
398+
399+
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_munlock0
400+
(JNIEnv *e, jclass cl, jlong addr, jlong len) {
401+
return munlock((void *) (uintptr_t) addr, (size_t) len);
402+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,3 +689,16 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_msync
689689
(void) async;
690690
return 0;
691691
}
692+
693+
/* Best-effort page pin. VirtualLock is quota-bound to the process working-set
694+
* minimum; callers treat any refusal as a soft downgrade, so no
695+
* SaveLastError() -- the refusal is not surfaced as an error. */
696+
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_mlock0
697+
(JNIEnv *e, jclass cl, jlong addr, jlong len) {
698+
return VirtualLock((LPVOID) (uintptr_t) addr, (SIZE_T) len) ? 0 : -1;
699+
}
700+
701+
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_munlock0
702+
(JNIEnv *e, jclass cl, jlong addr, jlong len) {
703+
return VirtualUnlock((LPVOID) (uintptr_t) addr, (SIZE_T) len) ? 0 : -1;
704+
}

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

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,12 @@
3131
import io.questdb.client.std.Os;
3232
import io.questdb.client.std.QuietCloseable;
3333
import io.questdb.client.std.Unsafe;
34+
import org.jetbrains.annotations.TestOnly;
3435
import org.slf4j.Logger;
3536
import org.slf4j.LoggerFactory;
3637

38+
import java.util.concurrent.atomic.AtomicBoolean;
39+
3740
/**
3841
* One mmap-backed SF segment file. The user thread (the single producer)
3942
* appends frames into the mapping; the I/O thread (the single consumer) reads
@@ -68,6 +71,10 @@ public final class MmapSegment implements QuietCloseable {
6871
public static final byte MANIFEST_REQUIRED_FLAG = 1;
6972
public static final byte VERSION = 1;
7073
private static final Logger LOG = LoggerFactory.getLogger(MmapSegment.class);
74+
// Deduplicates the process-wide "mlock refused" warning: the refusal is a
75+
// soft downgrade (see syncPublished) and must not spam the log once per
76+
// barrier when RLIMIT_MEMLOCK or the platform says no.
77+
private static final AtomicBoolean MLOCK_REFUSAL_WARNED = new AtomicBoolean();
7178
private static final int RECOVERY_BUFFER_SIZE = 64 * 1024;
7279

7380
private final FilesFacade filesFacade;
@@ -103,6 +110,9 @@ public final class MmapSegment implements QuietCloseable {
103110
// because the consumer must see writes in publication order — once the
104111
// producer bumps publishedCursor, every byte before it is fully written.
105112
private volatile long publishedCursor;
113+
// Number of failure-arm re-dirty passes performed by syncPublished.
114+
// Cold-path only; volatile so tests observing from another thread see it.
115+
private volatile long redirtyPasses;
106116
// Monotonic in-memory link to the segment that immediately follows this
107117
// one. SegmentRing publishes it before promoting the successor to active;
108118
// close deliberately retains it so a cursor can advance after head trim.
@@ -486,6 +496,21 @@ public void msync() {
486496
* captures {@link #publishedCursor}. A concurrent producer may publish
487497
* more bytes while the barrier runs; those bytes remain outside the
488498
* returned durable boundary until a later call.
499+
* <p>
500+
* The not-yet-durable range is pinned with a best-effort {@code mlock}
501+
* for the duration of the barrier and, on failure, re-dirtied before the
502+
* pin is released. Rationale (fsyncgate): after a failed writeback the
503+
* kernel marks the affected pages clean and reports the error once per
504+
* open file description, so a naive retry would msync+fsync clean pages
505+
* and return a vacuous 0 without persisting anything -- and an unpinned
506+
* clean page can be reclaimed and later re-faulted from stale disk
507+
* content, silently replacing the only good copy of the frames. The pin
508+
* guarantees the failure arm re-dirties the genuine in-memory bytes; the
509+
* re-dirty guarantees the next barrier performs real writeback and
510+
* reports real errors, so the manager's unlatch-on-success stays honest.
511+
* A refused mlock (RLIMIT_MEMLOCK, missing capability, stale native
512+
* library) never affects the barrier outcome; it only widens the
513+
* microseconds-scale window between the failed syscall and the re-dirty.
489514
*
490515
* @return the captured byte offset covered by the successful barrier
491516
*/
@@ -498,16 +523,55 @@ public long syncPublished() {
498523
if (published <= durableCursor) {
499524
return durableCursor;
500525
}
501-
if (filesFacade.msync(mmapAddress, published, false) != 0) {
502-
throw new MmapSegmentException("could not sync segment data " + path);
526+
long lockOffset = durableCursor & -Files.PAGE_SIZE;
527+
long lockAddr = mmapAddress + lockOffset;
528+
long lockLen = published - lockOffset;
529+
boolean locked = filesFacade.mlock(lockAddr, lockLen) == 0;
530+
if (!locked && MLOCK_REFUSAL_WARNED.compareAndSet(false, true)) {
531+
LOG.warn("mlock refused for SF barrier range in {} (degrading to re-dirty-only retry protection); "
532+
+ "raise RLIMIT_MEMLOCK or grant CAP_IPC_LOCK to close the post-failure reclaim window", path);
503533
}
504-
// FlushViewOfFile alone is not power-loss durable on Windows. Keep the
505-
// fd barrier on every platform so this method has one portable contract.
506-
if (filesFacade.fsync(fd) != 0) {
507-
throw new MmapSegmentException("could not sync segment file " + path);
534+
boolean durable = false;
535+
try {
536+
if (filesFacade.msync(mmapAddress, published, false) != 0) {
537+
throw new MmapSegmentException("could not sync segment data " + path);
538+
}
539+
// FlushViewOfFile alone is not power-loss durable on Windows. Keep the
540+
// fd barrier on every platform so this method has one portable contract.
541+
if (filesFacade.fsync(fd) != 0) {
542+
throw new MmapSegmentException("could not sync segment file " + path);
543+
}
544+
durable = true;
545+
durableCursor = published;
546+
return published;
547+
} finally {
548+
if (!durable) {
549+
redirtyRange(lockAddr, mmapAddress + published);
550+
}
551+
if (locked) {
552+
filesFacade.munlock(lockAddr, lockLen);
553+
}
554+
}
555+
}
556+
557+
/**
558+
* Marks every page overlapping {@code [fromAddr, endAddr)} dirty again by
559+
* re-storing one byte per page. Every touched offset lies inside the
560+
* published prefix (or the immutable header magic after aligning down),
561+
* so the same-value store races with nothing; kernel dirty tracking is
562+
* value-blind, so the store is a real dirtying event that forces the next
563+
* writeback to re-submit the whole page.
564+
*/
565+
private void redirtyRange(long fromAddr, long endAddr) {
566+
for (long addr = fromAddr; addr < endAddr; addr += Files.PAGE_SIZE) {
567+
Unsafe.getUnsafe().putByte(addr, Unsafe.getUnsafe().getByte(addr));
508568
}
509-
durableCursor = published;
510-
return published;
569+
redirtyPasses++;
570+
}
571+
572+
@TestOnly
573+
public long redirtyPassesForTest() {
574+
return redirtyPasses;
511575
}
512576

513577
/**

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,9 +1224,12 @@ private void servicePeriodicSync(RingEntry e, long now) {
12241224
syncScratch.getQuick(i).syncPublished();
12251225
}
12261226
e.ring.clearSyncRequestIfActiveDurable();
1227-
// The pass above covered every live segment's published range, so
1228-
// any earlier barrier failure has been remedied — unlatch it so a
1229-
// transient disk fault doesn't permanently brick the producer.
1227+
// The pass above covered every live segment's published range, and
1228+
// a failed barrier re-dirties its range under an mlock pin (see
1229+
// MmapSegment.syncPublished), so a success here is a genuine
1230+
// re-persist -- not a vacuous retry over pages a failed writeback
1231+
// marked clean (fsyncgate). Unlatch so a transient disk fault
1232+
// doesn't permanently brick the producer.
12301233
e.ring.clearDurabilityFailure();
12311234
e.nextDataSyncNanos = now + e.syncIntervalNanos;
12321235
if (e.syncFailureLogged) {

core/src/main/java/io/questdb/client/std/Files.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,53 @@ public static void munmap(long address, long len, int memoryTag) {
573573
*/
574574
public static native int msync(long addr, long len, boolean async);
575575

576+
// Guards the optional mlock/munlock natives: a packaged native library
577+
// built before these symbols existed throws UnsatisfiedLinkError on first
578+
// use. Flip to unavailable and report refusal (-1) from then on, so
579+
// callers degrade to their unpinned tier instead of crashing.
580+
private static volatile boolean mlockLinked = true;
581+
582+
/**
583+
* Best-effort page pin over {@code [addr, addr+len)} of an mmap'd region.
584+
* Whole pages containing any part of the range are locked; {@code addr}
585+
* should be page-aligned for portability. Returns 0 on success, -1 when
586+
* the platform refuses (RLIMIT_MEMLOCK, missing privilege) or the loaded
587+
* native library predates the symbol. Callers must treat refusal as a
588+
* soft downgrade, never an error.
589+
*/
590+
public static int mlock(long addr, long len) {
591+
if (!mlockLinked) {
592+
return -1;
593+
}
594+
try {
595+
return mlock0(addr, len);
596+
} catch (UnsatisfiedLinkError err) {
597+
mlockLinked = false;
598+
return -1;
599+
}
600+
}
601+
602+
/**
603+
* Releases a pin taken by {@link #mlock(long, long)}. Best-effort: a
604+
* refusal is ignorable ({@code munmap} implicitly drops any remaining
605+
* locks on the range).
606+
*/
607+
public static int munlock(long addr, long len) {
608+
if (!mlockLinked) {
609+
return -1;
610+
}
611+
try {
612+
return munlock0(addr, len);
613+
} catch (UnsatisfiedLinkError err) {
614+
mlockLinked = false;
615+
return -1;
616+
}
617+
}
618+
619+
private static native int mlock0(long addr, long len);
620+
621+
private static native int munlock0(long addr, long len);
622+
576623
/**
577624
* Returns a native pointer to the current entry's null-terminated name
578625
* (UTF-8). Pointer is valid only until the next {@link #findNext(long)}

core/src/main/java/io/questdb/client/std/FilesFacade.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,16 @@ default int fsyncDir(String dir) {
108108

109109
int mkdir(String path, int mode);
110110

111+
/**
112+
* Best-effort page pin over {@code [addr, addr+len)} of an mmap'd region.
113+
* Returns 0 when the range is locked, non-zero when the platform refuses
114+
* (RLIMIT_MEMLOCK, missing privilege, or a native library without the
115+
* symbol). Callers must treat refusal as a soft downgrade, never an error.
116+
*/
117+
default int mlock(long addr, long len) {
118+
return Files.mlock(addr, len);
119+
}
120+
111121
default long mmap(int fd, long len, long offset, int flags, int memoryTag) {
112122
return Files.mmap(fd, len, offset, flags, memoryTag);
113123
}
@@ -116,6 +126,14 @@ default int msync(long addr, long len, boolean async) {
116126
return Files.msync(addr, len, async);
117127
}
118128

129+
/**
130+
* Releases a pin taken by {@link #mlock(long, long)}. Best-effort;
131+
* refusals are ignorable ({@code munmap} implicitly unlocks).
132+
*/
133+
default int munlock(long addr, long len) {
134+
return Files.munlock(addr, len);
135+
}
136+
119137
default void munmap(long address, long len, int memoryTag) {
120138
Files.munmap(address, len, memoryTag);
121139
}

0 commit comments

Comments
 (0)