Skip to content

Commit 75d89b8

Browse files
glasstigerclaude
andcommitted
Preserve cap-gap episode; minor test/style nits
A cluster of small review follow-ups; only the first changes behavior. - The orphan-tail-retire re-anchor caught a CatchUpSendException and passed fail(e.getCause()), unwrapping the cap-gap marker so connectLoop's isCatchUpCapGap reset the orphan settle episode instead of preserving it. Route unwrap-aware: fail(isCatchUpCapGap(e) ? e : e.getCause()) keeps the attempt count and dwell anchor across the re-anchor recycle for a genuine cap gap, and unwraps to the raw cause otherwise, exactly as a normal send failure does. This only ever errs safe today -- more retries before an orphan drainer quarantines -- but the wrapper exists precisely to carry that signal. - Drop @testonly from BackgroundDrainer.connectWithDurableAckRetry: run() calls it in production, so the annotation was inaccurate. - Rename CatchUpSendException.capGap to isCapGap (boolean naming). - Restore the CursorWebSocketSendLoop final-field group and a QwpWebSocketSender import to alphabetical order. - DeltaDictCatchUpTest: fix a comment naming a test that does not exist. - DeltaDictRecoveryTest: delete the never-instantiated CountingHandler and the orphaned javadoc stacked above FullDiskDictFacade. - CursorSendEngineTest: rename the legacy-guard test to describe what it asserts (the guards force an FSN gap; current recovery adopts the v2 slot) -- it never drives a legacy reader to throw. - PoisonFrameTest: the two catch-up-only "does not strike" tests now assert poisonStrikes() == 0 explicitly instead of relying only on checkError() not throwing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0a9c3d6 commit 75d89b8

7 files changed

Lines changed: 18 additions & 30 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@
4747
import io.questdb.client.cutlass.qwp.client.sf.cursor.DefaultSenderErrorHandler;
4848
import io.questdb.client.cutlass.qwp.client.sf.cursor.DefaultSenderProgressHandler;
4949
import io.questdb.client.cutlass.qwp.client.sf.cursor.PersistedSymbolDict;
50-
import io.questdb.client.cutlass.qwp.client.sf.cursor.UnreplayableSlotException;
5150
import io.questdb.client.cutlass.qwp.client.sf.cursor.SenderConnectionDispatcher;
5251
import io.questdb.client.cutlass.qwp.client.sf.cursor.SenderErrorDispatcher;
5352
import io.questdb.client.cutlass.qwp.client.sf.cursor.SenderProgressDispatcher;
53+
import io.questdb.client.cutlass.qwp.client.sf.cursor.UnreplayableSlotException;
5454
import io.questdb.client.cutlass.qwp.protocol.QwpConstants;
5555
import io.questdb.client.cutlass.qwp.protocol.QwpTableBuffer;
5656
import io.questdb.client.std.CharSequenceObjHashMap;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,6 @@ public BackgroundDrainer() {
265265
* @return a fresh durable-ack-capable client, or {@code null} if
266266
* {@link #outcome} has been set to FAILED or STOPPED
267267
*/
268-
@TestOnly
269268
public WebSocketClient connectWithDurableAckRetry() {
270269
// run() already set runnerThread; setting it again here is a no-op
271270
// on that path but wires up direct @TestOnly calls so requestStop()

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,9 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
229229
// user-facing 5-minute default is applied at the config layer.
230230
private final long catchUpCapGapMinEscalationWindowNanos;
231231
private final CatchUpCapGapPolicy catchUpCapGapPolicy;
232-
private final ReconnectPolicy reconnectPolicy;
233232
private final CursorSendEngine engine;
234233
private final long parkNanos;
234+
private final ReconnectPolicy reconnectPolicy;
235235
// FIFO of OK-acked batches awaiting durable-upload confirmation. Used only
236236
// when durableAckMode is true. Each entry binds a wireSeq to the per-table
237237
// (name, seqTxn) pairs the server reported on the OK frame. The queue is
@@ -1548,7 +1548,7 @@ private void clearDurableAckTracking() {
15481548
}
15491549

15501550
private static boolean isCatchUpCapGap(Throwable t) {
1551-
return t instanceof CatchUpSendException && ((CatchUpSendException) t).capGap;
1551+
return t instanceof CatchUpSendException && ((CatchUpSendException) t).isCapGap;
15521552
}
15531553

15541554
private void resetCatchUpCapGapEpisode() {
@@ -3103,7 +3103,12 @@ private boolean trySendOne() {
31033103
// Re-anchor's catch-up send failed. fail() here is a fresh,
31043104
// non-re-entrant connectLoop entry from the I/O loop body --
31053105
// the same recovery a normal trySendOne send failure takes.
3106-
fail(e.getCause());
3106+
// Preserve the wrapper on a cap gap so connectLoop's
3107+
// isCatchUpCapGap keeps the orphan settle episode (attempt count
3108+
// + dwell anchor) alive across the re-anchor recycle; an ordinary
3109+
// failure unwraps to the raw cause and restarts the episode, like
3110+
// any normal send failure.
3111+
fail(isCatchUpCapGap(e) ? e : e.getCause());
31073112
return false;
31083113
}
31093114
return true;
@@ -3325,15 +3330,15 @@ public interface ReconnectFactory {
33253330
* send failures restart the orphan settle episode.
33263331
*/
33273332
private static final class CatchUpSendException extends RuntimeException {
3328-
private final boolean capGap;
3333+
private final boolean isCapGap;
33293334

33303335
CatchUpSendException(Throwable cause) {
33313336
this(cause, false);
33323337
}
33333338

3334-
CatchUpSendException(Throwable cause, boolean capGap) {
3339+
CatchUpSendException(Throwable cause, boolean isCapGap) {
33353340
super(cause);
3336-
this.capGap = capGap;
3341+
this.isCapGap = isCapGap;
33373342
}
33383343
}
33393344

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public void testFixedCapNearBoundarySymbolCatchesUpWithoutTerminal() throws Exce
164164
// table("t").symbol("s", <173 chars>).atNow() encodes to 198 bytes (<=200,
165165
// accepted), its dict entry is 2+173=175 bytes (> old budget 172 -> old
166166
// terminal), while the real solo catch-up frame is 12+1+1+175=189 (<=200 ->
167-
// fits). Unlike testCatchUpEntryTooLargeForCapFailsTerminally (a genuinely
167+
// fits). Unlike testCatchUpCapGapRetriesUntilBudgetThenLatches (a genuinely
168168
// oversized entry on a shrunk cap, which MUST still terminate), this entry
169169
// is legally shippable and must NOT terminate.
170170
final int cap = 200;

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,26 +1636,6 @@ public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] dat
16361636
}
16371637
}
16381638

1639-
/** Counts every binary frame it receives and acks it. */
1640-
private static class CountingHandler implements TestWebSocketServer.WebSocketServerHandler {
1641-
final AtomicInteger frames = new AtomicInteger();
1642-
private final AtomicLong nextSeq = new AtomicLong(0);
1643-
1644-
@Override
1645-
public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) {
1646-
frames.incrementAndGet();
1647-
try {
1648-
client.sendBinary(QwpWireTestUtils.buildAck(nextSeq.getAndIncrement()));
1649-
} catch (IOException e) {
1650-
throw new RuntimeException(e);
1651-
}
1652-
}
1653-
}
1654-
/**
1655-
* Short-writes the FIRST dictionary append after it is armed, modelling a disk that
1656-
* fills mid-flush. Offset 0 is left alone so the file header still writes -- only the
1657-
* entry append fails, which is the path under test.
1658-
*/
16591639
/**
16601640
* Refuses to grow the symbol dictionary's mmap append window -- the production
16611641
* shape of a full disk, since PersistedSymbolDict reserves that window through

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ public void testLegacyReaderGuardsSurviveReopenAndAreRepairedWhenDamaged() throw
356356
}
357357

358358
@Test
359-
public void testV2DiskSlotForcesLegacyReaderToFailClosed() throws Exception {
359+
public void testLegacyGuardsForceFsnGapAndCurrentRecoveryAdoptsV2Slot() throws Exception {
360360
TestUtils.assertMemoryLeak(() -> {
361361
long buf = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT);
362362
try {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,8 @@ public void testNonOrderlyCloseAfterOnlyCatchUpDoesNotStrike() throws Exception
270270
}
271271
// No strike was ever charged, so nothing escalated: the loop stays
272272
// retriable and the producer-facing error latch is clear.
273+
assertEquals("a catch-up-only non-orderly close must charge no poison strike",
274+
0, loop.poisonStrikes());
273275
loop.checkError();
274276
} finally {
275277
closeAll(clients);
@@ -306,6 +308,8 @@ public void testNonOrderlyRejectionAfterOnlyCatchUpDoesNotStrike() throws Except
306308
}
307309
// No strike was ever charged, so nothing escalated: the loop stays
308310
// retriable and the producer-facing error latch is clear.
311+
assertEquals("a catch-up-only NACK must charge no poison strike",
312+
0, loop.poisonStrikes());
309313
loop.checkError();
310314
} finally {
311315
closeAll(clients);

0 commit comments

Comments
 (0)