Skip to content

Commit 1699c0a

Browse files
glasstigerclaude
andcommitted
Snapshot server batch cap once in sendRow
sendRow read the volatile serverMaxBatchSize three times while checking one row against the cap. The I/O thread lowers that field mid-stream on a failover -- clearing it to 0 when the new node advertises no cap -- so a read torn across the guard and the throw could reject the row against a "cap" of 0, which means "no cap": the row rolled back and a retry succeeded, but the producer still saw a spurious "row too large for server batch cap" error. Snapshot the field into a local once, exactly as flushPendingRows already does, so the guard, the comparison and the message all use one consistent value. Also add QwpWebSocketSenderTest coverage for the per-row cap guard, which had none: an oversized row under a positive cap throws and rolls back to zero rows, and a large row commits when the server advertises no cap (the invariant the snapshot protects). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0a3010f commit 1699c0a

2 files changed

Lines changed: 80 additions & 3 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4155,12 +4155,18 @@ private void sendRow() {
41554155
// batch stay intact. (The check ignores the null-padding bytes
41564156
// nextRow() will add; that's bounded by numColumns * elemSize and
41574157
// far below any realistic cap.)
4158-
if (serverMaxBatchSize > 0) {
4158+
// Snapshot the volatile cap ONCE, as flushPendingRows does. The I/O thread
4159+
// lowers serverMaxBatchSize -- or clears it to 0 on a failover to a node that
4160+
// advertises no cap -- mid-stream via applyServerBatchSizeLimit. Re-reading the
4161+
// field across the guard and the throw could observe it drop to 0 between reads
4162+
// and reject the row against a "cap" of 0, which actually means "no cap".
4163+
int cap = serverMaxBatchSize;
4164+
if (cap > 0) {
41594165
long rowBytes = currentTableBuffer.getBufferedBytes() - currentTableBufferSnapshotBytes;
4160-
if (rowBytes > serverMaxBatchSize) {
4166+
if (rowBytes > cap) {
41614167
throw new LineSenderException("row too large for server batch cap")
41624168
.put(" [rowBytes=").put(rowBytes)
4163-
.put(", serverMaxBatchSize=").put(serverMaxBatchSize).put(']');
4169+
.put(", serverMaxBatchSize=").put(cap).put(']');
41644170
}
41654171
}
41664172

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,77 @@ public void testResetAfterCloseThrows() throws Exception {
624624
});
625625
}
626626

627+
@Test
628+
public void testRowAcceptedWhenServerAdvertisesNoCap() throws Exception {
629+
// serverMaxBatchSize == 0 means "no cap" (an older server, or a failover to a
630+
// node that advertises none). The per-row guard in sendRow must be skipped, so
631+
// even a large row is accepted rather than rejected against a "cap" of 0. This
632+
// is the invariant the sendRow cap snapshot protects: the I/O thread can lower
633+
// the volatile serverMaxBatchSize to 0 mid-row, and a torn read that observed
634+
// the drop between the guard and the throw must not spuriously reject the row.
635+
assertMemoryLeak(() -> {
636+
try (QwpWebSocketSender sender = QwpWebSocketSender.createForTesting(
637+
"localhost", 9000,
638+
/*autoFlushRows*/ Integer.MAX_VALUE,
639+
/*autoFlushBytes*/ 0,
640+
/*autoFlushIntervalNanos*/ 0L)) {
641+
sender.setConnectedForTest(true);
642+
sender.applyServerBatchSizeLimit(0);
643+
Assert.assertEquals(0, sender.getServerMaxBatchSize());
644+
645+
StringBuilder big = new StringBuilder();
646+
for (int i = 0; i < 256; i++) {
647+
big.append('x');
648+
}
649+
// A row far larger than any small cap still commits when the server
650+
// advertises no cap; the guard leaves it alone.
651+
sender.table("t").stringColumn("s", big.toString()).atNow();
652+
653+
QwpTableBuffer buf = sender.getTableBuffer("t");
654+
Assert.assertEquals(1, buf.getRowCount());
655+
}
656+
});
657+
}
658+
659+
@Test
660+
public void testRowExceedingServerBatchCapThrows() throws Exception {
661+
// The per-row guard in sendRow rejects a single row whose encoded bytes
662+
// already exceed the server's advertised cap, before nextRow() commits it, so
663+
// the flush cannot build an oversize WS frame the server closes with 1009.
664+
assertMemoryLeak(() -> {
665+
try (QwpWebSocketSender sender = QwpWebSocketSender.createForTesting(
666+
"localhost", 9000,
667+
/*autoFlushRows*/ Integer.MAX_VALUE,
668+
/*autoFlushBytes*/ 0,
669+
/*autoFlushIntervalNanos*/ 0L)) {
670+
sender.setConnectedForTest(true);
671+
sender.applyServerBatchSizeLimit(64);
672+
Assert.assertEquals(64, sender.getServerMaxBatchSize());
673+
674+
StringBuilder big = new StringBuilder();
675+
for (int i = 0; i < 256; i++) {
676+
big.append('x');
677+
}
678+
sender.table("t").stringColumn("s", big.toString());
679+
try {
680+
sender.atNow();
681+
Assert.fail("Expected LineSenderException");
682+
} catch (LineSenderException e) {
683+
Assert.assertTrue(
684+
"expected 'row too large for server batch cap', got: " + e.getMessage(),
685+
e.getMessage().contains("row too large for server batch cap"));
686+
// The message reports the single snapshotted cap, not a torn value.
687+
Assert.assertTrue(
688+
"expected the snapshotted cap in the message, got: " + e.getMessage(),
689+
e.getMessage().contains("serverMaxBatchSize=64"));
690+
}
691+
// The rejected row was rolled back before commit, leaving no row behind.
692+
QwpTableBuffer buf = sender.getTableBuffer("t");
693+
Assert.assertEquals(0, buf.getRowCount());
694+
}
695+
});
696+
}
697+
627698
@Test
628699
public void testStringColumnAfterCloseThrows() throws Exception {
629700
assertMemoryLeak(() -> {

0 commit comments

Comments
 (0)