fix(sqlite): seek to the positional resume point instead of replaying the window

[andinux]

The defect

The positional cursor on cloudsync_payload_chunks was designed to make each /check call an O(1) seek to where the previous call stopped. It does not do that today.

The resume lower bound is stated only inside the disjunction (db_version>? OR (db_version=? AND seq>=?)). SQLite will derive a range term from a disjunction like this — but only when it can see both arms compare against the same value. The two arms here carry distinct anonymous parameters, and SQLite does not assume two parameters hold the same value, so it derives nothing.

cloudsync_changes is a virtual table that string-builds its inner SQL from the constraints xBestIndex receives, so the clause it actually runs is:

WHERE db_version <= ? AND site_id != ? ORDER BY db_version, seq ASC

An upper bound and a site filter, and nothing saying where to start. Every call re-reads the window from the beginning and discards rows until it reaches the resume point — and each discarded row costs nearly as much as an emitted one, because the generated subquery evaluates cloudsync_col_value() plus two joins per row. A full drain is therefore quadratic in the number of chunks.

In production this took one tenant's change export to ~25 minutes, and before a server-side deadline was raised it never completed at all.

The distinction is invisible in the SQL text, which is why this survived review:

two distinct parameters  ->  WHERE db_version <= ? AND site_id != ?
one parameter reused     ->  WHERE db_version <= ? AND db_version >= ? AND site_id != ?

The fix

State the lower bound explicitly alongside the untouched disjunction:

WHERE db_version<=? AND site_id<>? AND db_version>=? AND (db_version>? OR (db_version=? AND seq>=?))

The term is logically implied by the disjunction, so it selects exactly the same rows. It exists only so the constraint loop emits a lower bound into the generated inner SQL and the (db_version) index can seek.

Reusing one parameter across both arms would work identically. I chose the explicit conjunct because it states the bound the code depends on, rather than relying on a planner inference that a later edit could silently undo by splitting the parameter again.

Measurements

make chunk-bench (added here) reproduces the defect and measures the fix locally — no network, no server.

rows	chunks	chunk size	drain, before	drain, after	speedup	per chunk, before	per chunk, after
3 000	94	256 KB	1 582 ms	100 ms	15.8x	16.8 ms	1.07 ms
6 000	188	256 KB	6 110 ms	210 ms	29.2x	32.5 ms	1.12 ms
200 000	194	5 MiB	186 058 ms	6 115 ms	30.4x	959.1 ms	31.5 ms

Per-chunk latency by decile of chunk index, same run:

before   2.2   5.2   9.4  14.8  14.0  18.8  20.0  24.2  29.2  31.9    last/first: 14.3x
after    1.04  1.00  1.29  1.04  1.39  1.12  1.01  1.00  1.00  1.02   last/first:  0.98x

There is no single speedup multiplier here, and that is the point: the multiplier is proportional to the window size. Doubling the chunks doubled it (15.8x -> 29.2x). Growth exponents: before 3.86x for 2x the chunks (~N^1.95), after 2.09x (~N^1.06). The invariant is the last column — per-chunk cost stops depending on the window at all. O(N^2) -> O(N).

The benchmark also asserts that all three SQL shapes select the identical row at every resume point (0 differences across 93 points), which is the correctness claim the fix rests on.

At a realistic window size

The third row is a ~1 GB window at the 5 MiB default payload_max_chunk_size, which is the size and
shape a large tenant actually produces: 200 000 rows of 5 KB across 20 000 db_versions, 194 chunks,
1 008 637 552 payload bytes. One seeded database (21 s), measured by two builds of the extension in
turn, each running alone on an idle machine.

per chunk, ms, by decile of chunk index
before   130  319  515  697  873  1055  1251  1396  1612  1781   last/first: 13.7x
after     34   29   30   30   30    29    34    32    30    35   last/first:  1.02x

Isolating the resume seek from the chunk encoding over the same 193 resume points:

	before	after
seek, total	182 920 ms	44 ms
seek, first -> last decile	156 ms -> 1 771 ms	0.24 ms -> 0.22 ms

Those two numbers decompose the drain exactly, which is the check that the measurement is sound:
replay alone is 183 s, chunk encoding (untouched by this fix) is 6.1 s, and 183 + 6.1 = 189 s against
a measured 186 s — 1.6% apart. Nothing else in the call scales with position.

Reproduce with:

CHUNK_BENCH_ROWS=200000 CHUNK_BENCH_ROW_BYTES=5000 CHUNK_BENCH_TXNS=20000 \
CHUNK_BENCH_CHUNK_SIZE=5242880 CHUNK_BENCH_REPEATS=1 \
CHUNK_BENCH_DB=dist/chunk-bench-1gb.sqlite CHUNK_BENCH_KEEP=1 \
CHUNK_BENCH_EXT=<build>/cloudsync.dylib make chunk-bench

Not a PostgreSQL defect

src/postgresql/cloudsync_postgresql.c:1400-1407 writes the same disjunction with $3 in both arms, so the parameter-identity trap does not apply, and PostgreSQL's planner handles OR'd range bounds natively rather than through a vtab constraint pipeline. No change there.

Testing

• dist/unit — 156 tests, all OK
• dist/review_regressions — 0 failures
• dist/network_unit — all passed
• make chunk-bench at 94 and 188 chunks, curve flat after the fix

Notes for reviewers

• The benchmark is built by CI along with the other test binaries (the TEST_SRC wildcard picks up everything in test/), but never run there: timings are machine-dependent, and the shape of the curve rather than the absolute numbers is the result. Being compiled on every target is deliberate — it is what keeps it from rotting on musl and MinGW.
• If you re-run it, make first. make dist/chunk_bench builds the test binary but not dist/cloudsync.dylib, which is what the benchmark load_extensions — measuring against a stale extension shows the fix doing nothing.
• CHUNK_BENCH_TXNS=1 is the negative control: one db_version for the whole window, where no bound on db_version can narrow anything. Cost there is flat and high (~65 ms/chunk) both before and after, as it should be.
• The existing comments at cloudsync_sqlite.c:1049 and :1342 claiming "O(1) seek per chunk" were aspirational; they are now accurate, so they are left alone.

🤖 Generated with Claude Code