Skip to content

Commit 6cc1307

Browse files
glasstigerclaude
andcommitted
Document split-flush and dict-getter contracts
Two documentation-only additions to the QWP store-and-forward client, making previously implicit contracts explicit. No behavior change. flushPendingRowsSplit: add a "Not atomic across frames" note. A sealAndSwapBuffer() failure on split frame k>1 leaves frames 1..k-1 published as deferred; the throw skips the trailing table-buffer reset, so the next flush re-emits the whole batch and the eventual commit commits the already-published prefix twice. This is pre-existing and within store-and-forward's at-least-once contract (absorbed by a DEDUP table or a durable-ack await); the note names the atomic-split fix (roll back or skip the published prefix on retry) as the larger follow-up. PersistedSymbolDict: mark loadedEntriesAddr(), loadedEntriesLen() and readLoadedSymbols() as construction-phase only. They read the native entry region that close() frees and nulls, with no closed-guard, so they are safe only before the slot's I/O thread and any producer append start; reading them from a running thread would risk a use-after-free. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cbaf474 commit 6cc1307

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3526,6 +3526,22 @@ private void flushPendingRows(boolean deferCommit) {
35263526
* own message. All messages except the last carry FLAG_DEFER_COMMIT
35273527
* so the server appends rows without committing until the final
35283528
* message arrives.
3529+
* <p>
3530+
* <b>Not atomic across frames.</b> The frames publish one at a time, so a
3531+
* publish failure partway through -- {@link #sealAndSwapBuffer()} throwing on
3532+
* frame k&gt;1, e.g. a backpressure deadline or the buffer-recycle timeout --
3533+
* leaves frames 1..k-1 already on the ring as deferred (appended, not yet
3534+
* committed). The throw propagates past the {@code resetTableBuffersAfterFlush}
3535+
* at the end of the loop, so the source rows survive in their table buffers
3536+
* and the NEXT flush re-emits the whole batch; the eventual commit then
3537+
* commits the already-published prefix alongside the re-sent copies,
3538+
* delivering those rows at-least-once (duplicated), not exactly-once. This is
3539+
* within store-and-forward's at-least-once contract -- a DEDUP table or a
3540+
* durable-ack await absorbs the duplicate, and the symbol-dict state stays
3541+
* consistent on the retry (the re-sent frames carry empty deltas and the
3542+
* write-ahead persist is a {@code pd.size()} no-op). Making the split atomic
3543+
* (rolling back the published prefix, or skipping it on retry) would be a
3544+
* larger change.
35293545
*
35303546
* @param deferCommit when true, ALL messages (including the last)
35313547
* carry FLAG_DEFER_COMMIT. When false, only the

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,22 @@ public synchronized void close() {
354354
* Base address of the loaded entry region -- the concatenated
355355
* {@code [len][utf8]} bytes of every recovered symbol in id order, exactly
356356
* as a delta section carries them. Zero when nothing was recovered.
357+
* <p>
358+
* <b>Construction-phase only.</b> This hands out a raw pointer into native
359+
* memory that {@link #close()} frees and nulls, with no closed-guard and no
360+
* synchronization. It is safe to read only BEFORE the slot's I/O thread and
361+
* any producer append start -- i.e. while the send loop is being constructed
362+
* or an orphan-drain is seeding its mirror, both of which happen-before those
363+
* threads. A caller that reads it from a running thread races {@code close()}
364+
* and can dereference freed memory (use-after-free).
357365
*/
358366
public long loadedEntriesAddr() {
359367
return loadedEntriesAddr;
360368
}
361369

362370
/**
363-
* Byte length of {@link #loadedEntriesAddr()}.
371+
* Byte length of {@link #loadedEntriesAddr()}. Construction-phase only, for
372+
* the same reason -- see {@link #loadedEntriesAddr()}.
364373
*/
365374
public int loadedEntriesLen() {
366375
return loadedEntriesLen;
@@ -371,6 +380,10 @@ public int loadedEntriesLen() {
371380
* (entry {@code i} is symbol id {@code i}). Used once on recovery to
372381
* repopulate the producer's global dictionary. Empty when nothing was
373382
* recovered.
383+
* <p>
384+
* <b>Construction-phase only</b> -- like {@link #loadedEntriesAddr()}, this
385+
* walks the native entry region {@link #close()} frees, with no closed-guard,
386+
* so it must run before the I/O thread and any producer append start.
374387
*/
375388
public ObjList<String> readLoadedSymbols() {
376389
ObjList<String> out = new ObjList<>(Math.max(size, 1));

0 commit comments

Comments
 (0)