Skip to content

Commit ab5a13a

Browse files
committed
perf(qwp): skip proven-durable sealed prefix in periodic SF sync
servicePeriodicSync copied and scanned the whole live sealed list under the ring monitor every tick and called syncPublished() on each. But a segment is made durable before it is sealed (the requestSyncBeforeRotation gate), so every sealed barrier early-returns -- the copy was O(live-sealed) monitor-held work that grows without bound under a producer-outpaces-drain backlog. Track a monotonic durability frontier (firstNonDurableSealed) and copy only [frontier, size) + active via copyPendingSyncSegments. The frontier only advances past segments observed durable and durability never regresses, so it is a conservative lower bound that can never skip a segment still needing a barrier; recovery-resumed non-durable sealed segments are covered because it starts at 0. Maintained under the ring monitor and shifted with sealed-list compaction, so steady-state copy is O(1). Adds a frontier invariant assert plus tests for the durable-prefix skip and the compaction index-shift. Also folds in minor local cleanups (Arrays.fill, RetainedSegmentMembership lambdas, assertTrue).
1 parent 5bd76e0 commit ab5a13a

4 files changed

Lines changed: 250 additions & 44 deletions

File tree

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.slf4j.Logger;
3535
import org.slf4j.LoggerFactory;
3636

37+
import java.util.Arrays;
3738
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
3839
import java.util.concurrent.atomic.AtomicLong;
3940
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
@@ -1094,9 +1095,7 @@ private boolean serviceRing0(RingEntry e) {
10941095
batchSize = e.ring.stagePendingTrims(
10951096
trimBatch, MAX_TRIMS_PER_RING_PASS, durableAck);
10961097
} catch (Throwable t) {
1097-
for (int i = 0; i < trimBatch.length; i++) {
1098-
trimBatch[i] = null;
1099-
}
1098+
Arrays.fill(trimBatch, null);
11001099
recordTrimFailure(e, TRIM_RETRY_UNLINK, now, t);
11011100
return false;
11021101
}
@@ -1219,17 +1218,20 @@ private void servicePeriodicSync(RingEntry e, long now) {
12191218
return;
12201219
}
12211220
try {
1222-
e.ring.copyLiveSegmentsForSync(syncScratch);
1221+
e.ring.copyPendingSyncSegments(syncScratch);
12231222
for (int i = 0, n = syncScratch.size(); i < n; i++) {
12241223
syncScratch.getQuick(i).syncPublished();
12251224
}
12261225
e.ring.clearSyncRequestIfActiveDurable();
1227-
// The pass above covered every live segment's published range, and
1228-
// a failed barrier re-dirties its range under an mlock pin (see
1229-
// MmapSegment.syncPublished), so a success here is a genuine
1230-
// re-persist -- not a vacuous retry over pages a failed writeback
1231-
// marked clean (fsyncgate). Unlatch so a transient disk fault
1232-
// doesn't permanently brick the producer.
1226+
// The pass above covered every not-yet-durable live range -- the
1227+
// proven-durable sealed prefix is skipped precisely because its
1228+
// syncPublished would early-return (see copyPendingSyncSegments),
1229+
// so any latched failure necessarily belongs to a range we just
1230+
// re-barriered. A failed barrier re-dirties its range under an
1231+
// mlock pin (see MmapSegment.syncPublished), so a success here is a
1232+
// genuine re-persist -- not a vacuous retry over pages a failed
1233+
// writeback marked clean (fsyncgate). Unlatch so a transient disk
1234+
// fault doesn't permanently brick the producer.
12331235
e.ring.clearDurabilityFailure();
12341236
e.nextDataSyncNanos = now + e.syncIntervalNanos;
12351237
if (e.syncFailureLogged) {

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

Lines changed: 96 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,19 @@ public final class SegmentRing implements QuietCloseable {
100100
// Logical head of sealedSegments. Head removal nulls one entry and advances
101101
// this index; occasional compaction bounds unused prefix slots.
102102
private int sealedHead;
103+
// Frontier index into sealedSegments: every sealed segment in
104+
// [sealedHead, firstNonDurableSealed) has been proven durable by an earlier
105+
// periodic pass and never needs re-scanning -- publishedCursor is frozen at
106+
// seal and durableCursor only advances, so durability never regresses. The
107+
// periodic sync (copyPendingSyncSegments) skips this proven-durable prefix,
108+
// keeping the steady-state copy-under-monitor O(1) instead of O(live-sealed)
109+
// work that would otherwise grow with a producer-outpaces-drain backlog.
110+
// Rotation seals only already-durable predecessors (the
111+
// requestSyncBeforeRotation gate), so the sole source of non-durable sealed
112+
// segments is a crash-recovery resume; those are covered because the
113+
// frontier starts at 0. Maintained entirely under this monitor, in the same
114+
// coordinate space as sealedHead (shifted by compaction, reset on clear).
115+
private int firstNonDurableSealed;
103116
// High-water byte offset within the active segment at which we proactively
104117
// ask the segment manager to provision a spare (if one isn't already
105118
// installed). Computed once as 3/4 of segment capacity -- leaves the manager
@@ -668,29 +681,23 @@ private static RetainedSegmentMembership newDefaultMembership(
668681
}
669682
if (DEFAULT_MEMBERSHIP_MODE == RetainedSegmentMembershipMode.LINEAR) {
670683
if (observer == null) {
671-
return new RetainedSegmentMembership() {
672-
@Override
673-
public boolean contains(MmapSegment segment) {
674-
for (int i = 0, n = chain.size(); i < n; i++) {
675-
if (chain.get(i) == segment) {
676-
return true;
677-
}
678-
}
679-
return false;
680-
}
681-
};
682-
}
683-
return new RetainedSegmentMembership() {
684-
@Override
685-
public boolean contains(MmapSegment segment) {
684+
return segment -> {
686685
for (int i = 0, n = chain.size(); i < n; i++) {
687-
observer.onMembershipOperation();
688686
if (chain.get(i) == segment) {
689687
return true;
690688
}
691689
}
692690
return false;
691+
};
692+
}
693+
return segment -> {
694+
for (int i = 0, n = chain.size(); i < n; i++) {
695+
observer.onMembershipOperation();
696+
if (chain.get(i) == segment) {
697+
return true;
698+
}
693699
}
700+
return false;
694701
};
695702
}
696703

@@ -699,19 +706,11 @@ public boolean contains(MmapSegment segment) {
699706
retained.put(chain.get(i), Boolean.TRUE);
700707
}
701708
if (observer == null) {
702-
return new RetainedSegmentMembership() {
703-
@Override
704-
public boolean contains(MmapSegment segment) {
705-
return retained.containsKey(segment);
706-
}
707-
};
709+
return retained::containsKey;
708710
}
709-
return new RetainedSegmentMembership() {
710-
@Override
711-
public boolean contains(MmapSegment segment) {
712-
observer.onMembershipOperation();
713-
return retained.containsKey(segment);
714-
}
711+
return segment -> {
712+
observer.onMembershipOperation();
713+
return retained.containsKey(segment);
715714
};
716715
}
717716

@@ -1070,6 +1069,46 @@ synchronized void copyLiveSegmentsForSync(ObjList<MmapSegment> target) {
10701069
}
10711070
}
10721071

1072+
/**
1073+
* Copies the live segments that may still need a durability barrier: every
1074+
* sealed segment from the {@link #firstNonDurableSealed} frontier onward,
1075+
* plus the active segment. First advances the frontier past any sealed
1076+
* segments an earlier pass (or rotation's pre-seal barrier) has since made
1077+
* durable. Used by the periodic sync path in place of
1078+
* {@link #copyLiveSegmentsForSync}: the proven-durable prefix would
1079+
* otherwise be re-copied under this monitor and re-scanned every tick as
1080+
* no-op {@link MmapSegment#syncPublished()} early-returns -- O(live-sealed)
1081+
* work that grows without bound under a producer-outpaces-drain backlog.
1082+
* The frontier is a conservative lower bound (it only ever advances past
1083+
* segments observed durable, and durability never regresses), so this can
1084+
* never skip a segment that still needs a barrier.
1085+
*/
1086+
synchronized void copyPendingSyncSegments(ObjList<MmapSegment> target) {
1087+
target.clear();
1088+
// Invariant maintained by every mutation site (rotation append, trim's
1089+
// removeSealedHead, compaction shift, close). A frontier that drifted
1090+
// above size would silently skip un-fsynced segments, so guard it in
1091+
// tests; the clamp below keeps production safe if it is ever violated.
1092+
assert firstNonDurableSealed >= sealedHead && firstNonDurableSealed <= sealedSegments.size()
1093+
: "durability frontier out of range: firstNonDurableSealed=" + firstNonDurableSealed
1094+
+ " sealedHead=" + sealedHead + " size=" + sealedSegments.size();
1095+
int i = firstNonDurableSealed;
1096+
if (i < sealedHead) {
1097+
i = sealedHead;
1098+
}
1099+
int n = sealedSegments.size();
1100+
while (i < n && sealedSegments.get(i).isPublishedDurable()) {
1101+
i++;
1102+
}
1103+
firstNonDurableSealed = i;
1104+
for (; i < n; i++) {
1105+
target.add(sealedSegments.get(i));
1106+
}
1107+
if (active != null) {
1108+
target.add(active);
1109+
}
1110+
}
1111+
10731112
void enablePeriodicSync() {
10741113
periodicSyncEnabled = true;
10751114
syncRequested = true;
@@ -1115,6 +1154,7 @@ public synchronized void close() {
11151154
}
11161155
sealedSegments.clear();
11171156
sealedHead = 0;
1157+
firstNonDurableSealed = 0;
11181158
for (int i = 0, n = pendingTrims.size(); i < n; i++) {
11191159
pendingTrims.getQuick(i).close();
11201160
}
@@ -1211,7 +1251,6 @@ public synchronized MmapSegment firstTrimmable() {
12111251
return lastSeq <= ackedFsn ? segment : null;
12121252
}
12131253

1214-
/** Active segment -- exposed for the I/O thread's "send next batch" path. */
12151254
/**
12161255
* Walks every published frame in the ring (sealed segments plus the active
12171256
* segment) and returns the FSN of the LAST frame whose payload does NOT
@@ -1419,6 +1458,20 @@ public synchronized int getPendingTrimCount() {
14191458
return pendingTrims.size();
14201459
}
14211460

1461+
/**
1462+
* Number of live segments the periodic path would barrier this tick,
1463+
* advancing the durability frontier exactly as a real tick does. In the
1464+
* steady state (every sealed segment proven durable) this collapses to 1
1465+
* -- the active segment -- proving the proven-durable sealed prefix is no
1466+
* longer copied/scanned under the monitor.
1467+
*/
1468+
@TestOnly
1469+
public synchronized int pendingSyncSegmentCountForTest() {
1470+
ObjList<MmapSegment> scratch = new ObjList<>();
1471+
copyPendingSyncSegments(scratch);
1472+
return scratch.size();
1473+
}
1474+
14221475
@TestOnly
14231476
public synchronized MmapSegment pinSegmentContainingForTest(long fsn) {
14241477
return pinSegmentContaining(fsn);
@@ -1618,16 +1671,30 @@ private void compactSealedSegments() {
16181671
int liveCount = sealedSegments.size() - sealedHead;
16191672
trimMovedReferences += liveCount;
16201673
sealedSegments.remove(0, sealedHead - 1);
1674+
// The durable-prefix frontier lives in the same index space as the
1675+
// entries we just shifted down by sealedHead; move it with them.
1676+
// The invariant firstNonDurableSealed >= sealedHead keeps this >= 0.
1677+
firstNonDurableSealed -= sealedHead;
1678+
if (firstNonDurableSealed < 0) {
1679+
firstNonDurableSealed = 0;
1680+
}
16211681
sealedHead = 0;
16221682
}
16231683
}
16241684

16251685
private void removeSealedHead() {
16261686
sealedSegments.setQuick(sealedHead++, null);
1687+
// If the removed head WAS the frontier (a non-durable but already-ACKed
1688+
// recovery-resumed segment can be trimmed before its first barrier),
1689+
// keep the frontier at or ahead of the live head.
1690+
if (firstNonDurableSealed < sealedHead) {
1691+
firstNonDurableSealed = sealedHead;
1692+
}
16271693
int size = sealedSegments.size();
16281694
if (sealedHead == size) {
16291695
sealedSegments.clear();
16301696
sealedHead = 0;
1697+
firstNonDurableSealed = 0;
16311698
} else if (sealedHead >= 64 && sealedHead >= size - sealedHead) {
16321699
compactSealedSegments();
16331700
}

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerPeriodicSyncTest.java

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,87 @@ public void testSyncPassStopsAtFirstFailureThenRetryCoversAllSegments() throws E
305305
});
306306
}
307307

308+
@Test
309+
public void testPeriodicFrontierSkipsSealedPrefixOnceDurable() throws Exception {
310+
TestUtils.assertMemoryLeak(() -> {
311+
// Recovery resumes a non-durable sealed segment (durableCursor at
312+
// the header) plus a non-durable active. The first periodic copy
313+
// must offer BOTH -- the sealed one still needs a barrier. Once a
314+
// pass has made them durable, the durability frontier advances and
315+
// later copies must offer ONLY the active: the proven-durable
316+
// sealed prefix is never re-copied/re-scanned under the ring
317+
// monitor. This guards the O(1)-steady-state contract that
318+
// replaced the old O(live-sealed) copyLiveSegmentsForSync pass.
319+
final long intervalNanos = 100L;
320+
final long segmentSize = MmapSegment.HEADER_SIZE
321+
+ 2 * (MmapSegment.FRAME_HEADER_SIZE + 16);
322+
AtomicLong ticks = new AtomicLong();
323+
CountingFilesFacade filesFacade = new CountingFilesFacade();
324+
String dir = TestUtils.createTmpDir("qdb-periodic-frontier-");
325+
long payload = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT);
326+
SegmentManager manager = null;
327+
SegmentRing ring = null;
328+
try {
329+
// Sealed chain member (full: FSNs 0..1) + active (FSN 2).
330+
try (MmapSegment s0 = MmapSegment.create(
331+
filesFacade, dir + "/r0.sfa", 0L, segmentSize)) {
332+
s0.tryAppend(payload, 16);
333+
s0.tryAppend(payload, 16);
334+
s0.msync();
335+
}
336+
try (MmapSegment s1 = MmapSegment.create(
337+
filesFacade, dir + "/r1.sfa", 2L, segmentSize)) {
338+
s1.tryAppend(payload, 16);
339+
s1.msync();
340+
}
341+
ring = SegmentRing.openExisting(filesFacade, dir, segmentSize);
342+
assertTrue(ring != null);
343+
assertEquals(1, ring.getSealedSegments().size());
344+
345+
manager = new SegmentManager(
346+
segmentSize,
347+
SegmentManager.DEFAULT_POLL_NANOS,
348+
segmentSize * 8L,
349+
filesFacade,
350+
ticks::get);
351+
manager.register(ring, dir, null, intervalNanos);
352+
353+
// Before any successful barrier the recovered sealed segment is
354+
// non-durable, so the frontier cannot advance past it: the copy
355+
// must offer sealed + active.
356+
assertEquals("non-durable recovered sealed segment must be offered for sync",
357+
2, ring.pendingSyncSegmentCountForTest());
358+
359+
// Run the pass (enablePeriodicSync requested it): both segments
360+
// are barriered and become durable.
361+
manager.serviceRingForTesting(ring);
362+
363+
// The sealed segment is still live (nothing ACKed, so no trim)
364+
// but now durable, so the frontier skips it: only the active is
365+
// offered. count == 1 with a still-present sealed segment proves
366+
// the skip.
367+
assertEquals("durable sealed segment must remain live (no trim without ACKs)",
368+
1, ring.getSealedSegments().size());
369+
assertEquals("durable sealed prefix must be skipped by the periodic copy",
370+
1, ring.pendingSyncSegmentCountForTest());
371+
// Idempotent: re-running the frontier advance changes nothing.
372+
assertEquals(1, ring.pendingSyncSegmentCountForTest());
373+
} finally {
374+
if (manager != null && ring != null) {
375+
manager.deregister(ring);
376+
}
377+
if (ring != null) {
378+
ring.close();
379+
}
380+
if (manager != null) {
381+
manager.close();
382+
}
383+
Unsafe.free(payload, 16, MemoryTag.NATIVE_DEFAULT);
384+
TestUtils.removeTmpDir(dir);
385+
}
386+
});
387+
}
388+
308389
@Test
309390
public void testTransientSyncFailureClearsOnNextSuccess() throws Exception {
310391
TestUtils.assertMemoryLeak(() -> {

0 commit comments

Comments
 (0)