Skip to content

libsql-server: memory-bounded streaming dump importer (opt-in, benchmarkable against the buffered one) - #51

Draft
tszymczyszyn-shopify wants to merge 6 commits into
v0.9.30-shopify-patchesfrom
tszymczyszyn/streaming-dump-importer
Draft

tszymczyszyn-shopify wants to merge 6 commits into
v0.9.30-shopify-patchesfrom
tszymczyszyn/streaming-dump-importer

Conversation

@tszymczyszyn-shopify

@tszymczyszyn-shopify tszymczyszyn-shopify commented Oct 7, 2026 •

Copy link
Copy Markdown

Summary

Adds an opt-in, memory-bounded streaming importer for POST /v1/namespaces/:ns/create + dump_url, selectable per request or server-wide, alongside the existing (now called buffered) importer, which is kept byte-for-byte so both can be benchmarked and validated on the same libsql-server instance.

Design: docs/STREAMING_DUMP_IMPORT_DESIGN.md. Context: Retail #35846 / LibSQL DB Mover P0 copy path.

📖 Reviewer walkthrough — request flow, suggested reading order, invariants, failure paths, compatibility differences, test anchors, and review focus.

Why

The current importer does read_to_string on the whole dump (plus a lowercase copy for the attach check), so import RSS is proportional to dump size. Everything downstream (SQLite page cache, replication log) is already bounded/flushed to disk — the importer's String was the only size-proportional allocation.

What

  • Runtime selection
    • request body: "dump_importer": "buffered" | "streaming" (400 if given without dump_url, 422 for unknown values)
    • server default: --dump-importer / SQLD_DUMP_IMPORTER (default buffered — no behavior change until opted in)
    • streaming knobs: --dump-import-max-statement-size (64 MiB → HTTP 413), --dump-import-queue-bytes (16 MiB), --dump-import-queue-depth (256)
  • Streaming pipeline: async reader → StatementFramer (single-pass resumable port of SQLite's complete.c state machine, absolute line/column tracking) → bounded mpsc + byte-budget Semaphore → one BLOCKING_RT executor thread owning the connection. Explicit End message; a channel closed without End (stream error, framing error, cancelled admin request) ⇒ ROLLBACK on the executor thread.
  • Per statement: UTF-8 check → sqlite3_parser for policy (empty frames, libsql_wasm_func_table skip, ATTACH/DETACH → 400 with the existing message, same n_stmt > 2 && autocommit ⇒ NoTxn rule) → execute the original statement text. Parser errors are remapped to absolute dump positions, so the existing error snapshots apply to both importers unchanged.
  • Metrics libsql_server_dump_import_{duration_seconds,bytes,statements,max_statement_bytes}{importer} plus failures{importer,kind}; start/progress/success logs and failure-category + partial-progress logs.
  • file: dumps are now read in 64 KiB chunks (was 4 KiB) — benefits both importers.

Behavior differences (streaming vs buffered) — see §10 of the design

  • The word "attach" inside data no longer rejects the dump (buffered's substring check is a false positive; the streaming check is statement-level, plus the authorizer backstop).
  • Schema SQL stored in sqlite_schema is the dump's DDL verbatim; buffered stores the parser's re-serialization (e.g. ON plain (a), multi-line trigger body). Data is identical.
  • Invalid UTF-8 / NUL → 400 (was 500); oversized statement → 413; standalone DETACH → 400 (buffered: 500); row-returning statements (EXPLAIN, stray SELECT) execute with rows discarded (was 500).

Results (debug build, macOS arm64, 36 MB dump / 300 007 statements, one server, scripts/bench-dump-import.sh)

importer wall RSS before → peak Δ RSS
streaming 15.2 s 38 → 54 MB +15 MB
buffered 19.7 s 56 → 195 MB +135 MB (≈3.7× dump)

Both: integrity_check ok, 300 300 identical data rows.

Adversarial review (commit 5: address adversarial review)

Independent reviewers (scope / correctness / security / performance / testing / architecture / operations) plus a manual pass; full table in design doc §18. Verdict before fixes: NEEDS CHANGES.

  • HIGH — fixed: framing called sqlite3_complete() once per candidate ;, i.e. O(n²) in interior semicolons, on a tokio worker. Measured: 1 MiB value with 50k ; → 20 s CPU; 200 KB CSS-like value → 0.36 s per row. Replaced with complete.rs, a resumable Rust port of complete.c (same tokenizer + 8×8 state machine), differential-tested against the real sqlite3_complete (2000 random token soups × 5 chunkings). No unsafe left in the importer. The CSS-shaped 10 MB dump now imports in 0.56 s.
  • MEDIUM — fixed: 2× transient copy of the largest statement; queue_bytes/queue_depth = 0 panics for programmatic configs; failure log duplicated error text that may quote dump SQL (now category + progress); no progress/partial stats for long imports (now every 10 s + on failure); dump_importer JSON value case-sensitive while the CLI flag wasn't; bench script portability (macOS date %N, bc, bare $(curl) under set -e, unverified PID).
  • LOW — fixed/documented: column in the 413 message; standalone DETACH is 400 (streaming) vs 500 (buffered), now documented; executor panic path (unwinding drops the connection → SQLite rolls back; documented).
  • Tests added: WASM-table skip + empty dump (both importers), EOF without ;, executor failure under a saturated queue, abandoned admin request (executor rolls back and releases the connection), linear framing of a semicolon-dense statement, zero-copy hand-over of large frames. 39 dump integration tests + 20 unit tests, stable over repeated runs.
  • Deferred (documented): per-statement policy duplicated between importers until buffered is removed; no admission control for concurrent streaming imports on BLOCKING_RT; PR could be split at commit 1 (pure refactor).

Tests

  • 20 unit tests (dump_import::{complete,framer,mod}), framer cases run for every chunk size 1..=len, plus the differential test against sqlite3_complete.
  • Every existing dump integration test now also runs with _streaming; existing snapshots are reused by name (incl. the absolute-position one: line 7, column 11).
  • New: 1/7-byte HTTP chunking, truncated body → 500 + rollback, 413, NUL/invalid UTF-8 → 400, dump_importer field validation, server-wide default, 50k-statement dump under a 64 KiB queue budget, and an equivalence test comparing both importers' exported data.
  • Full cargo test -p libsql-server: all pass except embedded_replica::local::local_sync_with_writes, which fails identically on the base commit in my environment (pre-existing, unrelated).

Commits

  1. extract dump loading into a selectable importer — pure refactor + plumbing; buffered importer moved verbatim, streaming stubbed; existing tests/snapshots untouched
  2. add incremental SQL statement framer
  3. add memory-bounded streaming dump importer + tests
  4. docs + benchmark script

Out of scope / known pre-existing issues (design §13)

  • Enormous statements/BLOBs (the size cap is a safety valve, not support).
  • Namespace lifecycle on failure (metastore row kept after a failed create, no cleanup when the admin request is cancelled) — DB Mover fence/quarantine work.
  • Local macOS build note (not in this PR): libsql-ffi/build.rs passes regexp/pcre2/pcre2_internal.h to cc as a source; current Xcode ar silently drops members when it meets the resulting non-Mach-O object, so sqld fails to link with undefined sqlite3_* symbols. GNU ar on Linux CI tolerates it. Deleting that one sqlean_patterns.push(...) line fixes it; happy to send it as a separate PR.

Rollout

Keep buffered as default until the §15 acceptance criteria are met on production-shaped data (settings/catalog schemas incl. FTS); then flip --dump-importer to streaming in a follow-up and remove the buffered importer after a soak.

Move the existing dump loader verbatim into namespace/dump_import/buffered.rs
and introduce the plumbing needed to select an importer at runtime:

- DumpImporterKind { Buffered, Streaming } and DumpImportConfig (default
  importer, max statement size, in-flight queue bytes/depth) on DbConfig and
  BaseNamespaceConfig.
- RestoreOption::Dump now carries a DumpSource { stream, importer }.
- Admin API: optional `dump_importer` field on POST /v1/namespaces/:ns/create
  (400 when given without dump_url).
- CLI: --dump-importer (SQLD_DUMP_IMPORTER, default buffered),
  --dump-import-max-statement-size, --dump-import-queue-bytes,
  --dump-import-queue-depth.
- LoadDumpError gains ImporterWithoutDumpUrl (400) and StatementTooLarge (413).
- file: dumps are read in 64 KiB chunks instead of 4 KiB.
- Import duration/bytes/statements/failure metrics and start/finish logs.

The streaming importer is a stub in this commit; the buffered importer's
behavior is unchanged and existing dump tests/snapshots pass as-is.
StatementFramer accumulates dump chunks and emits complete statements using
sqlite3_complete() as the boundary oracle, so semicolons inside strings,
quoted identifiers, comments and CREATE TRIGGER ... END bodies are handled
the same way the sqlite3 shell handles them. It tracks the absolute
line/column of every frame so parser errors can be reported against the
whole dump, rejects NUL bytes, and fails closed when a single statement
exceeds the configured size limit.

Unit tests cover every chunk size from 1 to the input length for each case.
Select it with `"dump_importer": "streaming"` on the create-namespace
request or `--dump-importer streaming` server-wide. The buffered importer
remains the default.

An async reader drives the dump stream through StatementFramer and hands
complete statements to a single executor thread (BLOCKING_RT) that owns the
connection for the whole import. A bounded mpsc channel plus a byte-budget
semaphore cap the statements in flight, so memory is bounded by
configuration plus the largest single statement instead of the dump size.

Per statement the executor validates UTF-8, parses with sqlite3_parser for
the policy checks (empty frames, libsql_wasm_func_table skip, ATTACH/DETACH
rejection, the same must-be-in-a-transaction rule as before) and then
executes the *original* statement text, so the schema SQL stored in
sqlite_schema is exactly what the dump contained. Parser errors are
remapped to absolute dump positions, so the existing error snapshots apply
to both importers. An explicit End message distinguishes a clean EOF from a
dropped reader (stream error, framing error, cancelled admin request); in
every failure path the transaction is rolled back on the executor thread.

Tests: every existing dump test now also runs against the streaming
importer, plus streaming-specific coverage for 1/7-byte HTTP chunking,
truncated bodies, statement size limit (413), NUL bytes and invalid UTF-8
(400), the dump_importer request field, the server-wide default, a
50k-statement dump under a tiny queue budget, and an equivalence test that
imports the same dump with both importers and compares the data.
- ADMIN_API.md: dump_url semantics and the new dump_importer field.
- USER_GUIDE.md: creating a database from a SQLite dump, importer selection
  and the --dump-import-* flags.
- STREAMING_DUMP_IMPORT_DESIGN.md: the design the implementation follows,
  with 'as built' notes where it was refined (422 for unknown importer,
  size limit also applies to terminated statements, exact UTF-8 error
  position, DDL text differences between importers) and first results.
- scripts/bench-dump-import.sh: runs both importers against one server,
  samples RSS, compares exported data and integrity_check.
Framing is now a single linear pass. The first implementation called
sqlite3_complete() for every candidate semicolon of the pending statement,
which is O(n^2) in interior semicolons: a 1 MiB text value with 50k
semicolons cost 20 s of CPU on a tokio worker, a 200 KB CSS-like value
0.36 s per row. complete.rs is a resumable port of complete.c (same
tokenizer rules and 8x8 state machine); a differential unit test frames
2000 random token soups under five chunkings and requires identical results
to the real sqlite3_complete, which is now only used in that test. No unsafe
code remains in the importer.

Other findings fixed:
- frames >= 1 MiB are handed over without copying, so peak memory is one
  copy of the largest statement rather than two;
- queue_bytes/queue_depth of 0 in a programmatically built DumpImportConfig
  no longer panic (clamped at the point of use);
- the failure log records the failure category plus statements/bytes
  processed instead of repeating the error text (which may quote dump SQL
  and is already logged by the HTTP layer); progress is logged every 10 s;
- dump_importer in the request body is parsed like the CLI flag
  (case-insensitive, trimmed);
- the 413 message includes the statement's column;
- bench script: no bc/date %N, explicit tool check, curl and
  integrity_check failures reported, caller-supplied PID must be sqld.

New tests: WASM-table skip and empty dump (both importers), EOF without a
trailing semicolon, executor failure under a saturated queue, abandoned
admin request (executor rolls back and releases the connection), linear
framing of a semicolon-dense statement, zero-copy hand-over of large frames.

Documented the remaining accepted differences (DETACH: 400 vs 500) and the
review outcome in the design doc.
@tszymczyszyn-shopify
tszymczyszyn-shopify force-pushed the tszymczyszyn/streaming-dump-importer branch from 3ca3765 to ecb9431 Compare October 8, 2026 13:35
@tszymczyszyn-shopify

Copy link
Copy Markdown
Author

Reviewer walkthrough

Pinned to ecb94314aa. This is a large diff, but the runtime path is concentrated in five new files under namespace/dump_import. Suggested review order below.

TL;DR

This adds an opt-in streaming implementation for namespace creation from dump_url. The historical importer remains the default and the control arm. The new path incrementally frames SQL, puts complete statements behind count + byte backpressure, and executes them in order on one blocking thread owning the connection. Its importer memory is bounded by the configured queue plus the largest allowed statement rather than the whole dump.

No default behavior changes: buffered remains the server default. Selection is per request ("dump_importer":"streaming") or server-wide (--dump-importer / SQLD_DUMP_IMPORTER).

End-to-end flow

flowchart LR
    A[POST namespace create] --> B[CreateNamespaceReq]
    B --> C[DumpSource<br/>stream + optional importer]
    C --> D[resolve request override<br/>or server default]
    D --> E{importer}
    E -->|buffered| F[legacy whole-dump importer]
    E -->|streaming| G[async DumpStream reader]
    G --> H[StatementFramer<br/>linear complete.c state machine]
    H --> I[mpsc queue<br/>depth bound]
    I --> J[Semaphore permit travels<br/>with frame: byte bound]
    J --> K[one BLOCKING_RT executor<br/>owns PrimaryConnection]
    K --> L[UTF-8 + parse/policy check]
    L --> M[execute original SQL text]
    M --> N[COMMIT or best-effort ROLLBACK]
Loading

The explicit Msg::End distinguishes clean EOF from a dropped reader. A closed channel without End means stream/framing failure or request cancellation and forces rollback.

Suggested reading order

1. API and selection plumbing (small)

2. Dispatcher and unchanged control arm

3. Statement boundary detection (most correctness-sensitive)

  • complete.rs: resumable Rust port of SQLite complete.c's tokenizer and 8×8 state machine. This handles quotes, comments, bracket identifiers and CREATE [TEMP] TRIGGER … END; across arbitrary chunk boundaries.
  • StatementFramer::push / finish: absolute positions, NUL/size checks, one linear scan, and zero-copy hand-over for frames ≥1 MiB.
  • Differential and pathological-shape tests: 2,000 deterministic token soups × five chunkings are compared with the real sqlite3_complete(); separate tests cover semicolon density and allocation identity.

Why the local port: calling non-resumable sqlite3_complete() for each candidate semicolon was quadratic. It measured 20 s CPU for one 1 MiB/50k-semicolon statement on a tokio worker. The port scans every dump byte once and keeps the FFI only as its test oracle.

4. Bounded reader/executor pipeline

  • load_dump_streaming: drives the stream and framer, joins the blocking executor, and resolves source-vs-executor error precedence.
  • send_frame: an owned semaphore permit travels inside each message. Frames larger than the queue budget acquire the whole budget and therefore travel alone.
  • run_executor: one thread owns the connection, installs the ATTACH/DETACH authorizer, consumes until explicit End, and rolls back on every error/channel-close path.
  • handle_statement and execution: UTF-8 → parse one command → legacy WASM-table skip → ATTACH/DETACH policy → legacy transaction rule → execute the original SQL while draining returned rows.

Memory model: one stream chunk + one unfinished frame + at most queue_bytes of queued frames + the statement being executed. max_statement_bytes is a separate safety cap (64 MiB default); huge individual BLOB literals are intentionally not optimized beyond that cap.

5. Failure semantics and tests

Event Result
Source/framing failure sender drops; executor sees close, rolls back; specific source error wins
Executor parse/execute/policy failure receiver drops; blocked reader wakes; executor error wins
Clean EOF final tail is sent, then explicit End; executor requires autocommit
EOF with open transaction explicit rollback + NoCommit
Admin request cancellation sender/stream drop; uncancelled blocking executor sees close and rolls back
Executor panic connection is dropped during unwind; SQLite rolls back an open transaction

The main integration anchors are chunking + truncated body, cross-importer data equivalence and large-dump backpressure, and EOF, saturated-queue failure, and cancellation.

Intentional compatibility differences

The full matrix is in design §10. The important ones:

  • streaming stores the original DDL text; buffered executes the parser's normalized rendering (data is equivalent);
  • the word attach inside data is accepted by streaming instead of triggering buffered's substring false positive;
  • invalid UTF-8/NUL is 400 instead of 500; oversized statements are 413;
  • standalone DETACH is 400 instead of buffered's execution-time 500;
  • row-returning statements execute with rows discarded instead of failing with ExecuteReturnedResults.

These differences are opt-in because buffered remains the default.

Evidence

  • 20 dump_import unit tests.
  • 39 dump integration tests, including cancellation and saturated backpressure.
  • CI was fully green on code head 967f8f637e; this pinned head only updates documentation to match that code and has CI re-running.
  • 36 MB / 300,007 statements: streaming 15.2 s / +15 MB RSS, buffered 19.7 s / +135 MB RSS; identical 300,300 data rows; integrity_check=ok.
  • Pathological 10 MB CSS-shaped dump (~5,000 interior semicolons per 200 KB row): streaming 0.56 s / +21 MB, buffered 0.53 s / +32 MB, identical data. The pre-fix framer took ~0.36 s per row.

Where reviewer judgment is most valuable

  1. Does CompletionScanner faithfully match the bundled SQLite completion rules? The differential test is the main guard.
  2. Do the channel-close/error-precedence paths always terminate and rollback, including cancellation?
  3. Is the stated memory bound convincing, especially the permit lifetime inside Msg::Stmt and oversized-frame behavior?
  4. Are the documented opt-in behavior differences acceptable for rollout?
  5. Is keeping buffered as default until production-shaped validation the right migration boundary?

Explicitly out of scope / deferred

  • Metastore cleanup when namespace creation fails or the admin request is cancelled (pre-existing lifecycle behavior).
  • A server-side import timeout.
  • Bottomless's own buffering.
  • Admission control for many concurrent long-lived imports on BLOCKING_RT.
  • Removing the buffered importer or flipping the default; both require production soak first.

The full rationale, memory model, behavior matrix, benchmark procedure and adversarial-review resolutions are in STREAMING_DUMP_IMPORT_DESIGN.md.

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