Skip to content

Commit b969bd9

Browse files
committed
fix(client): coordinate segment trim with I/O cursor
1 parent 429153a commit b969bd9

8 files changed

Lines changed: 466 additions & 156 deletions

File tree

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,10 @@ public MmapSegment activeSegment() {
528528
return ring.getActive();
529529
}
530530

531+
MmapSegment pinActiveSegment() {
532+
return ring.pinActiveSegment();
533+
}
534+
531535
/**
532536
* Append the payload, blocking up to {@link #appendDeadlineNanos} when
533537
* the cursor ring is at its memory/disk cap and waiting for ACK-driven
@@ -613,7 +617,7 @@ public synchronized void close() {
613617
closed = true;
614618
// Capture drain state BEFORE closing the ring — once the ring is
615619
// closed, its accessors aren't safe to read. The active segment is
616-
// never trimmed by drainTrimmable (only sealed segments are), so
620+
// never trim-eligible (only sealed segments are), so
617621
// when everything published has been acked we have to unlink the
618622
// residual .sfa files here. Without this, the next sender (or a
619623
// drainer adopting this slot) would replay already-acked data
@@ -1191,6 +1195,10 @@ public MmapSegment findSegmentContaining(long fsn) {
11911195
return ring.findSegmentContaining(fsn);
11921196
}
11931197

1198+
MmapSegment pinSegmentContaining(long fsn) {
1199+
return ring.pinSegmentContaining(fsn);
1200+
}
1201+
11941202
/**
11951203
* Pass-through to {@link SegmentRing#firstSealed()}.
11961204
*/
@@ -1215,6 +1223,14 @@ public MmapSegment nextSealedAfter(MmapSegment current) {
12151223
return ring.nextSealedAfter(current);
12161224
}
12171225

1226+
MmapSegment advancePinnedSegment(MmapSegment current) {
1227+
return ring.advancePinnedSegment(current);
1228+
}
1229+
1230+
void releasePinnedSegment(MmapSegment current) {
1231+
ring.releasePinnedSegment(current);
1232+
}
1233+
12181234
/**
12191235
* I/O thread accessor: highest FSN whose frame is fully written.
12201236
*/

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

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,9 @@ public synchronized void close() {
896896
}
897897
ioThread = null;
898898
}
899+
// Covers close-before-start and start failures; normal I/O-thread
900+
// exit already released the same pin in its finally block.
901+
releaseSendingSegment();
899902
// Close the current client. After a reconnect, swapClient has
900903
// replaced the original (and closed it); the owner only retains
901904
// the stale pre-reconnect reference. Without closing the live
@@ -1102,17 +1105,24 @@ public synchronized void start() {
11021105
// walks back to the lowest unacked frame so sealed-segment data
11031106
// actually reaches the wire — without it, start() would skip
11041107
// straight to the active and orphan everything in sealed.
1105-
positionCursorForStart();
1108+
try {
1109+
positionCursorForStart();
1110+
} catch (Throwable t) {
1111+
running = false;
1112+
releaseSendingSegment();
1113+
shutdownLatch.countDown();
1114+
throw t;
1115+
}
11061116
Thread t = new Thread(this::ioLoop, "qdb-cursor-ws-io");
11071117
t.setDaemon(true);
11081118
try {
11091119
t.start();
11101120
} catch (Throwable th) {
11111121
// Thread.start() failed (e.g. native stack alloc OOM). ioLoop
1112-
// never ran, so its finally{shutdownLatch.countDown()} never
1113-
// fires. Release the latch and reset state so a subsequent
1114-
// close() doesn't block on a thread that doesn't exist.
1122+
// never ran, so its finally block cannot release the segment pin
1123+
// or count down the latch.
11151124
running = false;
1125+
releaseSendingSegment();
11161126
shutdownLatch.countDown();
11171127
throw th;
11181128
}
@@ -1132,35 +1142,17 @@ private PendingDurableEntry acquirePendingEntry() {
11321142
* else the active). Returns the same segment if it's still being written
11331143
* (we're on the active and just need to wait for more publishedFsn).
11341144
* <p>
1135-
* Uses {@link CursorSendEngine#nextSealedAfter} so we never have to
1136-
* snapshot the full sealed list — important when the producer outpaces
1137-
* the I/O thread and the sealed list can grow to thousands of entries
1138-
* (cursor SF lets the producer fan out at memory speed; the wire path
1139-
* catches up at WebSocket speed).
1145+
* The ring switches the single I/O pin atomically with choosing the next
1146+
* segment, so trim can never unmap either side of the handoff. No sealed
1147+
* list snapshot is needed when the producer outpaces the wire path.
11401148
*/
11411149
private MmapSegment advanceSegment() {
11421150
MmapSegment current = sendingSegment;
1143-
MmapSegment liveActive = engine.activeSegment();
1144-
if (current == liveActive) {
1145-
// We're on the active — there's no "next", just wait for more
1146-
// bytes to be published into it. Caller's sendOne will see
1147-
// publishedOffset > sendOffset eventually and resume.
1148-
return current;
1151+
MmapSegment next = engine.advancePinnedSegment(current);
1152+
if (next != current) {
1153+
sendOffset = MmapSegment.HEADER_SIZE;
11491154
}
1150-
sendOffset = MmapSegment.HEADER_SIZE;
1151-
MmapSegment next = engine.nextSealedAfter(current);
1152-
if (next != null) {
1153-
return next;
1154-
}
1155-
// current was the newest sealed (no later sealed exists). If it's
1156-
// still in the sealed list, the next segment must be the active;
1157-
// if it's been trimmed out from under us, fall back to the oldest
1158-
// remaining sealed before resorting to the active.
1159-
next = engine.firstSealed();
1160-
if (next != null && next.baseSeq() > current.baseSeq()) {
1161-
return next;
1162-
}
1163-
return liveActive;
1155+
return next;
11641156
}
11651157

11661158
private void applyDurableAck() {
@@ -1760,6 +1752,9 @@ private void ioLoop() {
17601752
}
17611753
}
17621754
} finally {
1755+
// Release native-segment lifetime before publishing I/O-thread
1756+
// completion or running delegated engine cleanup.
1757+
releaseSendingSegment();
17631758
// Last act of the I/O thread: dispose of whatever client it
17641759
// holds. This is the airtight half of the close()-vs-reconnect
17651760
// race — when close()'s latch await is interrupted (drainer pool
@@ -1809,7 +1804,7 @@ private void ioLoop() {
18091804
* loop will then wait until the producer publishes more bytes.
18101805
*/
18111806
private void positionCursorAt(long targetFsn) {
1812-
MmapSegment seg = engine.findSegmentContaining(targetFsn);
1807+
MmapSegment seg = engine.pinSegmentContaining(targetFsn);
18131808
if (seg == null) {
18141809
// No segment currently advertises targetFsn. That normally means
18151810
// targetFsn is just past publishedFsn and there is nothing to
@@ -1818,14 +1813,14 @@ private void positionCursorAt(long targetFsn) {
18181813
// The producer is concurrent with this I/O thread, though. It can
18191814
// publish targetFsn after the first findSegmentContaining() returns
18201815
// null but before or during the active-tip snapshot below.
1821-
sendingSegment = engine.activeSegment();
1816+
sendingSegment = engine.pinActiveSegment();
18221817
sendOffset = sendingSegment.publishedOffset();
18231818
// The publishedOffset read is the producer's volatile publish
18241819
// barrier. If it saw the new frame bytes, the frameCount write that
18251820
// makes targetFsn discoverable is also visible, so a second lookup
18261821
// must now find it. If the producer publishes later, sendOffset is
18271822
// still at the old tip and trySendOne() will send the frame normally.
1828-
seg = engine.findSegmentContaining(targetFsn);
1823+
seg = engine.pinSegmentContaining(targetFsn);
18291824
if (seg != null) {
18301825
positionCursorInSegment(seg, targetFsn);
18311826
}
@@ -1905,6 +1900,14 @@ private void releasePendingEntry(PendingDurableEntry e) {
19051900
pendingDurablePool.addFirst(e);
19061901
}
19071902

1903+
private void releaseSendingSegment() {
1904+
MmapSegment segment = sendingSegment;
1905+
if (segment != null) {
1906+
engine.releasePinnedSegment(segment);
1907+
sendingSegment = null;
1908+
}
1909+
}
1910+
19081911
/**
19091912
* Send a WebSocket PING to prod the server into flushing pending
19101913
* STATUS_DURABLE_ACK frames, but only when the throttle interval has

0 commit comments

Comments
 (0)