Skip to content

Commit 07034cd

Browse files
committed
fix(qwp): recover SF segments with positioned reads
Validate persisted segment bytes through bounded positioned reads before mmap. Fail closed on read and size instability, preserve corruption quarantine semantics, and cover sparse tails, buffer boundaries, and recovery I/O faults.
1 parent f4e6271 commit 07034cd

9 files changed

Lines changed: 584 additions & 471 deletions

File tree

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

Lines changed: 251 additions & 245 deletions
Large diffs are not rendered by default.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
/**
2828
* Positively-identified segment corruption: the file's own bytes prove it is
2929
* not (or no longer) a readable SF segment — truncated below the fixed header,
30-
* wrong magic, a negative {@code baseSeq}, or an unreadable (unbacked/torn)
31-
* header page. Distinct from its parent {@link MmapSegmentException}, which
32-
* recovery treats as an <b>operational</b> failure (open/mmap error on a file
30+
* wrong magic, or a negative {@code baseSeq}. Distinct from its parent
31+
* {@link MmapSegmentException}, which recovery treats as an <b>operational</b>
32+
* failure (open/read/mmap error on a file
3333
* whose contents may be perfectly intact) and must therefore be fatal.
3434
* <p>
3535
* Recovery quarantines corruption (rename to {@code <name>.corrupt}) and

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
* Hard failure of the MmapSegment layer — bad header, mmap rejection, file
2929
* too short for header, etc. Indicates the segment is unusable, not that
3030
* the disk is full (the latter surfaces as backpressure on the producer
31-
* via {@link io.questdb.client.cutlass.qwp.client.LineSenderException}).
31+
* via {@link io.questdb.client.cutlass.line.LineSenderException}).
3232
* <p>
3333
* Recovery distinguishes two flavors: this base type marks <b>operational</b>
34-
* failures (open/mmap/enumeration errors — the file's contents may be fine,
34+
* failures (open/read/mmap/enumeration errors — the file's contents may be fine,
3535
* so recovery must fail closed), while the {@link MmapSegmentCorruptionException}
3636
* subtype marks <b>positively-identified corruption</b> in the file's own
3737
* bytes, which recovery may quarantine instead of aborting.

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -226,16 +226,16 @@ static Recovery recover(
226226

227227
ObjList<MmapSegment> all = new ObjList<>();
228228
// Files whose own bytes prove corruption (bad magic, sub-header size,
229-
// negative baseSeq, unreadable header page). They are excluded from
230-
// the chain and quarantined to <name>.corrupt — but only AFTER the
231-
// surviving chain validates (or resolves to EMPTY), so a failed
232-
// recovery never mutates the slot. Whether a quarantined file was
233-
// load-bearing is decided by the manifest-boundary / contiguity
234-
// checks below, not by the skip itself. Operational open errors
235-
// (EMFILE, EACCES, mmap rejection, unsupported version) are NOT in
236-
// this bucket: they throw the plain MmapSegmentException type and
237-
// abort recovery, because the underlying file may be perfectly
238-
// intact and silently dropping it could lose durable frames.
229+
// negative baseSeq). They are excluded from the chain and quarantined
230+
// to <name>.corrupt — but only AFTER the surviving chain validates (or
231+
// resolves to EMPTY), so a failed recovery never mutates the slot.
232+
// Whether a quarantined file was load-bearing is decided by the
233+
// manifest-boundary / contiguity checks below, not by the skip itself.
234+
// Operational open/stat/read/mmap errors, observed size instability,
235+
// and unsupported versions are NOT in this bucket: they throw the
236+
// plain MmapSegmentException type and abort recovery, because the
237+
// underlying file may be perfectly intact and silently dropping it
238+
// could lose durable frames.
239239
ObjList<String> corruptPaths = null;
240240
SfManifest manifest = null;
241241
try {

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,21 +89,18 @@ default int fsyncDir(String dir) {
8989
return Files.fsyncDir(dir);
9090
}
9191

92+
/**
93+
* Returns the current byte length of the file referenced by open descriptor
94+
* {@code fd}, or a negative value when the descriptor cannot be statted.
95+
*/
9296
long length(int fd);
9397

9498
/**
95-
* Stat length of the file at {@code path}, in bytes. Default delegates to
96-
* {@link Files#length(String)}.
97-
*
98-
* <p>Test injection point: {@code MmapSegment.openExisting} maps the file to
99-
* this length and scans straight out of the mapping, so a wrapping facade
100-
* that returns a value <em>larger</em> than the real file makes the mapping
101-
* extend past end-of-file. A read of a page beyond real EOF raises SIGBUS on
102-
* <em>every</em> filesystem (which HotSpot translates to a catchable
103-
* {@code InternalError} at an {@code Unsafe} intrinsic site) — the same fault
104-
* a genuinely unbacked/sparse page raises on ZFS, but reproduced
105-
* deterministically on ext4/xfs too. That is what lets recovery's mmap-fault
106-
* guard be regression-tested on any CI runner rather than only on ZFS.
99+
* Stat length of the file at {@code path}, in bytes.
100+
* {@link DefaultFilesFacade} delegates to {@link Files#length(String)}.
101+
* Code that already owns an open descriptor should prefer
102+
* {@link #length(int)} so path replacement cannot make the stat refer to a
103+
* different file.
107104
*/
108105
long length(String path);
109106

@@ -152,6 +149,13 @@ default int openRWExclusive(long pathPtr) {
152149
*/
153150
long length(long pathPtr);
154151

152+
/**
153+
* Reads up to {@code len} bytes from the absolute file {@code offset} into
154+
* native memory at {@code addr}, without changing the descriptor position.
155+
* A positive result may be shorter than requested and callers that require
156+
* the whole range must retry. For a non-zero request, {@code 0} means EOF
157+
* or no progress and a negative result indicates an operating-system error.
158+
*/
155159
long read(int fd, long addr, long len, long offset);
156160

157161
boolean remove(String path);

0 commit comments

Comments
 (0)