Skip to content

Commit 6937474

Browse files
committed
fix(qwp): drop cached timestamp columns and repair accounting on buffer clear/rollback
Senders cache designated-timestamp ColumnBuffer refs and derive bookkeeping from their table buffers. rollbackUncommittedColumns() (failed at()) and QwpTableBuffer.clear() close those columns, leaving dangling refs that NPE on the next at() and corrupt pending accounting. - Invalidate WebSocket timestamp caches in rollbackRow(); cancelRow() now routes through it. - Add QwpTableBuffer.Owner: clear() notifies the owning sender before releasing columns. WebSocket subtracts the buffer's exact accounted share (committedBytes - new baselineBytes, seed offsets excluded); UDP resets transient row state, datagram estimate, and headroom state (its base-estimate cache is keyed by column count alone). - Unify flush snapshot anchoring at post-reset storage so per-table contributions are exact under any reset/switch/flush history. Hot path unchanged. Regressions: clear between rows (both units, both senders), non-current/mid-row clear, accounting locality, stale headroom estimate.
1 parent f47859b commit 6937474

5 files changed

Lines changed: 418 additions & 9 deletions

File tree

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
* state until commit, so committed table data can be flushed without replaying
6868
* the row back into column storage.
6969
*/
70-
public class QwpUdpSender implements Sender {
70+
public class QwpUdpSender implements Sender, QwpTableBuffer.Owner {
7171
private static final int ADAPTIVE_HEADROOM_EWMA_SHIFT = 2;
7272
private static final Logger LOG = LoggerFactory.getLogger(QwpUdpSender.class);
7373
private static final int MAX_TABLE_NAME_LENGTH = 127;
@@ -195,6 +195,27 @@ public void cancelRow() {
195195
rollbackCurrentRowToCommittedState();
196196
}
197197

198+
/**
199+
* {@link QwpTableBuffer.Owner} callback: the buffer is about to close all
200+
* of its column buffers, so drop cached column references, in-progress
201+
* column states, and the committed datagram estimate derived from it.
202+
* The table's adaptive headroom state must go too: its base-estimate
203+
* cache and EWMA samples are keyed by column count alone, which is only
204+
* sound while columns are add-only — clear() can replace the schema with
205+
* a different one of the same size.
206+
*/
207+
@Override
208+
public void onTableBufferClear(QwpTableBuffer buffer) {
209+
if (buffer == currentTableBuffer) {
210+
clearTransientRowState();
211+
resetCommittedDatagramEstimate();
212+
}
213+
TableHeadroomState headroomState = tableHeadroomStates.get(buffer.getTableName());
214+
if (headroomState != null) {
215+
headroomState.reset();
216+
}
217+
}
218+
198219
@Override
199220
public void close() {
200221
if (!closed) {
@@ -580,7 +601,7 @@ public Sender table(CharSequence tableName) {
580601
currentTableName = currentTableBuffer.getTableName();
581602
} else {
582603
currentTableName = tableName.toString();
583-
currentTableBuffer = new QwpTableBuffer(currentTableName);
604+
currentTableBuffer = new QwpTableBuffer(currentTableName, this);
584605
tableBuffers.put(currentTableName, currentTableBuffer);
585606
}
586607
currentTableHeadroomState = tableHeadroomStates.get(currentTableName);
@@ -1490,6 +1511,15 @@ long predictNextRowGrowth() {
14901511
return Math.max(lastRowDatagramGrowth, ewmaRowDatagramGrowth);
14911512
}
14921513

1514+
void reset() {
1515+
cachedBaseEstimate = -1;
1516+
cachedBaseEstimateColumnCount = -1;
1517+
committedSampleCount = 0;
1518+
ewmaRowDatagramGrowth = 0;
1519+
lastRowDatagramGrowth = 0;
1520+
schemaColumnCount = -1;
1521+
}
1522+
14931523
void recordCommittedRow(int currentSchemaColumnCount, long rowDatagramGrowth) {
14941524
if (schemaColumnCount != currentSchemaColumnCount) {
14951525
committedSampleCount = 0;

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

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@
120120
* Initial connection failures are not retained as terminal sender state; a later
121121
* operation may try to connect again.
122122
*/
123-
public class QwpWebSocketSender implements Sender {
123+
public class QwpWebSocketSender implements Sender, QwpTableBuffer.Owner {
124124

125125
public static final long DEFAULT_AUTH_TIMEOUT_MS = 15_000L;
126126
// Soft per-batch byte budget. Trips a flush when raw column-buffer bytes
@@ -1011,10 +1011,7 @@ public QwpWebSocketSender byteColumn(CharSequence columnName, byte value) {
10111011
@Override
10121012
public void cancelRow() {
10131013
checkNotClosed();
1014-
if (currentTableBuffer != null) {
1015-
currentTableBuffer.cancelCurrentRow();
1016-
currentTableBuffer.rollbackUncommittedColumns();
1017-
}
1014+
rollbackRow();
10181015
}
10191016

10201017
/**
@@ -1811,6 +1808,15 @@ public long getPendingBytes() {
18111808
return pendingBytes;
18121809
}
18131810

1811+
/**
1812+
* Row counterpart of {@link #getPendingBytes()}: committed-but-unflushed
1813+
* rows across all tables, as tracked for the auto-flush row threshold.
1814+
*/
1815+
@TestOnly
1816+
public int getPendingRowCount() {
1817+
return pendingRowCount;
1818+
}
1819+
18141820
/**
18151821
* Server-advertised cap on the per-batch raw byte size. Zero before the
18161822
* first connect; updated by every successful reconnect via
@@ -1824,10 +1830,18 @@ public int getServerMaxBatchSize() {
18241830
@TestOnly
18251831
public QwpTableBuffer getTableBuffer(String tableName) {
18261832
QwpTableBuffer buffer = tableBuffers.get(tableName);
1833+
if (buffer == currentTableBuffer && buffer != null) {
1834+
// Keep the timestamp caches and byte snapshot intact: re-snapping
1835+
// mid-row would fold in-progress bytes into the snapshot and break
1836+
// sendRow()'s delta accounting.
1837+
return buffer;
1838+
}
18271839
if (buffer == null) {
18281840
buffer = new QwpTableBuffer(tableName, this);
18291841
tableBuffers.put(tableName, buffer);
18301842
}
1843+
cachedTimestampColumn = null;
1844+
cachedTimestampNanosColumn = null;
18311845
currentTableBuffer = buffer;
18321846
currentTableBufferSnapshotBytes = buffer.getBufferedBytes();
18331847
currentTableName = tableName;
@@ -2235,6 +2249,33 @@ public void reset() {
22352249
cachedTimestampNanosColumn = null;
22362250
}
22372251

2252+
/**
2253+
* {@link QwpTableBuffer.Owner} callback: the buffer is about to close all
2254+
* of its column buffers, so drop any cached column references and remove
2255+
* exactly the buffer's accounted share from the pending totals. That
2256+
* share is its committed bytes (the snapshot for the current table, which
2257+
* excludes any in-progress row; raw storage otherwise) minus its baseline
2258+
* bytes, which pendingBytes never included. Runs before the buffer wipes
2259+
* itself, so all three values still reflect the pre-clear state.
2260+
*/
2261+
@Override
2262+
public void onTableBufferClear(QwpTableBuffer buffer) {
2263+
long committedBytes;
2264+
if (buffer == currentTableBuffer) {
2265+
cachedTimestampColumn = null;
2266+
cachedTimestampNanosColumn = null;
2267+
committedBytes = currentTableBufferSnapshotBytes;
2268+
currentTableBufferSnapshotBytes = 0;
2269+
} else {
2270+
committedBytes = buffer.getBufferedBytes();
2271+
}
2272+
pendingBytes -= committedBytes - buffer.getBaselineBytes();
2273+
pendingRowCount -= buffer.getRowCount();
2274+
if (pendingRowCount == 0) {
2275+
firstPendingRowTimeNanos = 0;
2276+
}
2277+
}
2278+
22382279
/**
22392280
* Register an async listener for connection-state transitions: initial
22402281
* connect, primary failover, endpoint attempt failures, the full address
@@ -3823,7 +3864,11 @@ private void resetTableBuffersAfterFlush(ObjList<CharSequence> keys) {
38233864
}
38243865
currentBatchMaxSymbolId = -1;
38253866
pendingBytes = 0;
3826-
currentTableBufferSnapshotBytes = 0;
3867+
// Anchor at the post-reset storage (baseline seed bytes of var-width
3868+
// columns), matching the empty-flush path above, so per-row deltas
3869+
// accumulated from here always equal committedBytes - baselineBytes.
3870+
currentTableBufferSnapshotBytes = currentTableBuffer == null
3871+
? 0 : currentTableBuffer.getBufferedBytes();
38273872
pendingRowCount = 0;
38283873
firstPendingRowTimeNanos = 0;
38293874
}
@@ -3860,6 +3905,10 @@ private void resetSymbolDictStateForNewConnection() {
38603905
}
38613906

38623907
private void rollbackRow() {
3908+
// rollbackUncommittedColumns() may close a newly created designated
3909+
// timestamp column, so cached references must not survive rollback.
3910+
cachedTimestampColumn = null;
3911+
cachedTimestampNanosColumn = null;
38633912
if (currentTableBuffer != null) {
38643913
currentTableBuffer.cancelCurrentRow();
38653914
currentTableBuffer.rollbackUncommittedColumns();

core/src/main/java/io/questdb/client/cutlass/qwp/protocol/QwpTableBuffer.java

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,10 @@ public class QwpTableBuffer implements QuietCloseable {
6363
private static final int MAX_COLUMN_NAME_LENGTH = 127;
6464
private final LowerCaseCharSequenceIntHashMap columnNameToIndex;
6565
private final ObjList<ColumnBuffer> columns;
66+
private final Owner owner;
6667
private final QwpWebSocketSender sender;
6768
private final String tableName;
69+
private long baselineBytes; // storage retained by empty columns after reset() (e.g. var-width seed offsets)
6870
private QwpColumnDef[] cachedColumnDefs;
6971
private int columnAccessCursor; // tracks expected next column index
7072
private boolean columnDefsCacheValid;
@@ -74,18 +76,33 @@ public class QwpTableBuffer implements QuietCloseable {
7476
private int rowCount;
7577

7678
public QwpTableBuffer(String tableName) {
77-
this(tableName, null);
79+
this(tableName, (QwpWebSocketSender) null);
7880
}
7981

8082
/**
8183
* Use this constructor overload to allow writing to a symbol column.
8284
* {@link ColumnBuffer#addSymbol(CharSequence)} needs the sender to
8385
* call {@link QwpWebSocketSender#getOrAddGlobalSymbol(CharSequence)}, registering
8486
* the symbol in the global dictionary shared with the server.
87+
* The sender is also registered as the buffer's {@link Owner}.
8588
*/
8689
public QwpTableBuffer(String tableName, QwpWebSocketSender sender) {
90+
this(tableName, sender, sender);
91+
}
92+
93+
/**
94+
* Use this constructor overload for owners that derive state from the
95+
* buffer but do not use the global symbol dictionary (symbol columns fall
96+
* back to the per-buffer dictionary).
97+
*/
98+
public QwpTableBuffer(String tableName, Owner owner) {
99+
this(tableName, null, owner);
100+
}
101+
102+
private QwpTableBuffer(String tableName, QwpWebSocketSender sender, Owner owner) {
87103
this.tableName = tableName;
88104
this.sender = sender;
105+
this.owner = owner;
89106
this.columns = new ObjList<>();
90107
this.columnNameToIndex = new LowerCaseCharSequenceIntHashMap();
91108
this.rowCount = 0;
@@ -118,8 +135,15 @@ public void cancelCurrentRow() {
118135
/**
119136
* Clears the buffer completely, including column definitions.
120137
* Frees all off-heap memory.
138+
* <p>
139+
* The registered {@link Owner} is notified before any column state is
140+
* released, so it can drop cached {@link ColumnBuffer} references and
141+
* repair bookkeeping derived from the buffer's pre-clear contents.
121142
*/
122143
public void clear() {
144+
if (owner != null) {
145+
owner.onTableBufferClear(this);
146+
}
123147
for (int i = 0, n = columns.size(); i < n; i++) {
124148
columns.get(i).close();
125149
}
@@ -132,13 +156,26 @@ public void clear() {
132156
rowCount = 0;
133157
columnDefsCacheValid = false;
134158
cachedColumnDefs = null;
159+
baselineBytes = 0;
135160
}
136161

137162
@Override
138163
public void close() {
139164
clear();
140165
}
141166

167+
/**
168+
* Bytes of storage the columns retained through the last {@link #reset()}
169+
* (var-width columns re-seed a 4-byte initial offset entry, for example).
170+
* These bytes are part of {@link #getBufferedBytes()} but predate any row
171+
* committed since the reset, so an owner accounting for committed-row
172+
* bytes must exclude them: contribution = committed bytes - baseline.
173+
* Zero for a freshly created or {@link #clear()}ed buffer.
174+
*/
175+
public long getBaselineBytes() {
176+
return baselineBytes;
177+
}
178+
142179
/**
143180
* Returns the total bytes buffered across all columns.
144181
* This queries actual buffer sizes, not estimates.
@@ -312,6 +349,7 @@ public void reset() {
312349
committedColumnCount = columns.size();
313350
inProgressColumnCount = 0;
314351
rowCount = 0;
352+
baselineBytes = getBufferedBytes();
315353
}
316354

317355
public void retainInProgressRow(
@@ -489,6 +527,18 @@ static int elementSizeInBuffer(byte type) {
489527
}
490528
}
491529

530+
/**
531+
* A sender that owns this buffer and derives state from it: cached
532+
* {@link ColumnBuffer} references, pending-byte/row accounting, datagram
533+
* size estimates. {@link #clear()} closes every column buffer at once,
534+
* which invalidates all of that derived state, so it notifies the owner
535+
* <em>before</em> releasing anything -- the callback may still read the
536+
* buffer's pre-clear row count and byte sizes.
537+
*/
538+
public interface Owner {
539+
void onTableBufferClear(QwpTableBuffer buffer);
540+
}
541+
492542
/**
493543
* Helper class to capture array data from DoubleArray/LongArray.appendToBufPtr().
494544
*/

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,45 @@ public void testAtMicrosOversizeFailureRollsBackWithoutLeakingTimestampState() t
269269
});
270270
}
271271

272+
@Test
273+
public void testAtMicrosRecoversAfterTableBufferClear() throws Exception {
274+
assertUdpSenderRecoversAfterTableBufferClear(ChronoUnit.MICROS);
275+
}
276+
277+
@Test
278+
public void testAtNanosRecoversAfterTableBufferClear() throws Exception {
279+
assertUdpSenderRecoversAfterTableBufferClear(ChronoUnit.NANOS);
280+
}
281+
282+
@Test
283+
public void testTableBufferClearInvalidatesHeadroomBaseEstimate() throws Exception {
284+
assertMemoryLeak(() -> {
285+
// Same column count before and after clear(), but a far longer
286+
// column name: a stale base estimate cached for the old schema
287+
// (keyed by column count alone) would under-estimate the new one.
288+
String longName = repeat('n', 120);
289+
CapturingNetworkFacade nf = new CapturingNetworkFacade();
290+
try (QwpUdpSender sender = new QwpUdpSender(nf, 0, 0, 9000, 1, 1024 * 1024)) {
291+
sender.table("t")
292+
.longColumn("a", 1)
293+
.atNow();
294+
sender.currentTableBufferForTest().clear();
295+
296+
sender.table("t")
297+
.longColumn(longName, 2)
298+
.atNow();
299+
long estimate = sender.committedDatagramEstimateForTest();
300+
sender.flush();
301+
302+
int actual = nf.lengths.get(nf.lengths.size() - 1);
303+
Assert.assertTrue(
304+
"estimate must cover the rebuilt schema [estimate=" + estimate + ", actual=" + actual + "]",
305+
estimate >= actual
306+
);
307+
}
308+
});
309+
}
310+
272311
@Test
273312
public void testAtNanosOversizeFailureRollsBackWithoutLeakingTimestampState() throws Exception {
274313
assertMemoryLeak(() -> {
@@ -1955,6 +1994,32 @@ private static void assertEstimateAtLeastActual(List<ScenarioRow> rows) {
19551994
}
19561995
}
19571996

1997+
private static void assertUdpSenderRecoversAfterTableBufferClear(ChronoUnit unit) throws Exception {
1998+
assertMemoryLeak(() -> {
1999+
CapturingNetworkFacade nf = new CapturingNetworkFacade();
2000+
try (QwpUdpSender sender = new QwpUdpSender(nf, 0, 0, 9000, 1, 1024 * 1024)) {
2001+
sender.table("t")
2002+
.longColumn("a", 1)
2003+
.at(1_000_000L, unit);
2004+
Assert.assertTrue(sender.committedDatagramEstimateForTest() > 0);
2005+
2006+
// Raw clear() discards the buffered row; the sender must drop
2007+
// its cached designated-timestamp column and datagram estimate.
2008+
sender.currentTableBufferForTest().clear();
2009+
Assert.assertEquals(0, sender.committedDatagramEstimateForTest());
2010+
2011+
sender.table("t")
2012+
.longColumn("a", 2)
2013+
.at(2_000_000L, unit);
2014+
sender.flush();
2015+
}
2016+
2017+
assertRowsEqual(Collections.singletonList(
2018+
decodedRow("t", "a", 2L, "", 2_000_000L)
2019+
), decodeRows(nf.packets));
2020+
});
2021+
}
2022+
19582023
private static void assertNullableArrayNullState(QwpTableBuffer.ColumnBuffer column) {
19592024
Assert.assertNotNull(column);
19602025
Assert.assertEquals(1, column.getSize());

0 commit comments

Comments
 (0)