Skip to content

Commit 530920e

Browse files
committed
test(pool): make close interrupt deadline checks deterministic
Track the bounded creation wait and handled interrupts in one atomic test witness. Handshake repeated interrupts until the original wait exits so the regression tests reject deadline resets without depending on scheduler timing.
1 parent 6937474 commit 530920e

3 files changed

Lines changed: 132 additions & 70 deletions

File tree

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ public final class QueryClientPool implements AutoCloseable {
9797
// DEFAULT_CLOSE_QUERY_TIMEOUT_MILLIS. Volatile because QuestDBImpl sets it
9898
// once at build time on a different thread than the borrowers that read it.
9999
private volatile long closeQueryTimeoutMillis = DEFAULT_CLOSE_QUERY_TIMEOUT_MILLIS;
100+
// Atomic test witness for the bounded creation-wait loop. The sign bit
101+
// means active; the remaining bits count handled InterruptedExceptions.
102+
// Condition.hasWaiters() cannot serve this purpose because an interrupt
103+
// transiently transfers the closer out of the condition queue. Volatile
104+
// lets white-box tests observe active/count as one coherent snapshot.
105+
private volatile long creationWaitState;
100106
private int inFlightCreations;
101107

102108
public QueryClientPool(
@@ -339,13 +345,21 @@ public void close() {
339345
final long creationWaitDeadlineNanos = System.nanoTime() + creationWaitNanos;
340346
long creationRemainingNanos = creationWaitNanos;
341347
boolean creationWaitInterrupted = false;
342-
while (inFlightCreations > 0 && creationRemainingNanos > 0) {
343-
try {
344-
creationFinished.awaitNanos(creationRemainingNanos);
345-
} catch (InterruptedException e) {
346-
creationWaitInterrupted = true;
348+
creationWaitState = inFlightCreations > 0 && creationRemainingNanos > 0
349+
? Long.MIN_VALUE
350+
: 0;
351+
try {
352+
while (inFlightCreations > 0 && creationRemainingNanos > 0) {
353+
try {
354+
creationFinished.awaitNanos(creationRemainingNanos);
355+
} catch (InterruptedException e) {
356+
creationWaitInterrupted = true;
357+
creationWaitState++;
358+
}
359+
creationRemainingNanos = creationWaitDeadlineNanos - System.nanoTime();
347360
}
348-
creationRemainingNanos = creationWaitDeadlineNanos - System.nanoTime();
361+
} finally {
362+
creationWaitState &= Long.MAX_VALUE;
349363
}
350364
if (creationWaitInterrupted) {
351365
Thread.currentThread().interrupt();
@@ -528,11 +542,16 @@ void release(QueryWorker w, long gen) {
528542
}
529543
}
530544

545+
/**
546+
* Returns whether {@link #close()} is inside its bounded wait for
547+
* in-flight creations. This remains true while an interrupt transfers the
548+
* closer out of the condition queue and it re-enters with the same deadline.
549+
*/
531550
@TestOnly
532551
public boolean hasCreationWaiterForTesting() {
533552
lock.lock();
534553
try {
535-
return lock.hasWaiters(creationFinished);
554+
return creationWaitState < 0;
536555
} finally {
537556
lock.unlock();
538557
}

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,12 @@ public final class SenderPool implements AutoCloseable {
238238
// (cancelling recovery) WITHOUT making a later close() short-circuit the
239239
// teardown. Guarded by lock.
240240
private boolean closeStarted;
241+
// Atomic test witness for the bounded creation-wait loop. The sign bit
242+
// means active; the remaining bits count handled InterruptedExceptions.
243+
// Condition.hasWaiters() cannot serve this purpose because an interrupt
244+
// transiently transfers the closer out of the condition queue. Volatile
245+
// lets white-box tests observe active/count as one coherent snapshot.
246+
private volatile long creationWaitState;
241247
private int inFlightCreations;
242248
// Lease teardowns currently running on borrower threads (retireLease's
243249
// delegate-close section, outside the lock). close() counts these as
@@ -1339,11 +1345,16 @@ public Thread getStartupRecoveryThreadForTesting() {
13391345
return startupRecoveryThread;
13401346
}
13411347

1348+
/**
1349+
* Returns whether {@link #close()} is inside its bounded wait for
1350+
* in-flight creations. This remains true while an interrupt transfers the
1351+
* closer out of the condition queue and it re-enters with the same deadline.
1352+
*/
13421353
@TestOnly
13431354
public boolean hasCreationWaiterForTesting() {
13441355
lock.lock();
13451356
try {
1346-
return lock.hasWaiters(creationFinished);
1357+
return creationWaitState < 0;
13471358
} finally {
13481359
lock.unlock();
13491360
}
@@ -1483,13 +1494,21 @@ public void close() {
14831494
final long creationWaitDeadlineNanos = System.nanoTime() + creationWaitNanos;
14841495
long creationRemainingNanos = creationWaitNanos;
14851496
boolean creationWaitInterrupted = false;
1486-
while (inFlightCreations > 0 && creationRemainingNanos > 0) {
1487-
try {
1488-
creationFinished.awaitNanos(creationRemainingNanos);
1489-
} catch (InterruptedException e) {
1490-
creationWaitInterrupted = true;
1497+
creationWaitState = inFlightCreations > 0 && creationRemainingNanos > 0
1498+
? Long.MIN_VALUE
1499+
: 0;
1500+
try {
1501+
while (inFlightCreations > 0 && creationRemainingNanos > 0) {
1502+
try {
1503+
creationFinished.awaitNanos(creationRemainingNanos);
1504+
} catch (InterruptedException e) {
1505+
creationWaitInterrupted = true;
1506+
creationWaitState++;
1507+
}
1508+
creationRemainingNanos = creationWaitDeadlineNanos - System.nanoTime();
14911509
}
1492-
creationRemainingNanos = creationWaitDeadlineNanos - System.nanoTime();
1510+
} finally {
1511+
creationWaitState &= Long.MAX_VALUE;
14931512
}
14941513
if (creationWaitInterrupted) {
14951514
Thread.currentThread().interrupt();

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

Lines changed: 80 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,18 @@
4343
import java.util.concurrent.atomic.AtomicBoolean;
4444
import java.util.concurrent.atomic.AtomicInteger;
4545
import java.util.concurrent.atomic.AtomicReference;
46-
import java.util.function.BooleanSupplier;
46+
import java.util.concurrent.locks.LockSupport;
4747
import java.util.function.Consumer;
4848
import java.util.function.IntFunction;
49+
import java.util.function.LongSupplier;
4950

5051
public class QuestDBImplCloseLifecycleTest {
5152

53+
private static final long QUERY_CREATION_WAIT_STATE_OFFSET =
54+
Unsafe.getFieldOffset(QueryClientPool.class, "creationWaitState");
5255
private static final String QUERY_CFG = "ws::addr=127.0.0.1:9000;";
56+
private static final long SENDER_CREATION_WAIT_STATE_OFFSET =
57+
Unsafe.getFieldOffset(SenderPool.class, "creationWaitState");
5358
private static final String SENDER_CFG = "http::addr=127.0.0.1:1;protocol_version=2;auto_flush=off;";
5459

5560
@Test(timeout = 30_000)
@@ -186,18 +191,14 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringQueryCreation() thr
186191
inCreation.countDown();
187192
awaitOrFail(releaseCreation, "test never released query creation");
188193
};
189-
// A 1s creation-wait budget, not 100ms: the interrupt storm below must land at least
190-
// twice inside this window for the deadline-restart property to be exercised at all,
191-
// and a freshly started, yielding interrupter thread is not guaranteed two scheduler
192-
// quanta within 100ms on a saturated CI agent (observed on hosted 3-core mac agents,
193-
// where the post-join count assert failed with the product deadline honored exactly).
194+
// A 1s creation-wait budget, not 100ms, leaves enough room to
195+
// establish the handshaked interrupt loop before asserting the
196+
// original absolute deadline.
194197
QuestDBImpl db = newQuestDB(
195198
SENDER_CFG, 0, 0, 1000, slotIndex -> fakeSender(null, null, null), connectHook);
196199
QueryClientPool pool = db.getQueryPoolForTesting();
197200
AtomicReference<Throwable> borrowOutcome = new AtomicReference<>();
198201
AtomicBoolean closeReturnedInterrupted = new AtomicBoolean();
199-
AtomicBoolean keepInterrupting = new AtomicBoolean(true);
200-
AtomicInteger interruptCount = new AtomicInteger();
201202
Thread borrower = new Thread(() -> {
202203
try {
203204
db.borrowQuery();
@@ -209,13 +210,6 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringQueryCreation() thr
209210
db.close();
210211
closeReturnedInterrupted.set(Thread.currentThread().isInterrupted());
211212
}, "interrupted-query-closer");
212-
Thread interrupter = new Thread(() -> {
213-
while (keepInterrupting.get()) {
214-
interruptCount.incrementAndGet();
215-
closer.interrupt();
216-
Thread.yield();
217-
}
218-
}, "query-close-interrupter");
219213
long nativeBaseline = Unsafe.getMemUsedByTag(MemoryTag.NATIVE_DEFAULT);
220214
try {
221215
borrower.start();
@@ -226,14 +220,17 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringQueryCreation() thr
226220
closer.start();
227221
awaitCreationWaiter(pool,
228222
"facade close did not wait while query construction was internally owned");
229-
interrupter.start();
230-
awaitRepeatedInterrupts(interruptCount, pool::hasCreationWaiterForTesting,
231-
"query close left its creation wait before the interrupt storm landed twice");
223+
long handledInterrupts = interruptUntilCreationWaitEnds(
224+
closer,
225+
() -> getCreationWaitState(pool),
226+
"query creation wait did not honor its absolute deadline");
232227
closer.join(TimeUnit.SECONDS.toMillis(5));
233228
Assert.assertFalse(
234229
"repeated interrupts restarted the query creation-wait deadline",
235230
closer.isAlive());
236-
Assert.assertTrue("test did not repeatedly interrupt query close", interruptCount.get() > 1);
231+
Assert.assertTrue(
232+
"query close did not handle both interrupts",
233+
handledInterrupts > 1);
237234
Assert.assertTrue("facade close must restore query closer interruption",
238235
closeReturnedInterrupted.get());
239236
Assert.assertEquals(
@@ -252,9 +249,7 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringQueryCreation() thr
252249
borrowOutcome.get() instanceof QueryException
253250
&& String.valueOf(borrowOutcome.get().getMessage()).contains("closed"));
254251
} finally {
255-
keepInterrupting.set(false);
256252
releaseCreation.countDown();
257-
interrupter.join(TimeUnit.SECONDS.toMillis(10));
258253
db.close();
259254
borrower.join(TimeUnit.SECONDS.toMillis(10));
260255
closer.join(TimeUnit.SECONDS.toMillis(10));
@@ -275,15 +270,13 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringSenderCreation() th
275270
};
276271
String senderConfig = "ws::addr=localhost:1;sf_dir="
277272
+ System.getProperty("java.io.tmpdir") + "/qdb-interrupted-pool-" + System.nanoTime() + ";";
278-
// 1s creation-wait budget for the same reason as the query-interrupt test above: the
279-
// interrupt storm must land at least twice inside the window even on a saturated agent.
273+
// Use the same 1s budget and handshaked interrupt loop as the query
274+
// lifecycle test above.
280275
QuestDBImpl db = newQuestDB(senderConfig, 0, 0, 1000, senderFactory, client -> {
281276
});
282277
SenderPool pool = db.getSenderPoolForTesting();
283278
AtomicReference<Throwable> borrowOutcome = new AtomicReference<>();
284279
AtomicBoolean closeReturnedInterrupted = new AtomicBoolean();
285-
AtomicBoolean keepInterrupting = new AtomicBoolean(true);
286-
AtomicInteger interruptCount = new AtomicInteger();
287280
Thread borrower = new Thread(() -> {
288281
try {
289282
db.borrowSender();
@@ -295,13 +288,6 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringSenderCreation() th
295288
db.close();
296289
closeReturnedInterrupted.set(Thread.currentThread().isInterrupted());
297290
}, "interrupted-sender-closer");
298-
Thread interrupter = new Thread(() -> {
299-
while (keepInterrupting.get()) {
300-
interruptCount.incrementAndGet();
301-
closer.interrupt();
302-
Thread.yield();
303-
}
304-
}, "sender-close-interrupter");
305291
try {
306292
borrower.start();
307293
Assert.assertTrue("sender borrow never reached construction",
@@ -313,14 +299,17 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringSenderCreation() th
313299
closer.start();
314300
awaitCreationWaiter(pool,
315301
"facade close did not wait while sender construction was internally owned");
316-
interrupter.start();
317-
awaitRepeatedInterrupts(interruptCount, pool::hasCreationWaiterForTesting,
318-
"sender close left its creation wait before the interrupt storm landed twice");
302+
long handledInterrupts = interruptUntilCreationWaitEnds(
303+
closer,
304+
() -> getCreationWaitState(pool),
305+
"sender creation wait did not honor its absolute deadline");
319306
closer.join(TimeUnit.SECONDS.toMillis(5));
320307
Assert.assertFalse(
321308
"repeated interrupts restarted the sender creation-wait deadline",
322309
closer.isAlive());
323-
Assert.assertTrue("test did not repeatedly interrupt sender close", interruptCount.get() > 1);
310+
Assert.assertTrue(
311+
"sender close did not handle both interrupts",
312+
handledInterrupts > 1);
324313
Assert.assertTrue("facade close must restore sender closer interruption",
325314
closeReturnedInterrupted.get());
326315
Assert.assertEquals(
@@ -341,9 +330,7 @@ public void facadeCloseIsBoundedUnderRepeatedInterruptsDuringSenderCreation() th
341330
borrowOutcome.get() instanceof LineSenderException
342331
&& String.valueOf(borrowOutcome.get().getMessage()).contains("closed"));
343332
} finally {
344-
keepInterrupting.set(false);
345333
releaseCreation.countDown();
346-
interrupter.join(TimeUnit.SECONDS.toMillis(10));
347334
db.close();
348335
borrower.join(TimeUnit.SECONDS.toMillis(10));
349336
closer.join(TimeUnit.SECONDS.toMillis(10));
@@ -529,32 +516,69 @@ private static void awaitCreationWaiter(SenderPool pool, String message) {
529516
Assert.fail(message);
530517
}
531518

519+
private static long getCreationWaitState(QueryClientPool pool) {
520+
return Unsafe.getUnsafe().getLongVolatile(pool, QUERY_CREATION_WAIT_STATE_OFFSET);
521+
}
522+
523+
private static long getCreationWaitState(SenderPool pool) {
524+
return Unsafe.getUnsafe().getLongVolatile(pool, SENDER_CREATION_WAIT_STATE_OFFSET);
525+
}
526+
532527
/**
533-
* Holds the test until the interrupt storm has landed at least twice while the facade close is
534-
* still inside its bounded creation wait. The deadline-restart property is only exercised by
535-
* interrupts that arrive during that wait, and the scheduler owes the interrupter thread
536-
* nothing: with a post-join count assert alone, the run races the close budget against thread
537-
* scheduling and can fail with the product invariant intact. Failing here instead separates
538-
* "interrupter starved before the budget expired" from a genuine deadline bug.
528+
* Delivers one interrupt at a time, waiting until the creation-wait loop
529+
* handles each one before delivering the next. This prevents interrupt
530+
* coalescing while keeping pressure on the wait until its original absolute
531+
* deadline expires. The state snapshot encodes active in the sign bit and
532+
* the handled-interrupt count in the remaining bits.
539533
*/
540-
private static void awaitRepeatedInterrupts(
541-
AtomicInteger interruptCount,
542-
BooleanSupplier closerStillWaiting,
534+
private static long interruptUntilCreationWaitEnds(
535+
Thread closer,
536+
LongSupplier creationWaitState,
543537
String message
544538
) {
545-
long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
539+
long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
546540
while (System.nanoTime() < deadline) {
547-
// Count first: two interrupts observed while polling means the storm landed no matter
548-
// how quickly the wait ends afterwards, so a budget expiry seen next is not a failure.
549-
if (interruptCount.get() > 1) {
550-
return;
541+
long state = creationWaitState.getAsLong();
542+
long handledInterrupts = state & Long.MAX_VALUE;
543+
if (state >= 0) {
544+
Assert.assertTrue(
545+
message + "; wait ended after handling only " + handledInterrupts + " interrupt(s)",
546+
handledInterrupts > 1);
547+
return handledInterrupts;
551548
}
552-
if (!closerStillWaiting.getAsBoolean()) {
553-
Assert.fail(message + "; interrupts landed: " + interruptCount.get());
549+
550+
closer.interrupt();
551+
long countBeforeInterrupt = handledInterrupts;
552+
while (System.nanoTime() < deadline) {
553+
state = creationWaitState.getAsLong();
554+
handledInterrupts = state & Long.MAX_VALUE;
555+
if (state >= 0) {
556+
Assert.assertTrue(
557+
message + "; wait ended after handling only " + handledInterrupts + " interrupt(s)",
558+
handledInterrupts > 1);
559+
return handledInterrupts;
560+
}
561+
if (handledInterrupts > countBeforeInterrupt) {
562+
// Keep enough interrupt pressure to expose a restarted
563+
// deadline without monopolising a CI worker core.
564+
LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1));
565+
break;
566+
}
567+
Thread.yield();
554568
}
555-
Thread.yield();
556569
}
557-
Assert.fail(message + "; interrupts landed: " + interruptCount.get());
570+
571+
long state = creationWaitState.getAsLong();
572+
long handledInterrupts = state & Long.MAX_VALUE;
573+
if (state >= 0) {
574+
Assert.assertTrue(
575+
message + "; wait ended after handling only " + handledInterrupts + " interrupt(s)",
576+
handledInterrupts > 1);
577+
return handledInterrupts;
578+
}
579+
Assert.fail(message + "; creation wait still active after handling "
580+
+ handledInterrupts + " interrupt(s)");
581+
return 0;
558582
}
559583

560584
private static void awaitOrFail(CountDownLatch latch, String message) {

0 commit comments

Comments
 (0)