Skip to content

Add bencher_replica: in-process SQLite replication to replace Litestream#929

Draft
epompeii wants to merge 1 commit into
develfrom
u/ep/bencher-replica
Draft

Add bencher_replica: in-process SQLite replication to replace Litestream#929
epompeii wants to merge 1 commit into
develfrom
u/ep/bencher-replica

Conversation

@epompeii

Copy link
Copy Markdown
Member

Why

Litestream 0.5.13 blocked all Bencher Cloud API writes for ~5.5 minutes on 2026-07-10: when its verify step decides a full re-snapshot is needed, it copies the entire 4.35 GB database into a local LTX file while holding the SQLite write lock (via its _litestream_lock table), and no configuration knob reaches that code path. The same stall recurs on process restarts, and LTX compaction churns whole-database S3 transfers several times a day. Upstream 0.5.14 does not fix it.

bencher_replica is an in-process replacement. Being in-process is the structural fix: the app funnels writes through a single writer mutex, so the replicator coordinates checkpoints with the app's own write scheduling instead of fighting a separate process for locks. The prime invariant (I5): the SQLite write lock is only ever held for O(WAL-tail) work, never O(database). All six governing invariants are documented in plus/bencher_replica/src/lib.rs.

What

  • WAL parser with full salt and cumulative checksum-chain verification, cross-validated in both directions against SQLite itself
  • Storage: local filesystem XOR S3-compatible (endpoint override for R2/MinIO) behind one contract, plus a scripted fault-injection wrapper for tests
  • Sync engine: step-driven core; checkpoints run PASSIVE while the replicator holds BEGIN IMMEDIATE, closing the ship-vs-checkpoint race without ever needing blocking RESTART/TRUNCATE checkpoints
  • Snapshots: generation-based, sourced from a single-step SQLite online backup into a scratch file (transactionally consistent under concurrent checkpoints), throttled zstd multipart upload, snapshot.json as the atomic commit marker with a mandatory-replay boundary offset
  • Restore: latest-only, in the same startup handshake slot Litestream used; every epoch WAL is chain-pre-validated before application and checkpoint consumption is verified frame-for-frame
  • Verification: restore-and-compare at a pinned position (default daily, 0 disables); the backstop for externally-caused divergence
  • Shadow mode: with both plus.litestream and plus.replica configured, Litestream keeps checkpoint ownership and restore precedence during the burn-in
  • Fix: standalone sweep connections (stats, credit grants) now disable wal_autocheckpoint when replication is configured; previously the credit sweep could checkpoint and restart the WAL behind Litestream's back
  • Observability: Replica* otel counters and a critical-section histogram; JsonLitestream.metrics_port plus [[metrics]] in the Fly configs so litestream_* Prometheus metrics are scraped during the shadow period

Testing

  • 275 crate tests: WAL parser fixtures (real and synthetic, both checksum byte orders, golden cross-version canary), three-backend storage contract suite, 8 fault-injection scenarios, 6 crash-recovery kill points, 8 seeded 200-op randomized equivalence workloads, ignored soak and live-S3 tiers
  • 4 server-level integration tests: real API writes replicate and restore to a logically equivalent database; reboot-with-missing-DB auto-restores; shadow coexistence
  • An adversarial multi-agent review confirmed 18 findings; all are fixed with regression tests. Highlights: salt-match resume now proves CONTENT against the replica tip (post-power-loss rewind forks), the meta-verified resume path requires salt1 continuity (buried WAL cycles), restore hard-fails instead of soft-stopping when local errors could leave torn state, and shutdown checkpoints after the final ship so SQLite's checkpoint-on-close WAL deletion does not force a full snapshot on every deploy
  • Full gates: cargo nextest run --all-features (2047 passed), cargo test --doc, cargo clippy --no-deps --all-targets --all-features -- -Dwarnings, cargo check --no-default-features, cargo gen-types

Deployment plan

  1. Shadow burn-in: add plus.replica (S3 target, its own bucket/prefix) alongside the existing plus.litestream config; watch replica.verify.pass and replica.divergence metrics
  2. Run the ignored live-S3 tier against a real bucket before the shadow deploy: cargo nextest run -p bencher_replica --features plus,testing --run-ignored ignored-only with BENCHER_REPLICA_TEST_S3_* env vars
  3. Cutover: remove the plus.litestream section (forces one clean generation); the Litestream removal checklist (binary, Dockerfile stage, config types) follows after the burn-in

Notes for review

  • One new lint suppression needs sign-off per CLAUDE.md: #[expect(clippy::too_many_lines)] on ApiCounter::description() in bencher_otel (matches existing precedent for exhaustive metadata matches)
  • cargo deny check fails on two PRE-EXISTING advisories via bencher_plot's tree (ttf-parser unmaintained, crossbeam-epoch RUSTSEC-2026-0204), unrelated to this change
  • Documented residual limitation: an external writer that buries a WAL cycle AND deletes the WAL file while the server is down is undetectable at resume; the daily verification is the backstop

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

PR: #929
Base: devel
Head: u/ep/bencher-replica
Commit: 176b2893cd0fdc2ce461be3b14f450484fd39b93


Here is my consolidated review.

PR Review: bencher_replica — in-process SQLite replication

A large, high-quality PR (~21k lines) replacing Litestream with an in-process WAL replicator. I reviewed the integration layer directly and fanned out five parallel reviews across the new crate's core modules (WAL parsing, sync engine, restore/snapshot, storage backends, config). No Critical or High-severity defects were found. The code is unusually well-defended, thoroughly documented (invariants I1-I6), and heavily tested (unit + integration + fault-matrix + live-S3 CI tiers).

Strengths

  • Security fix done right: GET /v0/server/config now masks all secrets via Config::sanitized() (unconditional in debug and release), with tests covering base + plus secrets (security, DB, SMTP, OAuth, Litestream, and replica credentials).
  • Restore atomicity is sound: scratch DB → quick_check → atomic rename → parent-dir fsync, with scratch cleanup on every error path. A partial/prefix WAL can never be applied (checksum-chain pre-validation + checkpointed == expected_frames enforcement).
  • Single SQLite engine: rusqlite and diesel share one libsqlite3-sys (0.36.0), so WAL sharing is safe.
  • Checkpoint safety: the wal_autocheckpoint = 0 invariant is now centralized in one configure_standalone_connection helper routed through every writing connection (writer, pools, stats sweep, stats endpoint, test harness) — a good defense-in-depth consolidation.
  • CLAUDE.md compliance is clean across the board: no anyhow, no Box<dyn Error>, no select!, error variants wrap original types, camino for paths, dep.workspace = true shorthand, workspace-pinned deps, Dockerfile stubs added, cargo gen-types artifacts (openapi.json) regenerated. CI rust filter already covers plus/** and lib/**, so no path-filter change needed.

Findings worth addressing

Medium

  1. Data-losing restore soft-stops emit no adverse-event metric (restore.rs:176-177 and other soft-stop sites). When a broken WAL lineage forces a boot on the snapshot alone (dropping recent commits), only slog::error! fires, yet ApiCounter::ReplicaRestore still increments as success. CLAUDE.md explicitly requires adverse events be tracked via bencher_otel::ApiMeter::increment. As-is, a silent data regression looks like a successful restore to SRE. Recommend a dedicated counter on these soft-stop paths.

  2. ReplicaMeta version field is never validated (meta.rs:100). load deserializes but never checks version against META_VERSION = 1. Since the meta drives the "resume as epoch+1 vs re-snapshot" decision, a future structurally-compatible v2 meta would be silently trusted. The version field currently buys nothing.

  3. next_committed can buffer an unbounded rolled-back WAL tail (wal.rs:324-441). It calls scan with max_txn_bytes = u64::MAX in Retain mode, appending every frame before discovering no commit terminates the run. A multi-GB rolled-back transaction is fully heap-buffered then discarded, contradicting the scan doc's "never buffer unboundedly" claim (that guarantee only holds for scan_committed_extent). Safe only because callers front-run the bounded variant.

  4. Local storage backend has no reaper for crash-orphaned upload partials (local.rs:284, storage.rs:249). abort_incomplete_uploads is a no-op for the local backend. A LocalMultipart dropped on SIGKILL/OOM mid-snapshot leaves a .partial-<uuid> fragment (potentially multi-GB) forever; the S3 backend has a startup sweep, the local one does not.

  5. Resume-after-soft-stop salt-collision edge case (restore.rs:157-166, lower confidence). If soft-stopped epoch N+1 segments were corrupt (present, bad checksum) rather than absent, resuming ships new epoch N+1 segments under fresh salts into the same generation, and a later restore would see two salts in that epoch group → permanent soft-stop. Worth confirming against the engine's salt-rebind path.

Low (noting, not blocking)

  • wait_fatal takes the receiver eagerly at call-time (replicator.rs:119), not on first poll. Current main.rs usage (single call, immediately awaited in the shutdown race) is correct, but constructing-then-dropping the future would permanently disarm fatal detection. Moving .take() inside the async move would harden the contract.
  • Held writer-mutex guard across spawn_blocking().await in checkpoint_once/verify_once is not cancellation-safe; benign today because ticks are never cooperatively cancelled.
  • Final shutdown drain aborts on the first transient storage error (sync.rs:803) rather than retrying within remaining deadline budget — ships less WAL tail than possible on a momentary blip (lag, not loss).
  • String-typed fields where strong types exist: RestoreError::QuickCheck(String), snapshot_meta.rs created/sha256 as String, ReplicaMeta primitives. Mostly defensible (textual corruption output, must-round-trip advisory data).
  • testing/otel features don't imply plus (Cargo.toml); enabling either without plus compiles an empty crate with unused deps.
  • access_key_id stored as plaintext String (not Secret) in both replica and litestream config — matches existing convention and is less sensitive than the secret key, but is still a credential identifier.
  • Incomplete-upload sweep lists the whole bucket, not the prefix (s3.rs:504, deliberate for MinIO compat) — a boot-time cost in large shared buckets, correct behavior.

Verdict

Solid, carefully-engineered, and safe to merge after considering the Medium items — particularly #1 (adverse-event metric on data-losing restores), which is a direct CLAUDE.md requirement and the most operationally important, and #5 (confirm the salt-rebind resume path). The rest are hardening opportunities rather than blockers.


Model: claude-opus-4-8

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

ProjectBencher
Branchu/ep/bencher-replica
Testbedintel-v1

🚨 3 Alerts

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
Adapter::Magic (Rust)Latency
microseconds (µs)
📈 plot
🚷 threshold
🚨 alert (🔔)
26.78 µs
(+4.24%)Baseline: 25.69 µs
26.72 µs
(100.20%)

Adapter::RustLatency
microseconds (µs)
📈 plot
🚷 threshold
🚨 alert (🔔)
3.87 µs
(+10.30%)Baseline: 3.51 µs
3.71 µs
(104.31%)

Adapter::RustBenchLatency
microseconds (µs)
📈 plot
🚷 threshold
🚨 alert (🔔)
3.86 µs
(+10.10%)Baseline: 3.50 µs
3.71 µs
(104.07%)

Click to view all benchmark results
BenchmarkLatencyBenchmark Result
microseconds (µs)
(Result Δ%)
Upper Boundary
microseconds (µs)
(Limit %)
Adapter::Json📈 view plot
🚷 view threshold
4.85 µs
(+4.02%)Baseline: 4.66 µs
4.92 µs
(98.55%)
Adapter::Magic (JSON)📈 view plot
🚷 view threshold
4.72 µs
(+4.16%)Baseline: 4.53 µs
4.73 µs
(99.77%)
Adapter::Magic (Rust)📈 view plot
🚷 view threshold
🚨 view alert (🔔)
26.78 µs
(+4.24%)Baseline: 25.69 µs
26.72 µs
(100.20%)

Adapter::Rust📈 view plot
🚷 view threshold
🚨 view alert (🔔)
3.87 µs
(+10.30%)Baseline: 3.51 µs
3.71 µs
(104.31%)

Adapter::RustBench📈 view plot
🚷 view threshold
🚨 view alert (🔔)
3.86 µs
(+10.10%)Baseline: 3.50 µs
3.71 µs
(104.07%)

🐰 View full continuous benchmarking report in Bencher

Litestream 0.5.13 blocked all API writes for ~5.5 minutes in production
(2026-07-10): when it decides a full re-snapshot is needed, it copies the
entire database into a local LTX file while holding the SQLite write lock,
and no configuration reaches that code path. Its LTX compaction also churns
whole-database S3 transfers several times a day.

bencher_replica is an in-process replacement built around six invariants
(documented in src/lib.rs), the prime one being that the SQLite write lock
is only ever held for O(WAL-tail) work, never O(database):

- WAL parser with full salt and cumulative checksum-chain verification
- Local filesystem XOR S3-compatible storage behind one contract
- Step-driven sync engine; checkpoints are PASSIVE while the replicator
  itself holds BEGIN IMMEDIATE, closing the ship-vs-checkpoint race without
  ever needing RESTART or TRUNCATE checkpoints
- Generation-based snapshots via a single-step SQLite online backup into a
  scratch file (transactionally consistent under concurrent checkpoints),
  throttled zstd multipart upload, snapshot.json as the atomic commit marker
- Latest-only restore in the same startup handshake slot Litestream used,
  with chain pre-validation and checkpoint-consumption verification
- Restore-and-compare verification (default daily) and shadow mode: with
  both plus.litestream and plus.replica configured, Litestream keeps
  checkpoint ownership and restore precedence during the burn-in

Also included:
- Fix: standalone sweep connections (stats, credit grants) now disable
  wal_autocheckpoint when replication is configured; previously the credit
  sweep could checkpoint and restart the WAL behind Litestream's back
- plus.replica config (JsonReplication), otel Replica* counters, main.rs
  lifecycle wiring (restore precedence, fatal race arm, final ship inside
  the Fly kill budget), TestServer::new_with_replica, Dockerfile stubs
- JsonLitestream.metrics_port and [[metrics]] in the Fly configs so
  litestream_* Prometheus metrics are scraped during the shadow period

Testing: 275 crate tests (WAL fixtures cross-validated against SQLite
itself, a three-backend storage contract suite, 8 fault-injection
scenarios, 6 crash kill points, 8 seeded 200-op equivalence workloads,
ignored soak and live-S3 tiers) plus 4 server-level integration tests.
An adversarial multi-agent review confirmed 18 findings, all fixed with
regression tests, including a silent data-loss gap in resume (salt-match
resume now proves content against the replica tip, and the meta-verified
path requires salt1 continuity).
@epompeii
epompeii force-pushed the u/ep/bencher-replica branch from 6d14320 to 176b289 Compare July 19, 2026 13:31
@epompeii
epompeii deployed to Cloudflare July 19, 2026 13:32 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant