Skip to content

Commit 39e9e52

Browse files
glasstigerclaude
andcommitted
Cover the persisted symbol-dict short-write path
The failed-publish regression tests exercise a persist that SUCCEEDED (the publish is what fails), so nothing pinned the persist-FAILURE trigger: a short write (disk full / quota) mid-persist. Add a ShortWriteOnceFacade that lands one entry append short and two tests -- one per production persist path (appendRawEntries fast path, appendSymbols slow path) -- asserting the load-bearing idempotency invariant the write-ahead persist relies on: a short write throws WITHOUT advancing size()/appendOffset, so a retry keyed off pd.size() re-persists the same range at the same offset and recovers a gap-free, duplicate-free dictionary. Verified as regression guards: reordering the production code to advance size before the written==len check fails both tests with "short write must NOT advance size expected:<0> but was:<2>". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bbc01a8 commit 39e9e52

1 file changed

Lines changed: 146 additions & 0 deletions

File tree

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

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,78 @@ public void testAppendRawEntriesMatchesAppendSymbols() throws Exception {
146146
});
147147
}
148148

149+
@Test
150+
public void testAppendRawEntriesShortWriteThrowsWithoutAdvancingIsIdempotentOnRetry() throws Exception {
151+
// The producer's FAST path persists a frame's already-encoded delta bytes
152+
// via appendRawEntries. A short write (disk full / quota) mid-persist must
153+
// throw WITHOUT advancing size()/appendOffset, so a retry keyed off size()
154+
// re-writes the identical bytes at the same offset and recovers a gap-free,
155+
// duplicate-free dictionary. The failed-PUBLISH regressions
156+
// (DeltaDictRecoveryTest) exercise a persist that SUCCEEDED; only this pins
157+
// the persist-FAILURE trigger, whose idempotency the write-ahead
158+
// (persistNewSymbolsBeforePublish, resuming from pd.size()) relies on.
159+
assertMemoryLeak(() -> {
160+
Path src = Files.createTempDirectory("qwp-symdict-src");
161+
Path dst = Files.createTempDirectory("qwp-symdict-dst");
162+
try {
163+
GlobalSymbolDictionary dict = new GlobalSymbolDictionary();
164+
dict.getOrAddSymbol("AAPL"); // id 0
165+
dict.getOrAddSymbol("MSFT"); // id 1
166+
167+
// Encode the range once to obtain its on-disk [len][utf8]... bytes.
168+
PersistedSymbolDict encoded = PersistedSymbolDict.open(src.toString());
169+
encoded.appendSymbols(dict, 0, 1);
170+
encoded.close();
171+
172+
PersistedSymbolDict source = PersistedSymbolDict.open(src.toString());
173+
try {
174+
long addr = source.loadedEntriesAddr();
175+
int rawLen = source.loadedEntriesLen();
176+
int count = source.size();
177+
178+
ShortWriteOnceFacade ff = new ShortWriteOnceFacade();
179+
PersistedSymbolDict d = PersistedSymbolDict.open(ff, dst.toString());
180+
Assert.assertNotNull(d);
181+
try {
182+
ff.armed = true; // the next entry append lands short
183+
try {
184+
d.appendRawEntries(addr, rawLen, count);
185+
Assert.fail("a short write must throw");
186+
} catch (IllegalStateException expected) {
187+
Assert.assertTrue("short-write error: " + expected.getMessage(),
188+
expected.getMessage().contains("short write"));
189+
}
190+
// The throw preceded the size/offset advance: nothing persisted.
191+
Assert.assertEquals("short write must NOT advance size", 0, d.size());
192+
193+
// Retry the SAME bytes (the facade auto-disarmed): the write
194+
// lands at the unchanged offset, overwriting the torn prefix.
195+
d.appendRawEntries(addr, rawLen, count);
196+
Assert.assertEquals(2, d.size());
197+
} finally {
198+
d.close();
199+
}
200+
} finally {
201+
source.close();
202+
}
203+
204+
// Recovery sees a gap-free, duplicate-free dictionary (size 2, not 3).
205+
PersistedSymbolDict reopened = PersistedSymbolDict.open(dst.toString());
206+
try {
207+
Assert.assertEquals("retry must not duplicate or gap the dictionary", 2, reopened.size());
208+
ObjList<String> symbols = reopened.readLoadedSymbols();
209+
Assert.assertEquals("AAPL", symbols.getQuick(0));
210+
Assert.assertEquals("MSFT", symbols.getQuick(1));
211+
} finally {
212+
reopened.close();
213+
}
214+
} finally {
215+
rmDir(src);
216+
rmDir(dst);
217+
}
218+
});
219+
}
220+
149221
@Test
150222
public void testAppendSymbolsBatchWritesDenseRange() throws Exception {
151223
// appendSymbols persists a whole id range in one write (the hot-path
@@ -204,6 +276,60 @@ public void testAppendSymbolsBatchWritesDenseRange() throws Exception {
204276
});
205277
}
206278

279+
@Test
280+
public void testAppendSymbolsShortWriteThrowsWithoutAdvancingIsIdempotentOnRetry() throws Exception {
281+
// The producer's SLOW path (re-encode the [pd.size()..batchMax] suffix after
282+
// a prior partial persist) writes via appendSymbols. A short write (disk full
283+
// / quota) must throw WITHOUT advancing size()/appendOffset, so a retry keyed
284+
// off size() re-persists the SAME range at the SAME offset and recovers a
285+
// gap-free, duplicate-free dictionary. A regression that advanced size before
286+
// the written==len check would strand a torn/duplicated dictionary here.
287+
assertMemoryLeak(() -> {
288+
Path dir = Files.createTempDirectory("qwp-symdict");
289+
try {
290+
GlobalSymbolDictionary dict = new GlobalSymbolDictionary();
291+
dict.getOrAddSymbol("AAPL"); // id 0
292+
dict.getOrAddSymbol("GOOG"); // id 1
293+
294+
ShortWriteOnceFacade ff = new ShortWriteOnceFacade();
295+
PersistedSymbolDict d = PersistedSymbolDict.open(ff, dir.toString());
296+
Assert.assertNotNull(d);
297+
try {
298+
ff.armed = true; // the next entry append lands short
299+
try {
300+
d.appendSymbols(dict, 0, 1);
301+
Assert.fail("a short write must throw");
302+
} catch (IllegalStateException expected) {
303+
Assert.assertTrue("short-write error: " + expected.getMessage(),
304+
expected.getMessage().contains("short write"));
305+
}
306+
// The throw preceded the size/offset advance: nothing persisted.
307+
Assert.assertEquals("short write must NOT advance size", 0, d.size());
308+
309+
// Retry the SAME range (the facade auto-disarmed): re-writes at
310+
// the unchanged offset, overwriting the torn bytes.
311+
d.appendSymbols(dict, 0, 1);
312+
Assert.assertEquals(2, d.size());
313+
} finally {
314+
d.close();
315+
}
316+
317+
// Recovery sees a gap-free, duplicate-free dictionary (size 2, not 3).
318+
PersistedSymbolDict reopened = PersistedSymbolDict.open(dir.toString());
319+
try {
320+
Assert.assertEquals("retry must not duplicate or gap the dictionary", 2, reopened.size());
321+
ObjList<String> symbols = reopened.readLoadedSymbols();
322+
Assert.assertEquals("AAPL", symbols.getQuick(0));
323+
Assert.assertEquals("GOOG", symbols.getQuick(1));
324+
} finally {
325+
reopened.close();
326+
}
327+
} finally {
328+
rmDir(dir);
329+
}
330+
});
331+
}
332+
207333
@Test
208334
public void testBadMagicIsRecreatedEmpty() throws Exception {
209335
assertMemoryLeak(() -> {
@@ -776,4 +902,24 @@ public long length(String path) {
776902
return (long) Integer.MAX_VALUE + 1L;
777903
}
778904
}
905+
906+
/**
907+
* Lands ONE armed entry append short -- writes {@code len-1} of the {@code len}
908+
* requested bytes and reports {@code len-1} -- reproducing a disk-full / quota
909+
* short write mid-persist. Fires only on an entry append (offset past the
910+
* 8-byte header), never the header write, and disarms after firing so the retry
911+
* writes cleanly.
912+
*/
913+
private static final class ShortWriteOnceFacade extends DelegatingFilesFacade {
914+
boolean armed;
915+
916+
@Override
917+
public long write(int fd, long addr, long len, long offset) {
918+
if (armed && offset > 0 && len > 1) {
919+
armed = false;
920+
return INSTANCE.write(fd, addr, len - 1, offset);
921+
}
922+
return INSTANCE.write(fd, addr, len, offset);
923+
}
924+
}
779925
}

0 commit comments

Comments
 (0)