Skip to content

Commit 79e0847

Browse files
committed
Make final-probe regression deterministic
Release the retired slot from a test-only hook after the positive borrow wait has actually exhausted its budget. This removes the scheduler-dependent sleep and guarantees the test reaches the final post-wait probe.
1 parent 7df62d5 commit 79e0847

2 files changed

Lines changed: 49 additions & 44 deletions

File tree

core/src/main/java/io/questdb/client/impl/SenderPool.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,11 @@ public final class SenderPool implements AutoCloseable {
148148
private final Condition slotReleased;
149149
// True iff the configuration enables store-and-forward (sf_dir set).
150150
private final boolean storeAndForward;
151+
// Test seam: runs after a capacity-starved borrow's condition wait has
152+
// exhausted its positive timeout, before the loop's terminal pass. Null in
153+
// production; regression tests release a retired slot here to prove that
154+
// the terminal pass re-probes returned capacity before throwing.
155+
private volatile Runnable borrowWaitExpiredHook;
151156
// Slots removed from `all` whose delegate is still releasing its flock.
152157
// They keep reserving capacity (and their slotInUse mark) until the
153158
// flock drops, so the cap check and the slot allocator stay consistent
@@ -818,6 +823,12 @@ public PooledSender borrow() {
818823
}
819824
try {
820825
remainingNanos = slotReleased.awaitNanos(remainingNanos);
826+
if (remainingNanos <= 0) {
827+
Runnable hook = borrowWaitExpiredHook;
828+
if (hook != null) {
829+
hook.run();
830+
}
831+
}
821832
} catch (InterruptedException e) {
822833
Thread.currentThread().interrupt();
823834
throw new LineSenderException("interrupted while waiting for a Sender from the pool");
@@ -830,6 +841,11 @@ public PooledSender borrow() {
830841
}
831842
}
832843

844+
@TestOnly
845+
public void setBorrowWaitExpiredHook(Runnable hook) {
846+
this.borrowWaitExpiredHook = hook;
847+
}
848+
833849
/**
834850
* Raises the shutdown signal early -- without tearing down live delegates --
835851
* so an in-flight startup-recovery step driven on the {@link PoolHousekeeper}

core/src/test/java/io/questdb/client/test/impl/SenderPoolSfTest.java

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,14 +1290,13 @@ public void testZeroTimeoutBorrowProbesRetiredSlotBeforeThrowing() throws Except
12901290

12911291
@Test
12921292
public void testParkedBorrowerGetsFinalProbeAfterBudgetExpiry() throws Exception {
1293-
// Release-during-wait twin of the zero-timeout test. A borrower parks
1294-
// in awaitNanos while the retired slot's flock is genuinely held; the
1295-
// deferred cleanup then releases the flock mid-wait. Nothing signals
1296-
// slotReleased (the release is delegate-side, volatile writes only), so
1297-
// the borrower sleeps out its full budget. Pre-fix its wake-up pass hit
1298-
// the terminal timeout check before reprobeRetiredSlots() and threw --
1299-
// missing capacity that had already come back. Post-fix the wake-up
1300-
// pass probes first, recovers the index, and the creation is admitted.
1293+
// Positive-timeout twin of the zero-timeout test. A borrower parks in
1294+
// awaitNanos while the retired slot's flock is genuinely held and
1295+
// sleeps out its full budget. A test hook releases the flock after the
1296+
// wait reports expiry but before the terminal loop pass. Pre-fix that
1297+
// pass hit the timeout check before reprobeRetiredSlots() and threw --
1298+
// missing capacity that had already come back. Post-fix the terminal
1299+
// pass probes first, recovers the index, and admits the creation.
13011300
TestUtils.assertMemoryLeak(() -> {
13021301
// Phase 1: strand unacked data under default-0.
13031302
try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) {
@@ -1318,8 +1317,9 @@ public void testParkedBorrowerGetsFinalProbeAfterBudgetExpiry() throws Exception
13181317
Assert.assertTrue("unacked data must persist under default-0",
13191318
hasSegmentFile(slot("default-0")));
13201319

1321-
// Phase 2: maxSize=1, generous budget so the mid-wait release lands
1322-
// comfortably inside it.
1320+
// Phase 2: maxSize=1 and a positive acquire budget. A test hook
1321+
// releases the flock only after awaitNanos() has returned with that
1322+
// budget exhausted, so the terminal wake-up pass is deterministic.
13231323
CountingAckHandler handler = new CountingAckHandler();
13241324
try (TestWebSocketServer ack = new TestWebSocketServer(handler)) {
13251325
int ackPort = ack.getPort();
@@ -1340,49 +1340,38 @@ public void testParkedBorrowerGetsFinalProbeAfterBudgetExpiry() throws Exception
13401340
return real;
13411341
};
13421342

1343-
try (SenderPool pool = newPoolWithFactory(cfg2, 0, 1, 2_000, factory)) {
1343+
try (SenderPool pool = newPoolWithFactory(cfg2, 0, 1, 100, factory)) {
13441344
Assert.assertNotNull("recovery must have built slot 0", forged.get());
13451345
Assert.assertEquals("precondition: startup recovery must retire the slot",
13461346
1, pool.leakedSlotCount());
13471347

1348-
// Borrower parks: its pre-park probe correctly fails while
1349-
// the flock is genuinely held.
1350-
AtomicReference<Throwable> failure = new AtomicReference<>();
1351-
AtomicReference<PooledSender> borrowed = new AtomicReference<>();
1352-
Thread borrower = new Thread(() -> {
1348+
AtomicBoolean waitExpired = new AtomicBoolean();
1349+
pool.setBorrowWaitExpiredHook(() -> {
1350+
Assert.assertTrue("expired-wait hook must run exactly once",
1351+
waitExpired.compareAndSet(false, true));
1352+
Sender recoverer = forged.get();
13531353
try {
1354-
borrowed.set(pool.borrow());
1355-
} catch (Throwable e) {
1356-
failure.set(e);
1354+
setBooleanField(recoverer, "closed", false);
1355+
} catch (Exception e) {
1356+
throw new RuntimeException(e);
13571357
}
1358+
// The flock drops only after the positive awaitNanos()
1359+
// budget is exhausted. This delegate-side release does
1360+
// not signal slotReleased.
1361+
recoverer.close();
13581362
});
1359-
borrower.start();
1360-
1361-
// Let the borrower enter borrow() and park. If a CI stall
1362-
// delays it past the release below, the pre-park probe
1363-
// recovers instead and the test degrades to a pass -- never
1364-
// a flaky failure.
1365-
Thread.sleep(300);
1366-
1367-
// Mid-wait release: flock drops, no signal reaches the pool.
1368-
Sender recoverer = forged.get();
1369-
setBooleanField(recoverer, "closed", false);
1370-
recoverer.close();
1371-
1372-
borrower.join(10_000);
1373-
Assert.assertFalse("borrower must have finished", borrower.isAlive());
1374-
if (failure.get() != null) {
1375-
throw new AssertionError(
1376-
"borrower must recover the retired slot on its wake-up pass, not time out",
1377-
failure.get());
1378-
}
1379-
PooledSender b = borrowed.get();
1380-
Assert.assertNotNull("borrower must have obtained a sender", b);
13811363
try {
1382-
Assert.assertEquals("wake-up probe must recover the retired slot's capacity",
1383-
0, pool.leakedSlotCount());
1364+
PooledSender b = pool.borrow();
1365+
try {
1366+
Assert.assertTrue("borrow must exhaust its positive wait budget",
1367+
waitExpired.get());
1368+
Assert.assertEquals("wake-up probe must recover the retired slot's capacity",
1369+
0, pool.leakedSlotCount());
1370+
} finally {
1371+
b.close();
1372+
}
13841373
} finally {
1385-
b.close();
1374+
pool.setBorrowWaitExpiredHook(null);
13861375
}
13871376
}
13881377
}

0 commit comments

Comments
 (0)