Skip to content

Commit 36263c4

Browse files
bluestreak01claude
andcommitted
fix(ilp): cursor recovery — derive next FSN from on-disk segments
CursorSendEngine's constructor unconditionally created a fresh sf-initial.sfa at baseSeq=0 even when the SF directory contained sealed segments from a prior session, restarting the FSN sequence at 0 and overlapping with FSNs already on disk. ACK translation, trim, and replay would then operate on overlapping ranges. The recovery primitive — SegmentRing.openExisting — already exists and does the right thing (scans *.sfa, sorts by baseSeq, validates contiguity, picks the highest-baseSeq segment as the new active). The constructor just never called it. Now it does, in disk mode, before falling back to the fresh-create path on an empty dir. Adds a regression test that writes 5 frames to one engine, closes, reopens against the same dir, and asserts the next append's FSN continues at 5 instead of restarting at 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9781771 commit 36263c4

2 files changed

Lines changed: 78 additions & 17 deletions

File tree

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

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -144,25 +144,32 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
144144
this.ownsManager = ownsManager;
145145
this.appendDeadlineNanos = appendDeadlineNanos;
146146

147-
// Create the initial active segment with baseSeq=0. (No on-disk
148-
// recovery in PR1 — assumes the directory is empty.) The manager will
149-
// immediately notice that the ring needs a hot spare and provision one.
150-
MmapSegment initial;
151-
String initialPath = null;
152-
if (memoryMode) {
153-
initial = MmapSegment.createInMemory(0L, segmentSizeBytes);
147+
// Disk mode: try to recover any *.sfa files left behind by a prior
148+
// session before deciding to start fresh. Without this the engine
149+
// would create a new sf-initial.sfa at baseSeq=0, overlapping FSNs
150+
// already on disk and corrupting ACK translation, trim, and replay.
151+
SegmentRing recovered = memoryMode ? null
152+
: SegmentRing.openExisting(sfDir, segmentSizeBytes);
153+
if (recovered != null) {
154+
this.ring = recovered;
154155
} else {
155-
initialPath = sfDir + "/sf-initial.sfa";
156-
initial = MmapSegment.create(initialPath, 0L, segmentSizeBytes);
157-
}
158-
try {
159-
this.ring = new SegmentRing(initial, segmentSizeBytes);
160-
} catch (Throwable t) {
161-
initial.close();
162-
if (initialPath != null) {
163-
Files.remove(initialPath);
156+
MmapSegment initial;
157+
String initialPath = null;
158+
if (memoryMode) {
159+
initial = MmapSegment.createInMemory(0L, segmentSizeBytes);
160+
} else {
161+
initialPath = sfDir + "/sf-initial.sfa";
162+
initial = MmapSegment.create(initialPath, 0L, segmentSizeBytes);
163+
}
164+
try {
165+
this.ring = new SegmentRing(initial, segmentSizeBytes);
166+
} catch (Throwable t) {
167+
initial.close();
168+
if (initialPath != null) {
169+
Files.remove(initialPath);
170+
}
171+
throw t;
164172
}
165-
throw t;
166173
}
167174

168175
if (ownsManager) {

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,60 @@ public void testAppendBlockingThrowsOnDeadlineExpiryUnderCap() throws Exception
175175
}
176176
}
177177

178+
@Test
179+
public void testRestartIntoNonEmptySfDirContinuesFsnSequence() {
180+
// Red regression: restart against a populated SF dir must derive the
181+
// new active's baseSeq from the highest sealed segment on disk, not
182+
// hardcode 0. Previously CursorSendEngine always created a fresh
183+
// sf-initial.sfa at baseSeq=0, so the second session's FSNs collided
184+
// with frames the first session had already durably persisted.
185+
long segSize = MmapSegment.HEADER_SIZE
186+
+ 2 * (MmapSegment.FRAME_HEADER_SIZE + 64);
187+
int totalFrames = 5;
188+
long buf = Unsafe.malloc(64, MemoryTag.NATIVE_DEFAULT);
189+
try {
190+
// Session 1: write totalFrames, leaving the dir populated with
191+
// sealed segments + a (partially-filled) active at the end.
192+
try (CursorSendEngine engine = new CursorSendEngine(tmpDir, segSize)) {
193+
for (int i = 0; i < totalFrames; i++) {
194+
long fsn = engine.appendBlocking(buf, 64);
195+
assertEquals(i, fsn);
196+
}
197+
assertEquals(totalFrames - 1, engine.publishedFsn());
198+
}
199+
// Confirm the dir really has *.sfa files left over — otherwise
200+
// the test would pass for the wrong reason (empty dir == no bug).
201+
long find = Files.findFirst(tmpDir);
202+
assertTrue("findFirst() must succeed on populated tmpDir", find != 0);
203+
int sfaCount = 0;
204+
try {
205+
int rc = 1;
206+
while (rc > 0) {
207+
String name = Files.utf8ToString(Files.findName(find));
208+
if (name != null && name.endsWith(".sfa")) sfaCount++;
209+
rc = Files.findNext(find);
210+
}
211+
} finally {
212+
Files.findClose(find);
213+
}
214+
assertTrue("session 1 must leave .sfa files behind: count=" + sfaCount,
215+
sfaCount >= 1);
216+
217+
// Session 2: open the same dir. The next FSN must continue from
218+
// where session 1 left off, NOT restart at 0. Today this assertion
219+
// fails because the engine constructs a fresh ring at baseSeq=0
220+
// and ignores the on-disk segments.
221+
try (CursorSendEngine engine = new CursorSendEngine(tmpDir, segSize)) {
222+
long fsn = engine.appendBlocking(buf, 64);
223+
assertEquals("FSN must continue, not restart — overlapping "
224+
+ "FSNs would corrupt ACK translation, trim, and replay",
225+
totalFrames, fsn);
226+
}
227+
} finally {
228+
Unsafe.free(buf, 64, MemoryTag.NATIVE_DEFAULT);
229+
}
230+
}
231+
178232
@Test
179233
public void testMemoryModeSkipsDirAndStillWorks() {
180234
// sfDir == null → memory-only ring. No files, no mkdir, no path.

0 commit comments

Comments
 (0)