Skip to content

perf: stream the decoder page index and spill the encoder index to disk - #97

Open
corylanou wants to merge 3 commits into
perf/page-index-slicefrom
perf/page-index-spill
Open

perf: stream the decoder page index and spill the encoder index to disk#97
corylanou wants to merge 3 commits into
perf/page-index-slicefrom
perf/page-index-spill

Conversation

@corylanou

Copy link
Copy Markdown
Collaborator

Stacked on #95 (base branch perf/page-index-slice). Closes #96.

Summary

After #95 the encoder's index still grows with page count (24 B/page), and the decoder side turned out to be the larger remaining term: Decoder.Close slurped the tail of the file and built the full map[uint32]PageIndexElem even for compaction inputs that never read it (~80 B/page, ≈2.7 GiB per input at the 34M pages in benbjohnson/litestream#1477). Three commits, no wire-format change, no new dependencies, no platform-specific code:

  1. Decoder streams the page index on Close (decoder.go). Records are parsed straight from the reader through a small bufio buffer and hashed as consumed; the map is only built when retention is on (default, so PageIndex() keeps working). Compactor turns retention off for its inputs. The streamed path also validates what the old parser didn't: ascending page numbers within Commit, int64-bounded offsets/sizes, positive non-overlapping frames, size field equal to bytes read, nothing after the trailer, entry count equal to pages decoded, and a CRC64 over the index's page numbers equal to the same over the decoded pages (so the index must name exactly the decoded pages, without retaining them). DecodePageIndex shares the parser. Checksum coverage is unchanged — the perf(encoder): store page index in chunked slices, not a map #95 golden checksums still pass.

  2. Encoder spills the index to a temp file past a threshold (encoder.go). The index is append-only in ascending order and emitted sequentially on Close, so it never needed map semantics or random access: once it reaches SetSpillThreshold entries (default 1M, ~24 MiB) and SetSpillDir is configured, entries are appended to a buffered temp file in their final varint record format and copied into the output on Close. Buffered file I/O rides the OS page cache, which is the effect the mmap idea was after, without unsafe slices, remapping on growth, Windows unmap-before-delete ordering, SIGBUS on disk exhaustion, or 32-bit address-space limits (litestream ships armv6/armv7). Spilling is opt-in by directory because the encoder only sees an io.Writer and cannot pick a safe location itself; litestream's hardened image is FROM scratch with no /tmp, so it will pass its per-database meta directory.

    Lifecycle is strict, per review: Cleanup (idempotent, returns removal errors, keeps the path for retry) aborts an unclosed encoder so a later Close cannot emit a checksum-valid file with an empty index; Close installs its cleanup+abort defer before its first write and any failure is terminal (ErrEncoderAborted on retry); header/page/spill write failures abort; short writes are guarded everywhere. Spilled output is byte-identical to in-memory output, including across chunk boundaries, for deletion files and NoChecksum files.

  3. Compactor passthroughs SetSpillDir / Cleanup so callers can enable the spill for compaction output and clean up abandoned compactions.

Windows / platform check (asked before choosing this over mmap)

Both projects cross-build for Windows (ltx release.yml, litestream commit.yml + goreleaser), ltx also builds for plan9 and js/wasm, and syscall.Mmap is undefined on GOOS=windows — so an mmap design needs //go:build unix plus a fallback and gains nothing for a write-once/read-once sequential file. This branch has no build tags; GOOS=windows go vet ./..., go test ./..., and go test -race ./... are green. Details in #96.

Evidence

litestream-soak snapshot-compaction-overlap rig (corylanou/litestream-soak#196): the snapshot's replica stream is held at 95% so its index stays resident while an L1 compaction runs; heap growth = peak HeapInuse − post-GC baseline, GOGC=25, peak heap profile per phase. Litestream is benbjohnson/litestream#1479 plus the follow-up that sets SetSpillDir(db.MetaPath()) (linked below), built via local replace.

ltx fixture snapshot L1 compaction overlap (held snapshot + compaction)
v0.5.2 (litestream main) 4 GiB / 1,051,216 pages 168.6 MB 306.3 MB 470.6 MB
#95 4 GiB 68.8 MB 203.4 MB 199.6 MB
this PR 4 GiB 70.4 MB 73.7 MB 73.1 MB
this PR 8 GiB / 2,102,433 pages 73.1 MB 72.9 MB 73.9 MB

Doubling the database from 4 to 8 GiB no longer changes any phase: ~32 MB is the S3 multipart upload buffers (fixed), the rest is the ≤24 MiB of index chunks before the spill kicks in. Peak profile of the 8 GiB compaction phase: s3/manager.(*maxSlicePool).newSlice 32 MB, ltx.(*pageIndex).append 22.8 MB, and DecodePageIndex no longer appears at all (it was 81–84 MB at 4 GiB on #95).

Follow-ups (not here)

  • Litestream: Hydrator.ApplyLTX reads pages and returns without dec.Close(), so it never verifies the index/trailer/file checksum (pre-existing; noted by review). It should close the decoder with retention off.
  • Decoder.Close still cannot check index offsets against actual compressed frame positions, because the decoder tracks logical bytes, not file offsets.

Test plan

  • go test ./..., go test -race ./..., go vet ./..., GOOS=windows GOARCH=amd64 go vet ./...
  • Golden checksums from perf(encoder): store page index in chunked slices, not a map #95 unchanged; TestDecoder_PageIndexStructure builds checksum-valid files with phantom / missing / renamed / overlapping / out-of-range index entries and confirms only the structural checks reject them; TestEncoder_SpillLifecycle covers cleanup-before-close, first-write failure, non-retryable failed close, removal-error retry, deletion + NoChecksum parity
  • Rig evidence above (4 GiB, 8 GiB); litestream root suite green against this branch
  • Four adversarial Codex passes; every confirmed finding addressed (the first pass drove the strict lifecycle semantics)

Decoder.Close slurped the rest of the file with io.ReadAll and always
built a map[uint32]PageIndexElem for the page index, even for callers
that never look at it. Compactor inputs are the important case: it
streams pages and closes each input, so for a large database the map
was built and discarded per input (~80 B/page live, ~2.7 GiB at 34M
pages) and was the largest remaining per-page term after the encoder
change (litestream issue #1477, ltx issue #96).

Close now parses the index records straight from the reader through a
small bufio buffer, hashing bytes as they are consumed and validating
that page numbers ascend, that the size field matches the bytes read,
and that nothing follows the trailer. The map is only materialized when
retention is on (the default, so PageIndex keeps working); the
compactor turns it off for its inputs. File checksum coverage is
unchanged, which the golden checksum tests pin.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79888f3787

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread encoder.go
With the chunked index, encoder memory still scales with page count
(24 B/page, ~0.8 GiB for a 34M-page snapshot). The index is append-only
in ascending page order and is emitted sequentially on Close, so it
never needed map semantics or random access: once it passes
SetSpillThreshold entries (default 1M, ~24 MiB) and a spill directory
is configured with SetSpillDir, entries are appended to a buffered temp
file in their final varint record format and copied into the output on
Close. Encoder memory then stops growing with the file, and the OS page
cache carries the spill (ltx issue #96, litestream issue #1477).

The spill is opt-in via the directory because the encoder only receives
an io.Writer and cannot pick a safe location itself: litestream's
hardened image is FROM scratch with no /tmp, so it will pass its
per-database meta directory. No platform-specific code: buffered file
I/O works on every GOOS/GOARCH in both projects' release matrices,
including windows and 32-bit arm, which an mmap-based structure would
not without build tags.

Cleanup removes the spill file and is idempotent; Close calls it, and
callers that abandon an encoder without closing (cancelled snapshot
uploads) must call it themselves. Tests pin that spilled output is byte
identical to in-memory output, including across chunk boundaries.
Compactor owns its output Encoder, so callers need a passthrough to
enable the spill file for large compactions and to clean it up when a
compaction is abandoned mid-stream.
@corylanou
corylanou force-pushed the perf/page-index-spill branch from 79888f3 to 493c5d2 Compare August 28, 2026 13:46
corylanou added a commit to benbjohnson/litestream that referenced this pull request Aug 28, 2026
Enable the ltx encoder's page-index spill (superfly/ltx#97) for
snapshots and compactions, using the database meta directory as the
spill location: it is writable in every deployment including the
hardened scratch image, which has no /tmp. The compactor's spill dir is
set once the meta directory exists in Open.

Every ltx.NewCompactor and snapshot encoder now defers Cleanup so a
cancelled or failed operation cannot leave a spill file behind (and an
abandoned encoder cannot be closed into a checksum-valid file with an
empty index).

go.mod pins ltx to the head of superfly/ltx#97 for evaluation; replace
with the tagged ltx release before merging.
corylanou added a commit to benbjohnson/litestream that referenced this pull request Aug 28, 2026
Hydrator.ApplyLTX decoded pages until the end-of-pages marker and
returned, never calling Decoder.Close, so the page index, trailer, and
file checksum of hydrated LTX files were never checked: a truncated or
corrupted object was applied silently. Close the decoder after the page
loop with page-index retention disabled (superfly/ltx#97), so the file
is verified without building a database-sized index, and fail the
apply if verification fails.

Pages are applied as they stream, so a verification failure leaves
unverified content in the hydration file. The hydrator is marked
tainted in that case and Close discards the file and its meta instead
of persisting them, so a restart cannot resume from unverified pages.

Also reuse the page buffer across the loop instead of allocating one
per page, and add SetLogger to the vfs test mocks so the vfs-tagged
root test suite compiles again (it had drifted from the ReplicaClient
interface and is not run in CI).
corylanou added a commit to benbjohnson/litestream that referenced this pull request Aug 28, 2026
Hydrator.ApplyLTX decoded pages until the end-of-pages marker and
returned, never calling Decoder.Close, so the page index, trailer, and
file checksum of hydrated LTX files were never checked: a truncated or
corrupted object was applied silently. Close the decoder after the page
loop with page-index retention disabled (superfly/ltx#97), so the file
is verified without building a database-sized index, and fail the
apply if verification fails.

Pages are applied as they stream, so a verification failure leaves
unverified content in the hydration file. The hydrator is marked
tainted in that case and Close discards the file and its meta instead
of persisting them, so a restart cannot resume from unverified pages.

Also reuse the page buffer across the loop instead of allocating one
per page, and add SetLogger to the vfs test mocks so the vfs-tagged
root test suite compiles again (it had drifted from the ReplicaClient
interface and is not run in CI).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant