Skip to content

Commit e38b4d5

Browse files
bluestreak01claude
andcommitted
test(ilp): regression guards for SF disk-cap recovery
Two tests targeting the disk-cap deadlock scenario the reviewer flagged as a separate "high" severity finding. Both pass under the per-frame trim that landed in the previous commit; both would deadlock on the pre-fix code where trim left the active segment alone. testSingleActiveSegmentDoesNotDeadlockOnFullCap (new): Sets sf_max_bytes == sf_max_total_bytes, so no natural rotation can ever fire — the append-time projection check raises disk-full before rotate() is reached. Pre per-frame trim this state was permanent: the active was the only segment on disk, trim couldn't touch it, ACKs freed nothing. Force-rotate-on-fully-acked makes the active itself reclaimable, so an ACK covering every appended frame now restores capacity. The test stresses recovery further by refilling to disk-full a second time. testMaxTotalBytesTriggersDiskFullThenRecoversOnAck (renamed from testMaxTotalBytesTriggersDiskFull): The "Acceptable: only the active was on disk and active doesn't trim" branch in the catch block — which the reviewer specifically cited as evidence the deadlock was tolerated by the test suite — is gone. The recovery append after trim now must succeed; we assert it directly instead of swallowing a second disk-full. Full suite: 1976 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 91669d2 commit e38b4d5

1 file changed

Lines changed: 94 additions & 12 deletions

File tree

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

Lines changed: 94 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,91 @@ public void testReplayStopsWhenVisitorReturnsFalse() throws Exception {
537537
});
538538
}
539539

540+
/**
541+
* Direct repro for the "single active segment" deadlock the user
542+
* documented before per-frame trim landed: when {@code sf_max_bytes}
543+
* is generous (or never reached) but {@code sf_max_total_bytes} is
544+
* tight, every appended frame lives in one active segment with no
545+
* rotations. Pre per-frame trim, an ACK covering every frame freed
546+
* nothing — {@code trim} only reclaimed sealed segments — so the
547+
* sender stalled permanently even though the server had acknowledged
548+
* everything. The force-rotate-on-fully-acked path makes the active
549+
* itself reclaimable and breaks the deadlock.
550+
*/
551+
@Test
552+
public void testSingleActiveSegmentDoesNotDeadlockOnFullCap() throws Exception {
553+
TestUtils.assertMemoryLeak(() -> {
554+
// perSeg == totalCap: no natural rotation can ever fire,
555+
// because the projection check (writePos + total > perSeg
556+
// ⇒ projected += HEADER_SIZE) trips disk-full before rotate
557+
// is reached. Pre per-frame trim, the only path out of this
558+
// state was a natural rotation — which the projection
559+
// permanently blocks — so the sender deadlocked.
560+
final long perSeg = 200;
561+
final long totalCap = 200;
562+
byte[] payload = new byte[50];
563+
try (SegmentLog log = SegmentLog.open(tmpDir, perSeg, totalCap)) {
564+
long buf = alloc(payload);
565+
try {
566+
int appended = 0;
567+
SfDiskFullException dfe = null;
568+
for (int i = 0; i < 100 && dfe == null; i++) {
569+
try {
570+
log.append(buf, payload.length);
571+
appended++;
572+
} catch (SfDiskFullException e) {
573+
dfe = e;
574+
}
575+
}
576+
Assert.assertNotNull("expected disk-full once cap was hit", dfe);
577+
Assert.assertEquals("only the active segment should exist",
578+
1, log.segmentCount());
579+
580+
// Ack every appended frame. Force-rotate-on-fully-acked
581+
// must reclaim the active segment.
582+
log.trim(appended - 1);
583+
584+
Assert.assertEquals("oldestSeq -1 = active drained",
585+
-1L, log.oldestSeq());
586+
Assert.assertEquals("only the new empty active should remain",
587+
1, log.segmentCount());
588+
589+
// Stress the recovery: keep appending until the cap
590+
// is hit a second time. This proves the freed space
591+
// is genuinely reusable, not a one-shot trick.
592+
int reAppended = 0;
593+
SfDiskFullException secondDfe = null;
594+
for (int i = 0; i < 100 && secondDfe == null; i++) {
595+
try {
596+
log.append(buf, payload.length);
597+
reAppended++;
598+
} catch (SfDiskFullException e) {
599+
secondDfe = e;
600+
}
601+
}
602+
Assert.assertNotNull("cap should fire again after refill", secondDfe);
603+
Assert.assertTrue("second round must accept at least one frame",
604+
reAppended > 0);
605+
} finally {
606+
Unsafe.free(buf, payload.length, MemoryTag.NATIVE_DEFAULT);
607+
}
608+
}
609+
});
610+
}
611+
612+
/**
613+
* sf_max_total_bytes back-pressure: filling the cap raises
614+
* {@link SfDiskFullException}, and once every appended frame has been
615+
* acked, the next append must succeed. Pre per-frame trim, this case
616+
* could deadlock when the user's data fit entirely in the active
617+
* segment — trim only reclaimed sealed segments, so an ACK that
618+
* covered every appended frame still freed nothing. The
619+
* force-rotate-on-fully-acked path in {@link SegmentLog#trim} fixes
620+
* that: when every frame in the active segment is acked, the active
621+
* is sealed and immediately removed, so capacity returns.
622+
*/
540623
@Test
541-
public void testMaxTotalBytesTriggersDiskFull() throws Exception {
624+
public void testMaxTotalBytesTriggersDiskFullThenRecoversOnAck() throws Exception {
542625
TestUtils.assertMemoryLeak(() -> {
543626
// tiny: header (24) + ~4 frames of 50 bytes
544627
long perSeg = SegmentLog.HEADER_SIZE + 2L * (SegmentLog.FRAME_HEADER_SIZE + 50);
@@ -560,18 +643,17 @@ public void testMaxTotalBytesTriggersDiskFull() throws Exception {
560643
Assert.assertNotNull("eventually disk-full", dfe);
561644
Assert.assertTrue("appended at least one frame before disk-full", appended > 0);
562645

563-
// Trim what we have; active segment never trims, but if any sealed
564-
// exists it should go.
646+
// Ack every appended frame. trim must reclaim space
647+
// even when the unacked tail lives entirely in the
648+
// active segment.
565649
log.trim(appended - 1);
566-
// Try one more append after trim — could succeed if sealed segment was
567-
// dropped, freeing space.
568-
try {
569-
log.append(buf, payload.length);
570-
} catch (SfDiskFullException ignored) {
571-
// Acceptable: only the active was on disk and active doesn't trim.
572-
// The point is the disk-full exception fires, not that trim always
573-
// recovers from a single segment scenario.
574-
}
650+
651+
// Recovery is the load-bearing assertion. Pre per-frame
652+
// trim, this could throw SfDiskFullException again when
653+
// the active segment held all the capacity and trim
654+
// couldn't touch it — a permanent stall after every
655+
// frame had been acked.
656+
log.append(buf, payload.length);
575657
} finally {
576658
Unsafe.free(buf, payload.length, MemoryTag.NATIVE_DEFAULT);
577659
}

0 commit comments

Comments
 (0)