Skip to content

Commit 186ae10

Browse files
glasstigerclaude
andcommitted
Defer catch-up commit; fix stale delta comments
Three delta symbol-dict cleanups (a fourth reported item, a dead CursorSendEngine.isMemoryMode(), was already removed earlier on this branch, so nothing to do there): - sendCatchUpChunk now sets FLAG_DEFER_COMMIT on the catch-up frame. It carries dictionary entries but no rows, so it must never trigger a server-side commit. Today it is always the first frame on a fresh (empty-transaction) connection, so committing nothing is a no-op -- but that invariant is load-bearing and was unasserted. Deferring the empty commit removes the dependency, so a future mid-stream catch-up cannot prematurely commit an in-flight deferred transaction. The dictionary delta still registers, as any deferred data frame's does. - flushPendingRows' comment claimed delta mode is "memory-mode only"; file-mode store-and-forward ships monotonic deltas too once the persisted dictionary opened. Corrected. - The deltaDictEnabled field comment implied delta mode only applies to recovered / orphan-drained disk slots; it applies to fresh disk slots too. Clarified that recovery additionally seeds the mirror. testReconnectCatchUpRebuildsDictionary now asserts the catch-up frame carries FLAG_DEFER_COMMIT; it fails without the flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0b4ab3f commit 186ae10

3 files changed

Lines changed: 26 additions & 4 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3455,7 +3455,8 @@ private void flushPendingRows(boolean deferCommit) {
34553455
ensureActiveBufferReady();
34563456
// In full-dict mode every frame is self-sufficient: it carries the whole
34573457
// symbol dictionary from id 0 so orphan-drain / recovery replay to a fresh
3458-
// server never dangles a symbol id. In delta mode (memory-mode only) each
3458+
// server never dangles a symbol id. In delta mode (memory-mode, and
3459+
// file-mode store-and-forward once the persisted dictionary opened) each
34593460
// frame carries only ids above sentMaxSymbolId; a reconnect re-registers
34603461
// the dictionary via an I/O-thread catch-up frame before replay, so the
34613462
// producer's monotonic baseline stays valid across the wire boundary.

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,11 @@ public final class CursorWebSocketSendLoop implements QuietCloseable {
187187
private final WebSocketResponse response = new WebSocketResponse();
188188
private final ResponseHandler responseHandler = new ResponseHandler();
189189
// Delta symbol dictionary catch-up state (see swapClient). Active in memory
190-
// mode, and in disk mode on a recovered / orphan-drained slot (the constructor
191-
// seeds sentDict* from the persisted dictionary there).
190+
// mode, and in disk mode whenever the per-slot persisted dictionary opened --
191+
// fresh slots included, not just recovered / orphan-drained ones. On a
192+
// recovered / orphan-drained slot the constructor additionally SEEDS sentDict*
193+
// from that persisted dictionary; a fresh slot starts with an empty mirror and
194+
// grows it as frames are sent.
192195
// deltaDictEnabled gates all of it. The loop mirrors, in sentDict*, every
193196
// symbol it has ever sent -- the concatenated [len varint][utf8] bytes in
194197
// global-id order (sentDictBytes*) plus the count (sentDictCount) -- so that
@@ -2293,8 +2296,18 @@ private void sendCatchUpChunk(int deltaStart, int deltaCount, long symbolsAddr,
22932296
Unsafe.getUnsafe().putByte(frame + 2, (byte) 'P');
22942297
Unsafe.getUnsafe().putByte(frame + 3, (byte) '1');
22952298
Unsafe.getUnsafe().putByte(frame + 4, (byte) client.getServerQwpVersion());
2299+
// FLAG_DEFER_COMMIT: the catch-up carries dictionary entries but NO
2300+
// rows, so it must never trigger a server-side commit. Today it is
2301+
// always the first frame on a fresh (empty-transaction) connection, so
2302+
// committing nothing is a no-op -- but that invariant is load-bearing
2303+
// and unasserted. Deferring the (empty) commit removes the dependency:
2304+
// a future mid-stream catch-up cannot prematurely commit an in-flight
2305+
// deferred transaction. The dictionary delta still registers (as any
2306+
// deferred data frame's does); only the row commit is deferred, and the
2307+
// next real frame commits it.
22962308
Unsafe.getUnsafe().putByte(frame + QwpConstants.HEADER_OFFSET_FLAGS,
2297-
(byte) (QwpConstants.FLAG_GORILLA | QwpConstants.FLAG_DELTA_SYMBOL_DICT));
2309+
(byte) (QwpConstants.FLAG_GORILLA | QwpConstants.FLAG_DELTA_SYMBOL_DICT
2310+
| QwpConstants.FLAG_DEFER_COMMIT));
22982311
Unsafe.getUnsafe().putShort(frame + 6, (short) 0); // tableCount
22992312
Unsafe.getUnsafe().putInt(frame + 8, payloadLen);
23002313
long q = writeVarintAt(frame + QwpConstants.HEADER_SIZE, deltaStart);

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ public void testReconnectCatchUpRebuildsDictionary() throws Exception {
9292
List<String> conn2 = handler.dictFor(2);
9393
Assert.assertTrue("2nd connection saw a catch-up frame with 0 tables",
9494
handler.sawZeroTableFrameOnConn2);
95+
Assert.assertTrue("the catch-up frame carries no rows, so it must defer its "
96+
+ "(empty) commit -- FLAG_DEFER_COMMIT set",
97+
handler.catchUpDeferredOnConn2);
9598
Assert.assertEquals("2nd connection dictionary size", 2, conn2.size());
9699
Assert.assertEquals("alpha", conn2.get(0));
97100
Assert.assertEquals("beta", conn2.get(1));
@@ -326,6 +329,9 @@ private static class CatchUpHandler implements TestWebSocketServer.WebSocketServ
326329
// (rather than a fixed sleep) before sending batch 2, so batch 2 cannot
327330
// race into connection 1's pre-close window and must land on the reconnect.
328331
volatile boolean conn1Closed;
332+
// Set from the flags byte of the zero-table catch-up frame on connection 2:
333+
// the catch-up carries no rows and must defer its (empty) commit.
334+
volatile boolean catchUpDeferredOnConn2;
329335
volatile boolean sawZeroTableFrameOnConn2;
330336
private final List<List<String>> dictsByConn = new CopyOnWriteArrayList<>();
331337
private TestWebSocketServer.ClientHandler currentClient;
@@ -352,6 +358,8 @@ public synchronized void onBinaryMessage(TestWebSocketServer.ClientHandler clien
352358
accumulate(data, dict);
353359
if (connNumber == 2 && tableCount(data) == 0) {
354360
sawZeroTableFrameOnConn2 = true;
361+
// FLAG_DEFER_COMMIT is bit 0x01 of the flags byte (offset 5).
362+
catchUpDeferredOnConn2 = (data[5] & 0x01) != 0;
355363
}
356364
try {
357365
client.sendBinary(buildAck(nextSeq.getAndIncrement()));

0 commit comments

Comments
 (0)