Skip to content

Commit 282ac09

Browse files
glasstigerclaude
andcommitted
Truncate the torn tail when reopening the symbol dict
openExisting parsed complete entries and set appendOffset past the last one, but left the file at its full length. A crash mid-append leaves a torn trailing record; if the next append after recovery is SHORTER than that torn tail, it overwrites only the tail's prefix and leaves residue beyond its own end. A later recovery can then mis-parse that residue as a ghost symbol, shifting every subsequent dense id -- so the "self-healing tail" guarantee was not actually airtight. open() now truncates the file to the end of the last complete entry (ftruncate) so nothing survives past appendOffset. Best-effort: a failed truncate falls back to the prior overwrite-from-appendOffset behaviour. testTornTrailingEntrySelfHeals now asserts the file returns to its clean length after the reopen; reverting the truncate fails it (19 vs 16 bytes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2b78b0e commit 282ac09

2 files changed

Lines changed: 20 additions & 3 deletions

File tree

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,18 @@ private static PersistedSymbolDict openExisting(String filePath, long fileLen) {
328328
}
329329
Unsafe.free(buf, len, MemoryTag.NATIVE_DEFAULT);
330330
buf = 0L;
331-
// appendOffset lands just past the last complete entry, so the next
332-
// append overwrites any torn trailing bytes.
331+
// Physically drop any torn/stale trailing bytes (a crash mid-append)
332+
// so a LATER, shorter append cannot leave residue past its own end
333+
// that a future recovery mis-parses as a ghost symbol -- which would
334+
// shift every subsequent dense id. Without this, appendOffset already
335+
// lands just past the last complete entry, so the next append
336+
// overwrites FROM there, but only truncation guarantees nothing
337+
// survives BEYOND it. Best-effort: a failed truncate just forgoes the
338+
// hardening and falls back to the overwrite-from-appendOffset behaviour.
339+
long validLen = HEADER_SIZE + entriesLen;
340+
if (validLen < len) {
341+
Files.truncate(fd, validLen);
342+
}
333343
return new PersistedSymbolDict(fd, HEADER_SIZE + entriesLen, count, entriesAddr, entriesLen);
334344
} catch (Throwable t) {
335345
if (buf != 0L) {

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,17 +229,24 @@ public void testTornTrailingEntrySelfHeals() throws Exception {
229229
d.close();
230230

231231
Path f = dir.resolve(".symbol-dict");
232+
long cleanLen = Files.size(f); // header + "one" + "two", no tail
232233
Files.write(f, new byte[]{(byte) 5, (byte) 'x', (byte) 'y'},
233234
StandardOpenOption.APPEND);
235+
Assert.assertEquals("torn tail present before reopen", cleanLen + 3, Files.size(f));
234236

235237
// Reopen: the torn tail is ignored; only the two complete entries load.
236238
PersistedSymbolDict re = PersistedSymbolDict.open(dir.toString());
237239
try {
240+
// open() physically truncates the torn tail: the file returns
241+
// to its clean length, so a later SHORTER append can never
242+
// leave residue past its end that a future recovery mis-parses
243+
// as a ghost symbol.
244+
Assert.assertEquals("torn tail physically dropped by open", cleanLen, Files.size(f));
238245
Assert.assertEquals(2, re.size());
239246
ObjList<String> s = re.readLoadedSymbols();
240247
Assert.assertEquals("one", s.getQuick(0));
241248
Assert.assertEquals("two", s.getQuick(1));
242-
// The next append overwrites the torn tail, keeping the file consistent.
249+
// The next append continues from the truncated tail cleanly.
243250
re.appendSymbol("three");
244251
Assert.assertEquals(3, re.size());
245252
} finally {

0 commit comments

Comments
 (0)