Skip to content

Commit d130f2c

Browse files
committed
fix(client): fail closed in SF recovery and add crash-safe boundary manifest
SF crash recovery previously failed OPEN on operational errors: - findFirst() < 0 (permission/transient enumeration error) was treated as an empty directory, so the engine started fresh and openCleanRW(O_TRUNC) truncated the existing durable log (sf-initial.sfa) and deleted the ack watermark; - findNext() < 0 silently accepted an incomplete directory listing; - per-file openRW/mmap failures (EMFILE, ENOMEM, transient I/O) on valid segments were swallowed by the same catch as data corruption and skipped; - the FSN contiguity check only compares adjacent pairs, so a skipped LEADING segment silently dropped unacked rows (ackedFsn seeded past them) and a skipped TRAILING segment silently re-issued its FSNs to new payloads (overlapping FSNs on disk). Recovery now fails closed and proves slot integrity before mutating: - Enumeration errors (findFirst/findNext < 0) abort startup. - Operational open/mmap failures on recognized segments abort startup; only positively-identified corruption (new MmapSegmentCorruptionException: bad magic, sub-header size, negative baseSeq, unreadable header page) is skippable, and quarantine renames (<name>.corrupt) are deferred until the surviving chain validates, so a failed recovery mutates nothing. Unsupported-version segments stay fatal (they belong to another client build; renaming them would strand that build's frames). - sf-manifest.bin: dual-slot, CRC-protected, generation-versioned boundary record (headBase/activeBase) written on rotation and ack-driven trim. Recovery validates the chain against the committed boundaries, which catches missing leading/trailing segments that contiguity alone cannot. Segments carry a manifest-required header flag so a deleted manifest next to migrated segments fails closed. Boundary updates are monotonic (clamped) and serialized with trim under the ring monitor; the trim path recomputes the successor under that monitor so a concurrent rotation can never make the head leapfrog a still-unacked sealed segment. - Segment files are created with O_EXCL (new Files.openRWExclusive natives) instead of O_TRUNC, so no code path can truncate an existing log. - Crash-window protocol, verified state by state: fresh start orders initial-segment durability -> manifest create -> flag stamp; rotation syncs the promoted spare's rebased header before the manifest references it and commits the manifest before any ring mutation; clean-drain close durably collapses boundaries to head==active before unlinking and removes the manifest last. Every window either recovers (equivalent empties from a fresh-start kill, an empty active at the chain end after a rotation crash, record-less manifest creation debris, drain-window survivors, stale below-head files) or fails closed with intact evidence. The one deliberate mutation on a failing path is quarantining a provably record-less manifest (it contains no boundaries by construction; flagged segments still fail closed afterwards). Regression tests (SegmentRecoveryIntegrityTest, 24 cases) cover the full required matrix: findFirst=-1, partial findNext=-1, openRW=-1, mmap failure -- each asserting byte-for-byte slot preservation; the engine-level O_TRUNC reproduction; leading/trailing/interior boundary loss; corrupt stray quarantine with sibling recovery; deferred-quarantine no-mutation on failure; flagged-segments-without-manifest; manifest creation-crash debris self-heal (zero-byte and record-less); drain-crash spare survivor; corrupt-active-with-empty-stand-in fail-closed; dual-slot generation selection and torn-record fallback. Independently reviewed for crash-consistency holes, lock ordering (manager lock -> ring monitor -> manifest monitor, acyclic), unacked-frame loss, and fd/mmap leaks; both review blockers (creation-debris brick, drain-window spare brick) are fixed and regression-tested. Windows native openRWExclusive0 uses CREATE_NEW; CI rebuilds platform binaries from the .c change.
1 parent e8c2dcd commit d130f2c

15 files changed

Lines changed: 1975 additions & 230 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_openRW0
6565
return (jint) fd;
6666
}
6767

68+
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_openRWExclusive0
69+
(JNIEnv *e, jclass cl, jlong lpszName) {
70+
int fd;
71+
RESTARTABLE(open((const char *) (uintptr_t) lpszName, O_CREAT | O_EXCL | O_RDWR, 0644), fd);
72+
return (jint) fd;
73+
}
74+
6875
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_openAppend0
6976
(JNIEnv *e, jclass cl, jlong lpszName) {
7077
int fd;

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_openRW0
103103
FILE_ATTRIBUTE_NORMAL);
104104
}
105105

106+
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_openRWExclusive0
107+
(JNIEnv *e, jclass cl, jlong lpszName) {
108+
return open_file((const char *) (uintptr_t) lpszName,
109+
GENERIC_READ | GENERIC_WRITE,
110+
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
111+
CREATE_NEW,
112+
FILE_ATTRIBUTE_NORMAL);
113+
}
114+
106115
JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_openAppend0
107116
(JNIEnv *e, jclass cl, jlong lpszName) {
108117
jint fd = open_file((const char *) (uintptr_t) lpszName,

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

Lines changed: 102 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,11 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
286286
// session before deciding to start fresh. Without this the engine
287287
// would create a new sf-initial.sfa at baseSeq=0, overlapping FSNs
288288
// already on disk and corrupting ACK translation, trim, and replay.
289-
SegmentRing recovered = memoryMode ? null
290-
: SegmentRing.openExisting(sfDir, segmentSizeBytes);
291-
this.wasRecoveredFromDisk = recovered != null;
289+
SegmentRing.Recovery recovery = memoryMode ? null
290+
: SegmentRing.recover(filesFacade, sfDir, segmentSizeBytes);
291+
SegmentRing recovered = recovery == null ? null : recovery.ring();
292+
this.wasRecoveredFromDisk = recovery != null
293+
&& recovery.status() == SegmentRing.RecoveryStatus.RECOVERED;
292294
if (recovered != null) {
293295
ringInProgress = recovered;
294296
// Seed ackedFsn to one below the lowest segment's baseSeq.
@@ -394,18 +396,49 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
394396
}
395397
MmapSegment initial;
396398
String initialPath = null;
399+
SfManifest initialManifest = null;
397400
if (memoryMode) {
398401
initial = MmapSegment.createInMemory(0L, segmentSizeBytes);
399402
} else {
403+
// Created WITHOUT the manifest-required flag: the flag is
404+
// a durable promise that sf-manifest.bin exists, so it
405+
// must only be stamped after the manifest itself is on
406+
// disk. Stamping it at create time would leave a crash
407+
// window (initial durable, manifest not yet created)
408+
// whose recovery hard-fails with "segment requires
409+
// missing manifest" even though nothing was lost.
400410
initialPath = sfDir + "/sf-initial.sfa";
401-
initial = MmapSegment.create(initialPath, 0L, segmentSizeBytes);
411+
initial = MmapSegment.create(filesFacade, initialPath, 0L, segmentSizeBytes);
402412
}
403413
try {
404-
ringInProgress = new SegmentRing(initial, segmentSizeBytes);
414+
if (!memoryMode) {
415+
// Ordering is load-bearing for crash recovery:
416+
// 1. initial segment durable (header + dirent),
417+
// 2. manifest created (its own create fsyncs),
418+
// 3. flag stamped on the segment.
419+
// A crash after (1) recovers via the legacy
420+
// (manifest-less) path; after (2) via the manifest
421+
// path with an unflagged-but-valid empty active;
422+
// after (3) it is the steady state. Every window
423+
// self-heals without operator action.
424+
initial.syncHeader();
425+
if (filesFacade.fsyncDir(sfDir) != 0) {
426+
throw new MmapSegmentException(
427+
"could not sync fresh SF segment directory " + sfDir);
428+
}
429+
initialManifest = SfManifest.create(filesFacade, sfDir, 0L, 0L);
430+
initial.markManifestRequired();
431+
}
432+
ringInProgress = new SegmentRing(initial, segmentSizeBytes, initialManifest);
433+
initialManifest = null;
405434
} catch (Throwable t) {
406435
initial.close();
436+
if (initialManifest != null) {
437+
initialManifest.close();
438+
SfManifest.removeFile(filesFacade, sfDir);
439+
}
407440
if (initialPath != null) {
408-
Files.remove(initialPath);
441+
filesFacade.remove(initialPath);
409442
}
410443
throw t;
411444
}
@@ -1247,17 +1280,24 @@ private static long segmentCleanupRank(String name) {
12471280
}
12481281

12491282
/**
1250-
* Unlinks every {@code .sfa} file under {@code dir}. Called only on
1251-
* clean shutdown when the ring confirms every published FSN has been
1252-
* acked — at that moment the slot has no recoverable work and the
1253-
* files are pure noise that would mislead the next sender's recovery.
1283+
* Unlinks every {@code .sfa} file under {@code dir}, then the SF
1284+
* manifest. Called only on clean shutdown when the ring confirms every
1285+
* published FSN has been acked — at that moment the slot has no
1286+
* recoverable work and the files are pure noise that would mislead the
1287+
* next sender's recovery.
12541288
* <p>
1255-
* Removal runs in ascending segment order and STOPS at the first
1256-
* failure, so whatever survives (a failure here, or a crash mid-loop)
1257-
* is always a contiguous top slice of the ring: recovery's
1258-
* FSN-contiguity check still passes, and the retained ack watermark
1259-
* (== the final acked FSN == the highest frame on disk) still covers
1260-
* every surviving frame, so a successor replays nothing.
1289+
* Crash safety comes from a single durable manifest update committed
1290+
* BEFORE the first unlink: collapsing the boundaries to
1291+
* {@code head == active} declares every frame below the active base
1292+
* trimmed, so any subset of surviving files recovers cleanly (stale
1293+
* files are discarded, a surviving active recovers as the whole chain,
1294+
* zero files with a leftover manifest is the recognized drain window
1295+
* and recovers as EMPTY). Removal still runs in ascending segment order
1296+
* and STOPS at the first failure; the retained ack watermark (== the
1297+
* final acked FSN) covers every surviving frame, so a successor replays
1298+
* nothing. Legacy slots without a manifest keep the pre-manifest
1299+
* argument: ascending-order removal preserves a contiguous top slice
1300+
* that passes FSN-contiguity.
12611301
*
12621302
* @return {@code true} only when enumeration succeeded and every
12631303
* {@code .sfa} file was confirmed removed — the caller keeps the ack
@@ -1300,13 +1340,54 @@ private static boolean unlinkAllSegmentFiles(FilesFacade filesFacade, String dir
13001340
int byRank = Long.compare(segmentCleanupRank(a), segmentCleanupRank(b));
13011341
return byRank != 0 ? byRank : a.compareTo(b);
13021342
});
1303-
for (int i = 0, n = names.size(); i < n; i++) {
1304-
String path = dir + "/" + names.get(i);
1305-
if (!filesFacade.remove(path)) {
1306-
LOG.warn("Failed to unlink fully-acked segment {} on close; stopping so the "
1307-
+ "residual files stay a contiguous, watermark-covered range", path);
1343+
// Manifest-aware drain protocol. Before removing anything, durably
1344+
// collapse the committed boundaries to head == active. From that
1345+
// moment EVERY crash state is a valid recovery input:
1346+
// - surviving data below the active base is "stale below head"
1347+
// (acked by definition here) and is discarded by recovery;
1348+
// - a surviving active-base segment recovers as the whole chain;
1349+
// - empty spares are validated extras;
1350+
// - zero segment files with the manifest still present is the
1351+
// recognized drain window and recovers as EMPTY.
1352+
// Without this barrier, deleting the file the manifest calls "head"
1353+
// would make the next startup fail on a slot that lost nothing.
1354+
// The manifest itself is removed last, after every segment unlink.
1355+
SfManifest manifest = null;
1356+
try {
1357+
try {
1358+
manifest = SfManifest.open(filesFacade, dir);
1359+
} catch (Throwable t) {
1360+
LOG.warn("close-time unlink could not read the SF manifest in {}; leaving "
1361+
+ "all files for the next recovery", dir, t);
13081362
return false;
13091363
}
1364+
if (manifest != null && !names.isEmpty()) {
1365+
try {
1366+
long activeBase = manifest.activeBase();
1367+
manifest.update(activeBase, activeBase);
1368+
} catch (Throwable t) {
1369+
LOG.warn("close-time unlink could not commit drain boundaries in {}; "
1370+
+ "leaving all files for the next recovery", dir, t);
1371+
return false;
1372+
}
1373+
}
1374+
for (int i = 0, n = names.size(); i < n; i++) {
1375+
String path = dir + "/" + names.get(i);
1376+
if (!filesFacade.remove(path)) {
1377+
LOG.warn("Failed to unlink fully-acked segment {} on close; stopping so the "
1378+
+ "residual files stay a watermark-covered, manifest-consistent range", path);
1379+
return false;
1380+
}
1381+
}
1382+
} finally {
1383+
if (manifest != null) {
1384+
manifest.close();
1385+
}
1386+
}
1387+
if (!SfManifest.removeFile(filesFacade, dir)) {
1388+
// Safe to proceed: a manifest with zero segment files is the
1389+
// recognized drain window and recovers as EMPTY.
1390+
LOG.warn("could not remove SF manifest in {} after full segment cleanup", dir);
13101391
}
13111392
return true;
13121393
}

0 commit comments

Comments
 (0)