Skip to content

Commit 98a4ac0

Browse files
glasstigerclaude
andcommitted
Don't key the cap-gap episode off nanoTime's sign
sendDictCatchUp tested `catchUpCapGapFirstNanos < 0` to decide whether a symbol-dictionary cap-gap episode was already open. A System.nanoTime() instant is only meaningful as a difference: its origin is arbitrary and the spec permits negative values, so its sign cannot encode state. CursorWebSocketSendLoopCatchUpAlignmentTest backdates the episode anchor by two hours to prove that a cap gap outliving the escalation window latches a terminal. On Linux nanoTime() is nanos-since-boot, so on a freshly booted CI agent it sits far below two hours of nanos and the backdated anchor came out negative. The loop then read that real anchor as "unset", re-anchored it to now on every strike and pinned episodeNanos at ~0: the dwell was never satisfied, no terminal latched, and the test failed. It fails on every machine with under two hours of uptime and on no long-lived dev box, which is why only CI saw it. Key the episode off the strike count instead, exactly as recordHeadRejectionStrike keys its own off poisonStrikes. -1 survives as a debug marker but no longer decides anything, so no state rides on a timestamp's sign. Add testCapGapEpisodeWithANegativeAnchorStillEscalates, which plants the negative anchor directly and so pins the sentinel whatever the host's uptime; the existing test only reddens where uptime happens to fall under its two-hour backdate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3b73bc8 commit 98a4ac0

2 files changed

Lines changed: 89 additions & 10 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -301,15 +301,19 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
301301
// nor resets it. NOT reset per connection -- it measures the cap-gap episode
302302
// across reconnects so a persistent gap eventually latches. I/O-thread-only.
303303
private int catchUpCapGapAttempts;
304-
// System.nanoTime() of the FIRST cap gap of the current episode, or -1 when no
305-
// episode is open. Anchors the escalation dwell. Reset together with
306-
// catchUpCapGapAttempts on a successful catch-up, so a node that accepts the
307-
// dictionary ends the episode outright. Anchoring at the FIRST gap (not at loop
308-
// entry, and not refreshed per attempt) is what keeps a TRANSIENT from burning the
309-
// terminal budget: a transient never reaches the catch-up, so it neither increments
310-
// the counter nor moves the anchor -- it can only lengthen the wall clock, which
311-
// alone can never latch the terminal because the strike count still has to be
312-
// satisfied. I/O-thread-only, like catchUpCapGapAttempts.
304+
// System.nanoTime() of the FIRST cap gap of the current episode. Anchors the
305+
// escalation dwell. Reset together with catchUpCapGapAttempts on a successful
306+
// catch-up, so a node that accepts the dictionary ends the episode outright.
307+
// Anchoring at the FIRST gap (not at loop entry, and not refreshed per attempt) is
308+
// what keeps a TRANSIENT from burning the terminal budget: a transient never reaches
309+
// the catch-up, so it neither increments the counter nor moves the anchor -- it can
310+
// only lengthen the wall clock, which alone can never latch the terminal because the
311+
// strike count still has to be satisfied. I/O-thread-only, like catchUpCapGapAttempts.
312+
//
313+
// -1 marks "no episode open" for a debugger or a heap dump, but it is NOT the test
314+
// for one -- catchUpCapGapAttempts == 0 is (see sendDictCatchUp). A nanoTime instant
315+
// is only meaningful as a difference: its origin is arbitrary and the spec permits
316+
// negative values, so no state may ride on this field's sign.
313317
private long catchUpCapGapFirstNanos = -1L;
314318
// True once a real ring frame (data or commit) has been sent on the CURRENT
315319
// connection, as opposed to only the dictionary catch-up. The catch-up consumes
@@ -2442,8 +2446,18 @@ private int sendDictCatchUp() {
24422446
// an ordinary rolling restart of the larger-cap node. Escalating on the
24432447
// count alone would therefore hard-fail a live store-and-forward
24442448
// producer during a routine cluster operation.
2449+
//
2450+
// The STRIKE COUNT -- never the anchor's sign -- says whether an episode is
2451+
// already open. A System.nanoTime() instant is only meaningful as a
2452+
// difference: its origin is arbitrary and the spec permits negative values,
2453+
// so a sign test cannot tell an unset anchor from a real one. Wherever it
2454+
// misreads a real anchor as unset it re-anchors to now on EVERY strike,
2455+
// pinning episodeNanos at ~0, and the dwell can then never be satisfied --
2456+
// the terminal never latches, however long the gap truly persists.
2457+
// recordHeadRejectionStrike keys its episode off poisonStrikes for the same
2458+
// reason.
24452459
long nowNanos = System.nanoTime();
2446-
if (catchUpCapGapFirstNanos < 0) {
2460+
if (catchUpCapGapAttempts == 0) {
24472461
catchUpCapGapFirstNanos = nowNanos;
24482462
}
24492463
catchUpCapGapAttempts++;

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,71 @@ public void testCatchUpCapGapStrikesAloneDoNotLatchWithinTheEscalationWindow() t
330330
});
331331
}
332332

333+
@Test
334+
public void testCapGapEpisodeWithANegativeAnchorStillEscalates() throws Exception {
335+
// A cap-gap episode anchored at a NEGATIVE nanoTime instant must escalate like any
336+
// other. A System.nanoTime() value is only meaningful as a difference -- its origin
337+
// is arbitrary and the spec permits negative values -- so no state may ride on the
338+
// anchor's sign. sendDictCatchUp once tested catchUpCapGapFirstNanos < 0 to mean "no
339+
// episode open": that read a negative anchor as unset, re-anchored it to now on every
340+
// strike and pinned episodeNanos at ~0, so the dwell was never satisfied and the
341+
// terminal could never latch, however long the cap gap truly persisted.
342+
//
343+
// That is what reddened CI. The sibling test above backdates the anchor two hours,
344+
// and on Linux nanoTime() is nanos-since-boot: on a CI agent up ten minutes it is
345+
// ~6e11, so "two hours ago" comes out ~ -6.6e12 -- negative. The defect therefore
346+
// only surfaced where uptime is under that backdate: every fresh CI agent, and never
347+
// a long-lived dev box (which is why it passed locally). Planting the negative anchor
348+
// directly pins the sentinel on ANY machine, whatever its uptime.
349+
TestUtils.assertMemoryLeak(() -> {
350+
Field maxField = CursorWebSocketSendLoop.class.getDeclaredField("MAX_CATCHUP_CAP_GAP_ATTEMPTS");
351+
maxField.setAccessible(true);
352+
int maxAttempts = maxField.getInt(null);
353+
CatchUpCapturingClient client = new CatchUpCapturingClient(160);
354+
try (CursorSendEngine engine = newEngine()) {
355+
// A one-hour dwell, against an anchor two hours back: satisfied on elapsed,
356+
// but only if the negative anchor survives to the subtraction.
357+
CursorWebSocketSendLoop loop = newLoop(engine, client, 3_600_000L);
358+
try {
359+
seedMirror(loop, TestUtils.repeat("x", 200));
360+
// Satisfy the strike half of the AND, one short of the budget.
361+
for (int i = 1; i < maxAttempts; i++) {
362+
try {
363+
invokeSetWireBaselineWithCatchUp(loop, engine.ackedFsn() + 1L);
364+
fail("cap gap must raise a retriable CatchUpSendException (attempt " + i + ')');
365+
} catch (InvocationTargetException e) {
366+
assertEquals("CatchUpSendException", e.getCause().getClass().getSimpleName());
367+
}
368+
loop.checkError(); // dwell unmet => retriable, whatever the count
369+
}
370+
// The episode began two hours ago on a machine booted minutes ago.
371+
Field anchor = CursorWebSocketSendLoop.class.getDeclaredField("catchUpCapGapFirstNanos");
372+
anchor.setAccessible(true);
373+
anchor.setLong(loop, -TimeUnit.HOURS.toNanos(2));
374+
375+
// Both halves now hold, so this strike must latch the terminal.
376+
try {
377+
invokeSetWireBaselineWithCatchUp(loop, engine.ackedFsn() + 1L);
378+
fail("the escalating cap gap must still raise CatchUpSendException");
379+
} catch (InvocationTargetException e) {
380+
assertEquals("CatchUpSendException", e.getCause().getClass().getSimpleName());
381+
}
382+
try {
383+
loop.checkError();
384+
fail("a cap-gap episode anchored at a negative nanoTime instant must still "
385+
+ "escalate -- the anchor's sign carries no meaning");
386+
} catch (LineSenderException terminal) {
387+
assertTrue("terminal must name the exhausted catch-up cap gap: " + terminal.getMessage(),
388+
terminal.getMessage().contains("during catch-up")
389+
&& terminal.getMessage().contains("must be resent"));
390+
}
391+
} finally {
392+
loop.close();
393+
}
394+
}
395+
});
396+
}
397+
333398
@Test
334399
public void testTransientCatchUpFailureDoesNotBurnTheCapGapBudget() throws Exception {
335400
// A TRANSIENT catch-up failure (the wire drops mid-catch-up -- a flapping LB, a

0 commit comments

Comments
 (0)