Skip to content

Commit 50fc218

Browse files
glasstigerclaude
andcommitted
Give the catch-up cap gap a settle budget
Two hardening fixes to the split/catch-up paths for heterogeneous / rolling-cap clusters. Catch-up cap-gap settle budget (M1): when a symbol-dict catch-up entry is too large for the fresh server's advertised batch cap, sendDictCatchUp latched a terminal on first sight. A homogeneous cluster never trips this (an entry that fit its data frame under a cap always fits its bare catch-up frame under that same cap), but a failover to a smaller-cap node could kill the sender for an entry an earlier node accepted. It now retries across MAX_CATCHUP_CAP_GAP_ATTEMPTS consecutive reconnects (throwing a retriable CatchUpSendException, no recordFatal), riding out the transient window until a larger-cap node returns; only a persistent gap latches the terminal. A successful catch-up resets the budget, and a transient reconnect never reaches the catch-up so it neither increments nor burns it. Matches the orphan drainer's durable-ack settle budget. Split-flush cap snapshot (M2): flushPendingRows now snapshots the volatile serverMaxBatchSize once and threads it into flushPendingRowsSplit for both the pre-flight sizing and the publish-loop assert. A mid-flush failover (the I/O thread lowers the cap) could previously make the two passes size against different caps, breaking the all-or-nothing guarantee and firing the assert on a legitimate race. The next flush picks up the new cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ff2e841 commit 50fc218

4 files changed

Lines changed: 136 additions & 32 deletions

File tree

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

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3512,11 +3512,19 @@ private void flushPendingRows(boolean deferCommit) {
35123512
int messageSize = encoder.finishMessage();
35133513
QwpBufferWriter buffer = encoder.getBuffer();
35143514

3515-
if (serverMaxBatchSize > 0 && messageSize > serverMaxBatchSize) {
3515+
// Snapshot the volatile cap ONCE for this whole flush. The I/O thread lowers
3516+
// serverMaxBatchSize on a mid-stream failover to a smaller-cap node
3517+
// (applyServerBatchSizeLimit); if the split pre-flight and the publish loop
3518+
// re-read the field independently, a failover between them would size frames
3519+
// against two different caps -- breaking the all-or-nothing guarantee and
3520+
// firing the publish-loop assert on a legitimate race. Both use this snapshot;
3521+
// the next flush picks up the new cap.
3522+
int cap = serverMaxBatchSize;
3523+
if (cap > 0 && messageSize > cap) {
35163524
// The combined frame's delta-entry bytes are byte-identical to the first
35173525
// split frame's (same baseline + batch max), so capture the length now
35183526
// for the arithmetic frame-sizing in flushPendingRowsSplit.
3519-
flushPendingRowsSplit(keys, deferCommit, encoder.getDeltaEntriesLen());
3527+
flushPendingRowsSplit(keys, deferCommit, encoder.getDeltaEntriesLen(), cap);
35203528
return;
35213529
}
35223530

@@ -3568,9 +3576,9 @@ private void flushPendingRows(boolean deferCommit) {
35683576
* carry FLAG_DEFER_COMMIT. When false, only the
35693577
* last message omits the flag.
35703578
*/
3571-
private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferCommit, int combinedDeltaEntriesLen) {
3579+
private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferCommit, int combinedDeltaEntriesLen, int cap) {
35723580
if (LOG.isDebugEnabled()) {
3573-
LOG.debug("Splitting flush across multiple messages [serverMaxBatchSize={}, defer={}]", serverMaxBatchSize, deferCommit);
3581+
LOG.debug("Splitting flush across multiple messages [serverMaxBatchSize={}, defer={}]", cap, deferCommit);
35743582
}
35753583

35763584
// Collect non-empty table indices so we know which is last, AND pre-flight
@@ -3613,12 +3621,12 @@ private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferComm
36133621
+ (deltaCount > 0 ? combinedDeltaEntriesLen : 0)
36143622
+ splitFrameBodyBytes.getQuick(bodyIdx);
36153623
bodyIdx++;
3616-
if (messageSize > serverMaxBatchSize) {
3624+
if (messageSize > cap) {
36173625
resetTableBuffersAfterFlush(keys);
36183626
throw new LineSenderException("single table batch too large for server batch cap")
36193627
.put(" [table=").put(tableName)
36203628
.put(", messageSize=").put(messageSize)
3621-
.put(", serverMaxBatchSize=").put(serverMaxBatchSize).put(']');
3629+
.put(", serverMaxBatchSize=").put(cap).put(']');
36223630
}
36233631
// Mirror advanceSentMaxSymbolId: once the first frame ships the batch's
36243632
// new ids, the remaining frames carry an empty delta above the baseline.
@@ -3655,12 +3663,14 @@ private void flushPendingRowsSplit(ObjList<CharSequence> keys, boolean deferComm
36553663
// The pre-flight pass above already verified every split frame fits the
36563664
// cap, so none can be found oversized here -- which is what keeps this
36573665
// loop from publishing (and stranding) a deferred prefix before an
3658-
// oversized table. The assert guards a future divergence between the two
3659-
// passes; it deliberately does NOT reset+throw here, because by this
3660-
// point a prefix may already be on the ring.
3661-
assert messageSize <= serverMaxBatchSize
3666+
// oversized table. Both passes size against the SAME snapshot cap, so a
3667+
// mid-flush failover cannot make them disagree; the assert therefore only
3668+
// catches a genuine divergence between the pre-flight arithmetic and the
3669+
// real encode (a future bug), not a cap race. It deliberately does NOT
3670+
// reset+throw here, because by this point a prefix may already be on the ring.
3671+
assert messageSize <= cap
36623672
: "split frame exceeded serverMaxBatchSize after pre-flight [table=" + tableName
3663-
+ ", messageSize=" + messageSize + ", serverMaxBatchSize=" + serverMaxBatchSize + ']';
3673+
+ ", messageSize=" + messageSize + ", serverMaxBatchSize=" + cap + ']';
36643674

36653675
// Write-ahead persist before publish (see flushPendingRows). The
36663676
// first split frame carries the batch's new symbols; the rest are

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

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,18 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
133133
*/
134134
public static final long DEFAULT_POISON_MIN_ESCALATION_WINDOW_MILLIS = 5_000L;
135135
private static final Logger LOG = LoggerFactory.getLogger(CursorWebSocketSendLoop.class);
136+
// Settle budget for the symbol-dict catch-up cap gap: how many CONSECUTIVE
137+
// reconnect attempts may find a single dictionary entry too large for the fresh
138+
// server's advertised batch cap before the sender latches a terminal. A
139+
// homogeneous cluster never trips it -- an entry that fit its data frame under a
140+
// cap always fits its bare catch-up frame under that same cap -- so this only
141+
// affects a heterogeneous / rolling-cap cluster, where a failover to a
142+
// smaller-cap node can hit it for an entry an earlier node accepted. Retrying
143+
// rides out the transient window until a larger-cap node returns; only a
144+
// persistent gap (every reachable node too small for this many attempts) latches
145+
// terminal, matching the orphan drainer's DEFAULT_MAX_DURABLE_ACK_MISMATCH_ATTEMPTS.
146+
// A successful catch-up resets the counter (see sendDictCatchUp).
147+
private static final int MAX_CATCHUP_CAP_GAP_ATTEMPTS = 16;
136148
// Hard ceiling for the lifetime-monotonic sent-dictionary mirror. The mirror
137149
// fields are int, so it cannot exceed Integer.MAX_VALUE bytes; reaching even
138150
// this needs ~200M+ distinct symbols on a single connection, far past any real
@@ -206,6 +218,12 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
206218
// for the connection's lifetime (a reconnect may need the whole dictionary at
207219
// any moment), so it cannot be dropped; it is an intentional cost of the feature.
208220
private final boolean deltaDictEnabled;
221+
// Consecutive reconnect attempts whose symbol-dict catch-up found an entry too
222+
// large for the fresh server's batch cap (see MAX_CATCHUP_CAP_GAP_ATTEMPTS). A
223+
// successful catch-up resets it; it is NOT reset per connection -- it measures
224+
// the cap-gap episode across reconnects so a persistent gap eventually latches.
225+
// I/O-thread-only.
226+
private int catchUpCapGapAttempts;
209227
// True once a real ring frame (data or commit) has been sent on the CURRENT
210228
// connection, as opposed to only the dictionary catch-up. The catch-up
211229
// consumes wire sequences (nextWireSeq), so nextWireSeq > 0 no longer implies
@@ -2243,24 +2261,36 @@ private int sendDictCatchUp() {
22432261
+ NativeBufferWriter.varintSize(1)
22442262
+ entryBytes;
22452263
if (soloFrameLen > frameLimit) {
2246-
// Non-retriable: the entry will not shrink and the same cluster
2247-
// re-advertises the same cap, so reconnecting would livelock.
2248-
// Latch a terminal (the data must be resent after the cap is
2249-
// raised) rather than calling fail() -- which, from inside the
2250-
// catch-up, would re-enter connectLoop (see CatchUpSendException).
2264+
// Cap gap: this entry cannot be re-registered under the fresh
2265+
// server's advertised cap. A HOMOGENEOUS cluster never reaches here
2266+
// (an entry that fit its data frame under a cap always fits its bare
2267+
// catch-up frame under that same cap), so the only way in is a
2268+
// heterogeneous / rolling-cap failover to a smaller-cap node.
22512269
//
2252-
// Tradeoff (heterogeneous / rolling-cap clusters): a symbol
2253-
// accepted under a larger/absent cap can hit this on failover to a
2254-
// smaller-cap node, and the hard terminal does NOT self-recover if
2255-
// a later node advertises a larger cap -- the producer must be
2256-
// resumed after the data is resent (or the cap raised). Bounding
2257-
// symbol size at ingest, or a settle budget across reconnects
2258-
// before latching, would relax this, but both are larger changes;
2259-
// the terminal keeps the homogeneous common case livelock-free.
2270+
// Give the cluster a settle budget instead of latching on first
2271+
// sight: a larger-cap node may return, so retry across reconnects
2272+
// and only latch a terminal after MAX_CATCHUP_CAP_GAP_ATTEMPTS
2273+
// consecutive attempts still find it too large. Under budget the
2274+
// throw is RETRIABLE (no recordFatal) -- connectLoop reconnects with
2275+
// backoff and re-runs the catch-up, which resets the counter on a
2276+
// node that accepts it. A transient reconnect (connect/upgrade
2277+
// failure, role reject) never reaches the catch-up, so it neither
2278+
// increments nor burns this budget. On exhaustion latch via
2279+
// recordFatal, NOT fail() -- failing from inside the catch-up would
2280+
// re-enter connectLoop (see CatchUpSendException); the data must be
2281+
// resent after the cap is raised.
2282+
catchUpCapGapAttempts++;
2283+
boolean exhausted = catchUpCapGapAttempts >= MAX_CATCHUP_CAP_GAP_ATTEMPTS;
22602284
LineSenderException err = new LineSenderException(
22612285
"symbol dictionary entry too large for the server batch cap during catch-up ["
2262-
+ "frameLen=" + soloFrameLen + ", cap=" + cap + ']');
2263-
recordFatal(err);
2286+
+ "frameLen=" + soloFrameLen + ", cap=" + cap + ", attempt="
2287+
+ catchUpCapGapAttempts + '/' + MAX_CATCHUP_CAP_GAP_ATTEMPTS + ']'
2288+
+ (exhausted
2289+
? "; the data must be resent after the cap is raised"
2290+
: "; retrying -- a larger-cap node may return"));
2291+
if (exhausted) {
2292+
recordFatal(err);
2293+
}
22642294
throw new CatchUpSendException(err);
22652295
}
22662296
if (chunkSymbols > 0 && chunkBytes + entryBytes > budget) {
@@ -2279,6 +2309,9 @@ private int sendDictCatchUp() {
22792309
sendCatchUpChunk(chunkStartId, chunkSymbols, chunkStartAddr, (int) chunkBytes);
22802310
framesSent++;
22812311
}
2312+
// The whole dictionary re-registered without a cap gap: this node accepts
2313+
// every entry, so the cap-gap episode (if any) is over -- reset the budget.
2314+
catchUpCapGapAttempts = 0;
22822315
return framesSent;
22832316
}
22842317

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,15 @@ public void testFixedCapNearBoundarySymbolCatchesUpWithoutTerminal() throws Exce
212212
@Test
213213
public void testCatchUpEntryTooLargeForCapFailsTerminally() throws Exception {
214214
// A dictionary entry that exceeds the reconnect server's per-chunk budget
215-
// (cap - HEADER_SIZE - 16) cannot be shipped as a catch-up chunk.
216-
// sendDictCatchUp must latch a clean terminal ("... during catch-up")
217-
// rather than call fail(): pre-fix the oversized entry drove an endless
218-
// reconnect loop (the entry never shrinks and the same cluster
219-
// re-advertises the same cap) and re-entered connectLoop from the catch-up.
215+
// (cap - HEADER_SIZE - 16) cannot be shipped as a catch-up chunk. Every
216+
// reachable node re-advertises the same small cap here, so the gap never
217+
// resolves: sendDictCatchUp must retry across the settle budget and then
218+
// latch a clean terminal ("... during catch-up") rather than call fail()
219+
// (which from inside the catch-up re-enters connectLoop). Pre-fix it latched
220+
// on the FIRST cap gap; the settle budget (MAX_CATCHUP_CAP_GAP_ATTEMPTS)
221+
// rides out a transient smaller-cap window first (see the retry sibling in
222+
// CursorWebSocketSendLoopCatchUpAlignmentTest), and only a persistent gap
223+
// exhausts it. Small reconnect backoffs keep the budgeted attempts fast.
220224
//
221225
// Connection 1 advertises no cap, so the ~202-byte symbol registers and
222226
// enters the sent-dictionary mirror. The handler then shrinks the
@@ -234,7 +238,8 @@ public void testCatchUpEntryTooLargeForCapFailsTerminally() throws Exception {
234238

235239
String bigSymbol = TestUtils.repeat("x", 200); // ~202-byte dict entry
236240
LineSenderException terminal = null;
237-
Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";");
241+
Sender sender = Sender.fromConfig("ws::addr=localhost:" + port
242+
+ ";reconnect_initial_backoff_millis=10;reconnect_max_backoff_millis=50;");
238243
try {
239244
sender.table("t").symbol("s", bigSymbol).longColumn("v", 1L).atNow();
240245
sender.flush();

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import io.questdb.client.DefaultHttpClientConfiguration;
2828
import io.questdb.client.cutlass.http.client.WebSocketClient;
29+
import io.questdb.client.cutlass.line.LineSenderException;
2930
import io.questdb.client.cutlass.qwp.client.WebSocketResponse;
3031
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
3132
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop;
@@ -265,6 +266,61 @@ public void testCatchUpChunkFrameSizeOverflowFailsLoud() throws Exception {
265266
});
266267
}
267268

269+
@Test
270+
public void testCatchUpCapGapRetriesUntilBudgetThenLatches() throws Exception {
271+
// M1: an entry too large for the fresh server's cap during catch-up (a
272+
// heterogeneous / rolling-cap failover to a smaller-cap node) must NOT latch
273+
// on first sight. sendDictCatchUp throws a RETRIABLE CatchUpSendException so
274+
// the reconnect loop rides it out -- a larger-cap node may return -- and only
275+
// after MAX_CATCHUP_CAP_GAP_ATTEMPTS consecutive cap gaps does it recordFatal.
276+
// Pre-fix the first cap gap latched a terminal, so one transient failover to a
277+
// smaller-cap node killed the sender. (A successful catch-up resets the budget;
278+
// the other catch-up tests, which use a fitting cap, never trip it.)
279+
TestUtils.assertMemoryLeak(() -> {
280+
Field maxField = CursorWebSocketSendLoop.class.getDeclaredField("MAX_CATCHUP_CAP_GAP_ATTEMPTS");
281+
maxField.setAccessible(true);
282+
int maxAttempts = maxField.getInt(null);
283+
// cap 160 => catch-up budget is below a ~216-byte solo frame for a 200-char symbol.
284+
CatchUpCapturingClient client = new CatchUpCapturingClient(160);
285+
try (CursorSendEngine engine = newEngine()) {
286+
CursorWebSocketSendLoop loop = newLoop(engine, client);
287+
try {
288+
seedMirror(loop, TestUtils.repeat("x", 200));
289+
// Attempts 1 .. max-1 are retriable: no terminal is latched.
290+
for (int i = 1; i < maxAttempts; i++) {
291+
try {
292+
invokeSetWireBaselineWithCatchUp(loop, engine.ackedFsn() + 1L);
293+
fail("cap gap must raise a retriable CatchUpSendException (attempt " + i + ')');
294+
} catch (InvocationTargetException e) {
295+
assertEquals("CatchUpSendException", e.getCause().getClass().getSimpleName());
296+
assertTrue("attempt " + i + " must name the catch-up cap gap: "
297+
+ e.getCause().getMessage(),
298+
e.getCause().getMessage().contains("during catch-up"));
299+
}
300+
loop.checkError(); // under budget => retriable => no terminal
301+
}
302+
// The exhausting attempt still throws, and now latches the terminal.
303+
try {
304+
invokeSetWireBaselineWithCatchUp(loop, engine.ackedFsn() + 1L);
305+
fail("the exhausting cap gap must still raise CatchUpSendException");
306+
} catch (InvocationTargetException e) {
307+
assertEquals("CatchUpSendException", e.getCause().getClass().getSimpleName());
308+
}
309+
try {
310+
loop.checkError();
311+
fail("exhausting the cap-gap settle budget must latch a terminal");
312+
} catch (LineSenderException terminal) {
313+
assertTrue("terminal must name the exhausted catch-up cap gap: " + terminal.getMessage(),
314+
terminal.getMessage().contains("during catch-up")
315+
&& terminal.getMessage().contains("must be resent"));
316+
}
317+
} finally {
318+
loop.close();
319+
}
320+
}
321+
});
322+
}
323+
268324
private static void appendFrames(CursorSendEngine engine, int count) {
269325
long buf = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT);
270326
try {

0 commit comments

Comments
 (0)