Repository navigation
libsql-server: memory-bounded streaming dump importer (opt-in, benchmarkable against the buffered one) - #51
Draft
tszymczyszyn-shopify wants to merge 5 commits into
Conversation
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.
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.
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 samelibsql-serverinstance.Design:
docs/STREAMING_DUMP_IMPORT_DESIGN.md. Context: Retail #35846 / LibSQL DB Mover P0 copy path.Why
The current importer does
read_to_stringon the whole dump (plus a lowercase copy for theattachcheck), so import RSS is proportional to dump size. Everything downstream (SQLite page cache, replication log) is already bounded/flushed to disk — the importer'sStringwas the only size-proportional allocation.What
"dump_importer": "buffered" | "streaming"(400 if given withoutdump_url, 422 for unknown values)--dump-importer/SQLD_DUMP_IMPORTER(defaultbuffered— no behavior change until opted in)--dump-import-max-statement-size(64 MiB → HTTP 413),--dump-import-queue-bytes(16 MiB),--dump-import-queue-depth(256)StatementFramer(memchr(';')+sqlite3_complete()as the boundary oracle, absolute line/column tracking) → boundedmpsc+ byte-budgetSemaphore→ oneBLOCKING_RTexecutor thread owning the connection. ExplicitEndmessage; a channel closed withoutEnd(stream error, framing error, cancelled admin request) ⇒ROLLBACKon the executor thread.sqlite3_parserfor policy (empty frames,libsql_wasm_func_tableskip,ATTACH/DETACH→ 400 with the existing message, samen_stmt > 2 && autocommit ⇒ NoTxnrule) → execute the original statement text. Parser errors are remapped to absolute dump positions, so the existing error snapshots apply to both importers unchanged.libsql_server_dump_import_{duration_seconds,bytes,statements,failures,max_statement_bytes}{importer}and start/finish logs for both importers.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
sqlite_schemais the dump's DDL verbatim; buffered stores the parser's re-serialization (e.g.ON plain (a), multi-line trigger body). Data is identical.Results (debug build, macOS arm64, 36 MB dump / 300 007 statements, one server,
scripts/bench-dump-import.sh)Both:
integrity_checkok, 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.
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 withcomplete.rs, a resumable Rust port of complete.c (same tokenizer + 8×8 state machine), differential-tested against the realsqlite3_complete(2000 random token soups × 5 chunkings). Nounsafeleft in the importer. The CSS-shaped 10 MB dump now imports in 0.56 s.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_importerJSON value case-sensitive while the CLI flag wasn't; bench script portability (macOSdate %N,bc, bare$(curl)underset -e, unverified PID).DETACHis 400 (streaming) vs 500 (buffered), now documented; executor panic path (unwinding drops the connection → SQLite rolls back; documented).;, 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.BLOCKING_RT; PR could be split at commit 1 (pure refactor).Tests
dump_import::{complete,framer,mod}), framer cases run for every chunk size 1..=len, plus the differential test againstsqlite3_complete._streaming; existing snapshots are reused by name (incl. the absolute-position one:line 7, column 11).dump_importerfield validation, server-wide default, 50k-statement dump under a 64 KiB queue budget, and an equivalence test comparing both importers' exported data.cargo test -p libsql-server: all pass exceptembedded_replica::local::local_sync_with_writes, which fails identically on the base commit in my environment (pre-existing, unrelated).Commits
extract dump loading into a selectable importer— pure refactor + plumbing; buffered importer moved verbatim, streaming stubbed; existing tests/snapshots untouchedadd incremental SQL statement frameradd memory-bounded streaming dump importer+ testsdocs+ benchmark scriptOut of scope / known pre-existing issues (design §13)
libsql-ffi/build.rspassesregexp/pcre2/pcre2_internal.htoccas a source; current Xcodearsilently drops members when it meets the resulting non-Mach-O object, sosqldfails to link with undefinedsqlite3_*symbols. GNUaron Linux CI tolerates it. Deleting that onesqlean_patterns.push(...)line fixes it; happy to send it as a separate PR.Rollout
Keep
bufferedas default until the §15 acceptance criteria are met on production-shaped data (settings/catalog schemas incl. FTS); then flip--dump-importertostreamingin a follow-up and remove the buffered importer after a soak.