Skip to content

Commit ca7a2da

Browse files
committed
fix(qwp): fail closed on SF manifest with uncollapsed boundaries and no segment files
Recovery treated every valid manifest with zero .sfa files as EMPTY and removed it, even when headBase < activeBase. No in-protocol crash can produce that state: the close-time drain durably collapses the boundaries to head == active before its first unlink, and a fresh start writes (0,0). Uncollapsed boundaries with no segment files therefore prove durable, never-declared-acked frames vanished outside the protocol (manual wipe, partial restore) -- yet recovery silently started fresh and deleted the manifest, destroying the only evidence of the loss. Accept the segment-less slot as EMPTY only when headBase == activeBase, matching the existing guard on the some-files clean-drain window; otherwise throw without mutating the slot. Fix the drain-window test to model the real crash state (9,9) and add a fail-closed (7,9) test asserting the directory is left byte-identical.
1 parent 9c9e1dd commit ca7a2da

2 files changed

Lines changed: 59 additions & 13 deletions

File tree

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

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -276,16 +276,32 @@ static Recovery recover(
276276
return Recovery.empty();
277277
}
278278
if (manifest != null) {
279-
// No .sfa files at all. Two legitimate protocols produce
280-
// this: the close-time drain unlinks the last segment
281-
// before it removes the manifest, and a fresh-start crash
282-
// can leave a boundary-less (0,0) manifest behind. In
283-
// both cases nothing recoverable exists, so accept EMPTY
284-
// -- but shout, because a manual wipe of segment files
285-
// looks identical and the operator should know.
286-
LOG.warn("SF manifest exists in {} with no segment files "
287-
+ "(clean-drain or fresh-start crash window, or manual "
288-
+ "segment removal); discarding it and starting fresh", sfDir);
279+
// No .sfa files at all. Only two protocols legitimately
280+
// produce this, and both leave head == active: the
281+
// close-time drain durably collapses the boundaries
282+
// BEFORE its first unlink, and a fresh-start crash can
283+
// leave a boundary-less (0,0) manifest behind. Uncollapsed
284+
// boundaries therefore prove durable, never-declared-acked
285+
// frames existed in [headBase, activeBase] whose files
286+
// vanished outside the protocol (manual wipe, partial
287+
// restore) -- fail closed and keep the manifest as
288+
// evidence instead of silently starting fresh.
289+
long manifestHeadBase = manifest.headBase();
290+
long manifestActiveBase = manifest.activeBase();
291+
if (manifestHeadBase != manifestActiveBase) {
292+
throw new MmapSegmentException(SfManifest.FILE_NAME + " in " + sfDir
293+
+ " references durable data (headBase=" + manifestHeadBase
294+
+ ", activeBase=" + manifestActiveBase
295+
+ ") but no segment files exist");
296+
}
297+
// Collapsed boundaries: nothing recoverable exists, so
298+
// accept EMPTY -- but shout, because a manual wipe of a
299+
// fully-acked slot looks identical and the operator
300+
// should know.
301+
LOG.warn("SF manifest exists in {} with collapsed boundaries ({}) and "
302+
+ "no segment files (clean-drain or fresh-start crash window, or "
303+
+ "manual segment removal); discarding it and starting fresh",
304+
sfDir, manifestActiveBase);
289305
manifest.close();
290306
manifest = null;
291307
if (!SfManifest.removeFile(filesFacade, sfDir)) {

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentRecoveryIntegrityTest.java

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,11 @@ public void testRotationCrashWindowEmptyActiveAtChainEndRecovers() throws Except
423423
@Test
424424
public void testDrainWindowManifestWithoutSegmentsRecoversEmptyAndRemovesManifest() throws Exception {
425425
TestUtils.assertMemoryLeak(() -> {
426-
// Clean-drain close unlinks the last segment before the manifest;
427-
// a crash between the two leaves this state.
428-
writeManifest(3, 7, 9);
426+
// Clean-drain close durably collapses the boundaries to
427+
// head == active before unlinking, then removes segments and
428+
// finally the manifest; a crash between the last unlink and the
429+
// manifest removal leaves collapsed boundaries with no files.
430+
writeManifest(3, 9, 9);
429431

430432
SegmentRing ring = SegmentRing.openExisting(FilesFacade.INSTANCE, tmpDir, SEGMENT_SIZE);
431433
Assert.assertNull("segment-less slot must recover as EMPTY", ring);
@@ -434,6 +436,34 @@ public void testDrainWindowManifestWithoutSegmentsRecoversEmptyAndRemovesManifes
434436
});
435437
}
436438

439+
@Test
440+
public void testZeroSegmentFilesWithUncollapsedBoundariesFailsClosed() throws Exception {
441+
TestUtils.assertMemoryLeak(() -> {
442+
// headBase(7) < activeBase(9): the manifest durably committed that
443+
// segments [7..9] existed and were never declared acked. No
444+
// in-protocol crash leaves this next to zero .sfa files -- the
445+
// close-time drain collapses boundaries to head == active BEFORE
446+
// its first unlink, and a fresh start writes (0,0). Uncollapsed
447+
// boundaries with no segment files therefore prove durable data
448+
// vanished outside the protocol: recovery must fail closed and
449+
// keep the manifest as evidence, not silently start fresh.
450+
writeManifest(3, 7, 9);
451+
Map<String, byte[]> before = snapshotDir();
452+
453+
try {
454+
SegmentRing ring = SegmentRing.openExisting(FilesFacade.INSTANCE, tmpDir, SEGMENT_SIZE);
455+
if (ring != null) {
456+
ring.close();
457+
}
458+
Assert.fail("recovery must fail closed: manifest boundaries (7,9) reference "
459+
+ "durable frames but no segment file survives");
460+
} catch (MmapSegmentException expected) {
461+
TestUtils.assertContains(expected.getMessage(), "references durable data");
462+
}
463+
assertDirUnchanged(before);
464+
});
465+
}
466+
437467
@Test
438468
public void testFreshStartCrashBeforeManifestCreationRecoversViaLegacyPath() throws Exception {
439469
TestUtils.assertMemoryLeak(() -> {

0 commit comments

Comments
 (0)