Skip to content

Commit 68aa31a

Browse files
glasstigerclaude
andcommitted
Pin the quarantine test on the rename failure
testQuarantineRenameFailurePreservesOriginalSlot counted the slot's .sfa files BEFORE build(), then asserted the count was unchanged after the failed rename. Recovery legitimately unlinks an empty never-rotated hot-spare on the way in (SegmentRing drops a recovered segment whose frameCount() is 0 with no torn tail), and whether SegmentManager had provisioned one by then is a race -- so the assertion measured that race rather than the rename. It failed 3 of 3 standalone runs, and inverted (2 vs 3) under load. Snapshot inside the quarantineAfterCloseHook instead: after the engine closed, before the rename. That is exactly what "a failed rename preserves the slot" means, and it is immune to the hot-spare race because both counts observe the post-recovery state. Assert the snapshot is non-zero as well, so the equality cannot pass vacuously on an empty slot. The product was never at fault: the reclaimed segment holds no frames, and close() unlinks nothing -- instrumenting quarantineTornSlot shows the same segments before and after it. Mutating that method to delete a segment during the failed rename still fails the test (expected 2, was 1), so it continues to catch real destruction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9cc550f commit 68aa31a

1 file changed

Lines changed: 16 additions & 2 deletions

File tree

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/DeltaDictRecoveryTest.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,16 @@ public void testQuarantineRenameFailurePreservesOriginalSlot() throws Exception
471471
assertMemoryLeak(() -> {
472472
writeAndTearUnreplayableSlot();
473473
java.nio.file.Path slot = Paths.get(sfDir, "default");
474-
int segmentCount = countSegmentFiles(slot);
474+
// Snapshot when quarantine STARTS (after the engine closed, before the
475+
// rename), not before build(). Recovery legitimately unlinks an empty
476+
// never-rotated hot-spare on the way in (SegmentRing: frameCount()==0 and
477+
// no torn tail), and whether SegmentManager provisioned one is a race --
478+
// so a pre-build count measures that race, not this test's subject. What
479+
// the rename failure must preserve is whatever the slot holds at the
480+
// moment quarantine begins.
481+
AtomicInteger atQuarantineStart = new AtomicInteger(-1);
482+
Sender.LineSenderBuilder.setQuarantineAfterCloseHookForTest(
483+
() -> atQuarantineStart.set(countSegmentFiles(slot)));
475484
Sender.LineSenderBuilder.setQuarantineFilesFacadeForTest(new DelegatingFilesFacade() {
476485
@Override
477486
public int rename(String oldPath, String newPath) {
@@ -503,11 +512,16 @@ public int rename(String oldPath, String newPath) {
503512
}
504513
} finally {
505514
Sender.LineSenderBuilder.setQuarantineFilesFacadeForTest(null);
515+
Sender.LineSenderBuilder.setQuarantineAfterCloseHookForTest(null);
506516
}
507517
Assert.assertTrue("rename failure must preserve the original slot directory",
508518
java.nio.file.Files.isDirectory(slot));
519+
// Guards the equality below against passing vacuously on an empty slot:
520+
// 0 == 0 would "preserve every segment" while holding nothing.
521+
Assert.assertTrue("quarantine must have started with data-bearing segments, saw: "
522+
+ atQuarantineStart.get(), atQuarantineStart.get() > 0);
509523
Assert.assertEquals("rename failure must preserve every segment",
510-
segmentCount, countSegmentFiles(slot));
524+
atQuarantineStart.get(), countSegmentFiles(slot));
511525
Assert.assertFalse("failed rename must not leave a quarantine directory",
512526
java.nio.file.Files.exists(Paths.get(sfDir, "default.unreplayable-0")));
513527
});

0 commit comments

Comments
 (0)