Skip to content

Commit 0a345a1

Browse files
mtopolnikclaude
andcommitted
fix(qwp): read the SF recovery scan via pread instead of the mapping
Recovery-time reads of a segment file go through pread into a private buffer: the header block, the frame scan, the CRC fold, and the torn-tail probe. Unreadable regions are then ordinary read results with one deterministic outcome per input on every filesystem, JDK, and JIT state: a sparse hole reads back as zeros and fails the frame CRC, a region past real end-of-file gives a short read, and a media error gives a failed read -- each is the boundary of recoverable data. The mapping is created only after the scan, for the append path, whose writes materialize pages rather than faulting. Reading the same regions through the mapping raises SIGBUS, which HotSpot converts to an InternalError delivered at an imprecise point under a JIT-compiled caller -- not reliably catchable by any try/catch placement, which is the MmapSegmentRecoveryFaultTest flake on the JDK 8 CI (#69). With no fault to catch, the catchable-InternalError machinery (isMmapAccessFault, the table-CRC fallback, the per-method catch arms) is gone, and the frame CRC uses the native Crc32c again -- safe on a process-private buffer. The scan is one pass through a sliding 1 MiB pread window (header fields, lastGood, frame count, torn-tail probe together); frames larger than the window CRC-fold across refills. The recovery tests become single-outcome and gain a bad-sector case via the facade's read seam (renamed MmapSegmentRecoveryBoundaryTest -- there is no fault delivery left to branch on). Fixes #69. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5cc68a9 commit 0a345a1

6 files changed

Lines changed: 347 additions & 390 deletions

File tree

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

Lines changed: 215 additions & 236 deletions
Large diffs are not rendered by default.

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
197197
// CAUTION: only unlink when the file is genuinely
198198
// empty past the header. If frame[0] failed CRC
199199
// (bit-rot, partial-page-write at crash, etc.) but
200-
// valid frames followed, scanFrames returns
200+
// valid frames followed, the recovery scan returns
201201
// lastGood=HEADER_SIZE and frameCount=0 -- yet
202202
// tornTailBytes is non-zero. Treating that as
203203
// "empty hot-spare" would silently destroy every
@@ -269,8 +269,9 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
269269
// FSNs after recovery. A gap means a segment went missing (a
270270
// manual deletion) or a sealed segment under-recovered -- its tail
271271
// was cut short by a sparse/unbacked page or a mid-file media error
272-
// (bad sector), the same class of fault scanFrames tolerates on the
273-
// active segment but which corrupts the range on a sealed one.
272+
// (bad sector), the same unreadable-region boundary the recovery
273+
// scan tolerates on the active segment but which corrupts the
274+
// range on a sealed one.
274275
for (int i = 1, n = opened.size(); i < n; i++) {
275276
MmapSegment prev = opened.get(i - 1);
276277
MmapSegment curr = opened.get(i);

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,13 @@ public interface FilesFacade {
9191
* Stat length of the file at {@code path}, in bytes. Default delegates to
9292
* {@link Files#length(String)}.
9393
*
94-
* <p>Test injection point: {@code MmapSegment.openExisting} maps the file to
95-
* this length and scans straight out of the mapping, so a wrapping facade
96-
* that returns a value <em>larger</em> than the real file makes the mapping
97-
* extend past end-of-file. A read of a page beyond real EOF raises SIGBUS on
98-
* <em>every</em> filesystem (which HotSpot translates to a catchable
99-
* {@code InternalError} at an {@code Unsafe} intrinsic site) — the same fault
100-
* a genuinely unbacked/sparse page raises on ZFS, but reproduced
101-
* deterministically on ext4/xfs too. That is what lets recovery's mmap-fault
102-
* guard be regression-tested on any CI runner rather than only on ZFS.
94+
* <p>Test injection point: {@code MmapSegment.openExisting} treats this
95+
* length as the file's extent and preads its recovery scan against it, so a
96+
* wrapping facade that returns a value <em>larger</em> than the real file
97+
* makes the scan read past end-of-file. Those reads come back short, which
98+
* recovery must treat as the boundary of recoverable data — the same
99+
* outcome as a file whose size metadata survived a crash its data blocks
100+
* did not, reproduced deterministically on any filesystem and CI runner.
103101
*/
104102
long length(String path);
105103

0 commit comments

Comments
 (0)