Skip to content

Commit c448e34

Browse files
committed
fix(client): make SF recovery segment sort O(N log N) unconditionally
sortByBaseSeq was a median-of-three Lomuto quicksort. Median-of-three covers the readdir orders a healthy slot produces (lexicographic enumeration yields already-sorted baseSeqs, hashed directory order is effectively random), but Lomuto partitioning is O(N^2) on organ-pipe, duplicate-heavy and median-of-three-killer orders. Exact simulation at the documented 16K-segment ceiling: 22.6M comparisons for organ-pipe, 50.7M for Musser's med3-killer permutation, 134M (full N^2/2) for mass-duplicate baseSeqs -- versus ~221K on the healthy paths. Such orders are only reachable through corrupted-yet-parseable or operator-copied headers, and recovery validation rejects those slots -- but the quadratic stall lands BEFORE validation gets to reject them. The sort is now an introsort: the median-of-three fast path is unchanged, and each root-to-leaf partition path carries a budget of 2*floor(log2(N)) passes. Loop-on-larger iterations count against the budget, so bad-split chains cannot hide in the loop. Ranges that exhaust it fall back to in-place heapsort (still Long.compareUnsigned, zero allocation), capping every adversarial pattern at ~3.8*N*log2(N) comparisons (organ-pipe 773K, duplicates 507K, med3-killer 861K at N=16384). Recursion depth stays under log2(N). The sortComparisons counter ticks +2 per sift-down level so it remains a strict upper bound in the fallback. New adversarial test drives the sort directly with in-memory segments at N=16384 across organ-pipe, all-duplicate, few-distinct, med3-killer and sign-bit-key patterns, asserting unsigned order plus comparisons < 8*N*log2(N) -- >2x headroom over the worst measured pattern, ~12x below the mildest quadratic blow-up. The existing sorted-input regression test is unchanged and still passes.
1 parent d130f2c commit c448e34

2 files changed

Lines changed: 186 additions & 14 deletions

File tree

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

Lines changed: 91 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ public final class SegmentRing implements QuietCloseable {
7171
// openExisting() recovery on this JVM. Used by SegmentRingTest to
7272
// assert the sort stays O(N log N) without relying on wall-clock time
7373
// (CI runner variance makes elapsed-millisecond bounds flaky). Cheap
74-
// in production: one volatile-free add per partition pass, dwarfed by
75-
// the mmap I/O the recovery does on every segment.
74+
// in production: one volatile-free add per partition pass (plus two per
75+
// sift level in the rare heapsort fallback), dwarfed by the mmap I/O
76+
// the recovery does on every segment.
7677
private static long sortComparisons;
7778
// References copied while compacting the logical sealed-segment head.
7879
// Test-only operation count for deterministic trim-complexity assertions.
@@ -1110,9 +1111,10 @@ public static long getNextSealedComparisons() {
11101111
* {@link #sortByBaseSeq} since the last {@link #resetSortComparisons()}
11111112
* (or process start). The count is incremented once per partition pass
11121113
* for the median-of-three pivot pick plus once per element compared
1113-
* against the pivot, so a clean run on N segments adds roughly
1114-
* {@code 3 + (hi - lo - 1)} per recursive frame, summing to O(N log N).
1115-
* Exposed for {@code SegmentRingTest} to detect O(N²) regressions
1114+
* against the pivot ({@code 3 + (hi - lo - 1)} per pass), and by two per
1115+
* sift-down level when a range falls back to heapsort, so it strictly
1116+
* upper-bounds the true compare count and sums to O(N log N) on every
1117+
* input. Exposed for {@code SegmentRingTest} to detect O(N²) regressions
11161118
* deterministically.
11171119
*/
11181120
@TestOnly
@@ -1145,15 +1147,46 @@ public static void resetTrimMovedReferences() {
11451147
}
11461148

11471149
/**
1148-
* In-place quicksort over {@code list[lo, hi)} keyed by ascending
1149-
* {@code baseSeq}. Median-of-three pivot avoids the pathological O(N²)
1150-
* on already-sorted input that lexicographic readdir produces (our
1151-
* filenames are zero-padded hex of {@code baseSeq}). Recursion depth is
1152-
* bounded by ~2 log₂(N) -- for the documented 16K-segment ceiling, well
1153-
* under the JVM default stack.
1150+
* Drives the recovery-time baseSeq sort over the whole list. Exposed so
1151+
* {@code SegmentRingTest} can feed adversarial orders (organ-pipe, mass
1152+
* duplicates, median-of-three killer, unsigned-boundary keys) straight
1153+
* into the sort and assert comparison bounds without staging thousands
1154+
* of segment files on disk.
1155+
*/
1156+
@TestOnly
1157+
public static void sortByBaseSeqForTest(ObjList<MmapSegment> list) {
1158+
sortByBaseSeq(list, 0, list.size());
1159+
}
1160+
1161+
/**
1162+
* In-place introsort over {@code list[lo, hi)} keyed by ascending
1163+
* unsigned {@code baseSeq}. Median-of-three quicksort handles the readdir
1164+
* orders a healthy slot produces (lexicographic enumeration of the
1165+
* generation-numbered filenames yields already-sorted baseSeqs; hashed
1166+
* directory order is effectively random), and a partition-pass budget of
1167+
* 2·⌊log₂(N)⌋ demotes any range that keeps splitting badly to in-place
1168+
* heapsort. Without that budget, Lomuto with a median-of-three pivot is
1169+
* O(N²) on organ-pipe, duplicate-heavy and median-of-three-killer orders
1170+
* -- reachable only through corrupted-yet-parseable or operator-copied
1171+
* headers, but at the documented 16K-segment ceiling that is 10⁷..10⁸
1172+
* comparisons of startup stall before recovery validation gets to
1173+
* reject the slot, so the fallback makes O(N log N) unconditional.
1174+
* Recursion depth stays under log₂(N) (recurse on the smaller side,
1175+
* loop on the larger), well within the JVM default stack.
11541176
*/
11551177
private static void sortByBaseSeq(ObjList<MmapSegment> list, int lo, int hi) {
1178+
int n = hi - lo;
1179+
if (n > 1) {
1180+
sortByBaseSeq(list, lo, hi, 2 * (31 - Integer.numberOfLeadingZeros(n)));
1181+
}
1182+
}
1183+
1184+
private static void sortByBaseSeq(ObjList<MmapSegment> list, int lo, int hi, int budget) {
11561185
while (hi - lo > 1) {
1186+
if (budget-- == 0) {
1187+
heapSortByBaseSeq(list, lo, hi);
1188+
return;
1189+
}
11571190
int mid = (lo + hi) >>> 1;
11581191
long a = list.get(lo).baseSeq();
11591192
long b = list.get(mid).baseSeq();
@@ -1183,17 +1216,61 @@ private static void sortByBaseSeq(ObjList<MmapSegment> list, int lo, int hi) {
11831216
}
11841217
swap(list, store, hi - 1);
11851218
// Recurse on the smaller partition; loop on the larger to keep
1186-
// recursion depth bounded by log₂(N).
1219+
// recursion depth bounded by log₂(N). Children inherit the
1220+
// remaining pass budget: it counts passes along a root-to-leaf
1221+
// path, so a chain of bad splits exhausts it after ~2 log₂(N)
1222+
// levels no matter how the work is divided.
11871223
if (store - lo < hi - store - 1) {
1188-
sortByBaseSeq(list, lo, store);
1224+
sortByBaseSeq(list, lo, store, budget);
11891225
lo = store + 1;
11901226
} else {
1191-
sortByBaseSeq(list, store + 1, hi);
1227+
sortByBaseSeq(list, store + 1, hi, budget);
11921228
hi = store;
11931229
}
11941230
}
11951231
}
11961232

1233+
/**
1234+
* In-place heapsort over {@code list[lo, hi)} keyed by ascending unsigned
1235+
* {@code baseSeq}: the introsort fallback for ranges whose partition-pass
1236+
* budget ran out. Guaranteed O(N log N) for any key distribution and any
1237+
* initial order; no allocation.
1238+
*/
1239+
private static void heapSortByBaseSeq(ObjList<MmapSegment> list, int lo, int hi) {
1240+
int n = hi - lo;
1241+
for (int root = (n >>> 1) - 1; root >= 0; root--) {
1242+
siftDownByBaseSeq(list, lo, root, n);
1243+
}
1244+
for (int end = n - 1; end > 0; end--) {
1245+
swap(list, lo, lo + end);
1246+
siftDownByBaseSeq(list, lo, 0, end);
1247+
}
1248+
}
1249+
1250+
private static void siftDownByBaseSeq(ObjList<MmapSegment> list, int lo, int root, int heapSize) {
1251+
while (true) {
1252+
int child = (root << 1) + 1;
1253+
if (child >= heapSize) {
1254+
return;
1255+
}
1256+
// At most two unsigned compares per level (sibling pick + parent
1257+
// test); bump the counter by the constant 2 up front -- same
1258+
// cheap-upper-bound convention as the partition pass.
1259+
sortComparisons += 2;
1260+
if (child + 1 < heapSize
1261+
&& Long.compareUnsigned(list.get(lo + child).baseSeq(),
1262+
list.get(lo + child + 1).baseSeq()) < 0) {
1263+
child++;
1264+
}
1265+
if (Long.compareUnsigned(list.get(lo + root).baseSeq(),
1266+
list.get(lo + child).baseSeq()) >= 0) {
1267+
return;
1268+
}
1269+
swap(list, lo + root, lo + child);
1270+
root = child;
1271+
}
1272+
}
1273+
11971274
private static void swap(ObjList<MmapSegment> list, int i, int j) {
11981275
if (i == j) return;
11991276
MmapSegment tmp = list.get(i);

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,101 @@ public void testLargeSegmentCountReopensInOrder() throws Exception {
686686
});
687687
}
688688

689+
/**
690+
* Adversarial-order companion to
691+
* {@link #testLargeSegmentCountReopensInOrder}: that test covers the
692+
* already-sorted readdir order median-of-three was chosen for; this one
693+
* covers the orders median-of-three does NOT defend. On the
694+
* pre-introsort quicksort, exact simulation at N=16384 measured ~22.6M
695+
* comparisons for organ-pipe, ~134M (full N²/2) for mass-duplicate
696+
* baseSeqs and ~50.7M for Musser's median-of-three-killer permutation.
697+
* The heapsort fallback caps the worst of these at ~3.8·N·log₂(N)
698+
* (~860K), so the 8·N·log₂(N) bound below keeps >2x headroom against
699+
* harmless implementation drift while sitting ~26x under the mildest
700+
* quadratic blow-up. Also pins unsigned key ordering: baseSeqs with the
701+
* sign bit set must sort above {@code Long.MAX_VALUE}, not below zero.
702+
* <p>
703+
* A healthy client cannot produce these orders (recovery validation
704+
* rejects duplicate or non-contiguous baseSeqs moments after the sort),
705+
* but corrupted-yet-parseable headers or operator file copies feed the
706+
* sort BEFORE validation runs, so the sort itself must stay log-linear.
707+
* Sorts in-memory segments directly: staging 16K adversarial header
708+
* files on disk per pattern would dominate the test's runtime without
709+
* adding coverage.
710+
*/
711+
@Test
712+
public void testAdversarialSegmentOrdersSortLogLinear() throws Exception {
713+
TestUtils.assertMemoryLeak(() -> {
714+
final int n = 16384;
715+
final long bound = 8L * n * (long) (Math.log(n) / Math.log(2));
716+
717+
// Organ pipe: 0,1,...,n/2-1,n/2-1,...,1,0.
718+
long[] organPipe = new long[n];
719+
for (int i = 0; i < n / 2; i++) {
720+
organPipe[i] = i;
721+
organPipe[n - 1 - i] = i;
722+
}
723+
assertAdversarialSortWithinBound("organ-pipe", organPipe, bound);
724+
725+
long[] allDuplicates = new long[n];
726+
for (int i = 0; i < n; i++) {
727+
allDuplicates[i] = 42;
728+
}
729+
assertAdversarialSortWithinBound("all-duplicates", allDuplicates, bound);
730+
731+
long[] fewDistinct = new long[n];
732+
for (int i = 0; i < n; i++) {
733+
fewDistinct[i] = i % 4;
734+
}
735+
assertAdversarialSortWithinBound("few-distinct", fewDistinct, bound);
736+
737+
// Musser's median-of-three killer permutation of 1..n.
738+
long[] med3Killer = new long[n];
739+
int half = n / 2;
740+
for (int i = 1; i <= half; i++) {
741+
if (i % 2 == 1) {
742+
med3Killer[i - 1] = i;
743+
med3Killer[i] = half + i;
744+
}
745+
med3Killer[half + i - 1] = 2L * i;
746+
}
747+
assertAdversarialSortWithinBound("median-of-three-killer", med3Killer, bound);
748+
749+
// High-bit keys interleaved with small ones: exercises the
750+
// unsigned comparison contract alongside the comparison bound.
751+
long[] unsignedMix = new long[n];
752+
for (int i = 0; i < n; i++) {
753+
unsignedMix[i] = (i % 2 == 0) ? (0x8000000000000000L | i) : i;
754+
}
755+
assertAdversarialSortWithinBound("unsigned-mix", unsignedMix, bound);
756+
});
757+
}
758+
759+
private static void assertAdversarialSortWithinBound(String label, long[] baseSeqs, long bound) {
760+
final long segSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 1;
761+
ObjList<MmapSegment> list = new ObjList<>();
762+
try {
763+
for (long baseSeq : baseSeqs) {
764+
list.add(MmapSegment.createInMemory(baseSeq, segSize));
765+
}
766+
SegmentRing.resetSortComparisons();
767+
SegmentRing.sortByBaseSeqForTest(list);
768+
long comparisons = SegmentRing.getSortComparisons();
769+
for (int i = 1, size = list.size(); i < size; i++) {
770+
assertTrue(label + ": unsigned baseSeq order violated at index " + i,
771+
Long.compareUnsigned(list.get(i - 1).baseSeq(), list.get(i).baseSeq()) <= 0);
772+
}
773+
assertTrue(label + " sort took " + comparisons + " comparisons (expected < " + bound
774+
+ " = 8 * N * log2(N) for N=" + baseSeqs.length
775+
+ "); regression suggests the introsort heapsort fallback stopped engaging",
776+
comparisons < bound);
777+
} finally {
778+
for (int i = 0, size = list.size(); i < size; i++) {
779+
list.get(i).close();
780+
}
781+
}
782+
}
783+
689784
@Test
690785
public void testRemovingAcknowledgedPrefixMovesLinearReferences() throws Exception {
691786
TestUtils.assertMemoryLeak(() -> {

0 commit comments

Comments
 (0)