collections: paginate by commit sequence - #224
Merged
Merged
Conversation
A caller that wants a page at a time has nothing to resume from. The store has no key order, and adding one would mean an ordered index over the mutable arena. It does not need one: MVCC already stamps every record with the sequence it was born at, and records are appended in commit order, so each segment is a sorted run by sequence. At a fixed snapshot each key has exactly one visible version, which makes (seq, key) a total order over what a scan can see. QueryRawFromSeq walks that order, yielding at most limit records and a cursor to continue from. The cursor is per shard, because the sequence is: each shard stamps from its own counter, so a shard is finished before the next is opened, and each is frozen at the sequence it had when its first page was taken. That is the guarantee Scan already makes, which is likewise per shard. Deliberately not a physical position. A segment's id is its index in the shard's slice: compaction renumbers ids and reopen assigns them by sort order, so ids are reused and a stale (segment, offset) cursor would silently address a different record instead of failing. Sequences survive a rewrite, because compaction carries the stamps onto the destination records for MVCC's sake. Segments gain maxSeq beside minSeq so a resume skips whole segments that end before its cursor. Both are rebuilt by the recovery walk; both stay zero after a fast reopen, which is conservative (a zero costs a walk, it never skips a segment that should have been read) and matches how minSeq has always behaved. Tests cover the page-size sweep, order stability across page boundaries, exactly-once under concurrent writes, a started shard staying frozen, the constraint being applied, pagination across a compaction rewrite, and maxSeq surviving recovery — each set up to actually reach the path it claims: several segments per shard, a churn ratio that makes Compact rewrite rather than no-op, and the directory snapshot removed so Open walks records instead of restoring. SKIP=go-vet: the pre-commit hook vets from the root module, which cannot see this separate module's package.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
QueryRawProjectedFromSeq on the DB, opQueryRawProjSeq on the wire: at most limit projected rows after a cursor, then a cursor trailer saying where to continue and whether anything remains. A page costs a resume rather than a re-scan, and the cursor names no physical position, so a compaction between pages cannot invalidate it. Mutable tables only. An archive already answers "the newest K" in one shot -- ordered newest-first with the limit pushed down -- so the op says that rather than pretending to paginate, and a view falls through to the plain projection op. A server too old for the opcode answers stBadReq, which the client turns into ErrPaginationUnsupported so a caller can fall back to an unpaginated read. The collections merge also needed a fix that only the wire test caught. Records committed TOGETHER share a sequence and sit in the arena in insertion order, not key order -- so within one commit a record belonging after the cursor can sit physically before one belonging before it. The merge had assumed each segment was a sorted run by (seq, key) and lost records at every page boundary landing inside a commit: 95 of 100 over the wire, 192 of 200 in a reproduction. It now collects each commit's records across the segments, sorts that group by key, and emits it, so the order it walks is the order a cursor can resume from. Every earlier test wrote one record per commit, which is why they all passed. The regression test uses a transaction to get the shape deterministically -- concurrent Puts do coalesce, but only when they happen to queue together, which under suite load they stop doing, and a test resting on that would pass or fail with the machine rather than the code. SKIP=go-vet: the pre-commit hook vets from the root module, which cannot see these separate modules' packages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First layer of resumable pagination for the mutable store, for the htcondordb mirror to serve paginated job queries instead of declining them.
dbanddbrpcfollow; this is the primitive they need.Why not the obvious cursors
Not a key range. There is no key order, and adding one means an ordered index over the mutable arena.
Not a physical position. A segment's
idis its index in the shard's slice — compaction renumbers (baseID := uint32(len(sh.segs))) and reopen assigns by sort position (loaded[i].seg.id = uint32(i)), so ids are reused. A stale(segment, offset)cursor would not fail; it would silently address a different record.Not a held iterator. A scan pins its segments and a pin defers compaction's reap — which is why
colscan.gounpins each segment as soon as it is done rather than holding them across a scan. A cursor alive for minutes would defer reclamation for as long as a client sits on a page, in proportion to write traffic, and would not survive a restart.What it uses instead
The order MVCC already maintains. Every record carries the sequence it was born at, records are appended in commit order (so each segment is a sorted run by sequence), and at a fixed snapshot each key has exactly one visible version — which makes
(seq, key)a total order over what a scan can see. Sequences survive compaction because it carries the stamps onto the destination records; that is required for MVCC correctness, so the cursor rides on an invariant the store already has to keep.QueryRawFromSeq(q, cursor, limit)yields records in that order, at mostlimit, plus the cursor to continue from.The cursor is per shard, because the sequence is: each shard stamps from its own counter (
seq := sh.commitSeq + 1), so sequences from different shards are unrelated numbers. A shard is finished before the next is opened, and each is frozen at the sequence it had when its first page was taken — the same guaranteeScandocuments, which is likewise per shard.Segments gain
maxSeqbesideminSeq, so a resume skips whole segments ending before its cursor instead of walking them to find out. Both are rebuilt by the recovery walk; both stay zero after a fast reopen, which is conservative — a zero costs a walk, it never skips a segment that should have been read — and matches howminSeqhas always behaved.Tests
Page-size sweep (1, 7, 64, n, 2n) for coverage and exactly-once; order stability across page boundaries; exactly-once under writes landing mid-pagination; a started shard staying frozen; the constraint being applied; pagination across a compaction rewrite; and
maxSeqsurviving recovery.Each is set up to actually reach the path it names, which took a few tries: several segments per shard (a single active arena has nothing to merge), a churn ratio that makes
Compactrewrite rather than no-op, and the directory snapshot removed soOpenwalks records instead of restoring. Two of these initially passed for the wrong reason and are noted in the commit.Full
collectionssuite passes (220s);golangci-lintreports nothing in the new files.🤖 Generated with Claude Code