From cf6fab3ae38d03ddd6bca6a6ab75a69c403e36c5 Mon Sep 17 00:00:00 2001 From: Tomasz Szymczyszyn Date: Wed, 7 Oct 2026 17:46:56 +0200 Subject: [PATCH 1/6] libsql-server: extract dump loading into a selectable importer 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. --- libsql-server/src/config.rs | 3 + libsql-server/src/error.rs | 8 + libsql-server/src/http/admin/mod.rs | 26 +- libsql-server/src/lib.rs | 1 + libsql-server/src/main.rs | 36 +- .../src/namespace/configurator/helpers.rs | 130 +------ .../src/namespace/configurator/mod.rs | 2 + .../src/namespace/dump_import/buffered.rs | 105 ++++++ .../src/namespace/dump_import/framer.rs | 13 + .../src/namespace/dump_import/mod.rs | 323 ++++++++++++++++++ .../src/namespace/dump_import/streaming.rs | 17 + libsql-server/src/namespace/mod.rs | 25 +- libsql-server/src/schema/scheduler.rs | 1 + libsql-server/src/test/bottomless.rs | 1 + 14 files changed, 566 insertions(+), 125 deletions(-) create mode 100644 libsql-server/src/namespace/dump_import/buffered.rs create mode 100644 libsql-server/src/namespace/dump_import/framer.rs create mode 100644 libsql-server/src/namespace/dump_import/mod.rs create mode 100644 libsql-server/src/namespace/dump_import/streaming.rs diff --git a/libsql-server/src/config.rs b/libsql-server/src/config.rs index 2c3c302a6d..012585bcd8 100644 --- a/libsql-server/src/config.rs +++ b/libsql-server/src/config.rs @@ -11,6 +11,7 @@ use tonic::transport::Channel; use tower::ServiceExt; use crate::auth::{Auth, Disabled}; +pub use crate::namespace::dump_import::{DumpImportConfig, DumpImporterKind}; use crate::net::{AddrIncoming, Connector}; pub struct RpcClientConfig { @@ -103,6 +104,7 @@ pub struct DbConfig { pub max_concurrent_requests: u64, pub disable_intelligent_throttling: bool, pub connection_creation_timeout: Option, + pub dump_import: DumpImportConfig, } impl Default for DbConfig { @@ -123,6 +125,7 @@ impl Default for DbConfig { max_concurrent_requests: 128, disable_intelligent_throttling: false, connection_creation_timeout: None, + dump_import: DumpImportConfig::default(), } } } diff --git a/libsql-server/src/error.rs b/libsql-server/src/error.rs index bfe67f47c7..8f57ac7ce1 100644 --- a/libsql-server/src/error.rs +++ b/libsql-server/src/error.rs @@ -296,6 +296,12 @@ pub enum LoadDumpError { NotAFile, #[error("The passed dump sql is invalid: {0}")] InvalidSqlInput(String), + #[error("`dump_importer` requires `dump_url`")] + ImporterWithoutDumpUrl, + #[error( + "A dump statement starting at line {line} exceeds the maximum allowed size ({limit} bytes)" + )] + StatementTooLarge { line: u64, limit: usize }, } impl ResponseError for LoadDumpError {} @@ -315,7 +321,9 @@ impl IntoResponse for &LoadDumpError { | NoCommit | NotAFile | DumpFilePathNotAbsolute + | ImporterWithoutDumpUrl | InvalidSqlInput(_) => self.format_err(StatusCode::BAD_REQUEST), + StatementTooLarge { .. } => self.format_err(StatusCode::PAYLOAD_TOO_LARGE), } } } diff --git a/libsql-server/src/http/admin/mod.rs b/libsql-server/src/http/admin/mod.rs index 2d8de1cdd1..c1861ac74f 100644 --- a/libsql-server/src/http/admin/mod.rs +++ b/libsql-server/src/http/admin/mod.rs @@ -26,7 +26,8 @@ use crate::auth::parse_jwt_keys; use crate::connection::config::{DatabaseConfig, DurabilityMode}; use crate::error::{Error, LoadDumpError}; use crate::hrana; -use crate::namespace::{DumpStream, NamespaceName, NamespaceStore, RestoreOption}; +use crate::namespace::dump_import::DumpImporterKind; +use crate::namespace::{DumpSource, DumpStream, NamespaceName, NamespaceStore, RestoreOption}; use crate::net::Connector; use crate::LIBSQL_PAGE_SIZE; @@ -371,6 +372,10 @@ async fn handle_post_config( #[derive(Debug, Deserialize)] struct CreateNamespaceReq { dump_url: Option, + /// Which importer loads `dump_url`: `"buffered"` (historical, whole dump in memory) or + /// `"streaming"` (memory-bounded). Defaults to the server's `--dump-importer`. + #[serde(default)] + dump_importer: Option, max_db_size: Option, heartbeat_url: Option, bottomless_db_id: Option, @@ -408,6 +413,10 @@ async fn handle_create_namespace( )); } + if req.dump_importer.is_some() && req.dump_url.is_none() { + return Err(LoadDumpError::ImporterWithoutDumpUrl.into()); + } + if let Some(ns) = req.shared_schema_name { if req.shared_schema { return Err(Error::SharedSchemaCreationError( @@ -423,9 +432,10 @@ async fn handle_create_namespace( } let dump = match req.dump_url { - Some(ref url) => { - RestoreOption::Dump(dump_stream_from_url(url, app_state.connector.clone()).await?) - } + Some(ref url) => RestoreOption::Dump( + DumpSource::new(dump_stream_from_url(url, app_state.connector.clone()).await?) + .with_importer(req.dump_importer), + ), None => RestoreOption::Latest, }; @@ -474,6 +484,9 @@ async fn handle_fork_namespace( Ok(()) } +/// Read `file:` dumps in reasonably large chunks; both importers consume the resulting stream. +const DUMP_FILE_READ_CHUNK_SIZE: usize = 64 * 1024; + async fn dump_stream_from_url(url: &Url, connector: C) -> Result where C: Connector, @@ -507,7 +520,10 @@ where let f = tokio::fs::File::open(path).await?; - Ok(Box::new(ReaderStream::new(f))) + Ok(Box::new(ReaderStream::with_capacity( + f, + DUMP_FILE_READ_CHUNK_SIZE, + ))) } scheme => Err(LoadDumpError::UnsupportedUrlScheme(scheme.to_string())), } diff --git a/libsql-server/src/lib.rs b/libsql-server/src/lib.rs index 1642ad951a..10b46ae2d3 100644 --- a/libsql-server/src/lib.rs +++ b/libsql-server/src/lib.rs @@ -601,6 +601,7 @@ where encryption_config: self.db_config.encryption_config.clone(), disable_intelligent_throttling: self.db_config.disable_intelligent_throttling, connection_creation_timeout: self.db_config.connection_creation_timeout, + dump_import: self.db_config.dump_import.clone(), }; let (metastore_conn_maker, meta_store_wal_manager) = diff --git a/libsql-server/src/main.rs b/libsql-server/src/main.rs index 307d5482fe..0acf2e9133 100644 --- a/libsql-server/src/main.rs +++ b/libsql-server/src/main.rs @@ -16,8 +16,8 @@ use tracing_subscriber::Layer; use tracing_subscriber::{prelude::*, EnvFilter}; use libsql_server::config::{ - AdminApiConfig, BottomlessConfig, DbConfig, HeartbeatConfig, MetaStoreConfig, RpcClientConfig, - RpcServerConfig, TlsConfig, UserApiConfig, + AdminApiConfig, BottomlessConfig, DbConfig, DumpImportConfig, DumpImporterKind, + HeartbeatConfig, MetaStoreConfig, RpcClientConfig, RpcServerConfig, TlsConfig, UserApiConfig, }; use libsql_server::net::AddrIncoming; use libsql_server::version::Version; @@ -253,6 +253,28 @@ struct Cli { #[clap(long, env = "SQLD_CONNECTION_CREATION_TIMEOUT_SEC")] connection_creation_timeout_sec: Option, + /// Importer used by `POST /v1/namespaces/:ns/create` with `dump_url` when the request + /// doesn't specify `dump_importer`. `buffered` reads the whole dump into memory; + /// `streaming` executes statements as they arrive with bounded memory. + #[clap(long, env = "SQLD_DUMP_IMPORTER", default_value = "buffered")] + dump_importer: DumpImporterKind, + + /// Streaming dump importer: reject any single SQL statement larger than this. + #[clap( + long, + env = "SQLD_DUMP_IMPORT_MAX_STATEMENT_SIZE", + default_value = "64MiB" + )] + dump_import_max_statement_size: ByteSize, + + /// Streaming dump importer: maximum bytes of statements framed but not yet executed. + #[clap(long, env = "SQLD_DUMP_IMPORT_QUEUE_BYTES", default_value = "16MiB")] + dump_import_queue_bytes: ByteSize, + + /// Streaming dump importer: maximum number of statements framed but not yet executed. + #[clap(long, env = "SQLD_DUMP_IMPORT_QUEUE_DEPTH", default_value = "256")] + dump_import_queue_depth: usize, + /// Allow meta store to recover config from filesystem from older version, if meta store is /// empty on startup #[clap(long, env = "SQLD_ALLOW_METASTORE_RECOVERY")] @@ -401,6 +423,15 @@ fn make_db_config(config: &Cli) -> anyhow::Result { bottomless_replication.encryption_config = encryption_config.clone(); } } + let dump_import = DumpImportConfig { + default_importer: config.dump_importer, + max_statement_bytes: usize::try_from(config.dump_import_max_statement_size.as_u64()) + .context("dump import max statement size doesn't fit in usize")?, + queue_bytes: usize::try_from(config.dump_import_queue_bytes.as_u64()) + .context("dump import queue bytes doesn't fit in usize")?, + queue_depth: config.dump_import_queue_depth, + }; + dump_import.validate()?; Ok(DbConfig { extensions_path: config.extensions_path.clone().map(Into::into), bottomless_replication, @@ -419,6 +450,7 @@ fn make_db_config(config: &Cli) -> anyhow::Result { connection_creation_timeout: config .connection_creation_timeout_sec .map(|x| Duration::from_secs(x)), + dump_import, }) } diff --git a/libsql-server/src/namespace/configurator/helpers.rs b/libsql-server/src/namespace/configurator/helpers.rs index 599320783d..023333be30 100644 --- a/libsql-server/src/namespace/configurator/helpers.rs +++ b/libsql-server/src/namespace/configurator/helpers.rs @@ -5,23 +5,15 @@ use std::time::Duration; use anyhow::Context as _; use bottomless::replicator::Options; -use bytes::Bytes; use enclose::enclose; -use fallible_iterator::FallibleIterator; -use futures::Stream; use libsql_sys::EncryptionConfig; -use rusqlite::hooks::{AuthAction, AuthContext, Authorization}; -use sqlite3_parser::ast::{Cmd, Stmt}; -use sqlite3_parser::lexer::sql::{Parser, ParserError}; -use tokio::io::AsyncReadExt; use tokio::task::JoinSet; -use tokio_util::io::StreamReader; use crate::connection::config::DatabaseConfig; use crate::connection::connection_manager::InnerWalManager; use crate::connection::legacy::MakeLegacyConnection; use crate::connection::{Connection as _, MakeConnection, MakeThrottledConnection}; -use crate::database::{PrimaryConnection, PrimaryConnectionMaker}; +use crate::database::PrimaryConnectionMaker; use crate::error::LoadDumpError; use crate::namespace::broadcasters::BroadcasterHandle; use crate::namespace::meta_store::MetaStoreHandle; @@ -201,11 +193,19 @@ pub(super) async fn make_primary_connection_maker( RestoreOption::Dump(_) if !is_fresh_db => { Err(LoadDumpError::LoadDumpExistingDb)?; } - RestoreOption::Dump(dump) => { + RestoreOption::Dump(source) => { let conn = connection_maker.create().await?; - tracing::debug!("Loading dump"); - load_dump(dump, conn).await?; - tracing::debug!("Done loading dump"); + let kind = source + .importer + .unwrap_or(base_config.dump_import.default_importer); + crate::namespace::dump_import::load_dump( + kind, + source.stream, + conn, + &base_config.dump_import, + name, + ) + .await?; } _ => { /* other cases were already handled when creating bottomless */ } } @@ -290,110 +290,6 @@ async fn run_periodic_compactions(logger: Arc) -> anyhow::Res } } -async fn load_dump(dump: S, conn: PrimaryConnection) -> crate::Result<(), LoadDumpError> -where - S: Stream> + Unpin, -{ - let mut reader = tokio::io::BufReader::new(StreamReader::new(dump)); - let mut dump_content = String::new(); - reader - .read_to_string(&mut dump_content) - .await - .map_err(|e| LoadDumpError::Internal(format!("Failed to read dump content: {}", e)))?; - - if dump_content.to_lowercase().contains("attach") { - return Err(LoadDumpError::InvalidSqlInput( - "attach statements are not allowed in dumps".to_string(), - )); - } - - let mut parser = Box::new(Parser::new(dump_content.as_bytes())); - let mut skipped_wasm_table = false; - let mut n_stmt = 0; - - loop { - match parser.next() { - Ok(Some(cmd)) => { - n_stmt += 1; - - if !skipped_wasm_table { - if let Cmd::Stmt(Stmt::CreateTable { tbl_name, .. }) = &cmd { - if tbl_name.name.0 == "libsql_wasm_func_table" { - skipped_wasm_table = true; - tracing::debug!("Skipping WASM table creation"); - continue; - } - } - } - - if n_stmt > 2 && conn.is_autocommit().await.unwrap() { - return Err(LoadDumpError::NoTxn); - } - - let stmt_sql = cmd.to_string(); - tokio::task::spawn_blocking({ - let conn = conn.clone(); - move || -> crate::Result<(), LoadDumpError> { - conn.with_raw(|conn| { - conn.authorizer(Some(|auth: AuthContext<'_>| match auth.action { - AuthAction::Attach { filename: _ } => Authorization::Deny, - _ => Authorization::Allow, - })); - conn.execute(&stmt_sql, ()) - }) - .map_err(|e| match e { - rusqlite::Error::SqlInputError { - msg, sql, offset, .. - } => LoadDumpError::InvalidSqlInput(format!( - "msg: {}, sql: {}, offset: {}", - msg, sql, offset - )), - e => LoadDumpError::Internal(format!( - "statement: {}, error: {}", - n_stmt, e - )), - })?; - Ok(()) - } - }) - .await??; - } - Ok(None) => break, - Err(e) => { - let error_msg = match e { - sqlite3_parser::lexer::sql::Error::ParserError( - ParserError::SyntaxError { token_type, found }, - Some((line, col)), - ) => { - let near_token = found.as_deref().unwrap_or(&token_type); - format!( - "syntax error near '{}' at line {}, column {}", - near_token, line, col - ) - } - _ => format!("parse error: {}", e), - }; - - return Err(LoadDumpError::InvalidSqlInput(error_msg)); - } - } - } - - if !conn.is_autocommit().await.unwrap() { - tokio::task::spawn_blocking({ - let conn = conn.clone(); - move || -> crate::Result<(), LoadDumpError> { - conn.with_raw(|conn| conn.execute("rollback", ()))?; - Ok(()) - } - }) - .await??; - return Err(LoadDumpError::NoCommit); - } - - Ok(()) -} - fn check_fresh_db(path: &Path) -> crate::Result { let is_fresh = !path.join("wallog").try_exists()?; Ok(is_fresh) diff --git a/libsql-server/src/namespace/configurator/mod.rs b/libsql-server/src/namespace/configurator/mod.rs index 517b21ca5a..064f9a8f7c 100644 --- a/libsql-server/src/namespace/configurator/mod.rs +++ b/libsql-server/src/namespace/configurator/mod.rs @@ -13,6 +13,7 @@ use crate::replication::script_backup_manager::ScriptBackupManager; use crate::StatsSender; use super::broadcasters::BroadcasterHandle; +use super::dump_import::DumpImportConfig; use super::meta_store::MetaStoreHandle; use super::{ Namespace, NamespaceBottomlessDbIdInit, NamespaceName, NamespaceStore, ResetCb, @@ -41,6 +42,7 @@ pub struct BaseNamespaceConfig { pub(crate) encryption_config: Option, pub(crate) disable_intelligent_throttling: bool, pub(crate) connection_creation_timeout: Option, + pub(crate) dump_import: DumpImportConfig, } #[derive(Clone)] diff --git a/libsql-server/src/namespace/dump_import/buffered.rs b/libsql-server/src/namespace/dump_import/buffered.rs new file mode 100644 index 0000000000..850bdc867e --- /dev/null +++ b/libsql-server/src/namespace/dump_import/buffered.rs @@ -0,0 +1,105 @@ +//! The historical dump importer: reads the whole dump in memory, then parses and executes it. +//! +//! This is intentionally kept identical in behavior to the pre-streaming implementation so it +//! can serve as the reference/control implementation when benchmarking and validating the +//! streaming importer. + +use bytes::Bytes; +use fallible_iterator::FallibleIterator; +use futures::Stream; +use rusqlite::hooks::{AuthAction, AuthContext, Authorization}; +use sqlite3_parser::ast::{Cmd, Stmt}; +use sqlite3_parser::lexer::sql::Parser; +use tokio::io::AsyncReadExt; +use tokio_util::io::StreamReader; + +use crate::connection::Connection as _; +use crate::database::PrimaryConnection; +use crate::error::LoadDumpError; + +use super::{map_exec_error, map_parse_error, DumpImportStats}; + +pub(super) async fn load_dump_buffered( + dump: S, + conn: PrimaryConnection, +) -> crate::Result +where + S: Stream> + Unpin, +{ + let mut stats = DumpImportStats::default(); + let mut reader = tokio::io::BufReader::new(StreamReader::new(dump)); + let mut dump_content = String::new(); + reader + .read_to_string(&mut dump_content) + .await + .map_err(|e| LoadDumpError::Internal(format!("Failed to read dump content: {}", e)))?; + stats.bytes = dump_content.len() as u64; + + if dump_content.to_lowercase().contains("attach") { + return Err(LoadDumpError::InvalidSqlInput( + "attach statements are not allowed in dumps".to_string(), + )); + } + + let mut parser = Box::new(Parser::new(dump_content.as_bytes())); + let mut skipped_wasm_table = false; + let mut n_stmt = 0; + + loop { + match parser.next() { + Ok(Some(cmd)) => { + n_stmt += 1; + + if !skipped_wasm_table { + if let Cmd::Stmt(Stmt::CreateTable { tbl_name, .. }) = &cmd { + if tbl_name.name.0 == "libsql_wasm_func_table" { + skipped_wasm_table = true; + stats.statements_skipped += 1; + tracing::debug!("Skipping WASM table creation"); + continue; + } + } + } + + if n_stmt > 2 && conn.is_autocommit().await.unwrap() { + return Err(LoadDumpError::NoTxn); + } + + let stmt_sql = cmd.to_string(); + stats.max_statement_bytes = stats.max_statement_bytes.max(stmt_sql.len()); + tokio::task::spawn_blocking({ + let conn = conn.clone(); + move || -> crate::Result<(), LoadDumpError> { + conn.with_raw(|conn| { + conn.authorizer(Some(|auth: AuthContext<'_>| match auth.action { + AuthAction::Attach { filename: _ } => Authorization::Deny, + _ => Authorization::Allow, + })); + conn.execute(&stmt_sql, ()) + }) + .map_err(|e| map_exec_error(e, n_stmt))?; + Ok(()) + } + }) + .await??; + stats.statements_executed += 1; + } + Ok(None) => break, + Err(e) => return Err(map_parse_error(e, None)), + } + } + + if !conn.is_autocommit().await.unwrap() { + tokio::task::spawn_blocking({ + let conn = conn.clone(); + move || -> crate::Result<(), LoadDumpError> { + conn.with_raw(|conn| conn.execute("rollback", ()))?; + Ok(()) + } + }) + .await??; + return Err(LoadDumpError::NoCommit); + } + + Ok(stats) +} diff --git a/libsql-server/src/namespace/dump_import/framer.rs b/libsql-server/src/namespace/dump_import/framer.rs new file mode 100644 index 0000000000..05d76d1117 --- /dev/null +++ b/libsql-server/src/namespace/dump_import/framer.rs @@ -0,0 +1,13 @@ +//! Incremental SQL statement framing (see `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §7). + +/// Translate a position reported relative to a frame into a position in the whole dump. +/// +/// `frame` is the 1-based `(line, column)` of the frame's first byte; `rel` is the 1-based +/// position within the frame. +pub(super) fn absolute_position(frame: (u64, usize), rel: (u64, usize)) -> (u64, usize) { + if rel.0 <= 1 { + (frame.0, frame.1 + rel.1.saturating_sub(1)) + } else { + (frame.0 + rel.0 - 1, rel.1) + } +} diff --git a/libsql-server/src/namespace/dump_import/mod.rs b/libsql-server/src/namespace/dump_import/mod.rs new file mode 100644 index 0000000000..747fe87db9 --- /dev/null +++ b/libsql-server/src/namespace/dump_import/mod.rs @@ -0,0 +1,323 @@ +//! Loading a namespace from a SQL dump. +//! +//! Two importers are available and can be selected per request (`dump_importer` in the admin +//! create-namespace body) or server-wide (`--dump-importer`): +//! +//! - [`DumpImporterKind::Buffered`]: the historical importer. Reads the whole dump into memory, +//! parses it, and executes each statement. Memory usage is proportional to the dump size. +//! - [`DumpImporterKind::Streaming`]: frames complete statements incrementally with +//! `sqlite3_complete()` and executes them on a dedicated blocking thread while the dump is +//! still being read. Memory usage is bounded by [`DumpImportConfig`] plus the largest statement. +//! +//! See `docs/STREAMING_DUMP_IMPORT_DESIGN.md` for the full design. + +use std::fmt; +use std::str::FromStr; +use std::time::{Duration, Instant}; + +use sqlite3_parser::lexer::sql::ParserError; + +use crate::database::PrimaryConnection; +use crate::error::LoadDumpError; +use crate::namespace::{DumpStream, NamespaceName}; + +mod buffered; +mod framer; +mod streaming; + +/// Which implementation loads a dump into a fresh namespace. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DumpImporterKind { + /// Read the entire dump into memory before executing it. + Buffered, + /// Execute statements as they are framed from the incoming stream. + Streaming, +} + +impl DumpImporterKind { + pub fn as_str(&self) -> &'static str { + match self { + DumpImporterKind::Buffered => "buffered", + DumpImporterKind::Streaming => "streaming", + } + } +} + +impl fmt::Display for DumpImporterKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl FromStr for DumpImporterKind { + type Err = String; + + fn from_str(s: &str) -> Result { + match s.trim().to_ascii_lowercase().as_str() { + "buffered" => Ok(DumpImporterKind::Buffered), + "streaming" => Ok(DumpImporterKind::Streaming), + other => Err(format!( + "unknown dump importer `{other}`, expected `buffered` or `streaming`" + )), + } + } +} + +/// Server-wide dump import settings. +#[derive(Debug, Clone)] +pub struct DumpImportConfig { + /// Importer used when the create-namespace request doesn't specify one. + pub default_importer: DumpImporterKind, + /// Streaming importer: a single statement larger than this is rejected. + pub max_statement_bytes: usize, + /// Streaming importer: maximum bytes of framed statements waiting to be executed. + pub queue_bytes: usize, + /// Streaming importer: maximum number of framed statements waiting to be executed. + pub queue_depth: usize, +} + +impl DumpImportConfig { + pub const DEFAULT_MAX_STATEMENT_BYTES: usize = 64 * 1024 * 1024; + pub const DEFAULT_QUEUE_BYTES: usize = 16 * 1024 * 1024; + pub const DEFAULT_QUEUE_DEPTH: usize = 256; + + pub const MIN_MAX_STATEMENT_BYTES: usize = 4 * 1024; + pub const MIN_QUEUE_BYTES: usize = 64 * 1024; + pub const MAX_QUEUE_BYTES: usize = 1024 * 1024 * 1024; + + /// Check that the limits are sane and fit the implementation (the byte budget is a + /// `tokio::sync::Semaphore` acquired with `u32` permit counts). + pub fn validate(&self) -> anyhow::Result<()> { + anyhow::ensure!( + self.max_statement_bytes >= Self::MIN_MAX_STATEMENT_BYTES, + "dump import max statement size must be at least {} bytes", + Self::MIN_MAX_STATEMENT_BYTES + ); + anyhow::ensure!( + (Self::MIN_QUEUE_BYTES..=Self::MAX_QUEUE_BYTES).contains(&self.queue_bytes), + "dump import queue bytes must be between {} and {} bytes", + Self::MIN_QUEUE_BYTES, + Self::MAX_QUEUE_BYTES + ); + anyhow::ensure!( + self.queue_depth >= 1, + "dump import queue depth must be at least 1" + ); + Ok(()) + } +} + +impl Default for DumpImportConfig { + fn default() -> Self { + Self { + default_importer: DumpImporterKind::Buffered, + max_statement_bytes: Self::DEFAULT_MAX_STATEMENT_BYTES, + queue_bytes: Self::DEFAULT_QUEUE_BYTES, + queue_depth: Self::DEFAULT_QUEUE_DEPTH, + } + } +} + +/// What a completed import did. +#[derive(Debug, Default, Clone)] +pub struct DumpImportStats { + /// Statements handed to SQLite. + pub statements_executed: u64, + /// Frames that were not executed: whitespace/comment-only frames and the skipped WASM table. + pub statements_skipped: u64, + /// Dump bytes consumed. + pub bytes: u64, + /// Size of the largest statement executed. + pub max_statement_bytes: usize, + pub elapsed: Duration, +} + +/// Load `stream` into the fresh database behind `conn` using the requested importer. +/// +/// On error the dump transaction has been rolled back (or was never started). +pub(crate) async fn load_dump( + kind: DumpImporterKind, + stream: DumpStream, + conn: PrimaryConnection, + cfg: &DumpImportConfig, + namespace: &NamespaceName, +) -> Result { + let started = Instant::now(); + tracing::info!(namespace = %namespace, importer = %kind, "loading dump"); + + let res = match kind { + DumpImporterKind::Buffered => buffered::load_dump_buffered(stream, conn).await, + DumpImporterKind::Streaming => streaming::load_dump_streaming(stream, conn, cfg).await, + }; + + let elapsed = started.elapsed(); + metrics::histogram!( + "libsql_server_dump_import_duration_seconds", + elapsed.as_secs_f64(), + "importer" => kind.as_str() + ); + + match res { + Ok(mut stats) => { + stats.elapsed = elapsed; + metrics::counter!( + "libsql_server_dump_import_bytes", + stats.bytes, + "importer" => kind.as_str() + ); + metrics::counter!( + "libsql_server_dump_import_statements", + stats.statements_executed, + "importer" => kind.as_str() + ); + metrics::histogram!( + "libsql_server_dump_import_max_statement_bytes", + stats.max_statement_bytes as f64, + "importer" => kind.as_str() + ); + tracing::info!( + namespace = %namespace, + importer = %kind, + statements = stats.statements_executed, + skipped = stats.statements_skipped, + bytes = stats.bytes, + max_statement_bytes = stats.max_statement_bytes, + elapsed_ms = elapsed.as_millis() as u64, + "dump loaded" + ); + Ok(stats) + } + Err(e) => { + metrics::increment_counter!( + "libsql_server_dump_import_failures", + "importer" => kind.as_str(), + "kind" => failure_kind(&e) + ); + tracing::warn!( + namespace = %namespace, + importer = %kind, + error = %e, + elapsed_ms = elapsed.as_millis() as u64, + "dump load failed; transaction rolled back" + ); + Err(e) + } + } +} + +fn failure_kind(e: &LoadDumpError) -> &'static str { + match e { + LoadDumpError::NoTxn | LoadDumpError::NoCommit => "txn", + LoadDumpError::InvalidSqlInput(_) => "parse", + LoadDumpError::StatementTooLarge { .. } => "limit", + LoadDumpError::Internal(_) => "exec", + _ => "other", + } +} + +/// Convert a parser error into the user-facing `InvalidSqlInput` error. +/// +/// `frame_pos` is the 1-based `(line, column)` of the parsed text within the whole dump; when +/// `None`, positions reported by the parser are already absolute. +pub(super) fn map_parse_error( + mut e: sqlite3_parser::lexer::sql::Error, + frame_pos: Option<(u64, usize)>, +) -> LoadDumpError { + use sqlite3_parser::lexer::sql::Error as E; + use sqlite3_parser::lexer::ScanError as _; + + if let Some(frame_pos) = frame_pos { + let rel = match &e { + E::ParserError(_, pos) + | E::UnrecognizedToken(pos) + | E::UnterminatedLiteral(pos) + | E::UnterminatedBracket(pos) + | E::UnterminatedBlockComment(pos) + | E::BadVariableName(pos) + | E::BadNumber(pos) + | E::ExpectedEqualsSign(pos) + | E::MalformedBlobLiteral(pos) + | E::MalformedHexInteger(pos) => *pos, + E::Io(_) => None, + _ => None, + }; + if let Some(rel) = rel { + let (line, column) = framer::absolute_position(frame_pos, rel); + e.position(line, column); + } + } + + let msg = match e { + E::ParserError(ParserError::SyntaxError { token_type, found }, Some((line, col))) => { + let near_token = found.as_deref().unwrap_or(&token_type); + format!( + "syntax error near '{}' at line {}, column {}", + near_token, line, col + ) + } + other => format!("parse error: {}", other), + }; + + LoadDumpError::InvalidSqlInput(msg) +} + +/// Convert a SQLite execution error into the user-facing error, mirroring the historical format. +pub(super) fn map_exec_error(e: rusqlite::Error, n_stmt: u64) -> LoadDumpError { + match e { + rusqlite::Error::SqlInputError { + msg, sql, offset, .. + } => LoadDumpError::InvalidSqlInput(format!( + "msg: {}, sql: {}, offset: {}", + msg, sql, offset + )), + e => LoadDumpError::Internal(format!("statement: {}, error: {}", n_stmt, e)), + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn importer_kind_from_str() { + assert_eq!( + "buffered".parse::().unwrap(), + DumpImporterKind::Buffered + ); + assert_eq!( + " Streaming ".parse::().unwrap(), + DumpImporterKind::Streaming + ); + assert!("nope".parse::().is_err()); + } + + #[test] + fn importer_kind_serde() { + assert_eq!( + serde_json::from_str::("\"streaming\"").unwrap(), + DumpImporterKind::Streaming + ); + assert!(serde_json::from_str::("\"Streaming\"").is_err()); + } + + #[test] + fn config_validation() { + assert!(DumpImportConfig::default().validate().is_ok()); + let bad = DumpImportConfig { + queue_bytes: 1, + ..Default::default() + }; + assert!(bad.validate().is_err()); + let bad = DumpImportConfig { + queue_depth: 0, + ..Default::default() + }; + assert!(bad.validate().is_err()); + let bad = DumpImportConfig { + max_statement_bytes: 1, + ..Default::default() + }; + assert!(bad.validate().is_err()); + } +} diff --git a/libsql-server/src/namespace/dump_import/streaming.rs b/libsql-server/src/namespace/dump_import/streaming.rs new file mode 100644 index 0000000000..f99ab4ee20 --- /dev/null +++ b/libsql-server/src/namespace/dump_import/streaming.rs @@ -0,0 +1,17 @@ +//! Memory-bounded dump importer (see `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §8–9). + +use crate::database::PrimaryConnection; +use crate::error::LoadDumpError; +use crate::namespace::DumpStream; + +use super::{DumpImportConfig, DumpImportStats}; + +pub(super) async fn load_dump_streaming( + _stream: DumpStream, + _conn: PrimaryConnection, + _cfg: &DumpImportConfig, +) -> Result { + Err(LoadDumpError::Internal( + "streaming dump importer is not available".to_string(), + )) +} diff --git a/libsql-server/src/namespace/mod.rs b/libsql-server/src/namespace/mod.rs index ec45b50445..2cea87ed75 100644 --- a/libsql-server/src/namespace/mod.rs +++ b/libsql-server/src/namespace/mod.rs @@ -20,6 +20,7 @@ pub use self::store::NamespaceStore; pub mod broadcasters; pub(crate) mod configurator; +pub mod dump_import; pub mod meta_store; mod name; pub mod replication_wal; @@ -128,13 +129,35 @@ impl Namespace { pub type DumpStream = Box> + Send + Sync + 'static + Unpin>; +/// A SQL dump to load into a fresh namespace, and how to load it. +pub struct DumpSource { + pub stream: DumpStream, + /// Importer to use. `None` selects the server-wide default + /// (`BaseNamespaceConfig::dump_import.default_importer`). + pub importer: Option, +} + +impl DumpSource { + pub fn new(stream: DumpStream) -> Self { + Self { + stream, + importer: None, + } + } + + pub fn with_importer(mut self, importer: Option) -> Self { + self.importer = importer; + self + } +} + #[derive(Default)] pub enum RestoreOption { /// Restore database state from the most recent version found in a backup. #[default] Latest, /// Restore database from SQLite dump. - Dump(DumpStream), + Dump(DumpSource), /// Restore database state to a backup version equal to specific generation. Generation(Uuid), /// Restore database state to a backup version present at a specific point in time. diff --git a/libsql-server/src/schema/scheduler.rs b/libsql-server/src/schema/scheduler.rs index d9431b2d86..fc6f809ff0 100644 --- a/libsql-server/src/schema/scheduler.rs +++ b/libsql-server/src/schema/scheduler.rs @@ -940,6 +940,7 @@ mod test { encryption_config: None, connection_creation_timeout: None, disable_intelligent_throttling: false, + dump_import: Default::default(), }; let primary_config = PrimaryConfig { diff --git a/libsql-server/src/test/bottomless.rs b/libsql-server/src/test/bottomless.rs index f2f23583c2..69e638c948 100644 --- a/libsql-server/src/test/bottomless.rs +++ b/libsql-server/src/test/bottomless.rs @@ -98,6 +98,7 @@ async fn configure_server( max_concurrent_requests: 128, connection_creation_timeout: None, disable_intelligent_throttling: false, + dump_import: Default::default(), }, admin_api_config: None, disable_namespaces: true, From 6841279511a692fe6652a1947cfcd929fb675828 Mon Sep 17 00:00:00 2001 From: Tomasz Szymczyszyn Date: Wed, 7 Oct 2026 17:50:08 +0200 Subject: [PATCH 2/6] libsql-server: add incremental SQL statement framer for dump import 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. --- Cargo.lock | 1 + libsql-server/Cargo.toml | 1 + .../src/namespace/dump_import/framer.rs | 410 +++++++++++++++++- 3 files changed, 411 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 02f25a01f9..436299524e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3056,6 +3056,7 @@ dependencies = [ "libsql-sys", "libsql_replication", "md-5", + "memchr", "metrics", "metrics-exporter-prometheus", "metrics-util", diff --git a/libsql-server/Cargo.toml b/libsql-server/Cargo.toml index 06107d0557..e9721a32ba 100644 --- a/libsql-server/Cargo.toml +++ b/libsql-server/Cargo.toml @@ -38,6 +38,7 @@ itertools = "0.10.5" jsonwebtoken = "9" libsql = { path = "../libsql/", optional = true } libsql_replication = { path = "../libsql-replication" } +memchr = "2" metrics = "0.21.1" metrics-util = "0.15" metrics-exporter-prometheus = "0.12.2" diff --git a/libsql-server/src/namespace/dump_import/framer.rs b/libsql-server/src/namespace/dump_import/framer.rs index 05d76d1117..5eb752dbcd 100644 --- a/libsql-server/src/namespace/dump_import/framer.rs +++ b/libsql-server/src/namespace/dump_import/framer.rs @@ -1,4 +1,172 @@ -//! Incremental SQL statement framing (see `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §7). +//! Incremental SQL statement framing. +//! +//! [`StatementFramer`] turns an arbitrary sequence of byte chunks into complete SQL statements, +//! each ending at the `;` that terminates it, while only ever holding one unfinished statement +//! in memory. Statement boundaries are decided by SQLite's own `sqlite3_complete()`, so +//! semicolons inside string literals, quoted identifiers, comments and `CREATE TRIGGER ... END` +//! bodies are handled exactly like the `sqlite3` shell does. +//! +//! See `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §7. + +use std::ffi::c_char; + +use memchr::{memchr, memchr_iter, memrchr}; +use rusqlite::ffi::sqlite3_complete; + +/// One complete statement (plus any whitespace/comments that preceded it in the dump), with the +/// 1-based position of its first byte in the whole dump. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct Frame { + pub sql: Vec, + pub line: u64, + pub column: usize, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum FrameError { + /// Dumps are text; a NUL byte can't be part of valid SQL and would confuse the C framing + /// oracle, so it is rejected outright. + NulByte { line: u64, column: usize }, + /// The statement starting at `(line, column)` grew past the configured limit without + /// terminating. + StatementTooLarge { + line: u64, + column: usize, + limit: usize, + }, +} + +pub(super) struct StatementFramer { + /// Bytes after the last emitted frame; starts with the next statement's leading + /// whitespace/comments. + buf: Vec, + /// Offset in `buf` from which to look for the next `;`. Semicolons before it were already + /// tested and found not to terminate the statement. + scan_from: usize, + /// 1-based line of `buf[0]` in the whole dump. + line: u64, + /// 1-based byte column of `buf[0]` in the whole dump. + column: usize, + max_statement_bytes: usize, + bytes_seen: u64, +} + +impl StatementFramer { + pub fn new(max_statement_bytes: usize) -> Self { + Self { + buf: Vec::new(), + scan_from: 0, + line: 1, + column: 1, + max_statement_bytes, + bytes_seen: 0, + } + } + + /// Total number of bytes accepted by [`push`](Self::push). + pub fn bytes_seen(&self) -> u64 { + self.bytes_seen + } + + /// Append a chunk and return every statement completed by it, in input order. + /// + /// On error, the framer must not be used further. + pub fn push(&mut self, chunk: &[u8]) -> Result, FrameError> { + if let Some(k) = memchr(0, chunk) { + let (line, column) = advance_position((self.line, self.column), &self.buf); + let (line, column) = advance_position((line, column), &chunk[..k]); + return Err(FrameError::NulByte { line, column }); + } + + self.bytes_seen += chunk.len() as u64; + self.buf.extend_from_slice(chunk); + + let mut frames = Vec::new(); + let mut start = 0; + while let Some(rel) = memchr(b';', &self.buf[self.scan_from..]) { + let end = self.scan_from + rel; + if is_complete_statement(&mut self.buf, start, end) { + let sql = self.buf[start..=end].to_vec(); + frames.push(Frame { + sql, + line: self.line, + column: self.column, + }); + (self.line, self.column) = + advance_position((self.line, self.column), &self.buf[start..=end]); + start = end + 1; + } + self.scan_from = end + 1; + } + + if start > 0 { + self.buf.drain(..start); + self.scan_from -= start; + } + + if self.buf.len() > self.max_statement_bytes { + return Err(FrameError::StatementTooLarge { + line: self.line, + column: self.column, + limit: self.max_statement_bytes, + }); + } + + Ok(frames) + } + + /// Signal end of input. Returns whatever follows the last terminated statement: possibly + /// nothing, possibly whitespace/comments only, possibly a final statement without a trailing + /// `;`. The caller decides what that tail means. + pub fn finish(&mut self) -> Option { + if self.buf.is_empty() { + return None; + } + let frame = Frame { + sql: std::mem::take(&mut self.buf), + line: self.line, + column: self.column, + }; + self.scan_from = 0; + (self.line, self.column) = advance_position((self.line, self.column), &frame.sql); + Some(frame) + } +} + +/// Does `buf[start..=end]` (whose last byte is `;`) look like a complete SQL statement? +/// +/// `sqlite3_complete` needs a NUL-terminated string, so a terminator is temporarily placed right +/// after the `;`. The function only tokenizes; it does not parse or touch any database. +fn is_complete_statement(buf: &mut Vec, start: usize, end: usize) -> bool { + debug_assert_eq!(buf[end], b';'); + let pushed = if end + 1 == buf.len() { + buf.push(0); + true + } else { + false + }; + let saved = buf[end + 1]; + buf[end + 1] = 0; + // SAFETY: `buf[start..]` is NUL-terminated at `end + 1`, which is within bounds, and + // `sqlite3_complete` only reads the string. + let complete = unsafe { sqlite3_complete(buf[start..].as_ptr() as *const c_char) } != 0; + buf[end + 1] = saved; + if pushed { + buf.pop(); + } + complete +} + +/// Position of the byte following `bytes`, given the position of its first byte. +fn advance_position((line, column): (u64, usize), bytes: &[u8]) -> (u64, usize) { + match memrchr(b'\n', bytes) { + Some(last) => ( + line + memchr_iter(b'\n', bytes).count() as u64, + bytes.len() - last, + ), + None => (line, column + bytes.len()), + } +} /// Translate a position reported relative to a frame into a position in the whole dump. /// @@ -11,3 +179,243 @@ pub(super) fn absolute_position(frame: (u64, usize), rel: (u64, usize)) -> (u64, (frame.0 + rel.0 - 1, rel.1) } } + +#[cfg(test)] +mod test { + use super::*; + + const LIMIT: usize = 1 << 20; + + /// Feed `input` in chunks of `chunk_size` and collect frames plus the final tail. + fn frame_all(input: &[u8], chunk_size: usize) -> (Vec, Option) { + let mut framer = StatementFramer::new(LIMIT); + let mut frames = Vec::new(); + for chunk in input.chunks(chunk_size) { + frames.extend(framer.push(chunk).unwrap()); + } + assert_eq!(framer.bytes_seen(), input.len() as u64); + (frames, framer.finish()) + } + + fn sqls(frames: &[Frame]) -> Vec { + frames + .iter() + .map(|f| String::from_utf8(f.sql.clone()).unwrap()) + .collect() + } + + /// Framing must not depend on chunk boundaries, and frames + tail must reproduce the input. + fn assert_chunk_invariant(input: &str, expected: &[&str], expected_tail: Option<&str>) { + let reference = frame_all(input.as_bytes(), input.len().max(1)); + assert_eq!(sqls(&reference.0), expected, "input: {input:?}"); + assert_eq!( + reference + .1 + .as_ref() + .map(|f| String::from_utf8(f.sql.clone()).unwrap()), + expected_tail.map(str::to_owned), + "tail for input: {input:?}" + ); + for chunk_size in 1..=input.len().max(1) { + let got = frame_all(input.as_bytes(), chunk_size); + assert_eq!( + got, reference, + "chunk size {chunk_size} for input {input:?}" + ); + let mut rebuilt: Vec = got.0.iter().flat_map(|f| f.sql.clone()).collect(); + if let Some(tail) = &got.1 { + rebuilt.extend_from_slice(&tail.sql); + } + assert_eq!(rebuilt, input.as_bytes()); + } + } + + #[test] + fn simple_statements() { + assert_chunk_invariant( + "PRAGMA foreign_keys=OFF;\nBEGIN TRANSACTION;\nCREATE TABLE t(x);\nCOMMIT;\n", + &[ + "PRAGMA foreign_keys=OFF;", + "\nBEGIN TRANSACTION;", + "\nCREATE TABLE t(x);", + "\nCOMMIT;", + ], + Some("\n"), + ); + } + + #[test] + fn semicolons_inside_tokens_do_not_terminate() { + assert_chunk_invariant( + "INSERT INTO t VALUES('a;b');INSERT INTO t VALUES(\"q;i\");SELECT [b;r];", + &[ + "INSERT INTO t VALUES('a;b');", + "INSERT INTO t VALUES(\"q;i\");", + "SELECT [b;r];", + ], + None, + ); + assert_chunk_invariant( + "-- comment; with semicolon\nSELECT 1;/* block ; comment */SELECT 2;", + &[ + "-- comment; with semicolon\nSELECT 1;", + "/* block ; comment */SELECT 2;", + ], + None, + ); + assert_chunk_invariant( + "CREATE TABLE t(x CHECK(x <> ';'));", + &["CREATE TABLE t(x CHECK(x <> ';'));"], + None, + ); + } + + #[test] + fn trigger_body_is_one_frame() { + let trigger = "CREATE TRIGGER tr AFTER INSERT ON t BEGIN\n INSERT INTO u VALUES(1);\n UPDATE u SET x = 2;\nEND;"; + assert_chunk_invariant( + &format!("{trigger}\nINSERT INTO t VALUES(1);"), + &[trigger, "\nINSERT INTO t VALUES(1);"], + None, + ); + let temp = "CREATE TEMP TRIGGER tr AFTER INSERT ON t BEGIN SELECT 1; END;"; + assert_chunk_invariant(temp, &[temp], None); + } + + #[test] + fn multibyte_utf8_across_chunks() { + let input = "INSERT INTO t VALUES('żółć 🎉; nie koniec');INSERT INTO t VALUES('日本語');"; + assert_chunk_invariant( + input, + &[ + "INSERT INTO t VALUES('żółć 🎉; nie koniec');", + "INSERT INTO t VALUES('日本語');", + ], + None, + ); + for f in frame_all(input.as_bytes(), 1).0 { + assert!(std::str::from_utf8(&f.sql).is_ok()); + } + } + + #[test] + fn empty_statements_are_frames() { + assert_chunk_invariant( + "SELECT 1;;\n;SELECT 2;", + &["SELECT 1;", ";", "\n;", "SELECT 2;"], + None, + ); + } + + #[test] + fn finish_returns_tail() { + assert_chunk_invariant("SELECT 1;", &["SELECT 1;"], None); + assert_chunk_invariant("SELECT 1;\nCOMMIT", &["SELECT 1;"], Some("\nCOMMIT")); + assert_chunk_invariant( + "SELECT 1;\n-- trailing comment\n", + &["SELECT 1;"], + Some("\n-- trailing comment\n"), + ); + assert_chunk_invariant("", &[], None); + let mut framer = StatementFramer::new(LIMIT); + assert!(framer.push(b"SELECT 1;").unwrap().len() == 1); + assert_eq!(framer.finish(), None); + assert_eq!(framer.finish(), None); + } + + #[test] + fn nul_byte_is_rejected_with_position() { + let mut framer = StatementFramer::new(LIMIT); + framer.push(b"SELECT 1;\nSELECT ").unwrap(); + assert_eq!( + framer.push(b"2\0;"), + Err(FrameError::NulByte { line: 2, column: 9 }) + ); + } + + #[test] + fn statement_too_large() { + let mut framer = StatementFramer::new(16); + // exactly at the limit is fine as long as it terminates + assert_eq!(framer.push(b"SELECT 1234567;").unwrap().len(), 1); + // a pending statement longer than the limit is rejected at the start position + framer.push(b"\n").unwrap(); + assert_eq!( + framer.push(b"SELECT 'this is too long"), + Err(FrameError::StatementTooLarge { + line: 1, + column: 16, + limit: 16 + }) + ); + // a terminated statement longer than the limit arriving in one chunk is accepted: + // it never needs to be buffered unterminated + let mut framer = StatementFramer::new(16); + assert_eq!( + framer + .push(b"SELECT 'this is longer than sixteen bytes';") + .unwrap() + .len(), + 1 + ); + } + + #[test] + fn position_tracking() { + let input = + "PRAGMA x;\n\nBEGIN;\n INSERT INTO t\n VALUES(1); INSERT INTO t VALUES(2);\nCOMMIT;"; + let (frames, tail) = frame_all(input.as_bytes(), 3); + let positions: Vec<_> = frames.iter().map(|f| (f.line, f.column)).collect(); + assert_eq!( + positions, + vec![(1, 1), (1, 10), (3, 7), (5, 13), (5, 38)], + "{:?}", + sqls(&frames) + ); + assert_eq!(tail, None); + } + + #[test] + fn absolute_position_mapping() { + // Worked example from the design doc: the frame starts right after the `;` on line 5, + // column 32; the parser reports (3, 11) relative to the frame. + assert_eq!(absolute_position((5, 33), (3, 11)), (7, 11)); + // Error on the frame's first line: columns add up. + assert_eq!(absolute_position((5, 33), (1, 4)), (5, 36)); + assert_eq!(absolute_position((1, 1), (1, 1)), (1, 1)); + } + + #[test] + fn completeness_oracle() { + fn complete(s: &str) -> bool { + let mut buf = s.as_bytes().to_vec(); + let end = buf.len() - 1; + is_complete_statement(&mut buf, 0, end) + } + assert!(complete("SELECT 1;")); + assert!(complete(";")); + assert!(complete(" \n ;")); + assert!(complete("PRAGMA foreign_keys=OFF;")); + assert!(complete("EXPLAIN SELECT 1;")); + assert!(complete("SELECT 'a;b';")); + assert!(complete("/* a; */ SELECT 1;")); + assert!(complete("-- c;\nSELECT 1;")); + assert!(complete( + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; END;" + )); + assert!(!complete("SELECT ';")); + assert!(!complete("-- c;")); + assert!(!complete( + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;" + )); + assert!(!complete( + "CREATE TEMP TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;" + )); + + // the oracle only looks at `buf[start..=end]` + let mut buf = b"SELECT 1;SELECT ';".to_vec(); + assert!(is_complete_statement(&mut buf, 0, 8)); + assert!(!is_complete_statement(&mut buf, 9, 17)); + assert_eq!(buf, b"SELECT 1;SELECT ';"); + } +} From 0adb160402c481575ffc4af30d5a523a74100fd7 Mon Sep 17 00:00:00 2001 From: Tomasz Szymczyszyn Date: Wed, 7 Oct 2026 18:08:52 +0200 Subject: [PATCH 3/6] libsql-server: add memory-bounded streaming dump importer 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. --- .../src/namespace/dump_import/framer.rs | 63 +- .../src/namespace/dump_import/mod.rs | 2 +- .../src/namespace/dump_import/streaming.rs | 299 ++++- libsql-server/tests/common/http.rs | 13 + libsql-server/tests/namespaces/dumps.rs | 1010 +++++++++++++---- libsql-server/tests/namespaces/mod.rs | 8 +- ...umps__dump_importer_requires_dump_url.snap | 7 + ...importers_produce_identical_databases.snap | 23 + 8 files changed, 1187 insertions(+), 238 deletions(-) create mode 100644 libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__dump_importer_requires_dump_url.snap create mode 100644 libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__importers_produce_identical_databases.snap diff --git a/libsql-server/src/namespace/dump_import/framer.rs b/libsql-server/src/namespace/dump_import/framer.rs index 5eb752dbcd..3251557103 100644 --- a/libsql-server/src/namespace/dump_import/framer.rs +++ b/libsql-server/src/namespace/dump_import/framer.rs @@ -27,8 +27,8 @@ pub(super) enum FrameError { /// Dumps are text; a NUL byte can't be part of valid SQL and would confuse the C framing /// oracle, so it is rejected outright. NulByte { line: u64, column: usize }, - /// The statement starting at `(line, column)` grew past the configured limit without - /// terminating. + /// The statement starting at `(line, column)` is larger than the configured limit (or grew + /// past it without terminating). StatementTooLarge { line: u64, column: usize, @@ -86,6 +86,9 @@ impl StatementFramer { while let Some(rel) = memchr(b';', &self.buf[self.scan_from..]) { let end = self.scan_from + rel; if is_complete_statement(&mut self.buf, start, end) { + if end + 1 - start > self.max_statement_bytes { + return Err(self.too_large(&self.buf[start..=end])); + } let sql = self.buf[start..=end].to_vec(); frames.push(Frame { sql, @@ -105,16 +108,24 @@ impl StatementFramer { } if self.buf.len() > self.max_statement_bytes { - return Err(FrameError::StatementTooLarge { - line: self.line, - column: self.column, - limit: self.max_statement_bytes, - }); + return Err(self.too_large(&self.buf)); } Ok(frames) } + /// `pending` starts at the framer's current position; report the statement's first + /// non-whitespace byte so the message points at the statement rather than at the end of + /// the previous line. + fn too_large(&self, pending: &[u8]) -> FrameError { + let (line, column) = statement_start((self.line, self.column), pending); + FrameError::StatementTooLarge { + line, + column, + limit: self.max_statement_bytes, + } + } + /// Signal end of input. Returns whatever follows the last terminated statement: possibly /// nothing, possibly whitespace/comments only, possibly a final statement without a trailing /// `;`. The caller decides what that tail means. @@ -158,7 +169,7 @@ fn is_complete_statement(buf: &mut Vec, start: usize, end: usize) -> bool { } /// Position of the byte following `bytes`, given the position of its first byte. -fn advance_position((line, column): (u64, usize), bytes: &[u8]) -> (u64, usize) { +pub(super) fn advance_position((line, column): (u64, usize), bytes: &[u8]) -> (u64, usize) { match memrchr(b'\n', bytes) { Some(last) => ( line + memchr_iter(b'\n', bytes).count() as u64, @@ -168,6 +179,12 @@ fn advance_position((line, column): (u64, usize), bytes: &[u8]) -> (u64, usize) } } +/// Position of the first non-whitespace byte of `frame`, whose first byte is at `pos`. +pub(super) fn statement_start(pos: (u64, usize), frame: &[u8]) -> (u64, usize) { + let ws = frame.iter().take_while(|b| b.is_ascii_whitespace()).count(); + advance_position(pos, &frame[..ws]) +} + /// Translate a position reported relative to a frame into a position in the whole dump. /// /// `frame` is the 1-based `(line, column)` of the frame's first byte; `rel` is the 1-based @@ -338,28 +355,38 @@ mod test { let mut framer = StatementFramer::new(16); // exactly at the limit is fine as long as it terminates assert_eq!(framer.push(b"SELECT 1234567;").unwrap().len(), 1); - // a pending statement longer than the limit is rejected at the start position + // a pending statement longer than the limit is rejected; the position points at the + // statement itself, not at the whitespace that precedes it framer.push(b"\n").unwrap(); assert_eq!( framer.push(b"SELECT 'this is too long"), Err(FrameError::StatementTooLarge { - line: 1, - column: 16, + line: 2, + column: 1, limit: 16 }) ); - // a terminated statement longer than the limit arriving in one chunk is accepted: - // it never needs to be buffered unterminated + // a terminated statement longer than the limit is rejected too, even when it arrives + // in one chunk let mut framer = StatementFramer::new(16); + assert_eq!(framer.push(b"SELECT 1;\n").unwrap().len(), 1); assert_eq!( - framer - .push(b"SELECT 'this is longer than sixteen bytes';") - .unwrap() - .len(), - 1 + framer.push(b" SELECT 'this is longer than sixteen bytes';"), + Err(FrameError::StatementTooLarge { + line: 2, + column: 3, + limit: 16 + }) ); } + #[test] + fn statement_start_skips_leading_whitespace() { + assert_eq!(statement_start((5, 33), b"\n SELECT 1;"), (6, 5)); + assert_eq!(statement_start((5, 33), b"SELECT 1;"), (5, 33)); + assert_eq!(statement_start((5, 33), b" \n"), (6, 1)); + } + #[test] fn position_tracking() { let input = diff --git a/libsql-server/src/namespace/dump_import/mod.rs b/libsql-server/src/namespace/dump_import/mod.rs index 747fe87db9..3ba9b7674d 100644 --- a/libsql-server/src/namespace/dump_import/mod.rs +++ b/libsql-server/src/namespace/dump_import/mod.rs @@ -250,7 +250,7 @@ pub(super) fn map_parse_error( let msg = match e { E::ParserError(ParserError::SyntaxError { token_type, found }, Some((line, col))) => { - let near_token = found.as_deref().unwrap_or(&token_type); + let near_token = found.as_deref().unwrap_or(token_type); format!( "syntax error near '{}' at line {}, column {}", near_token, line, col diff --git a/libsql-server/src/namespace/dump_import/streaming.rs b/libsql-server/src/namespace/dump_import/streaming.rs index f99ab4ee20..325cf99326 100644 --- a/libsql-server/src/namespace/dump_import/streaming.rs +++ b/libsql-server/src/namespace/dump_import/streaming.rs @@ -1,17 +1,300 @@ -//! Memory-bounded dump importer (see `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §8–9). +//! Memory-bounded dump importer. +//! +//! ```text +//! async (tokio runtime) blocking thread (BLOCKING_RT) +//! DumpStream ──chunks──▶ StatementFramer ──frames──▶ mpsc + byte budget ──▶ run_executor ──▶ PrimaryConnection +//! ``` +//! +//! The reader task drives the stream, frames statements and hands them to a single executor +//! thread that owns the connection for the whole import. Memory is bounded by the configured +//! queue budget plus the largest single statement; the dump itself is never held in memory. +//! +//! See `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §8–9. +use std::sync::Arc; + +use fallible_iterator::FallibleIterator; +use futures::StreamExt; +use rusqlite::hooks::{AuthAction, AuthContext, Authorization}; +use sqlite3_parser::ast::{Cmd, Stmt}; +use sqlite3_parser::lexer::sql::Parser; +use tokio::sync::{mpsc, OwnedSemaphorePermit, Semaphore}; + +use crate::connection::Connection as _; use crate::database::PrimaryConnection; use crate::error::LoadDumpError; use crate::namespace::DumpStream; +use crate::BLOCKING_RT; + +use super::framer::{advance_position, Frame, FrameError, StatementFramer}; +use super::{map_exec_error, map_parse_error, DumpImportConfig, DumpImportStats}; + +enum Msg { + Stmt { + sql: Vec, + line: u64, + column: usize, + /// Released when the executor drops the message, freeing queue budget. + _budget: OwnedSemaphorePermit, + }, + /// The reader reached EOF cleanly (after sending the final unterminated tail, if any). + End, +} -use super::{DumpImportConfig, DumpImportStats}; +enum FeedError { + /// The stream or the framer failed; carries the user-facing error. + Source(LoadDumpError), + /// The executor went away (it failed first); its error is the one to report. + ExecutorGone, +} + +impl From for LoadDumpError { + fn from(e: FrameError) -> Self { + match e { + FrameError::NulByte { line, column } => LoadDumpError::InvalidSqlInput(format!( + "dump contains a NUL byte at line {line}, column {column}" + )), + FrameError::StatementTooLarge { line, limit, .. } => { + LoadDumpError::StatementTooLarge { line, limit } + } + } + } +} pub(super) async fn load_dump_streaming( - _stream: DumpStream, - _conn: PrimaryConnection, - _cfg: &DumpImportConfig, + mut stream: DumpStream, + conn: PrimaryConnection, + cfg: &DumpImportConfig, +) -> Result { + let (tx, rx) = mpsc::channel::(cfg.queue_depth); + let budget = Arc::new(Semaphore::new(cfg.queue_bytes)); + let queue_bytes = cfg.queue_bytes; + let executor = BLOCKING_RT.spawn_blocking(move || run_executor(conn, rx)); + let mut framer = StatementFramer::new(cfg.max_statement_bytes); + + let feed: Result<(), FeedError> = async { + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|e| { + FeedError::Source(LoadDumpError::Internal(format!( + "Failed to read dump content: {e}" + ))) + })?; + for frame in framer + .push(&chunk) + .map_err(|e| FeedError::Source(e.into()))? + { + send_frame(&tx, &budget, queue_bytes, frame).await?; + } + } + if let Some(tail) = framer.finish() { + send_frame(&tx, &budget, queue_bytes, tail).await?; + } + tx.send(Msg::End).await.map_err(|_| FeedError::ExecutorGone) + } + .await; + + // Guarantees the executor observes channel closure if we bailed out before sending `End`. + drop(tx); + + let exec = executor + .await + .map_err(|e| LoadDumpError::Internal(format!("dump executor task failed: {e}")))?; + + match (feed, exec) { + (Ok(()), exec) => exec.map(|mut stats| { + stats.bytes = framer.bytes_seen(); + stats + }), + // The executor failed first and closed the channel; report its error. + (Err(FeedError::ExecutorGone), Err(e)) => Err(e), + // Can't happen: the executor only returns Ok after receiving `End`. + (Err(FeedError::ExecutorGone), Ok(_)) => Err(LoadDumpError::Internal( + "dump executor finished before the dump was fully read".to_string(), + )), + // A stream or framing error wins over the executor's generic "ended before completion". + (Err(FeedError::Source(e)), _) => Err(e), + } +} + +async fn send_frame( + tx: &mpsc::Sender, + budget: &Arc, + queue_bytes: usize, + frame: Frame, +) -> Result<(), FeedError> { + // Oversized statements take the whole budget and therefore travel alone. + let permits = frame.sql.len().clamp(1, queue_bytes) as u32; + let permit = budget + .clone() + .acquire_many_owned(permits) + .await + .expect("dump import budget semaphore is never closed"); + tx.send(Msg::Stmt { + sql: frame.sql, + line: frame.line, + column: frame.column, + _budget: permit, + }) + .await + .map_err(|_| FeedError::ExecutorGone) +} + +struct Executor { + conn: PrimaryConnection, + n_stmt: u64, + skipped_wasm_table: bool, + stats: DumpImportStats, +} + +/// Runs on a blocking thread for the whole import. Never cancelled by tokio: it always reaches +/// its own COMMIT-or-ROLLBACK decision, even if the reader (and the admin request) go away. +fn run_executor( + conn: PrimaryConnection, + mut rx: mpsc::Receiver, ) -> Result { - Err(LoadDumpError::Internal( - "streaming dump importer is not available".to_string(), - )) + let mut ex = Executor { + conn, + n_stmt: 0, + skipped_wasm_table: false, + stats: DumpImportStats::default(), + }; + + ex.conn.with_raw(|c| { + c.authorizer(Some(|auth: AuthContext<'_>| match auth.action { + AuthAction::Attach { .. } | AuthAction::Detach { .. } => Authorization::Deny, + _ => Authorization::Allow, + })) + }); + + let res = ex.run(&mut rx); + + if res.is_err() { + ex.rollback_best_effort(); + } + ex.conn + .with_raw(|c| c.authorizer(None::) -> Authorization>)); + res +} + +impl Executor { + fn run(&mut self, rx: &mut mpsc::Receiver) -> Result { + loop { + match rx.blocking_recv() { + Some(Msg::Stmt { + sql, line, column, .. + }) => self.handle_statement(&sql, line, column)?, + Some(Msg::End) => break, + None => { + // Reader dropped the sender without `End`: stream error, framing error or + // request cancellation. The reader reports the specific cause if it's alive. + return Err(LoadDumpError::Internal( + "dump stream ended before completion".to_string(), + )); + } + } + } + + if !self.is_autocommit() { + self.conn.with_raw(|c| c.execute_batch("ROLLBACK"))?; + return Err(LoadDumpError::NoCommit); + } + + Ok(std::mem::take(&mut self.stats)) + } + + fn is_autocommit(&self) -> bool { + self.conn.with_raw(|c| c.is_autocommit()) + } + + fn rollback_best_effort(&self) { + self.conn.with_raw(|c| { + if !c.is_autocommit() { + if let Err(e) = c.execute_batch("ROLLBACK") { + tracing::warn!("failed to roll back dump transaction: {e}"); + } + } + }); + } + + fn handle_statement( + &mut self, + sql: &[u8], + line: u64, + column: usize, + ) -> Result<(), LoadDumpError> { + // 1. UTF-8: the parser converts lossily, so validate first. Frames end at an ASCII `;`, + // so a frame is valid UTF-8 iff the dump is valid UTF-8 over that range. + let sql = std::str::from_utf8(sql).map_err(|e| { + let (line, column) = advance_position((line, column), &sql[..e.valid_up_to()]); + LoadDumpError::InvalidSqlInput(format!( + "dump is not valid UTF-8 at line {line}, column {column}: {e}" + )) + })?; + + // 2. Parse (for policy checks and error reporting; the original text is what runs). + let mut parser = Parser::new(sql.as_bytes()); + let cmd = match parser.next() { + Ok(Some(cmd)) => cmd, + Ok(None) => { + // whitespace / comments / a bare `;` + self.stats.statements_skipped += 1; + return Ok(()); + } + Err(e) => return Err(map_parse_error(e, Some((line, column)))), + }; + if let Ok(Some(_)) = parser.next() { + return Err(LoadDumpError::Internal(format!( + "framing produced more than one statement at line {line}, column {column}" + ))); + } + + // 3. + self.n_stmt += 1; + + // 4. Same special case as the buffered importer. + if !self.skipped_wasm_table { + if let Cmd::Stmt(Stmt::CreateTable { tbl_name, .. }) = &cmd { + if tbl_name.name.0 == "libsql_wasm_func_table" { + self.skipped_wasm_table = true; + self.stats.statements_skipped += 1; + tracing::debug!("Skipping WASM table creation"); + return Ok(()); + } + } + } + + // 5. ATTACH policy (the authorizer is the backstop). + if matches!( + cmd, + Cmd::Stmt(Stmt::Attach { .. }) | Cmd::Stmt(Stmt::Detach(_)) + ) { + return Err(LoadDumpError::InvalidSqlInput( + "attach statements are not allowed in dumps".to_string(), + )); + } + + // 6. Everything after the first two statements must run inside the dump's transaction. + if self.n_stmt > 2 && self.is_autocommit() { + return Err(LoadDumpError::NoTxn); + } + + // 7. Execute the original text. + let n_stmt = self.n_stmt; + self.conn + .with_raw(|c| execute_single(c, sql)) + .map_err(|e| map_exec_error(e, n_stmt))?; + + self.stats.statements_executed += 1; + self.stats.max_statement_bytes = self.stats.max_statement_bytes.max(sql.len()); + Ok(()) + } +} + +/// Run one statement to completion, discarding any rows it returns (EXPLAIN, PRAGMA, a stray +/// SELECT), like the `sqlite3` shell does. Framing guarantees `sql` holds a single statement. +fn execute_single(conn: &mut rusqlite::Connection, sql: &str) -> rusqlite::Result<()> { + let mut stmt = conn.prepare(sql)?; + let mut rows = stmt.raw_query(); + while rows.next()?.is_some() {} + Ok(()) } diff --git a/libsql-server/tests/common/http.rs b/libsql-server/tests/common/http.rs index 8716a60503..a86bf87c11 100644 --- a/libsql-server/tests/common/http.rs +++ b/libsql-server/tests/common/http.rs @@ -45,6 +45,19 @@ impl Client { self.post_with_headers(url, &[], body).await } + /// Like [`post`](Self::post), but returns 5xx responses instead of failing. + pub(crate) async fn post_raw( + &self, + url: &str, + body: T, + ) -> anyhow::Result { + let bytes: Bytes = serde_json::to_vec(&body)?.into(); + let request = hyper::Request::post(url) + .header("Content-Type", "application/json") + .body(Body::from(bytes))?; + Ok(Response(self.0.request(request).await?)) + } + pub(crate) async fn post_with_headers( &self, url: &str, diff --git a/libsql-server/tests/namespaces/dumps.rs b/libsql-server/tests/namespaces/dumps.rs index 859130f773..8a759cef39 100644 --- a/libsql-server/tests/namespaces/dumps.rs +++ b/libsql-server/tests/namespaces/dumps.rs @@ -1,67 +1,144 @@ use std::convert::Infallible; +use std::path::Path; use std::time::Duration; -use hyper::{service::make_service_fn, Body, Response, StatusCode}; +use bytes::Bytes; +use futures::StreamExt; +use hyper::{service::make_service_fn, Body, Response as HyperResponse, StatusCode}; use insta::{assert_json_snapshot, assert_snapshot}; -use libsql::{Database, Value}; +use libsql::Database; +use libsql_server::config::{DbConfig, DumpImportConfig, DumpImporterKind}; use serde_json::json; use tempfile::tempdir; use tower::service_fn; -use turmoil::Builder; +use turmoil::{Builder, Sim}; -use crate::common::http::Client; +use crate::common::http::{Client, Response}; use crate::common::net::{TurmoilAcceptor, TurmoilConnector}; -use crate::namespaces::make_primary; +use crate::namespaces::{make_primary, make_primary_with_db_config}; + +const BUFFERED: Option<&str> = Some("buffered"); +const STREAMING: Option<&str> = Some("streaming"); + +/// `POST /v1/namespaces/:ns/create` with `dump_url` and, optionally, `dump_importer`. +async fn create_from_dump( + client: &Client, + ns: &str, + dump_url: &str, + importer: Option<&str>, +) -> anyhow::Result { + let mut body = json!({ "dump_url": dump_url }); + if let Some(importer) = importer { + body["dump_importer"] = json!(importer); + } + client + .post_raw( + &format!("http://primary:9090/v1/namespaces/{ns}/create"), + body, + ) + .await +} -#[test] -fn load_namespace_from_dump_from_url() { - const DUMP: &str = r#" +fn file_url(path: &Path) -> String { + format!("file:{}", path.display()) +} + +fn sim() -> Sim<'static> { + Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build() +} + +async fn count_rows(ns: &str, table: &str) -> anyhow::Result { + let db = Database::open_remote_with_connector( + &format!("http://{ns}.primary:8080"), + "", + TurmoilConnector, + )?; + let conn = db.connect()?; + let mut rows = conn + .query(&format!("select count(*) from {table}"), ()) + .await?; + Ok(rows.next().await?.unwrap().get::(0)?) +} + +/// Serve `chunks` as one HTTP response body on `dump-store:8080`, each chunk as its own body +/// frame. A trailing `Err` chunk aborts the body mid-way. +fn make_dump_store(sim: &mut Sim, chunks: Vec>) { + // turmoil may call the host closure more than once; the chunks are cloned per call. + let chunks: Vec> = chunks + .into_iter() + .map(|c| c.map_err(|e| (e.kind(), e.to_string()))) + .collect(); + sim.host("dump-store", move || { + let chunks = chunks.clone(); + async move { + let incoming = TurmoilAcceptor::bind(([0, 0, 0, 0], 8080)).await?; + let server = + hyper::server::Server::builder(incoming).serve(make_service_fn(move |_conn| { + let chunks = chunks.clone(); + async move { + Ok::<_, Infallible>(service_fn(move |_req| { + let chunks = chunks.clone(); + async move { + // Yield between chunks so hyper flushes each one before the + // next (or an error) is produced. + let stream = futures::stream::iter(chunks).then(|c| async move { + tokio::time::sleep(Duration::from_millis(1)).await; + c.map_err(|(kind, msg)| std::io::Error::new(kind, msg)) + }); + Ok::<_, Infallible>(HyperResponse::new(Body::wrap_stream(stream))) + } + })) + } + })); + + server.await.unwrap(); + + Ok(()) + } + }); +} + +fn make_dump_store_whole(sim: &mut Sim, dump: &'static str) { + make_dump_store(sim, vec![Ok(Bytes::from_static(dump.as_bytes()))]); +} + +fn make_dump_store_chunked(sim: &mut Sim, dump: &'static str, chunk_size: usize) { + make_dump_store( + sim, + dump.as_bytes() + .chunks(chunk_size) + .map(|c| Ok(Bytes::copy_from_slice(c))) + .collect(), + ); +} + +const SIMPLE_DUMP: &str = r#" PRAGMA foreign_keys=OFF; BEGIN TRANSACTION; CREATE TABLE test (x); INSERT INTO test VALUES(42); COMMIT;"#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); +fn load_namespace_from_dump_from_url_with(importer: Option<&'static str>) { + let mut sim = sim(); let tmp = tempdir().unwrap(); make_primary(&mut sim, tmp.path().to_path_buf()); + make_dump_store_whole(&mut sim, SIMPLE_DUMP); - sim.host("dump-store", || async { - let incoming = TurmoilAcceptor::bind(([0, 0, 0, 0], 8080)).await?; - let server = - hyper::server::Server::builder(incoming).serve(make_service_fn(|_conn| async { - Ok::<_, Infallible>(service_fn(|_req| async { - Ok::<_, Infallible>(Response::new(Body::from(DUMP))) - })) - })); - - server.await.unwrap(); - - Ok(()) - }); - - sim.client("client", async { + sim.client("client", async move { let client = Client::new(); - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "http://dump-store:8080/"}), - ) + let resp = create_from_dump(&client, "foo", "http://dump-store:8080/", importer) .await .unwrap(); assert_eq!(resp.status(), 200); - assert_snapshot!(resp.body_string().await.unwrap()); + assert_snapshot!( + "load_namespace_from_dump_from_url", + resp.body_string().await.unwrap() + ); - let foo = - Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; - let foo_conn = foo.connect()?; - let mut rows = foo_conn.query("select count(*) from test", ()).await?; - assert!(matches!( - rows.next().await.unwrap().unwrap().get_value(0)?, - Value::Integer(1) - )); + assert_eq!(count_rows("foo", "test").await?, 1); Ok(()) }); @@ -70,21 +147,21 @@ fn load_namespace_from_dump_from_url() { } #[test] -fn load_namespace_from_dump_from_file() { - const DUMP: &str = r#" - PRAGMA foreign_keys=OFF; - BEGIN TRANSACTION; - CREATE TABLE test (x); - INSERT INTO test VALUES(42); - COMMIT;"#; +fn load_namespace_from_dump_from_url() { + load_namespace_from_dump_from_url_with(None); +} - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); +#[test] +fn load_namespace_from_dump_from_url_streaming() { + load_namespace_from_dump_from_url_with(STREAMING); +} + +fn load_namespace_from_dump_from_file_with(importer: Option<&'static str>) { + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); - std::fs::write(tmp_path.join("dump.sql"), DUMP).unwrap(); + std::fs::write(tmp_path.join("dump.sql"), SIMPLE_DUMP).unwrap(); make_primary(&mut sim, tmp.path().to_path_buf()); @@ -92,32 +169,25 @@ fn load_namespace_from_dump_from_file() { let client = Client::new(); // path is not absolute is an error - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "file:dump.sql"}), - ) + let resp = create_from_dump(&client, "foo", "file:dump.sql", importer) .await .unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); // path doesn't exist is an error - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "file:/dump.sql"}), - ) + let resp = create_from_dump(&client, "foo", "file:/dump.sql", importer) .await .unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await - .unwrap(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await + .unwrap(); assert_eq!( resp.status(), StatusCode::OK, @@ -125,14 +195,7 @@ fn load_namespace_from_dump_from_file() { resp.json::().await.unwrap_or_default() ); - let foo = - Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; - let foo_conn = foo.connect()?; - let mut rows = foo_conn.query("select count(*) from test", ()).await?; - assert!(matches!( - rows.next().await.unwrap().unwrap().get_value(0)?, - Value::Integer(1) - )); + assert_eq!(count_rows("foo", "test").await?, 1); Ok(()) }); @@ -141,7 +204,16 @@ fn load_namespace_from_dump_from_file() { } #[test] -fn load_namespace_from_no_commit() { +fn load_namespace_from_dump_from_file() { + load_namespace_from_dump_from_file_with(None); +} + +#[test] +fn load_namespace_from_dump_from_file_streaming() { + load_namespace_from_dump_from_file_with(STREAMING); +} + +fn load_namespace_from_no_commit_with(importer: Option<&'static str>) { const DUMP: &str = r#" PRAGMA foreign_keys=OFF; BEGIN TRANSACTION; @@ -149,9 +221,7 @@ fn load_namespace_from_no_commit() { INSERT INTO test VALUES(42); "#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -161,13 +231,14 @@ fn load_namespace_from_no_commit() { sim.client("client", async move { let client = Client::new(); - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await - .unwrap(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await + .unwrap(); // the dump is malformed assert_eq!( resp.status(), @@ -177,13 +248,7 @@ fn load_namespace_from_no_commit() { ); // namespace doesn't exist - let foo = - Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; - let foo_conn = foo.connect()?; - assert!(foo_conn - .query("select count(*) from test", ()) - .await - .is_err()); + assert!(count_rows("foo", "test").await.is_err()); Ok(()) }); @@ -192,7 +257,16 @@ fn load_namespace_from_no_commit() { } #[test] -fn load_namespace_from_no_txn() { +fn load_namespace_from_no_commit() { + load_namespace_from_no_commit_with(None); +} + +#[test] +fn load_namespace_from_no_commit_streaming() { + load_namespace_from_no_commit_with(STREAMING); +} + +fn load_namespace_from_no_txn_with(importer: Option<&'static str>) { const DUMP: &str = r#" PRAGMA foreign_keys=OFF; CREATE TABLE test (x); @@ -200,9 +274,7 @@ fn load_namespace_from_no_txn() { COMMIT; "#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -212,12 +284,13 @@ fn load_namespace_from_no_txn() { sim.client("client", async move { let client = Client::new(); - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await?; + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await?; // the dump is malformed assert_eq!( resp.status(), @@ -225,16 +298,13 @@ fn load_namespace_from_no_txn() { "{}", resp.json::().await.unwrap_or_default() ); - assert_json_snapshot!(resp.json_value().await.unwrap()); + assert_json_snapshot!( + "load_namespace_from_no_txn", + resp.json_value().await.unwrap() + ); // namespace doesn't exist - let foo = - Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; - let foo_conn = foo.connect()?; - assert!(foo_conn - .query("select count(*) from test", ()) - .await - .is_err()); + assert!(count_rows("foo", "test").await.is_err()); Ok(()) }); @@ -242,11 +312,19 @@ fn load_namespace_from_no_txn() { sim.run().unwrap(); } +#[test] +fn load_namespace_from_no_txn() { + load_namespace_from_no_txn_with(None); +} + +#[test] +fn load_namespace_from_no_txn_streaming() { + load_namespace_from_no_txn_with(STREAMING); +} + #[test] fn export_dump() { - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); make_primary(&mut sim, tmp.path().to_path_buf()); @@ -280,6 +358,9 @@ fn export_dump() { sim.run().unwrap(); } +/// Rejects a dump on a (malformed) ATTACH. The buffered importer rejects on a substring +/// match before parsing; the streaming importer parses statement by statement, so the same +/// input is a syntax error there (see `load_dump_with_attach_statement_rejected_streaming`). #[test] fn load_dump_with_attach_rejected() { const DUMP: &str = r#" @@ -290,9 +371,7 @@ fn load_dump_with_attach_rejected() { ATTACH foo/bar.sql COMMIT;"#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -304,30 +383,18 @@ fn load_dump_with_attach_rejected() { let client = Client::new(); // path is not absolute is an error - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "file:dump.sql"}), - ) + let resp = create_from_dump(&client, "foo", "file:dump.sql", None) .await .unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); // path doesn't exist is an error - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "file:/dump.sql"}), - ) + let resp = create_from_dump(&client, "foo", "file:/dump.sql", None) .await .unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) + let resp = create_from_dump(&client, "foo", &file_url(&tmp_path.join("dump.sql")), None) .await .unwrap(); assert_eq!( @@ -339,13 +406,45 @@ fn load_dump_with_attach_rejected() { assert_snapshot!(resp.body_string().await?); - let foo = - Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; - let foo_conn = foo.connect()?; - - let res = foo_conn.query("select count(*) from test", ()).await; // This should error since the dump should have failed! - assert!(res.is_err()); + assert!(count_rows("foo", "test").await.is_err()); + + Ok(()) + }); + + sim.run().unwrap(); +} + +/// A well-formed ATTACH statement is rejected by both importers with the same message. +fn load_dump_with_attach_statement_rejected_with(importer: Option<&'static str>) { + const DUMP: &str = r#" + PRAGMA foreign_keys=OFF; + BEGIN TRANSACTION; + CREATE TABLE test (x); + ATTACH 'other.db' AS other; + INSERT INTO test VALUES(42); + COMMIT;"#; + + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + std::fs::write(tmp_path.join("dump.sql"), DUMP).unwrap(); + + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + assert_snapshot!("load_dump_with_attach_rejected", resp.body_string().await?); + assert!(count_rows("foo", "test").await.is_err()); Ok(()) }); @@ -354,7 +453,16 @@ fn load_dump_with_attach_rejected() { } #[test] -fn load_dump_with_invalid_sql() { +fn load_dump_with_attach_statement_rejected() { + load_dump_with_attach_statement_rejected_with(BUFFERED); +} + +#[test] +fn load_dump_with_attach_statement_rejected_streaming() { + load_dump_with_attach_statement_rejected_with(STREAMING); +} + +fn load_dump_with_invalid_sql_with(importer: Option<&'static str>) { const DUMP: &str = r#" PRAGMA foreign_keys=OFF; BEGIN TRANSACTION; @@ -363,9 +471,7 @@ fn load_dump_with_invalid_sql() { SELECT abs(-9223372036854775808) COMMIT;"#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -377,32 +483,25 @@ fn load_dump_with_invalid_sql() { let client = Client::new(); // path is not absolute is an error - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "file:dump.sql"}), - ) + let resp = create_from_dump(&client, "foo", "file:dump.sql", importer) .await .unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); // path doesn't exist is an error - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": "file:/dump.sql"}), - ) + let resp = create_from_dump(&client, "foo", "file:/dump.sql", importer) .await .unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = client - .post( - "http://primary:9090/v1/namespaces/foo/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await - .unwrap(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await + .unwrap(); assert_eq!( resp.status(), StatusCode::BAD_REQUEST, @@ -410,15 +509,11 @@ fn load_dump_with_invalid_sql() { resp.json::().await.unwrap_or_default() ); - assert_snapshot!(resp.body_string().await?); - - let foo = - Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; - let foo_conn = foo.connect()?; + // Both importers report the position in the whole dump (line 7, column 11). + assert_snapshot!("load_dump_with_invalid_sql", resp.body_string().await?); - let res = foo_conn.query("select count(*) from test", ()).await; // This should error since the dump should have failed! - assert!(res.is_err()); + assert!(count_rows("foo", "test").await.is_err()); Ok(()) }); @@ -427,7 +522,16 @@ fn load_dump_with_invalid_sql() { } #[test] -fn load_dump_with_trigger() { +fn load_dump_with_invalid_sql() { + load_dump_with_invalid_sql_with(None); +} + +#[test] +fn load_dump_with_invalid_sql_streaming() { + load_dump_with_invalid_sql_with(STREAMING); +} + +fn load_dump_with_trigger_with(importer: Option<&'static str>) { const DUMP: &str = r#" BEGIN TRANSACTION; CREATE TABLE test (x); @@ -439,9 +543,7 @@ fn load_dump_with_trigger() { INSERT INTO test VALUES (1); COMMIT;"#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -452,26 +554,18 @@ fn load_dump_with_trigger() { sim.client("client", async move { let client = Client::new(); - let resp = client - .post( - "http://primary:9090/v1/namespaces/debug_test/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await - .unwrap(); + let resp = create_from_dump( + &client, + "debug_test", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await + .unwrap(); assert_eq!(resp.status(), StatusCode::OK); - let db = Database::open_remote_with_connector( - "http://debug_test.primary:8080", - "", - TurmoilConnector, - )?; - let conn = db.connect()?; - // Original INSERT: 1, Trigger INSERT: 999 = 2 total rows - let mut rows = conn.query("SELECT COUNT(*) FROM test", ()).await?; - let row = rows.next().await?.unwrap(); - assert_eq!(row.get::(0)?, 2); + assert_eq!(count_rows("debug_test", "test").await?, 2); Ok(()) }); @@ -480,7 +574,16 @@ fn load_dump_with_trigger() { } #[test] -fn load_dump_with_case_trigger() { +fn load_dump_with_trigger() { + load_dump_with_trigger_with(None); +} + +#[test] +fn load_dump_with_trigger_streaming() { + load_dump_with_trigger_with(STREAMING); +} + +fn load_dump_with_case_trigger_with(importer: Option<&'static str>) { const DUMP: &str = r#" BEGIN TRANSACTION; CREATE TABLE test (id INTEGER, rate REAL DEFAULT 0.0); @@ -500,9 +603,7 @@ fn load_dump_with_case_trigger() { INSERT INTO test (id) VALUES (1); COMMIT;"#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -513,13 +614,14 @@ fn load_dump_with_case_trigger() { sim.client("client", async move { let client = Client::new(); - let resp = client - .post( - "http://primary:9090/v1/namespaces/case_test/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await - .unwrap(); + let resp = create_from_dump( + &client, + "case_test", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await + .unwrap(); assert_eq!(resp.status(), StatusCode::OK); let db = Database::open_remote_with_connector( @@ -541,7 +643,16 @@ fn load_dump_with_case_trigger() { } #[test] -fn load_dump_with_nested_case() { +fn load_dump_with_case_trigger() { + load_dump_with_case_trigger_with(None); +} + +#[test] +fn load_dump_with_case_trigger_streaming() { + load_dump_with_case_trigger_with(STREAMING); +} + +fn load_dump_with_nested_case_with(importer: Option<&'static str>) { const DUMP: &str = r#" BEGIN TRANSACTION; CREATE TABLE orders (id INTEGER, amount REAL, status TEXT); @@ -566,9 +677,7 @@ fn load_dump_with_nested_case() { INSERT INTO orders (id, amount, status) VALUES (1, 100.0, 'pending'); COMMIT;"#; - let mut sim = Builder::new() - .simulation_duration(Duration::from_secs(1000)) - .build(); + let mut sim = sim(); let tmp = tempdir().unwrap(); let tmp_path = tmp.path().to_path_buf(); @@ -579,13 +688,14 @@ fn load_dump_with_nested_case() { sim.client("client", async move { let client = Client::new(); - let resp = client - .post( - "http://primary:9090/v1/namespaces/nested_test/create", - json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), - ) - .await - .unwrap(); + let resp = create_from_dump( + &client, + "nested_test", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await + .unwrap(); assert_eq!(resp.status(), StatusCode::OK); let db = Database::open_remote_with_connector( @@ -608,3 +718,483 @@ fn load_dump_with_nested_case() { sim.run().unwrap(); } + +#[test] +fn load_dump_with_nested_case() { + load_dump_with_nested_case_with(None); +} + +#[test] +fn load_dump_with_nested_case_streaming() { + load_dump_with_nested_case_with(STREAMING); +} + +// --------------------------------------------------------------------------------------------- +// Streaming-specific behavior +// --------------------------------------------------------------------------------------------- + +/// Exercises framing across arbitrary HTTP body chunk boundaries, including inside multibyte +/// characters, string literals with semicolons, comments and a trigger body. +const CHUNKY_DUMP: &str = r#"PRAGMA foreign_keys=OFF; +BEGIN TRANSACTION; +-- a comment; with a semicolon +CREATE TABLE test (id INTEGER PRIMARY KEY, name TEXT); +CREATE TABLE audit (id INTEGER, note TEXT); +CREATE TRIGGER tr AFTER INSERT ON test BEGIN + INSERT INTO audit VALUES (NEW.id, 'inserted; ' || NEW.name); +END; +INSERT INTO test VALUES (1, 'żółć; 🎉'); +INSERT INTO test VALUES (2, 'say "hi"; bye'); +/* block + comment; */ +INSERT INTO test VALUES (3, 'attachment'); +; +COMMIT"#; + +fn streaming_chunked_http_delivery_with(chunk_size: usize) { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + make_dump_store_chunked(&mut sim, CHUNKY_DUMP, chunk_size); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump(&client, "foo", "http://dump-store:8080/", STREAMING).await?; + assert_eq!( + resp.status(), + StatusCode::OK, + "{}", + resp.body_string().await.unwrap_or_default() + ); + + assert_eq!(count_rows("foo", "test").await?, 3); + assert_eq!(count_rows("foo", "audit").await?, 3); + + let db = + Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; + let conn = db.connect()?; + let mut rows = conn.query("SELECT name FROM test WHERE id = 1", ()).await?; + assert_eq!(rows.next().await?.unwrap().get::(0)?, "żółć; 🎉"); + let mut rows = conn + .query("SELECT note FROM audit WHERE id = 3", ()) + .await?; + assert_eq!( + rows.next().await?.unwrap().get::(0)?, + "inserted; attachment" + ); + + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn streaming_chunked_http_delivery_1_byte() { + streaming_chunked_http_delivery_with(1); +} + +#[test] +fn streaming_chunked_http_delivery_7_bytes() { + streaming_chunked_http_delivery_with(7); +} + +#[test] +fn streaming_chunked_http_delivery_whole() { + streaming_chunked_http_delivery_with(CHUNKY_DUMP.len()); +} + +/// The same dump works with the buffered importer, except that the word "attachment" in the +/// data trips its substring check. +#[test] +fn buffered_rejects_word_attach_in_data() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + make_dump_store_whole(&mut sim, CHUNKY_DUMP); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump(&client, "foo", "http://dump-store:8080/", BUFFERED).await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + assert_snapshot!("load_dump_with_attach_rejected", resp.body_string().await?); + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn streaming_truncated_http_body_rolls_back() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + let cut = CHUNKY_DUMP.find("INSERT INTO test VALUES (2").unwrap(); + make_dump_store( + &mut sim, + vec![ + Ok(Bytes::copy_from_slice(&CHUNKY_DUMP.as_bytes()[..cut])), + Err(std::io::Error::new( + std::io::ErrorKind::ConnectionReset, + "dump store went away", + )), + ], + ); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump(&client, "foo", "http://dump-store:8080/", STREAMING).await?; + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + let body = resp.body_string().await?; + assert!( + body.contains("Failed to read dump content"), + "unexpected body: {body}" + ); + + // nothing was committed + assert!(count_rows("foo", "test").await.is_err()); + + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn streaming_statement_too_large() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + let big_value = "x".repeat(DumpImportConfig::MIN_MAX_STATEMENT_BYTES); + let dump = format!( + "BEGIN TRANSACTION;\nCREATE TABLE test (x);\nINSERT INTO test VALUES ('{big_value}');\nCOMMIT;\n" + ); + std::fs::write(tmp_path.join("dump.sql"), dump).unwrap(); + + make_primary_with_db_config( + &mut sim, + tmp.path().to_path_buf(), + DbConfig { + dump_import: DumpImportConfig { + max_statement_bytes: DumpImportConfig::MIN_MAX_STATEMENT_BYTES, + ..Default::default() + }, + ..Default::default() + }, + ); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE); + let body = resp.body_string().await?; + assert!( + body.contains("starting at line 3 exceeds the maximum allowed size"), + "unexpected body: {body}" + ); + assert!(count_rows("foo", "test").await.is_err()); + + // the buffered importer has no such limit + let resp = create_from_dump( + &client, + "bar", + &file_url(&tmp_path.join("dump.sql")), + BUFFERED, + ) + .await?; + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!(count_rows("bar", "test").await?, 1); + + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn streaming_rejects_nul_byte() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + let mut dump = + b"BEGIN TRANSACTION;\nCREATE TABLE test (x);\nINSERT INTO test VALUES ('a".to_vec(); + dump.push(0); + dump.extend_from_slice(b"b');\nCOMMIT;\n"); + std::fs::write(tmp_path.join("dump.sql"), dump).unwrap(); + + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = resp.body_string().await?; + assert!( + body.contains("NUL byte at line 3, column 28"), + "unexpected body: {body}" + ); + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn streaming_rejects_invalid_utf8() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + let mut dump = + b"BEGIN TRANSACTION;\nCREATE TABLE test (x);\nINSERT INTO test VALUES ('".to_vec(); + dump.extend_from_slice(&[0xff, 0xfe]); + dump.extend_from_slice(b"');\nCOMMIT;\n"); + std::fs::write(tmp_path.join("dump.sql"), dump).unwrap(); + + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = resp.body_string().await?; + assert!( + body.contains("not valid UTF-8 at line 3, column 27"), + "unexpected body: {body}" + ); + assert!(count_rows("foo", "test").await.is_err()); + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn dump_importer_requires_dump_url() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_importer": "streaming" }), + ) + .await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + assert_json_snapshot!(resp.json_value().await?); + + // unknown importer names are rejected by deserialization (axum answers 422) + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": "file:/dump.sql", "dump_importer": "turbo" }), + ) + .await?; + assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY); + + Ok(()) + }); + + sim.run().unwrap(); +} + +/// With `--dump-importer streaming`, requests that omit `dump_importer` use the streaming +/// importer. The dump contains the word "attachment", which only the buffered importer rejects. +#[test] +fn server_default_streaming() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + make_primary_with_db_config( + &mut sim, + tmp.path().to_path_buf(), + DbConfig { + dump_import: DumpImportConfig { + default_importer: DumpImporterKind::Streaming, + ..Default::default() + }, + ..Default::default() + }, + ); + make_dump_store_whole(&mut sim, CHUNKY_DUMP); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump(&client, "foo", "http://dump-store:8080/", None).await?; + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!(count_rows("foo", "test").await?, 3); + + // explicit opt-out still works + let resp = create_from_dump(&client, "bar", "http://dump-store:8080/", BUFFERED).await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + Ok(()) + }); + + sim.run().unwrap(); +} + +/// Importing the same dump with both importers must yield the same data; the streaming +/// importer additionally preserves the schema SQL text verbatim. +#[test] +fn importers_produce_identical_databases() { + const DUMP: &str = r#"PRAGMA foreign_keys=OFF; +BEGIN TRANSACTION; +CREATE TABLE plain (a, b REAL, c BLOB, d TEXT); +INSERT INTO plain VALUES(1, 1.0e10, X'00ff10', 'it''s "quoted"'); +INSERT INTO plain VALUES(NULL, -0.5, X'', 'multi +line'); +INSERT INTO plain VALUES(3, 2.5, NULL, 'tab inside'); +CREATE TABLE seq (id INTEGER PRIMARY KEY AUTOINCREMENT, v TEXT); +INSERT INTO seq VALUES(1,'one'); +INSERT INTO seq VALUES(5,'five'); +DELETE FROM sqlite_sequence; +INSERT INTO sqlite_sequence VALUES('seq',5); +CREATE TABLE norowid (k TEXT PRIMARY KEY, v) WITHOUT ROWID; +INSERT INTO norowid VALUES('k1', 1); +CREATE TABLE "rowid_preserved"(rowid_ INTEGER, v); +INSERT INTO "rowid_preserved"(rowid, rowid_, v) VALUES(7, 70, 'seven'); +CREATE INDEX plain_a ON plain(a); +CREATE VIEW v_plain AS SELECT a, d FROM plain WHERE a IS NOT NULL; +CREATE TRIGGER tr AFTER INSERT ON seq BEGIN UPDATE seq SET v = v || '!' WHERE id = NEW.id; END; +COMMIT; +"#; + + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + std::fs::write(tmp_path.join("dump.sql"), DUMP).unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let url = file_url(&tmp_path.join("dump.sql")); + + for (ns, importer) in [("ns_buffered", BUFFERED), ("ns_streaming", STREAMING)] { + let resp = create_from_dump(&client, ns, &url, importer).await?; + assert_eq!( + resp.status(), + StatusCode::OK, + "{importer:?}: {}", + resp.body_string().await.unwrap_or_default() + ); + } + + let mut dumps = Vec::new(); + for ns in ["ns_buffered", "ns_streaming"] { + let resp = client + .get(&format!( + "http://{ns}.primary:8080/dump?preserve_row_ids=true" + )) + .await?; + assert_eq!(resp.status(), StatusCode::OK); + dumps.push(resp.body_string().await?); + } + let [buffered, streaming]: [String; 2] = dumps.try_into().unwrap(); + + // Same data, row for row (including preserved rowids and sqlite_sequence). + let data = |dump: &str| -> Vec { + dump.lines() + .filter(|l| l.starts_with("INSERT INTO") || l.starts_with("DELETE FROM")) + .map(str::to_owned) + .collect() + }; + assert_eq!(data(&buffered), data(&streaming)); + assert!(streaming + .contains("INSERT INTO rowid_preserved(rowid,rowid_,v) VALUES(7,70,'seven');")); + + // The streaming importer executes the original statement text, so the schema SQL + // stored in sqlite_schema is exactly what the dump contained. (The buffered importer + // executes the parser's re-serialization, which normalizes whitespace in DDL, e.g. + // `ON plain (a)` and a multi-line trigger body.) + for ddl in [ + "CREATE TABLE IF NOT EXISTS \"rowid_preserved\"(rowid_ INTEGER, v);", + "CREATE INDEX plain_a ON plain(a);", + "CREATE TRIGGER tr AFTER INSERT ON seq BEGIN UPDATE seq SET v = v || '!' WHERE id = NEW.id; END;", + ] { + assert!(streaming.contains(ddl), "missing {ddl:?} in:\n{streaming}"); + } + assert_snapshot!(streaming); + + Ok(()) + }); + + sim.run().unwrap(); +} + +/// A dump much larger than the in-flight queue budget: exercises backpressure and guards +/// against accidental quadratic behavior in framing. +#[test] +fn streaming_large_dump() { + const ROWS: usize = 50_000; + + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + let mut dump = String::from("PRAGMA foreign_keys=OFF;\nBEGIN TRANSACTION;\nCREATE TABLE test (id INTEGER PRIMARY KEY, payload TEXT);\n"); + for i in 0..ROWS { + dump.push_str(&format!( + "INSERT INTO test VALUES({i}, 'row {i}; padding padding padding padding padding');\n" + )); + } + dump.push_str("COMMIT;\n"); + std::fs::write(tmp_path.join("dump.sql"), &dump).unwrap(); + + make_primary_with_db_config( + &mut sim, + tmp.path().to_path_buf(), + DbConfig { + dump_import: DumpImportConfig { + queue_bytes: DumpImportConfig::MIN_QUEUE_BYTES, + queue_depth: 8, + ..Default::default() + }, + ..Default::default() + }, + ); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + STREAMING, + ) + .await?; + assert_eq!( + resp.status(), + StatusCode::OK, + "{}", + resp.body_string().await.unwrap_or_default() + ); + assert_eq!(count_rows("foo", "test").await?, ROWS as i64); + Ok(()) + }); + + sim.run().unwrap(); +} diff --git a/libsql-server/tests/namespaces/mod.rs b/libsql-server/tests/namespaces/mod.rs index 37b373b76e..c33cc229c3 100644 --- a/libsql-server/tests/namespaces/mod.rs +++ b/libsql-server/tests/namespaces/mod.rs @@ -10,18 +10,24 @@ use std::time::Duration; use crate::common::http::Client; use crate::common::net::{init_tracing, SimServer, TestServer, TurmoilAcceptor, TurmoilConnector}; use libsql::{Database, Value}; -use libsql_server::config::{AdminApiConfig, RpcServerConfig, UserApiConfig}; +use libsql_server::config::{AdminApiConfig, DbConfig, RpcServerConfig, UserApiConfig}; use serde_json::json; use tempfile::tempdir; use turmoil::{Builder, Sim}; fn make_primary(sim: &mut Sim, path: PathBuf) { + make_primary_with_db_config(sim, path, DbConfig::default()); +} + +fn make_primary_with_db_config(sim: &mut Sim, path: PathBuf, db_config: DbConfig) { init_tracing(); sim.host("primary", move || { let path = path.clone(); + let db_config = db_config.clone(); async move { let server = TestServer { path: path.into(), + db_config, user_api_config: UserApiConfig { ..Default::default() }, diff --git a/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__dump_importer_requires_dump_url.snap b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__dump_importer_requires_dump_url.snap new file mode 100644 index 0000000000..02070f9867 --- /dev/null +++ b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__dump_importer_requires_dump_url.snap @@ -0,0 +1,7 @@ +--- +source: libsql-server/tests/namespaces/dumps.rs +expression: resp.json_value().await? +--- +{ + "error": "`dump_importer` requires `dump_url`" +} diff --git a/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__importers_produce_identical_databases.snap b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__importers_produce_identical_databases.snap new file mode 100644 index 0000000000..f45916f0f8 --- /dev/null +++ b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__importers_produce_identical_databases.snap @@ -0,0 +1,23 @@ +--- +source: libsql-server/tests/namespaces/dumps.rs +expression: streaming +--- +PRAGMA foreign_keys=OFF; +BEGIN TRANSACTION; +CREATE TABLE IF NOT EXISTS plain (a, b REAL, c BLOB, d TEXT); +INSERT INTO plain(rowid,a,b,c,d) VALUES(1,1,10000000000,X'00ff10','it''s "quoted"'); +INSERT INTO plain(rowid,a,b,c,d) VALUES(2,NULL,-0.5,X'',replace('multi\nline','\n',char(10))); +INSERT INTO plain(rowid,a,b,c,d) VALUES(3,3,2.5,NULL,'tab inside'); +CREATE TABLE IF NOT EXISTS seq (id INTEGER PRIMARY KEY AUTOINCREMENT, v TEXT); +INSERT INTO seq(rowid,id,v) VALUES(1,1,'one'); +INSERT INTO seq(rowid,id,v) VALUES(5,5,'five'); +CREATE TABLE IF NOT EXISTS norowid (k TEXT PRIMARY KEY, v) WITHOUT ROWID; +INSERT INTO norowid VALUES('k1',1); +CREATE TABLE IF NOT EXISTS "rowid_preserved"(rowid_ INTEGER, v); +INSERT INTO rowid_preserved(rowid,rowid_,v) VALUES(7,70,'seven'); +DELETE FROM sqlite_sequence; +INSERT INTO sqlite_sequence(rowid,name,seq) VALUES(1,'seq',5); +CREATE INDEX plain_a ON plain(a); +CREATE VIEW v_plain AS SELECT a, d FROM plain WHERE a IS NOT NULL; +CREATE TRIGGER tr AFTER INSERT ON seq BEGIN UPDATE seq SET v = v || '!' WHERE id = NEW.id; END; +COMMIT; From 0bbfca62f090c72fcc13675c0dc9935d19b8aaf9 Mon Sep 17 00:00:00 2001 From: Tomasz Szymczyszyn Date: Wed, 7 Oct 2026 18:16:59 +0200 Subject: [PATCH 4/6] docs: document dump importers, add design doc and benchmark script - 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. --- docs/ADMIN_API.md | 21 + docs/STREAMING_DUMP_IMPORT_DESIGN.md | 731 +++++++++++++++++++++++++++ docs/USER_GUIDE.md | 26 + scripts/bench-dump-import.sh | 122 +++++ 4 files changed, 900 insertions(+) create mode 100644 docs/STREAMING_DUMP_IMPORT_DESIGN.md create mode 100755 scripts/bench-dump-import.sh diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index d5a7fe2478..ecd5aabea3 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -21,9 +21,30 @@ body: ```json { "dump_url"?: string, + "dump_importer"?: "buffered" | "streaming", } ``` +`dump_url` initializes the new namespace from a SQLite SQL dump (`sqlite3 db .dump` or this +server's `GET /dump` output). Supported schemes are `file:` (an absolute path on the server +host) and `http(s):`. The dump must run inside a transaction and end with `COMMIT`; `ATTACH` is +rejected. + +`dump_importer` selects how the dump is loaded (it requires `dump_url`): + +- `buffered` (historical): the whole dump is read into memory, parsed, then executed. Memory + usage is proportional to the dump size. +- `streaming`: statements are framed with `sqlite3_complete()` and executed while the dump is + still being read. Memory usage is bounded by the server's queue settings plus the largest + single statement (see `--dump-import-*` flags); a statement larger than + `--dump-import-max-statement-size` is rejected with `413`. + +When omitted, the server's `--dump-importer` setting (`SQLD_DUMP_IMPORTER`, default `buffered`) +applies. Both importers produce the same data; the streaming importer additionally stores the +schema SQL exactly as written in the dump, whereas the buffered importer stores the parser's +normalized rendering. A dump whose *data* contains the word "attach" is rejected by the buffered +importer (substring check) but accepted by the streaming one (statement-level check). + ```HTTP DELETE /v1/namespaces/:namespace ``` diff --git a/docs/STREAMING_DUMP_IMPORT_DESIGN.md b/docs/STREAMING_DUMP_IMPORT_DESIGN.md new file mode 100644 index 0000000000..73959c49d0 --- /dev/null +++ b/docs/STREAMING_DUMP_IMPORT_DESIGN.md @@ -0,0 +1,731 @@ +# Design: memory-bounded streaming SQL dump importer for libsql-server + +Status: implemented (see `libsql-server/src/namespace/dump_import/`) · Target: `Shopify/libsql`, branch `v0.9.30-shopify-patches` · Related: Retail #35846, LibSQL DB Mover P0 + +Sections marked *as built* record where the implementation refined the original proposal. + +This document is written so that an implementer can follow it step by step without re-deriving decisions. Every file, type, flag, error and test is named. Section 16 is the ordered implementation plan; sections 7–9 are the normative specifications. + +--- + +## 1. Goals + +1. Import a SQLite SQL dump into a **new** namespace with peak importer memory that does **not** grow with total dump size. Memory is bounded by configuration plus the size of the single largest statement. +2. Keep the existing importer (hereafter **buffered**) byte-for-byte intact and selectable, so both importers can be run on the **same** `libsql-server` instance for benchmarking and validation. +3. Select the importer at runtime: + - per request: `"dump_importer": "buffered" | "streaming"` in the `POST /v1/namespaces/:ns/create` body; + - server default: `--dump-importer` / `SQLD_DUMP_IMPORTER` (default `buffered`, so behavior is unchanged until opted in). +4. Preserve the Admin API contract, HTTP status codes and — where practical — the exact error messages, so existing insta snapshots can be reused. +5. Fail closed: any malformed, truncated, or cancelled dump leaves **no committed partial data**; the SQL transaction is rolled back and an HTTP error is returned. + +## 2. Non-goals + +- Enormous single statements / large BLOBs. A per-statement size cap exists purely as a safety valve; dumps exceeding it are rejected, not supported. +- Fixing pre-existing namespace lifecycle issues (metastore row kept after a failed create; directory not cleaned when the request is cancelled). Documented in §13; owned by the DB Mover fence/quarantine work. +- Binary SQLite file ingestion, bottomless memory behavior, Admin request timeouts, export-side changes. +- Changing what the buffered importer does. It is moved, not modified. + +--- + +## 3. Baseline: what exists today + +| Concern | Location | +|---|---| +| Admin request parsing, `dump_url` → `DumpStream` | `libsql-server/src/http/admin/mod.rs` — `CreateNamespaceReq`, `handle_create_namespace`, `dump_stream_from_url` | +| `RestoreOption::Dump(DumpStream)` | `libsql-server/src/namespace/mod.rs` | +| Namespace creation (stores config in metastore **before** loading) | `libsql-server/src/namespace/store.rs` — `NamespaceStore::create` → `load_namespace` → `make_namespace` | +| Primary setup, dir cleanup on failure | `libsql-server/src/namespace/configurator/primary.rs` — `PrimaryConfigurator::setup` | +| Dump loading | `libsql-server/src/namespace/configurator/helpers.rs` — `make_primary_connection_maker` (match on `restore_option`), `load_dump`, `check_fresh_db` | +| Errors / HTTP mapping | `libsql-server/src/error.rs` — `LoadDumpError`, `IntoResponse for &LoadDumpError` | +| Tests | `libsql-server/tests/namespaces/dumps.rs` + `snapshots/` | + +Current `load_dump` sequence (helpers.rs:293–395): + +1. `StreamReader` → `BufReader` → `read_to_string(&mut dump_content)`: **whole dump in one `String`**. Invalid UTF-8 → `LoadDumpError::Internal` (HTTP 500). +2. `dump_content.to_lowercase().contains("attach")` → rejects the **entire** dump if the substring appears anywhere, including inside data (`'attachment'`). Also allocates a second lowercase copy of the whole dump. +3. `sqlite3_parser::Parser::new(dump_content.as_bytes())`, loop `parser.next()`: + - `Ok(Some(cmd))`: count `n_stmt`; skip the first `CREATE TABLE libsql_wasm_func_table`; if `n_stmt > 2` and connection is in autocommit → `NoTxn`; **execute `cmd.to_string()`** (AST re-serialized, not the original text) via `tokio::task::spawn_blocking` **per statement**, installing the ATTACH-denying authorizer each time. + - `Ok(None)`: end of input. (The parser absorbs empty statements such as `;;` and comment-only input — verified against the vendored parser — so this is only reached at EOF.) + - `Err(e)`: `InvalidSqlInput("syntax error near '…' at line L, column C")` or `"parse error: …"`. +4. After the loop, if not autocommit → `ROLLBACK` → `NoCommit`. + +Where the WAL bytes go during the import transaction: `ReplicationLoggerWalWrapper::insert_frames` flushes every page batch SQLite spills to the replication log file immediately (`replication_logger_wal.rs:101–110`), so the replication layer does **not** buffer the transaction in memory. The importer's `String` is the only size-proportional allocation. This is what we remove. + +--- + +## 4. Architecture + +``` + async (tokio runtime) blocking thread (BLOCKING_RT) +DumpStream ──chunks──▶ StatementFramer ──frames──▶ mpsc(depth) + byte budget ──▶ StatementExecutor ──▶ PrimaryConnection +(hyper body / memchr(';') + (Semaphore permits UTF-8 check → sqlite3_parser → + tokio file) sqlite3_complete() travel with each frame) policy checks → execute original SQL + → ROLLBACK on any failure +``` + +Three components, each independently testable: + +- **`StatementFramer`** (sync, pure): accumulates bytes, emits complete SQL statements with their 1-based `(line, column)` in the original dump. Uses SQLite's own `sqlite3_complete()` so semicolons inside strings, comments and `CREATE TRIGGER … END` bodies are handled exactly as the `sqlite3` shell does. +- **Reader task** (async): drives the stream, feeds the framer, pushes frames into a bounded channel, sends an explicit `End` marker. +- **`StatementExecutor`** (blocking, one thread for the whole import): owns the `PrimaryConnection`, validates/parses/executes each frame in order inside the dump's own transaction, enforces the transaction rules, rolls back on abort. + +Why a dedicated blocking thread instead of `spawn_blocking` per statement: it removes a thread-pool handoff per row, keeps the connection on one thread, and lets network I/O overlap with execution. + +--- + +## 5. Interface changes + +### 5.1 Admin API + +`POST /v1/namespaces/:namespace/create` body gains one optional field: + +```json +{ + "dump_url": "file:///abs/path/dump.sql", + "dump_importer": "streaming" +} +``` + +- Allowed values: `"buffered"`, `"streaming"` (serde `rename_all = "snake_case"`). Unknown value → serde rejection → HTTP **422** (axum's `JsonRejection` for deserialization errors; *as built*). +- Omitted → server default (`DumpImportConfig::default_importer`). +- Present without `dump_url` → HTTP 400 `LoadDumpError::ImporterWithoutDumpUrl` ("`dump_importer` requires `dump_url`"). +- Existing rule unchanged: `shared_schema_name` + `dump_url` → 400. + +Document in `docs/ADMIN_API.md`. + +### 5.2 CLI flags / environment (`libsql-server/src/main.rs`, `struct Cli`) + +| Flag | Env | Type / default | Meaning | +|---|---|---|---| +| `--dump-importer` | `SQLD_DUMP_IMPORTER` | `DumpImporterKind`, `buffered` | Importer used when the request omits `dump_importer`. | +| `--dump-import-max-statement-size` | `SQLD_DUMP_IMPORT_MAX_STATEMENT_SIZE` | `bytesize::ByteSize`, `64MiB` | Streaming only. A single statement larger than this is rejected (HTTP 413), whether it arrives terminated in one chunk or grows past the limit unterminated. Safety valve, not a sizing target. | +| `--dump-import-queue-bytes` | `SQLD_DUMP_IMPORT_QUEUE_BYTES` | `ByteSize`, `16MiB` | Streaming only. Max bytes of framed-but-not-yet-executed statements in flight. | +| `--dump-import-queue-depth` | `SQLD_DUMP_IMPORT_QUEUE_DEPTH` | `usize`, `256` | Streaming only. Max number of statements in flight. | + +Validation in `make_db_config`: `max_statement_size ≥ 4 KiB`, `64 KiB ≤ queue_bytes ≤ 1 GiB` (must fit `u32` for `Semaphore::acquire_many`), `queue_depth ≥ 1`. Fail startup with a clear `anyhow` error otherwise. + +### 5.3 Config plumbing + +```rust +// libsql-server/src/namespace/dump_import/mod.rs (new) +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DumpImporterKind { Buffered, Streaming } + +impl std::str::FromStr for DumpImporterKind { /* "buffered" | "streaming", case-insensitive; used by clap */ } +impl std::fmt::Display for DumpImporterKind { /* "buffered" / "streaming" — used as metric label */ } + +#[derive(Debug, Clone)] +pub struct DumpImportConfig { + pub default_importer: DumpImporterKind, // Buffered + pub max_statement_bytes: usize, // 64 MiB + pub queue_bytes: usize, // 16 MiB + pub queue_depth: usize, // 256 +} +impl Default for DumpImportConfig { /* values above */ } +``` + +- `config.rs`: `DbConfig` gets `pub dump_import: DumpImportConfig` (add to `Default`). +- `main.rs::make_db_config`: fill it from the flags. +- `namespace/configurator/mod.rs`: `BaseNamespaceConfig` gets `pub(crate) dump_import: DumpImportConfig`; `lib.rs` (~line 593) copies `self.db_config.dump_import.clone()`. Also update the two test constructors of `BaseNamespaceConfig` in `schema/scheduler.rs` tests (`..` is not available there; add the field with `Default::default()`). +- `namespace/mod.rs`: + +```rust +pub struct DumpSource { + pub stream: DumpStream, + /// `None` → use `BaseNamespaceConfig::dump_import.default_importer`. + pub importer: Option, +} + +pub enum RestoreOption { + #[default] Latest, + Dump(DumpSource), // was Dump(DumpStream) + Generation(Uuid), + PointInTime(NaiveDateTime), +} +``` + +Every existing `RestoreOption::Dump(_)` pattern still compiles; only `helpers.rs:204` destructures the payload (see §6). + +--- + +## 6. Module layout and call flow + +``` +libsql-server/src/namespace/dump_import/ +├── mod.rs DumpImporterKind, DumpImportConfig, DumpImportStats, pub(crate) async fn load_dump(...) dispatcher, +│ shared error-mapping helpers (map_parse_error, map_exec_error), metrics/logging +├── buffered.rs legacy load_dump moved verbatim (renamed load_dump_buffered), returns DumpImportStats +├── framer.rs StatementFramer + is_complete_statement() + position helpers + unit tests +└── streaming.rs load_dump_streaming (reader task) + run_executor (blocking) + Msg type +``` + +Register `pub(crate) mod dump_import;` in `namespace/mod.rs`. Add `memchr = "2"` to `libsql-server/Cargo.toml` (already in the lockfile via `sqlite3-parser`). + +`helpers.rs` after the change (only the `match restore_option` block changes): + +```rust +match restore_option { + RestoreOption::Dump(_) if !is_fresh_db => { + Err(LoadDumpError::LoadDumpExistingDb)?; + } + RestoreOption::Dump(source) => { + let conn = connection_maker.create().await?; + let kind = source + .importer + .unwrap_or(base_config.dump_import.default_importer); + crate::namespace::dump_import::load_dump( + kind, + source.stream, + conn, + &base_config.dump_import, + name, + ) + .await?; + } + _ => { /* other cases were already handled when creating bottomless */ } +} +``` + +Dispatcher: + +```rust +pub(crate) async fn load_dump( + kind: DumpImporterKind, + stream: DumpStream, + conn: PrimaryConnection, + cfg: &DumpImportConfig, + namespace: &NamespaceName, +) -> Result { + let started = Instant::now(); + tracing::info!(namespace = %namespace, importer = %kind, "loading dump"); + let res = match kind { + DumpImporterKind::Buffered => buffered::load_dump_buffered(stream, conn).await, + DumpImporterKind::Streaming => streaming::load_dump_streaming(stream, conn, cfg).await, + }; + // metrics + one info!/warn! line (see §12), then return res +} + +#[derive(Debug, Default, Clone)] +pub struct DumpImportStats { + pub statements_executed: u64, + pub statements_skipped: u64, // empty frames + the wasm table + pub bytes: u64, // dump bytes consumed + pub max_statement_bytes: usize, + pub elapsed: Duration, +} +``` + +`buffered.rs`: cut/paste the existing `load_dump` body unchanged; set `stats.bytes = dump_content.len()`, `statements_executed = n_stmt` (minus the skipped wasm table), return stats. **No other edits** — this is the control arm of the benchmark. + +--- + +## 7. `StatementFramer` — normative spec (`framer.rs`) + +### 7.1 Purpose + +Turn an arbitrary sequence of byte chunks into complete SQL statements, each ending at the `;` that terminates it, without ever holding more than one unfinished statement in memory. + +### 7.2 Completeness oracle + +```rust +use rusqlite::ffi::sqlite3_complete; // re-exported libsql_ffi binding: fn(*const c_char) -> c_int + +/// `buf[start..=end]` is a candidate statement whose last byte is b';'. +/// sqlite3_complete needs a NUL-terminated string, so a terminator is temporarily placed at end+1. +fn is_complete_statement(buf: &mut Vec, start: usize, end: usize) -> bool { + debug_assert_eq!(buf[end], b';'); + let pushed = if end + 1 == buf.len() { buf.push(0); true } else { false }; + let saved = buf[end + 1]; + buf[end + 1] = 0; + // SAFETY: buf[start..] is a NUL-terminated byte string; sqlite3_complete only reads it. + let complete = unsafe { sqlite3_complete(buf[start..].as_ptr() as *const _) } != 0; + buf[end + 1] = saved; + if pushed { buf.pop(); } + complete +} +``` + +Semantics of `sqlite3_complete` (sqlite3.h:2770–2805): returns non-zero iff the string ends with a semicolon token that is not inside a string/identifier/comment and the text is not an unfinished `CREATE TRIGGER … BEGIN … END`. It does not parse; it tokenizes only. Verified against the bundled SQLite (probe run on this branch): + +| input | result | +|---|---| +| `SELECT 1;`, `;`, ` \n ;`, `PRAGMA foreign_keys=OFF;`, `EXPLAIN SELECT 1;` | complete | +| `SELECT 'a;b';`, `SELECT "a;b";`, `SELECT [a;b];`, `CREATE TABLE t(x CHECK(x <> ';'));`, `INSERT INTO t VALUES(X'00ff');` | complete (interior `;` ignored) | +| `/* a; */ SELECT 1;`, `-- c;\nSELECT 1;` | complete | +| `SELECT ';`, `-- c;`, `COMMIT` | **not** complete | +| `CREATE [TEMP] TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;` | **not** complete; `... END;` complete | + +Embedded NUL bytes would truncate the view, hence §7.4 rule 1. + +### 7.3 State + +```rust +pub struct StatementFramer { + buf: Vec, // bytes after the last emitted frame; starts with the next statement's leading whitespace/comments + scan_from: usize, // offset in buf from which to look for the next ';' (avoids re-testing rejected candidates) + line: u64, // 1-based line of buf[0] in the whole dump + column: usize, // 1-based byte column of buf[0] + max_statement_bytes: usize, + bytes_seen: u64, +} + +pub struct Frame { pub sql: Vec, pub line: u64, pub column: usize } + +#[derive(Debug)] +pub enum FrameError { + NulByte { line: u64, column: usize }, + StatementTooLarge { line: u64, column: usize, limit: usize }, +} +``` + +`new(max_statement_bytes)` → `line = 1, column = 1`, empty buffer. + +### 7.4 `push(&mut self, chunk: &[u8]) -> Result, FrameError>` + +1. If `memchr(0, chunk)` finds a NUL at offset `k`: compute its position (advance a copy of `(line, column)` over `buf[..]` then `chunk[..k]`) and return `NulByte`. Nothing is appended. +2. `bytes_seen += chunk.len()`; `buf.extend_from_slice(chunk)`. +3. `let mut start = 0; let mut frames = Vec::new();` +4. Loop: `let Some(rel) = memchr(b';', &buf[scan_from..]) else break; let end = scan_from + rel;` + - If `is_complete_statement(&mut buf, start, end)`: + - *as built:* if `end + 1 - start > max_statement_bytes` → `StatementTooLarge` (position = first non-whitespace byte of the statement, via `statement_start`) + - `frames.push(Frame { sql: buf[start..=end].to_vec(), line: self.line, column: self.column })` + - advance position over `buf[start..=end]` (§7.6) + - `start = end + 1; scan_from = start;` + - else `scan_from = end + 1;` +5. After the loop: `buf.drain(..start)` (one memmove per push, not per frame); `scan_from -= start`. +6. If `buf.len() > max_statement_bytes` → `StatementTooLarge { line, column, limit }` where `(line, column)` is the first non-whitespace byte of the pending statement (*as built*: `statement_start`, so the message points at the statement rather than at the end of the previous line). +7. Return `frames`. + +Complexity: each candidate `;` costs one `sqlite3_complete` scan from the statement start, so a statement containing *k* interior semicolons (trigger bodies, string literals) costs O(k·len). Normal dumps have k ≤ a few. A frame is emitted at the first terminating `;`, so a frame contains exactly one statement plus any leading whitespace/comments. + +### 7.5 `finish(&mut self) -> Option` + +Called at EOF. If `buf` is empty → `None`. Otherwise return the remaining bytes as a frame (with current `line, column`) and clear the buffer. The **executor** decides what the tail means (§8.5 step 2): only whitespace/comments → ignored; a complete statement without a trailing `;` (e.g. `COMMIT` at EOF) → executed, matching the buffered importer, whose parser appends a virtual `;` at EOF; anything else → parse error. + +### 7.6 Position tracking + +After emitting a frame `f`: + +```rust +match memchr::memrchr(b'\n', &f) { + Some(last) => { self.line += memchr::memchr_iter(b'\n', &f).count() as u64; self.column = f.len() - last; } + None => { self.column += f.len(); } +} +``` + +(`f.len() - last` = 1-based column of the byte that follows the frame.) + +Absolute position of a parser error reported at `(rel_line, rel_col)` relative to a frame starting at `(line, column)`: + +```rust +pub fn absolute_position(frame: (u64, usize), rel: (u64, usize)) -> (u64, usize) { + if rel.0 <= 1 { (frame.0, frame.1 + rel.1.saturating_sub(1)) } else { (frame.0 + rel.0 - 1, rel.1) } +} +``` + +Worked example (existing snapshot `load_dump_with_invalid_sql`: `syntax error near 'COMMIT' at line 7, column 11`): the frame is `"\n SELECT abs(-9223372036854775808) \n COMMIT;"` starting at (5, 33) right after the `;` of line 5; the parser reports (3, 11); `absolute_position` → (5+3−1, 11) = (7, 11). Snapshot preserved. + +### 7.7 Invariants + +- `buf` never contains bytes of an already-emitted frame. +- Frames are emitted in input order and concatenating all frames plus the final `finish()` tail reproduces the input byte-for-byte. +- Frame boundaries are ASCII `;`, so if the whole dump is valid UTF-8, every frame is valid UTF-8 on its own (no multibyte char can straddle a frame boundary). +- Memory held by the framer ≤ `max_statement_bytes + len(last chunk)`. + +--- + +## 8. `StatementExecutor` — normative spec (`streaming.rs`) + +### 8.1 Message type + +```rust +enum Msg { + Stmt { + sql: Vec, + line: u64, + column: usize, + _budget: tokio::sync::OwnedSemaphorePermit, // released when the executor drops the message + }, + End, // reader reached EOF cleanly and sent the finish() tail (if any) +} +``` + +### 8.2 Entry point + +```rust +fn run_executor(conn: PrimaryConnection, mut rx: mpsc::Receiver) -> Result +``` + +Spawned once with `crate::BLOCKING_RT.spawn_blocking(move || run_executor(conn, rx))`. Receives with `rx.blocking_recv()`. Holds the only clone of the connection; drops it on return. + +### 8.3 Setup + +Install the authorizer **once**: + +```rust +conn.with_raw(|c| c.authorizer(Some(|auth: AuthContext<'_>| match auth.action { + AuthAction::Attach { .. } | AuthAction::Detach { .. } => Authorization::Deny, + _ => Authorization::Allow, +}))); +``` + +State: `n_stmt: u64 = 0`, `skipped_wasm_table = false`, `stats: DumpImportStats`. + +### 8.4 Main loop + +``` +loop { + match rx.blocking_recv() { + Some(Msg::Stmt{sql,line,column,..}) => if let Err(e) = handle_statement(...) { rollback_best_effort(); clear_authorizer(); return Err(e) } + Some(Msg::End) => break, + None => { rollback_best_effort(); clear_authorizer(); + return Err(LoadDumpError::Internal("dump stream ended before completion".into())) } + } +} +// End received +if !conn.with_raw(|c| c.is_autocommit()) { + conn.with_raw(|c| c.execute_batch("ROLLBACK"))?; // propagate error like the buffered importer + clear_authorizer(); + return Err(LoadDumpError::NoCommit); +} +clear_authorizer(); +Ok(stats) +``` + +`None` (channel closed without `End`) means the reader died — stream error, framer error, or request cancellation. The reader (if still alive) replaces this generic error with the real cause (§9.3). + +`rollback_best_effort`: `conn.with_raw(|c| if !c.is_autocommit() { if let Err(e) = c.execute_batch("ROLLBACK") { tracing::warn!(...) } })`. +`clear_authorizer`: `conn.with_raw(|c| c.authorizer(None::) -> Authorization>))`. + +### 8.5 `handle_statement(sql: &[u8], line, column)` — in this exact order + +1. **UTF-8**: `std::str::from_utf8(sql)` → on error `InvalidSqlInput("dump is not valid UTF-8 at line L, column C: …")` where `(L, C)` is the exact position of the offending byte (*as built*: `advance_position(frame_pos, &sql[..e.valid_up_to()])`). The vendored parser does *lossy* conversion internally — commit 4feb2b2e46 — so validation must happen here, before parsing. +2. **Parse** with `sqlite3_parser::lexer::sql::Parser::new(sql.as_bytes())`: + - `Ok(None)` → frame holds only whitespace/comments (or a bare `;`) → `stats.statements_skipped += 1`; return `Ok(())`. (Per-frame parsing makes empty frames explicit; the buffered importer absorbs them inside one parser instance. Net behavior is identical.) + - `Err(e)` → `map_parse_error(e, (line, column))` → `InvalidSqlInput` (§8.6). + - `Ok(Some(cmd))` → continue. Then call `parser.next()` once more; if it is `Ok(Some(_))`, return `Internal("framing produced more than one statement")` — must never happen; guards against oracle/parser disagreement. +3. `n_stmt += 1`. +4. **WASM table skip** (identical to buffered): if `!skipped_wasm_table` and `cmd` is `Cmd::Stmt(Stmt::CreateTable { tbl_name, .. })` with `tbl_name.name.0 == "libsql_wasm_func_table"` → set flag, `statements_skipped += 1`, return `Ok(())`. +5. **ATTACH policy**: if `cmd` is `Cmd::Stmt(Stmt::Attach { .. })` or `Cmd::Stmt(Stmt::Detach(_))` → `InvalidSqlInput("attach statements are not allowed in dumps".into())` (same text as the buffered importer's snapshot). The authorizer remains as defense in depth (e.g. ATTACH reached through a trigger body). +6. **Transaction rule** (identical to buffered): `if n_stmt > 2 && conn.with_raw(|c| c.is_autocommit()) { return Err(LoadDumpError::NoTxn) }`. +7. **Execute the original text**, not `cmd.to_string()`: + +```rust +fn execute_single(c: &mut rusqlite::Connection, sql: &str) -> rusqlite::Result<()> { + let mut stmt = c.prepare(sql)?; // single statement guaranteed by framing + step 2 guard (rusqlite's own tail check + // needs the `extra_check` feature, which this workspace does not enable — do not rely on it) + let mut rows = stmt.raw_query(); + while rows.next()?.is_some() {} // tolerate row-returning statements (EXPLAIN, PRAGMA x, stray SELECT) like the sqlite3 shell + Ok(()) +} +``` + + Errors → `map_exec_error(e, n_stmt)` (§8.6). Update `stats.statements_executed`, `stats.max_statement_bytes`. + +Rationale for executing original text: the AST round-trip is a known source of fidelity bugs in this code path (commits abd0b525fe "parse dumps with triggers correctly", fcfde9edcc) and silently alters the text (probe: `VALUES(1.0e10, 'a''b', X'00ff')` is re-emitted as `VALUES (1.0e10, 'a''b', X'00ff')` — harmless here, but it demonstrates the two texts differ). SQLite itself is the authority on its own dump format. The parser is still needed for the policy checks (steps 4–5), for empty-frame detection, and to report parse errors in the same format as before. Equivalence between the two importers is verified by diffing `GET /dump` output (§15). + +### 8.6 Error mapping (shared in `mod.rs`) + +```rust +fn map_parse_error(mut e: sqlite3_parser::lexer::sql::Error, frame_pos: (u64, usize)) -> LoadDumpError { + use sqlite3_parser::lexer::sql::Error as E; + // 1. extract relative position (every variant except Io carries Option<(u64, usize)>) + let rel = match &e { E::ParserError(_, p) | E::UnrecognizedToken(p) | E::UnterminatedLiteral(p) | /* …all others… */ => *p, E::Io(_) => None, _ => None }; + // 2. rewrite it to absolute using framer::absolute_position and ScanError::position + if let Some(rel) = rel { let (l, c) = absolute_position(frame_pos, rel); sqlite3_parser::lexer::ScanError::position(&mut e, l, c); } // `lexer::ScanError` is the public re-export (`lexer::scan` is private) + // 3. format exactly like the buffered importer + let msg = match e { + E::ParserError(ParserError::SyntaxError { token_type, found }, Some((line, col))) => { + let near = found.as_deref().unwrap_or(&token_type); + format!("syntax error near '{near}' at line {line}, column {col}") + } + other => format!("parse error: {other}"), + }; + LoadDumpError::InvalidSqlInput(msg) +} + +fn map_exec_error(e: rusqlite::Error, n_stmt: u64) -> LoadDumpError { + match e { + rusqlite::Error::SqlInputError { msg, sql, offset, .. } => + LoadDumpError::InvalidSqlInput(format!("msg: {msg}, sql: {sql}, offset: {offset}")), + e => LoadDumpError::Internal(format!("statement: {n_stmt}, error: {e}")), + } +} +``` + +`Error` is `#[non_exhaustive]`; keep a `_ => None` arm. + +--- + +## 9. Reader / pipeline — normative spec (`streaming.rs`) + +### 9.1 Signature + +```rust +pub(super) async fn load_dump_streaming( + mut stream: DumpStream, + conn: PrimaryConnection, + cfg: &DumpImportConfig, +) -> Result +``` + +### 9.2 Body + +```rust +let (tx, rx) = tokio::sync::mpsc::channel::(cfg.queue_depth); +let budget = Arc::new(tokio::sync::Semaphore::new(cfg.queue_bytes)); +let executor = crate::BLOCKING_RT.spawn_blocking(move || run_executor(conn, rx)); +let mut framer = StatementFramer::new(cfg.max_statement_bytes); + +enum FeedError { Stream(LoadDumpError), ExecutorGone } + +let feed: Result<(), FeedError> = async { + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|e| FeedError::Stream(LoadDumpError::Internal(format!("Failed to read dump content: {e}"))))?; + for frame in framer.push(&chunk).map_err(|e| FeedError::Stream(e.into()))? { + send_frame(&tx, &budget, cfg.queue_bytes, frame).await?; + } + } + if let Some(tail) = framer.finish() { send_frame(&tx, &budget, cfg.queue_bytes, tail).await?; } + tx.send(Msg::End).await.map_err(|_| FeedError::ExecutorGone) +}.await; + +drop(tx); // guarantees the executor observes closure if we failed before End +let exec = executor.await + .map_err(|e| LoadDumpError::Internal(format!("dump executor task failed: {e}")))?; + +match (feed, exec) { + (Ok(()), exec) => exec, // normal path: executor's verdict (Ok / NoCommit / …) + (Err(FeedError::ExecutorGone), Err(e)) => Err(e), // executor failed first; report its error + (Err(FeedError::ExecutorGone), Ok(s)) => Err(LoadDumpError::Internal("executor finished before the dump was fully read".into())), // impossible, defensive + (Err(FeedError::Stream(e)), _) => Err(e), // I/O or framing error wins over the executor's generic "ended before completion" +} +``` + +```rust +async fn send_frame(tx, budget: &Arc, queue_bytes: usize, f: Frame) -> Result<(), FeedError> { + let permits = f.sql.len().clamp(1, queue_bytes) as u32; // oversized statements take the whole budget → alone in flight + let permit = budget.clone().acquire_many_owned(permits).await.expect("semaphore never closed"); + tx.send(Msg::Stmt { sql: f.sql, line: f.line, column: f.column, _budget: permit }) + .await.map_err(|_| FeedError::ExecutorGone) +} +``` + +`From for LoadDumpError`: `NulByte` → `InvalidSqlInput("dump contains a NUL byte at line L, column C")`; `StatementTooLarge` → `LoadDumpError::StatementTooLarge { line, limit }` (new variant, HTTP 413). + +### 9.3 Failure and cancellation semantics + +| Event | Reader | Executor | Result | +|---|---|---|---| +| Stream yields `Err` | stops, drops `tx` | sees `None` → ROLLBACK | `Internal("Failed to read dump content: …")` → 500 (same as buffered) | +| Framer error (NUL / too large) | stops, drops `tx` | `None` → ROLLBACK | 400 / 413 | +| Executor error (parse/exec/NoTxn) | `send` fails → `ExecutorGone` | returns `Err(e)` after ROLLBACK | `e` → 400/500 as today | +| EOF without `COMMIT` | sends `End` | not autocommit → ROLLBACK → `NoCommit` | 400 (same as buffered) | +| Admin HTTP request dropped (client timeout) | future dropped → `tx` dropped | `None` → ROLLBACK, drops connection, exits | no response; SQLite state clean. Directory/metastore cleanup is **not** run (pre-existing, §13) | +| Executor panics | `executor.await` → `JoinError` | — | `Internal(...)` → 500; connection dropped by unwinding → SQLite rolls back on close | + +The executor is never cancelled by tokio; it always reaches a ROLLBACK or COMMIT decision itself. That is the robustness win over the buffered importer, whose per-statement `spawn_blocking` can be orphaned mid-loop. + +### 9.4 Stream chunking + +In `dump_stream_from_url` change `ReaderStream::new(f)` to `ReaderStream::with_capacity(f, 64 * 1024)`. Hyper bodies arrive as they come (typically 8–64 KiB). The framer is chunk-size agnostic; this is only an efficiency tweak and benefits both importers. + +--- + +## 10. Behavior parity matrix + +| Case | Buffered (unchanged) | Streaming | Snapshot reuse | +|---|---|---|---| +| Valid `sqlite3 .dump` / `GET /dump` output | 200 | 200 | — | +| `BEGIN`/`COMMIT` missing (`NoTxn`, `NoCommit`) | 400, same messages | 400, same messages | yes | +| Syntax error | 400 `syntax error near 'X' at line L, column C` (absolute) | identical (absolute via §7.6) | yes | +| `ATTACH 'f' AS x;` as a statement | 400 "attach statements are not allowed in dumps" (substring check) | 400, same message (AST check) | yes | +| Word "attach" inside data (`'attachment'`) | **400** (false positive) | 200 | new test, streaming only | +| `ATTACH foo/bar.sql` without `;` (existing test) | 400 "attach statements are not allowed" | 400 `syntax error near 'COMMIT' …` | keep legacy test; add streaming test with a well-formed ATTACH | +| Empty statement `;;`, comment-only segments | absorbed by the parser | empty frame skipped | new test, both | +| `COMMIT` without trailing `;` at EOF | executed | executed | new test, both | +| Invalid UTF-8 | 500 `Internal` | 400 `InvalidSqlInput` | document | +| NUL byte in dump | 500 (`read_to_string` fails) | 400 | document | +| Statement > `max_statement_bytes` | accepted (unbounded) | 413 | new test, streaming only | +| Row-returning statement (stray `SELECT`) | 500 (`ExecuteReturnedResults`) | executed, rows discarded | document | +| Executed SQL text | `cmd.to_string()` | original bytes | §8.5 (7). *As built:* the equivalence test showed the two `/dump` outputs differ in **DDL text only** — buffered stores the parser's normalized rendering (`ON plain (a)`, multi-line trigger body), streaming stores the dump's DDL verbatim. Data rows are identical, so validation compares `INSERT`/`DELETE` lines and asserts the streaming DDL matches the source. | +| Trigger / CASE / nested CASE (existing tests) | 200 | 200 | yes | +| Peak memory | O(dump size) | O(config + largest statement) | §15 | + +--- + +## 11. Memory model (streaming) + +| Component | Bound | Default | +|---|---|---| +| Last stream chunk | chunk size | ≤ 64 KiB | +| Framer pending buffer | `max_statement_bytes` (+ one chunk transiently) | 64 MiB cap; typically a few KiB | +| Channel payload | `min(queue_bytes + one statement, queue_depth × stmt)` | 16 MiB | +| Per-statement parse AST | O(statement) | transient | +| SQLite page cache | `cache_size` pragma, independent of importer | unchanged | +| Replication log / WAL | on disk, flushed per spill (`replication_logger_wal.rs`) | disk ≈ 2× DB size during import | + +Expected steady-state importer RSS: < 32 MiB regardless of dump size, versus ≈ 2× dump size for buffered (`String` + lowercase copy during the ATTACH check). + +Disk: the single dump transaction means the SQLite WAL and the replication log each grow to roughly the database size before `COMMIT`; the auto-checkpoint runs after commit. Identical for both importers; worth stating in the runbook. + +--- + +## 12. Observability + +Logging (in the dispatcher, `tracing`): + +- start: `info!(namespace, importer, "loading dump")` +- success: `info!(namespace, importer, statements, skipped, bytes, max_statement_bytes, elapsed_ms, "dump loaded")` +- failure: `warn!(namespace, importer, error = %e, elapsed_ms, "dump load failed; transaction rolled back")` + +Metrics (`libsql-server/src/metrics.rs` style; `metrics` 0.21 macros with labels): + +- `libsql_server_dump_import_duration_seconds{importer}` histogram +- `libsql_server_dump_import_bytes{importer}` counter +- `libsql_server_dump_import_statements{importer}` counter +- `libsql_server_dump_import_failures{importer, kind}` counter, `kind ∈ {stream, parse, exec, txn, limit}` +- `libsql_server_dump_import_max_statement_bytes{importer}` histogram + +--- + +## 13. Pre-existing issues this design does not fix (state them in the PR) + +1. `NamespaceStore::create` stores the namespace config in the metastore **before** loading. On import failure the directory is removed (`PrimaryConfigurator::setup`) but the metastore row stays, so a later request to the namespace lazily creates an **empty** database. Existing tests rely on this (`select … from test` errors because the table is missing, not because the namespace is missing). +2. If the Admin HTTP request is cancelled mid-import, `setup`'s cleanup does not run (the future is dropped). The streaming executor still rolls back SQLite state, but the directory and `.sentinel` remain. +3. No server-side timeout exists for dump imports; the Admin HTTP request stays open for the whole import. Clients must not time out, or must tolerate (2). +4. Bottomless, if enabled, has its own frame buffering/backpressure outside this design. + +These are tracked by the DB Mover quarantine/fence work (Retail #35846/#35848). + +--- + +## 14. Testing + +### 14.1 Unit tests — `framer.rs` + +Use a helper that feeds a dump in every chunk size from 1 to N (`for cs in 1..=input.len()`) and asserts identical frames. Cases: + +1. Two simple statements; `;` at the very end of a chunk; `;` as the first byte of a chunk. +2. `;` inside a string literal `'a;b'`, inside `"quoted;ident"`, inside `-- comment;\n`, inside `/* block ; comment */`. +3. `CREATE TRIGGER … BEGIN INSERT …; UPDATE …; END;` → one frame. +4. Multibyte UTF-8 (`'żółć'`, emoji) split across chunk boundaries → frames are valid UTF-8. +5. Empty statements `;;` and `;\n;` → frames emitted; concatenation reproduces input. +6. `finish()` returns `None` after a trailing `;`, returns the tail for `COMMIT` without `;` and for `-- trailing comment\n`. +7. NUL byte → `NulByte` with correct line/column. +8. Oversized pending statement → `StatementTooLarge` at the correct position; a statement exactly at the limit passes. +9. Position tracking: construct a multi-line dump, assert each frame's `(line, column)`; assert `absolute_position` on the §7.6 worked example. +10. `is_complete_statement` directly: `"SELECT 1;"` true, `"SELECT ';"` false, `"CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;"` false, `"… END;"` true, `";"` true. + +### 14.2 Integration tests — `libsql-server/tests/namespaces/dumps.rs` + +Add helper: + +```rust +async fn create_from_dump(client: &Client, ns: &str, dump_url: String, importer: Option<&str>) -> Response { + let mut body = json!({ "dump_url": dump_url }); + if let Some(i) = importer { body["dump_importer"] = json!(i); } + client.post(&format!("http://primary:9090/v1/namespaces/{ns}/create"), body).await.unwrap() +} +``` + +Refactor each existing test body into `fn _with(importer: Option<&str>)` and keep the existing `#[test] fn ()` calling `_with(None)` (snapshots untouched), plus `#[test] fn _streaming()` calling `_with(Some("streaming"))`. Streaming variants that need a snapshot use `insta::assert_snapshot!("", value)` (named snapshot) when the message is identical, otherwise a new snapshot. + +New tests (streaming unless noted): + +- `streaming_chunked_http_delivery`: the turmoil `dump-store` host serves the dump as `hyper::Body::wrap_stream(futures::stream::iter(chunks))` with 1-, 3- and 7-byte chunks; assert 200 and row count. End-to-end framing test. +- `streaming_truncated_http_body`: body stream yields half the dump then `Err(io::Error)` → 500; `select count(*) from test` fails. +- `streaming_empty_statements`, `streaming_commit_without_semicolon`, `streaming_semicolon_in_string_and_comment`. +- `streaming_attach_statement_rejected` (`ATTACH 'x.db' AS x;`) → 400 with the legacy message; `streaming_word_attach_in_data_accepted`. +- `streaming_statement_too_large` → 413, requires a server with `DbConfig { dump_import: DumpImportConfig { max_statement_bytes: 1024, .. } }` — add `make_primary_with_db_config(sim, path, DbConfig)` beside `make_primary`. +- `dump_importer_without_dump_url` → 400; `dump_importer_unknown_value` → 400. +- `server_default_streaming`: server started with `default_importer: Streaming`, request omits `dump_importer`, dump whose data contains the word `'attachment'` succeeds (buffered would reject it with 400 — proves streaming was used). +- `importers_produce_identical_databases`: same dump (triggers, views, indexes, `sqlite_sequence`, text with quotes/newlines, blobs, REAL values like `1.0e10`, a `WITHOUT ROWID` table, a table whose rowid must be preserved) into `ns_buffered` and `ns_streaming`; `GET /dump?preserve_row_ids=true` on both; assert identical data lines, assert the streaming output contains the source DDL verbatim, snapshot the streaming output (*as built*; see §10 for why not byte-equal). +- `streaming_large_dump`: generate 200k single-row INSERTs into a temp file; assert count. Guards against accidental O(n²) in framing. + +### 14.3 Lints + +`cargo clippy -p libsql-server -- -D warnings` and `cargo fmt` (toolchain 1.98.1, workspace enforces `-D warnings`). + +--- + +## 15. Benchmark and validation on one instance + +1. Start one server: `sqld --enable-namespaces --admin-listen-addr 127.0.0.1:9090 --dump-importer buffered` (defaults). Record `PID`. +2. Dataset: export a representative namespace with `GET /dump?preserve_row_ids=true` (or `sqlite3 x.db .dump`) into `/tmp/dump.sql`; sizes 100 MB, 1 GB, 4 GB. +3. For `imp in streaming buffered` (streaming first — `VmHWM` is a lifetime high-water mark): + - background sampler: every 200 ms append `date +%s%3N, $(grep VmRSS /proc/$PID/status)` to `rss_$imp.csv`; + - `time curl -sS -X POST localhost:9090/v1/namespaces/ns_$imp/create -H 'content-type: application/json' -d "{\"dump_url\":\"file:///tmp/dump.sql\",\"dump_importer\":\"$imp\"}"`; + - stop sampler; record `max(VmRSS) − baseline`, wall time, `du -sh data.sqld/dbs/ns_$imp`. +4. Validation: + - `curl -H 'x-namespace: ns_buffered' localhost:8080/dump?preserve_row_ids=true > a.sql`; same for `ns_streaming` → the `INSERT`/`DELETE` lines must be identical (DDL text legitimately differs, §10); + - `PRAGMA integrity_check` via hrana (`POST /v2/pipeline`) on both → `ok`; + - compare `libsql_server_dump_import_*` metrics from `/metrics`. +5. Acceptance: streaming peak RSS delta plateaus (< 64 MiB) across the three sizes while buffered scales ≈ 2× dump; streaming wall time ≤ buffered wall time; dumps byte-identical. + +The script is `scripts/bench-dump-import.sh`; it implements steps 3–4 against a running server and prints a results table. + +First run (*as built*; debug build, macOS arm64, 36 MB dump, 300 007 statements, one `sqld` instance, importers run back to back): + +| importer | HTTP | wall | RSS before → peak | Δ RSS | +|---|---|---|---|---| +| streaming | 200 | 15.2 s | 38 MB → 54 MB | **+15 MB** | +| buffered | 200 | 19.7 s | 56 MB → 195 MB | **+135 MB** (≈ 3.7× the dump) | + +Both namespaces: `PRAGMA integrity_check` = `ok`, 300 300 identical data rows. The buffered importer had earlier rejected a variant of the same dump whose data contained the word "attachment" (§10). + +--- + +## 16. Implementation plan (ordered; each step compiles and passes tests) + +**Step 1 — refactor, no behavior change** +- Create `namespace/dump_import/{mod.rs,buffered.rs}`; move `load_dump` verbatim to `buffered::load_dump_buffered`, returning `DumpImportStats`. +- Add `DumpImporterKind`, `DumpImportConfig`, `DumpImportStats`, dispatcher `load_dump` (streaming arm temporarily `unimplemented!()`-free: return `Internal("streaming importer not available")`). +- `DumpSource`, `RestoreOption::Dump(DumpSource)`; update `helpers.rs` match; `admin/mod.rs` builds `DumpSource { stream, importer: req.dump_importer }`. +- `DbConfig.dump_import`, `BaseNamespaceConfig.dump_import`, `lib.rs` plumbing, `scheduler.rs` test constructors, CLI flags in `main.rs`, `make_db_config` validation. +- New `LoadDumpError` variants: `ImporterWithoutDumpUrl` (400), `StatementTooLarge { line: u64, limit: usize }` (413, `StatusCode::PAYLOAD_TOO_LARGE`). Update `IntoResponse for &LoadDumpError`. +- `ReaderStream::with_capacity(f, 64 * 1024)`. +- Run the existing dump tests: all green, snapshots unchanged. + +**Step 2 — framer** +- `framer.rs` per §7 with the unit tests of §14.1. Add `memchr` dependency. + +**Step 3 — streaming importer** +- `streaming.rs` per §8–9; wire the dispatcher arm; `From for LoadDumpError`. +- Integration tests of §14.2; `make_primary_with_db_config`. +- Metrics + logging (§12). +- Docs: `docs/ADMIN_API.md` (`dump_importer`), `docs/USER_GUIDE.md` or `docs/BUILD-RUN.md` (flags), note the parity matrix differences. + +**Step 4 — benchmark** +- Script + results table in the PR; decide whether to flip `--dump-importer` default to `streaming` in a follow-up. Removal of the buffered importer is a later, separate change after production soak. + +File change list: + +| File | Change | +|---|---| +| `libsql-server/Cargo.toml` | `memchr = "2"` | +| `libsql-server/src/namespace/dump_import/{mod,buffered,framer,streaming}.rs` | new | +| `libsql-server/src/namespace/mod.rs` | `pub(crate) mod dump_import;`, `DumpSource`, `RestoreOption::Dump(DumpSource)` | +| `libsql-server/src/namespace/configurator/helpers.rs` | remove `load_dump` + its imports; new match arm | +| `libsql-server/src/namespace/configurator/mod.rs` | `BaseNamespaceConfig.dump_import` | +| `libsql-server/src/config.rs` | `DbConfig.dump_import` | +| `libsql-server/src/lib.rs` | copy into `BaseNamespaceConfig` | +| `libsql-server/src/main.rs` | 4 flags, `make_db_config`, validation | +| `libsql-server/src/http/admin/mod.rs` | `dump_importer` field, check, `DumpSource`, `ReaderStream::with_capacity` | +| `libsql-server/src/error.rs` | 2 variants + response mapping | +| `libsql-server/src/metrics.rs` | 5 metrics | +| `libsql-server/src/schema/scheduler.rs` | test `BaseNamespaceConfig` literals | +| `libsql-server/tests/namespaces/{mod,dumps}.rs` + snapshots | helpers, variants, new tests | +| `docs/ADMIN_API.md`, `docs/USER_GUIDE.md` | docs | + +--- + +## 17. Risks and open questions + +- **Oracle/parser disagreement.** `sqlite3_complete` and `sqlite3_parser` are two tokenizers. If they ever disagree on where a statement ends, the result is a parse error (fail closed) or the §8.5 step 2 guard, never silent corruption. Known shared rules: strings, quoted identifiers, `--`/`/* */` comments, `CREATE [TEMP] TRIGGER … END`. +- **Per-statement `Parser` allocation.** `Parser::new` allocates a lemon stack per call. If profiling shows it matters, reuse via `Parser::reset` behind a small wrapper; not needed for correctness. +- **Original-text execution vs AST text.** Deliberate (§8.5). If an A/B shows a divergence, the `/dump` diff in §15 will surface it; the fix belongs in the parser, not in the importer. +- **`EXPLAIN`/row-returning statements** are executed and their rows discarded; the buffered importer fails on them. Acceptable and documented. +- **413 vs 400 for oversized statements.** Chosen 413 so operators can tell "raise the cap" from "fix the dump". Revisit if admin clients treat 413 specially. +- **Default flip timing.** Keep `buffered` as default until the §15 acceptance criteria are met on production-shaped data (settings and catalog schemas, FTS tables included). diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index d23892bc80..dbb6ea99da 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -246,6 +246,32 @@ For example, to create a database named `db1`, send the following HTTP request: curl -X POST http://localhost:8080/v1/namespaces/db1/create ``` +### Creating a database from a SQLite dump + +A new database can be initialized from a SQL dump produced by `sqlite3 source.db .dump` (or by +this server's `GET /dump` endpoint): + +```shell +sqlite3 source.db .dump > /srv/dumps/db1.sql +curl -X POST http://localhost:9090/v1/namespaces/db1/create \ + -H 'content-type: application/json' \ + -d '{"dump_url": "file:///srv/dumps/db1.sql", "dump_importer": "streaming"}' +``` + +Two importers are available. `buffered` (the default) reads the whole dump into memory before +executing it. `streaming` executes statements as they arrive and keeps memory usage independent +of the dump size; it is selected per request with `dump_importer` or server-wide with +`--dump-importer streaming` (`SQLD_DUMP_IMPORTER`). The streaming importer is tuned with: + +- `--dump-import-max-statement-size` (`SQLD_DUMP_IMPORT_MAX_STATEMENT_SIZE`, default `64MiB`): + a single statement larger than this fails the import with HTTP 413. +- `--dump-import-queue-bytes` (`SQLD_DUMP_IMPORT_QUEUE_BYTES`, default `16MiB`) and + `--dump-import-queue-depth` (`SQLD_DUMP_IMPORT_QUEUE_DEPTH`, default `256`): how many + framed-but-not-yet-executed statements may be in flight. + +Either way the dump runs as one transaction, so the WAL and the replication log grow to roughly +the database size before the final `COMMIT`; make sure the disk has room for that. + The name of the database is determined from the `Host` header in the HTTP request. For example, if you have the following entries in your `/etc/hosts` file: diff --git a/scripts/bench-dump-import.sh b/scripts/bench-dump-import.sh new file mode 100755 index 0000000000..4e05577bfd --- /dev/null +++ b/scripts/bench-dump-import.sh @@ -0,0 +1,122 @@ +#!/usr/bin/env bash +# +# Compare the buffered and streaming dump importers on ONE running libsql-server instance. +# +# For each importer it creates a namespace from the same dump, samples the server's RSS while +# the import runs, records wall time and on-disk size, then checks that both namespaces export +# the same data and pass PRAGMA integrity_check. +# +# Usage: +# scripts/bench-dump-import.sh [admin_url] [user_host:port] [sqld_pid] +# +# dump.sql absolute path readable by the server (it is passed as a file: URL) +# admin_url default http://127.0.0.1:9090 +# user_host:port default 127.0.0.1:8080 (namespaces are selected with x-namespace) +# sqld_pid default: pgrep -x sqld (RSS sampling is skipped if unavailable) +# +# Start the server with e.g.: +# sqld --enable-namespaces --admin-listen-addr 127.0.0.1:9090 --http-listen-addr 127.0.0.1:8080 +# +# Linux reads VmRSS from /proc; macOS falls back to `ps -o rss`. + +set -euo pipefail + +DUMP=${1:?usage: $0 [admin_url] [user_host:port] [sqld_pid]} +ADMIN=${2:-http://127.0.0.1:9090} +USER_HOST=${3:-127.0.0.1:8080} +PID=${4:-$(pgrep -x sqld | head -n1 || true)} +RUN_ID=$(date +%s) +OUT=${BENCH_OUT:-/tmp/bench-dump-import-$RUN_ID} +mkdir -p "$OUT" + +case "$DUMP" in + /*) ;; + *) echo "dump path must be absolute: $DUMP" >&2; exit 2 ;; +esac + +rss_kb() { + if [ -z "$PID" ]; then echo 0; return; fi + if [ -r "/proc/$PID/status" ]; then + awk '/^VmRSS:/ {print $2}' "/proc/$PID/status" + else + ps -o rss= -p "$PID" | tr -d ' ' + fi +} + +sample_rss() { # $1 = output csv; samples every 200ms until killed + while :; do + printf '%s,%s\n' "$(date +%s%3N 2>/dev/null || python3 -c 'import time;print(int(time.time()*1000))')" "$(rss_kb)" + sleep 0.2 + done >"$1" +} + +import_with() { # $1 = importer, $2 = namespace + local importer=$1 ns=$2 sampler= + local csv="$OUT/rss_$importer.csv" + local baseline; baseline=$(rss_kb) + if [ -n "$PID" ]; then sample_rss "$csv" & sampler=$!; fi + local start; start=$(date +%s.%N) + local code + code=$(curl -sS -o "$OUT/create_$importer.json" -w '%{http_code}' \ + -X POST "$ADMIN/v1/namespaces/$ns/create" \ + -H 'content-type: application/json' \ + -d "{\"dump_url\":\"file://$DUMP\",\"dump_importer\":\"$importer\"}") + local end; end=$(date +%s.%N) + if [ -n "$sampler" ]; then kill "$sampler" 2>/dev/null || true; wait "$sampler" 2>/dev/null || true; fi + local peak=0 + if [ -s "$csv" ]; then peak=$(cut -d, -f2 "$csv" | sort -n | tail -n1); fi + printf '%s\t%s\t%s\t%.2f\t%s\t%s\t%s\n' \ + "$importer" "$ns" "$code" "$(echo "$end - $start" | bc)" "$baseline" "$peak" "$(( (peak - baseline) / 1024 ))" \ + >>"$OUT/results.tsv" + [ "$code" = 200 ] || { echo "import with $importer failed ($code): $(cat "$OUT/create_$importer.json")" >&2; exit 1; } +} + +data_lines() { # print the INSERT/DELETE lines of a dump that are not inside a CREATE statement + awk ' + mode == 0 { + if ($0 ~ /^CREATE (TEMP |TEMPORARY )?TRIGGER/) { if ($0 !~ /END;[[:space:]]*$/) mode = 2; next } + if ($0 ~ /^CREATE /) { if ($0 !~ /;[[:space:]]*$/) mode = 1; next } + if ($0 ~ /^(INSERT INTO|DELETE FROM)/) print + next + } + mode == 1 && $0 ~ /;[[:space:]]*$/ { mode = 0 } + mode == 2 && $0 ~ /END;[[:space:]]*$/ { mode = 0 } + ' "$1" +} + +printf 'importer\tnamespace\thttp\twall_s\trss_baseline_kb\trss_peak_kb\trss_delta_mb\n' >"$OUT/results.tsv" + +# streaming first: on Linux VmHWM is a lifetime high-water mark, and the buffered importer's +# peak would otherwise hide the streaming one. +import_with streaming "bench_streaming_$RUN_ID" +import_with buffered "bench_buffered_$RUN_ID" + +echo +echo "== results ($OUT/results.tsv)" +column -t -s $'\t' "$OUT/results.tsv" + +echo +echo "== validation" +for importer in buffered streaming; do + ns="bench_${importer}_$RUN_ID" + curl -sS -H "x-namespace: $ns" "http://$USER_HOST/dump?preserve_row_ids=true" >"$OUT/dump_$importer.sql" + # data rows only: the buffered importer stores parser-normalized DDL text, the streaming one + # stores the dump's DDL verbatim, so schema lines may legitimately differ in whitespace + # (including a trigger body spread over several lines, whose INSERTs are not data). + data_lines "$OUT/dump_$importer.sql" >"$OUT/data_$importer.sql" + integrity=$(curl -sS -H "x-namespace: $ns" -H 'content-type: application/json' \ + -X POST "http://$USER_HOST/v2/pipeline" \ + -d '{"requests":[{"type":"execute","stmt":{"sql":"PRAGMA integrity_check"}},{"type":"close"}]}' \ + | python3 -c 'import json,sys; r=json.load(sys.stdin)["results"][0]; print(r["response"]["result"]["rows"][0][0]["value"] if r["type"]=="ok" else r)') + echo "$importer: integrity_check=$integrity rows=$(wc -l <"$OUT/data_$importer.sql") size=$(wc -c <"$OUT/dump_$importer.sql")" +done +if cmp -s "$OUT/data_buffered.sql" "$OUT/data_streaming.sql"; then + echo "data: identical" +else + echo "data: DIFFERENT (see $OUT/data_*.sql)" >&2 + exit 1 +fi + +echo +echo "== server metrics" +curl -sS "$ADMIN/metrics" | grep -E '^libsql_server_dump_import' || echo "(metrics disabled or unavailable)" From 967f8f637e8368287cc1b82b444b5544060d0979 Mon Sep 17 00:00:00 2001 From: Tomasz Szymczyszyn Date: Thu, 8 Oct 2026 00:17:10 +0200 Subject: [PATCH 5/6] libsql-server: address adversarial review of the streaming dump importer 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. --- docs/ADMIN_API.md | 16 +- docs/STREAMING_DUMP_IMPORT_DESIGN.md | 30 +- libsql-server/src/error.rs | 8 +- .../src/namespace/dump_import/complete.rs | 374 ++++++++++++++++++ .../src/namespace/dump_import/framer.rs | 272 +++++++++---- .../src/namespace/dump_import/mod.rs | 57 ++- .../src/namespace/dump_import/streaming.rs | 82 +++- libsql-server/tests/namespaces/dumps.rs | 314 ++++++++++++++- scripts/bench-dump-import.sh | 36 +- 9 files changed, 1059 insertions(+), 130 deletions(-) create mode 100644 libsql-server/src/namespace/dump_import/complete.rs diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index ecd5aabea3..c606effcf0 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -40,10 +40,18 @@ rejected. `--dump-import-max-statement-size` is rejected with `413`. When omitted, the server's `--dump-importer` setting (`SQLD_DUMP_IMPORTER`, default `buffered`) -applies. Both importers produce the same data; the streaming importer additionally stores the -schema SQL exactly as written in the dump, whereas the buffered importer stores the parser's -normalized rendering. A dump whose *data* contains the word "attach" is rejected by the buffered -importer (substring check) but accepted by the streaming one (statement-level check). +applies. Values are matched case-insensitively; an unknown value is rejected with `422`. + +Both importers produce the same data. Known differences: + +- the streaming importer stores the schema SQL exactly as written in the dump, whereas the + buffered importer stores the parser's normalized rendering; +- a dump whose *data* contains the word "attach" is rejected by the buffered importer (substring + check) but accepted by the streaming one (statement-level check); a standalone `DETACH` + statement is rejected with `400` by the streaming importer and fails at execution (`500`) with + the buffered one; +- invalid UTF-8 or NUL bytes yield `400` (buffered: `500`), and statements that return rows are + executed with their rows discarded (buffered: `500`). ```HTTP DELETE /v1/namespaces/:namespace diff --git a/docs/STREAMING_DUMP_IMPORT_DESIGN.md b/docs/STREAMING_DUMP_IMPORT_DESIGN.md index 73959c49d0..a5777e6175 100644 --- a/docs/STREAMING_DUMP_IMPORT_DESIGN.md +++ b/docs/STREAMING_DUMP_IMPORT_DESIGN.md @@ -2,7 +2,7 @@ Status: implemented (see `libsql-server/src/namespace/dump_import/`) · Target: `Shopify/libsql`, branch `v0.9.30-shopify-patches` · Related: Retail #35846, LibSQL DB Mover P0 -Sections marked *as built* record where the implementation refined the original proposal. +Sections marked *as built* record where the implementation refined the original proposal, including the changes made after the adversarial review of PR #51 (§18). This document is written so that an implementer can follow it step by step without re-deriving decisions. Every file, type, flag, error and test is named. Section 16 is the ordered implementation plan; sections 7–9 are the normative specifications. @@ -228,6 +228,8 @@ Turn an arbitrary sequence of byte chunks into complete SQL statements, each end ### 7.2 Completeness oracle +> *As built:* the FFI call below was the first implementation. Because `sqlite3_complete` has no resumable form, calling it for every candidate `;` costs O(statement length) each time, which is quadratic for statements with many interior semicolons (measured: a 1 MiB text value with 50k semicolons took 20 s of CPU on a tokio worker). The shipped framer instead uses `complete.rs`, a resumable Rust port of complete.c's tokenizer and 8×8 state machine (`CompletionScanner::find_statement_end`), so a dump is scanned exactly once. `sqlite3_complete` is kept only as the reference in a differential unit test (`framing_matches_sqlite3_complete`: random token soups × chunkings must frame identically). The rest of this section describes the semantics both implementations share. + ```rust use rusqlite::ffi::sqlite3_complete; // re-exported libsql_ffi binding: fn(*const c_char) -> c_int @@ -541,6 +543,7 @@ In `dump_stream_from_url` change `ReaderStream::new(f)` to `ReaderStream::with_c | `BEGIN`/`COMMIT` missing (`NoTxn`, `NoCommit`) | 400, same messages | 400, same messages | yes | | Syntax error | 400 `syntax error near 'X' at line L, column C` (absolute) | identical (absolute via §7.6) | yes | | `ATTACH 'f' AS x;` as a statement | 400 "attach statements are not allowed in dumps" (substring check) | 400, same message (AST check) | yes | +| standalone `DETACH x;` | passes the substring check, fails at execution → 500 | 400 (AST check) | documented | | Word "attach" inside data (`'attachment'`) | **400** (false positive) | 200 | new test, streaming only | | `ATTACH foo/bar.sql` without `;` (existing test) | 400 "attach statements are not allowed" | 400 `syntax error near 'COMMIT' …` | keep legacy test; add streaming test with a well-formed ATTACH | | Empty statement `;;`, comment-only segments | absorbed by the parser | empty frame skipped | new test, both | @@ -675,6 +678,8 @@ First run (*as built*; debug build, macOS arm64, 36 MB dump, 300 007 statements, Both namespaces: `PRAGMA integrity_check` = `ok`, 300 300 identical data rows. The buffered importer had earlier rejected a variant of the same dump whose data contained the word "attachment" (§10). +After the review fixes (§18), a 10 MB dump of 50 rows holding 200 KB CSS-like values (~5 000 interior semicolons each — the shape that was quadratic): streaming 0.56 s / +21 MB RSS, buffered 0.53 s / +32 MB RSS, identical data, `integrity_check` ok. The pre-fix framer needed ≈ 0.36 s *per row* for this shape. + --- ## 16. Implementation plan (ordered; each step compiles and passes tests) @@ -729,3 +734,26 @@ File change list: - **`EXPLAIN`/row-returning statements** are executed and their rows discarded; the buffered importer fails on them. Acceptable and documented. - **413 vs 400 for oversized statements.** Chosen 413 so operators can tell "raise the cap" from "fix the dump". Revisit if admin clients treat 413 specially. - **Default flip timing.** Keep `buffered` as default until the §15 acceptance criteria are met on production-shaped data (settings and catalog schemas, FTS tables included). + +--- + +## 18. Adversarial review of PR #51 — findings and resolutions + +Independent reviewers (scope, correctness, security, performance, testing, architecture, operations) plus a manual pass. Verdict before fixes: **NEEDS CHANGES** (one HIGH, several MEDIUM). All items below are resolved in the PR unless marked *deferred*. + +| Sev | Finding | Resolution | +|---|---|---| +| HIGH | Framing was O(n²) in interior semicolons and ran on a tokio worker: 1 MiB/50k `;` → 20 s CPU; a 200 KB CSS-like value → 0.36 s per row (perf, security, architecture reviewers; measured). | Resumable port of `sqlite3_complete` (`complete.rs`), differential-tested against the FFI; `semicolon_dense_statement_is_linear` test. Framing is now a single linear pass. | +| MEDIUM | Peak memory ≈ 2× largest statement: `to_vec()` copy while `buf` still held the bytes until `drain`. | Frames ≥ 1 MiB are handed over with `split_off`/`mem::replace` (no copy); `large_frames_are_handed_over_without_copy` test. | +| MEDIUM | `DumpImportConfig { queue_bytes: 0 }` / `{ queue_depth: 0 }` built without `validate()` panicked (`clamp(1, 0)`, `mpsc::channel(0)`). | Clamped at the point of use in `load_dump_streaming`. | +| MEDIUM | Failure log repeated the full error (which may quote dump SQL) at WARN, duplicating the HTTP layer's ERROR log. | WARN now logs the failure *category* plus partial progress (statements, bytes); the error text stays in the HTTP-layer log/response. | +| MEDIUM | No progress visibility or partial stats for multi-minute imports. | Executor logs `dump import in progress` every 10 s; failures report statements/bytes processed. | +| MEDIUM | `dump_importer` JSON value was case-sensitive while the CLI flag was lenient. | `Deserialize` now delegates to `FromStr` (case-insensitive, trimmed). | +| MEDIUM | Untested: WASM-table skip, empty dump, EOF without `;`, executor failure under backpressure, request cancellation. | Tests added for all five (cancellation test tolerates the pre-existing "namespace may or may not be registered" nondeterminism). | +| MEDIUM | Bench script: macOS `date +%N` prints garbage silently; hard `bc`/`python3` dependencies; bare `$(curl …)` under `set -e`; caller-supplied PID not verified. | `python3` for timestamps, `awk` arithmetic, explicit tool check, curl/`integrity_check` failures reported, PID must be a `sqld` process. | +| LOW | 413 message dropped the computed column. | `StatementTooLarge` carries `column`. | +| LOW | Bare `DETACH` is 400 (streaming) vs 500 (buffered) — undocumented difference. | Documented in ADMIN_API.md and §10. | +| LOW | Executor panic bypasses `rollback_best_effort`. | Not a correctness gap: unwinding drops the connection and SQLite rolls back on close; documented on `run_executor`. | +| *deferred* | Per-statement policy (WASM skip, `n_stmt > 2` rule) duplicated between importers. | Keep until the buffered importer is removed; the parameterized tests pin both. | +| *deferred* | `BLOCKING_RT` hosts one long-lived thread per concurrent streaming import with no admission control. | 50 000-thread pool and low create concurrency today; revisit with bulk provisioning. | +| *deferred* | Splitting the PR (refactor vs feature). | Commit 1 is the pure refactor and can be reviewed in isolation. | diff --git a/libsql-server/src/error.rs b/libsql-server/src/error.rs index 8f57ac7ce1..691730e4a0 100644 --- a/libsql-server/src/error.rs +++ b/libsql-server/src/error.rs @@ -299,9 +299,13 @@ pub enum LoadDumpError { #[error("`dump_importer` requires `dump_url`")] ImporterWithoutDumpUrl, #[error( - "A dump statement starting at line {line} exceeds the maximum allowed size ({limit} bytes)" + "A dump statement starting at line {line}, column {column} exceeds the maximum allowed size ({limit} bytes)" )] - StatementTooLarge { line: u64, limit: usize }, + StatementTooLarge { + line: u64, + column: usize, + limit: usize, + }, } impl ResponseError for LoadDumpError {} diff --git a/libsql-server/src/namespace/dump_import/complete.rs b/libsql-server/src/namespace/dump_import/complete.rs new file mode 100644 index 0000000000..2cb356b8ca --- /dev/null +++ b/libsql-server/src/namespace/dump_import/complete.rs @@ -0,0 +1,374 @@ +//! Resumable port of SQLite's `sqlite3_complete()` (complete.c). +//! +//! `sqlite3_complete()` decides whether a string "appears to be a complete SQL statement": it +//! ends with a `;` token that is not inside a string, quoted identifier or comment, and is not +//! the interior of a `CREATE [TEMP] TRIGGER ... BEGIN ... END;` body. The C function tokenizes its +//! whole input on every call and has no resumable form, so calling it once per candidate `;` +//! of a growing statement is quadratic. [`CompletionScanner`] keeps the tokenizer and state +//! machine state across calls instead, so a dump is scanned exactly once. +//! +//! The token classes, keyword detection, comment/quote/bracket handling and the transition +//! table are copied from complete.c; `find_statement_end` must agree with +//! `sqlite3_complete(&buf[start..=semicolon])` for every candidate `;` (see the differential +//! test in `framer.rs`). + +// Token classes (indices into TRANS rows). +const SEMI: usize = 0; +const WS: usize = 1; +const OTHER: usize = 2; +const EXPLAIN: usize = 3; +const CREATE: usize = 4; +const TEMP: usize = 5; +const TRIGGER: usize = 6; +const END: usize = 7; + +/// `START`: at the beginning or end of a statement. The scanner reports a terminating `;` when +/// the transition on it lands here. +const STATE_START: u8 = 1; + +/// Transition table from complete.c (states: 0 INVALID, 1 START, 2 NORMAL, 3 EXPLAIN, +/// 4 CREATE, 5 TRIGGER, 6 SEMI, 7 END). +const TRANS: [[u8; 8]; 8] = [ + // SEMI WS OTHER EXPLAIN CREATE TEMP TRIGGER END + [1, 0, 2, 3, 4, 2, 2, 2], // 0 INVALID + [1, 1, 2, 3, 4, 2, 2, 2], // 1 START + [1, 2, 2, 2, 2, 2, 2, 2], // 2 NORMAL + [1, 3, 3, 2, 4, 2, 2, 2], // 3 EXPLAIN + [1, 4, 2, 2, 2, 4, 5, 2], // 4 CREATE + [6, 5, 5, 5, 5, 5, 5, 5], // 5 TRIGGER + [6, 6, 5, 5, 5, 5, 5, 7], // 6 SEMI + [1, 7, 5, 5, 5, 5, 5, 5], // 7 END +]; + +/// Longest keyword the state machine cares about (`temporary`). +const MAX_KEYWORD_LEN: usize = 9; + +/// Where the tokenizer is inside a multi-byte construct that may span chunk boundaries. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Lex { + Normal, + /// Saw `/`; a following `*` opens a block comment, anything else makes `/` an OTHER token. + Slash, + /// Saw `-`; a following `-` opens a line comment, anything else makes `-` an OTHER token. + Dash, + /// Inside `-- ...`, until `\n`. + LineComment, + /// Inside `/* ... */`. + BlockComment, + /// Inside a block comment, just saw `*`. + BlockCommentStar, + /// Inside `[...]`. + Bracket, + /// Inside a quoted string/identifier delimited by this byte (`'`, `"` or `` ` ``). + Quote(u8), + /// Inside an identifier/keyword run. + Ident, +} + +#[derive(Debug, Clone)] +pub(super) struct CompletionScanner { + state: u8, + lex: Lex, + /// Bytes of the identifier being scanned, only kept while it could still be a keyword. + ident: [u8; MAX_KEYWORD_LEN], + /// Length of the current identifier; `MAX_KEYWORD_LEN + 1` once it is too long to be a + /// keyword. + ident_len: usize, +} + +impl Default for CompletionScanner { + fn default() -> Self { + Self::new() + } +} + +impl CompletionScanner { + pub fn new() -> Self { + Self { + state: 0, + lex: Lex::Normal, + ident: [0; MAX_KEYWORD_LEN], + ident_len: 0, + } + } + + /// Consume `bytes`. If a `;` that completes a statement is found, stop right after it and + /// return its index within `bytes`; the scanner is then positioned at the start of the next + /// statement and the caller must continue with `bytes[index + 1..]`. Otherwise all of + /// `bytes` is consumed and `None` is returned. + pub fn find_statement_end(&mut self, bytes: &[u8]) -> Option { + for (i, &b) in bytes.iter().enumerate() { + if self.step(b) { + return Some(i); + } + } + None + } + + /// Feed one byte. Returns `true` iff `b` is a `;` that completes a statement. + fn step(&mut self, b: u8) -> bool { + loop { + match self.lex { + Lex::Ident => { + if is_id_char(b) { + self.push_ident(b); + return false; + } + self.flush_ident(); + // fall through: `b` still has to be processed in the Normal state + } + Lex::Slash => { + self.lex = Lex::Normal; + if b == b'*' { + self.lex = Lex::BlockComment; + return false; + } + self.transition(OTHER); + // reprocess `b` + } + Lex::Dash => { + self.lex = Lex::Normal; + if b == b'-' { + self.lex = Lex::LineComment; + return false; + } + self.transition(OTHER); + // reprocess `b` + } + Lex::LineComment => { + if b == b'\n' { + self.lex = Lex::Normal; + self.transition(WS); + } + return false; + } + Lex::BlockComment => { + if b == b'*' { + self.lex = Lex::BlockCommentStar; + } + return false; + } + Lex::BlockCommentStar => { + if b == b'/' { + self.lex = Lex::Normal; + self.transition(WS); + } else if b != b'*' { + self.lex = Lex::BlockComment; + } + return false; + } + Lex::Bracket => { + if b == b']' { + self.lex = Lex::Normal; + self.transition(OTHER); + } + return false; + } + Lex::Quote(q) => { + if b == q { + self.lex = Lex::Normal; + self.transition(OTHER); + } + return false; + } + Lex::Normal => { + return match b { + b';' => { + self.transition(SEMI); + self.state == STATE_START + } + b' ' | b'\r' | b'\t' | b'\n' | 0x0c => { + self.transition(WS); + false + } + b'/' => { + self.lex = Lex::Slash; + false + } + b'-' => { + self.lex = Lex::Dash; + false + } + b'[' => { + self.lex = Lex::Bracket; + false + } + b'`' | b'"' | b'\'' => { + self.lex = Lex::Quote(b); + false + } + c if is_id_char(c) => { + self.lex = Lex::Ident; + self.ident_len = 0; + self.push_ident(c); + false + } + _ => { + self.transition(OTHER); + false + } + }; + } + } + } + } + + fn transition(&mut self, token: usize) { + self.state = TRANS[self.state as usize][token]; + } + + fn push_ident(&mut self, b: u8) { + if self.ident_len < MAX_KEYWORD_LEN { + self.ident[self.ident_len] = b; + self.ident_len += 1; + } else { + self.ident_len = MAX_KEYWORD_LEN + 1; + } + } + + fn flush_ident(&mut self) { + let token = if self.ident_len <= MAX_KEYWORD_LEN { + classify_keyword(&self.ident[..self.ident_len]) + } else { + OTHER + }; + self.ident_len = 0; + self.lex = Lex::Normal; + self.transition(token); + } +} + +/// `IdChar()` from tokenize.c: alphanumerics, `_`, `$` and every non-ASCII byte. +#[inline] +fn is_id_char(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' || b == b'$' || b >= 0x80 +} + +fn classify_keyword(ident: &[u8]) -> usize { + if ident.eq_ignore_ascii_case(b"create") { + CREATE + } else if ident.eq_ignore_ascii_case(b"trigger") { + TRIGGER + } else if ident.eq_ignore_ascii_case(b"temp") || ident.eq_ignore_ascii_case(b"temporary") { + TEMP + } else if ident.eq_ignore_ascii_case(b"end") { + END + } else if ident.eq_ignore_ascii_case(b"explain") { + EXPLAIN + } else { + OTHER + } +} + +#[cfg(test)] +mod test { + use super::*; + + /// Feed `sql` and report whether its final byte is a terminating `;` (and nothing earlier + /// was): the single-statement question `sqlite3_complete` answers. + fn complete(sql: &str) -> bool { + let mut s = CompletionScanner::new(); + match s.find_statement_end(sql.as_bytes()) { + Some(i) => i + 1 == sql.len(), + None => false, + } + } + + #[test] + fn mirrors_sqlite3_complete_on_known_inputs() { + for ok in [ + "SELECT 1;", + ";", + " \n ;", + "PRAGMA foreign_keys=OFF;", + "EXPLAIN SELECT 1;", + "SELECT 'a;b';", + "SELECT \"a;b\";", + "SELECT [a;b];", + "SELECT `a;b`;", + "CREATE TABLE t(x CHECK(x <> ';'));", + "INSERT INTO t VALUES(X'00ff');", + "/* a; */ SELECT 1;", + "-- c;\nSELECT 1;", + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; END;", + "create temp trigger t after insert on x begin select 1; end;", + "CREATE TEMPORARY TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; UPDATE y SET z = 1; END;", + "EXPLAIN CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; END;", + // CASE ... END inside the body does not end the trigger (END must follow a `;`) + "CREATE TRIGGER t AFTER INSERT ON x BEGIN UPDATE y SET z = CASE WHEN 1 THEN 1 END; END;", + // a trigger body statement ending in END right before `;` still needs ;END; + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT CASE WHEN 1 THEN 1 END; END;", + // `END` is only special in a trigger + "SELECT CASE WHEN 1 THEN 1 END;", + "CREATE TABLE end(x);", + "CREATE TABLE trigger_log(x);", + "/**/SELECT 1;", + "/* ** */SELECT 1;", + "SELECT 1 -- trailing\n;", + "SELECT 1/2;", + "SELECT 1-2;", + ] { + assert!(complete(ok), "expected complete: {ok:?}"); + } + for not in [ + "SELECT ';", + "SELECT \";", + "SELECT [;", + "-- c;", + "/* c;", + "/* c; *", + "/*/ c;", + "COMMIT", + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;", + "CREATE TEMP TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;", + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; END", + "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; END x;", + "SELECT 1;SELECT 2;", // two statements: the first `;` terminates + ] { + assert!(!complete(not), "expected incomplete: {not:?}"); + } + } + + #[test] + fn resumes_across_arbitrary_splits() { + let sql = "CREATE TRIGGER t AFTER INSERT ON x BEGIN /* ; */ SELECT 'a;b' -- ;\n; END; -- done\nSELECT 1;"; + let ends: Vec = sql + .bytes() + .enumerate() + .filter(|(_, b)| *b == b';') + .map(|(i, _)| i) + .collect(); + // semicolons: block comment, string, line comment, body statement, "END;", "SELECT 1;" + assert_eq!(ends.len(), 6); + let expected = vec![ends[4], ends[5]]; + for chunk in 1..=sql.len() { + let mut s = CompletionScanner::new(); + let mut found = Vec::new(); + let mut offset = 0; + for piece in sql.as_bytes().chunks(chunk) { + let mut from = 0; + while let Some(i) = s.find_statement_end(&piece[from..]) { + found.push(offset + from + i); + from += i + 1; + } + offset += piece.len(); + } + assert_eq!(found, expected, "chunk size {chunk}"); + } + } + + #[test] + fn identifier_classification() { + assert_eq!(classify_keyword(b"CREATE"), CREATE); + assert_eq!(classify_keyword(b"Trigger"), TRIGGER); + assert_eq!(classify_keyword(b"temp"), TEMP); + assert_eq!(classify_keyword(b"TEMPORARY"), TEMP); + assert_eq!(classify_keyword(b"end"), END); + assert_eq!(classify_keyword(b"explain"), EXPLAIN); + assert_eq!(classify_keyword(b"ends"), OTHER); + assert_eq!(classify_keyword(b"temporarily"), OTHER); + assert_eq!(classify_keyword(b""), OTHER); + assert!(is_id_char(b'$') && is_id_char(b'_') && is_id_char(0xc3) && is_id_char(b'9')); + assert!(!is_id_char(b';') && !is_id_char(b' ') && !is_id_char(0x0b)); + } +} diff --git a/libsql-server/src/namespace/dump_import/framer.rs b/libsql-server/src/namespace/dump_import/framer.rs index 3251557103..4c1beb5939 100644 --- a/libsql-server/src/namespace/dump_import/framer.rs +++ b/libsql-server/src/namespace/dump_import/framer.rs @@ -2,16 +2,16 @@ //! //! [`StatementFramer`] turns an arbitrary sequence of byte chunks into complete SQL statements, //! each ending at the `;` that terminates it, while only ever holding one unfinished statement -//! in memory. Statement boundaries are decided by SQLite's own `sqlite3_complete()`, so -//! semicolons inside string literals, quoted identifiers, comments and `CREATE TRIGGER ... END` -//! bodies are handled exactly like the `sqlite3` shell does. +//! in memory. Statement boundaries follow the rules of SQLite's `sqlite3_complete()` (via the +//! resumable port in [`super::complete`]), so semicolons inside string literals, quoted +//! identifiers, comments and `CREATE TRIGGER ... END` bodies are handled exactly like the +//! `sqlite3` shell does, in a single linear pass over the dump. //! //! See `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §7. -use std::ffi::c_char; - use memchr::{memchr, memchr_iter, memrchr}; -use rusqlite::ffi::sqlite3_complete; + +use super::complete::CompletionScanner; /// One complete statement (plus any whitespace/comments that preceded it in the dump), with the /// 1-based position of its first byte in the whole dump. @@ -36,13 +36,16 @@ pub(super) enum FrameError { }, } +/// Frames at least this large are handed over without copying (see `push`). +const LARGE_FRAME_BYTES: usize = 1024 * 1024; + pub(super) struct StatementFramer { /// Bytes after the last emitted frame; starts with the next statement's leading /// whitespace/comments. buf: Vec, - /// Offset in `buf` from which to look for the next `;`. Semicolons before it were already - /// tested and found not to terminate the statement. - scan_from: usize, + /// Number of leading bytes of `buf` the scanner has already consumed. + fed: usize, + scanner: CompletionScanner, /// 1-based line of `buf[0]` in the whole dump. line: u64, /// 1-based byte column of `buf[0]` in the whole dump. @@ -55,7 +58,8 @@ impl StatementFramer { pub fn new(max_statement_bytes: usize) -> Self { Self { buf: Vec::new(), - scan_from: 0, + fed: 0, + scanner: CompletionScanner::new(), line: 1, column: 1, max_statement_bytes, @@ -82,29 +86,37 @@ impl StatementFramer { self.buf.extend_from_slice(chunk); let mut frames = Vec::new(); + // `start` is the offset in `buf` of the statement being scanned; frames before it are + // removed from `buf` in one `drain` at the end rather than one per frame. let mut start = 0; - while let Some(rel) = memchr(b';', &self.buf[self.scan_from..]) { - let end = self.scan_from + rel; - if is_complete_statement(&mut self.buf, start, end) { - if end + 1 - start > self.max_statement_bytes { - return Err(self.too_large(&self.buf[start..=end])); - } - let sql = self.buf[start..=end].to_vec(); - frames.push(Frame { - sql, - line: self.line, - column: self.column, - }); - (self.line, self.column) = - advance_position((self.line, self.column), &self.buf[start..=end]); - start = end + 1; + while let Some(rel) = self.scanner.find_statement_end(&self.buf[self.fed..]) { + let end = self.fed + rel; + self.fed = end + 1; + let len = end + 1 - start; + if len > self.max_statement_bytes { + return Err(self.too_large(&self.buf[start..=end])); } - self.scan_from = end + 1; + let (line, column) = (self.line, self.column); + (self.line, self.column) = advance_position((line, column), &self.buf[start..=end]); + let sql = if start == 0 && len >= LARGE_FRAME_BYTES { + // A statement that spanned several pushes always starts at 0 (everything before + // it was drained by an earlier push). Hand its buffer over instead of copying it, + // so the peak is one copy of the largest statement, not two. + let rest = self.buf.split_off(end + 1); + self.fed -= end + 1; + std::mem::replace(&mut self.buf, rest) + } else { + start = end + 1; + self.buf[start - len..start].to_vec() + }; + frames.push(Frame { sql, line, column }); } + // Everything in `buf` has now been fed to the scanner exactly once. + self.fed = self.buf.len(); if start > 0 { self.buf.drain(..start); - self.scan_from -= start; + self.fed -= start; } if self.buf.len() > self.max_statement_bytes { @@ -138,36 +150,12 @@ impl StatementFramer { line: self.line, column: self.column, }; - self.scan_from = 0; + self.fed = 0; (self.line, self.column) = advance_position((self.line, self.column), &frame.sql); Some(frame) } } -/// Does `buf[start..=end]` (whose last byte is `;`) look like a complete SQL statement? -/// -/// `sqlite3_complete` needs a NUL-terminated string, so a terminator is temporarily placed right -/// after the `;`. The function only tokenizes; it does not parse or touch any database. -fn is_complete_statement(buf: &mut Vec, start: usize, end: usize) -> bool { - debug_assert_eq!(buf[end], b';'); - let pushed = if end + 1 == buf.len() { - buf.push(0); - true - } else { - false - }; - let saved = buf[end + 1]; - buf[end + 1] = 0; - // SAFETY: `buf[start..]` is NUL-terminated at `end + 1`, which is within bounds, and - // `sqlite3_complete` only reads the string. - let complete = unsafe { sqlite3_complete(buf[start..].as_ptr() as *const c_char) } != 0; - buf[end + 1] = saved; - if pushed { - buf.pop(); - } - complete -} - /// Position of the byte following `bytes`, given the position of its first byte. pub(super) fn advance_position((line, column): (u64, usize), bytes: &[u8]) -> (u64, usize) { match memrchr(b'\n', bytes) { @@ -412,37 +400,157 @@ mod test { assert_eq!(absolute_position((1, 1), (1, 1)), (1, 1)); } + /// The reference: SQLite's own `sqlite3_complete()` applied to `buf[start..=end]` for every + /// candidate `;`, i.e. the quadratic algorithm the resumable scanner replaces. + fn reference_frames(input: &[u8]) -> Vec> { + use std::ffi::{c_char, CString}; + let mut frames = Vec::new(); + let mut start = 0; + for end in memchr_iter(b';', input) { + let candidate = CString::new(&input[start..=end]).unwrap(); + // SAFETY: `candidate` is a valid NUL-terminated string that outlives the call. + let complete = + unsafe { rusqlite::ffi::sqlite3_complete(candidate.as_ptr() as *const c_char) }; + if complete != 0 { + frames.push(input[start..=end].to_vec()); + start = end + 1; + } + } + frames + } + + /// Tiny deterministic PRNG so the differential test needs no extra dependencies. + struct Lcg(u64); + impl Lcg { + fn next(&mut self) -> u64 { + self.0 = self + .0 + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + self.0 >> 33 + } + fn pick<'a, T>(&mut self, items: &'a [T]) -> &'a T { + &items[(self.next() as usize) % items.len()] + } + } + + /// Differential test: frame boundaries must match `sqlite3_complete` for random token soups + /// built from everything the state machine cares about, under every chunking. #[test] - fn completeness_oracle() { - fn complete(s: &str) -> bool { - let mut buf = s.as_bytes().to_vec(); - let end = buf.len() - 1; - is_complete_statement(&mut buf, 0, end) + fn framing_matches_sqlite3_complete() { + const PIECES: &[&str] = &[ + ";", + ";", + ";", + " ", + "\n", + "\t", + "\r", + "\x0c", + "\x0b", + "/", + "*", + "-", + "--", + "/*", + "*/", + "[", + "]", + "'", + "\"", + "`", + "''", + "CREATE", + "create", + "TEMP", + "Temporary", + "TRIGGER", + "END", + "end", + "EXPLAIN", + "BEGIN", + "SELECT", + "x", + "1", + "_a", + "$b", + "ends", + "temps", + "\u{17c}", + "é", + "(", + ")", + ",", + "=", + "+", + ".", + "CASE", + "WHEN", + "THEN", + ]; + let mut rng = Lcg(0x5eed); + for case in 0..2000 { + let n = 1 + (rng.next() as usize) % 40; + let mut input = String::new(); + for _ in 0..n { + input.push_str(rng.pick(PIECES)); + } + // make sure something terminates so the interesting path is exercised often + if case % 2 == 0 { + input.push(';'); + } + let bytes = input.as_bytes(); + let expected = reference_frames(bytes); + for chunk in [1usize, 2, 3, 7, bytes.len().max(1)] { + let mut framer = StatementFramer::new(LIMIT); + let mut got = Vec::new(); + for piece in bytes.chunks(chunk) { + got.extend(framer.push(piece).unwrap().into_iter().map(|f| f.sql)); + } + assert_eq!(got, expected, "input {input:?} chunk {chunk}"); + } + } + } + + /// A statement full of interior semicolons is scanned once, not once per semicolon. + #[test] + fn semicolon_dense_statement_is_linear() { + let semis = 200_000; + let mut s = String::from("INSERT INTO t VALUES('"); + for _ in 0..semis { + s.push_str("abc;"); + } + s.push_str("');"); + let mut framer = StatementFramer::new(1 << 30); + let started = std::time::Instant::now(); + let mut frames = 0; + for chunk in s.as_bytes().chunks(64 * 1024) { + frames += framer.push(chunk).unwrap().len(); + } + assert_eq!(frames, 1); + // ~800 KB; the quadratic version needed tens of seconds for this shape. + assert!( + started.elapsed() < std::time::Duration::from_secs(2), + "framing took {:?}", + started.elapsed() + ); + } + + #[test] + fn large_frames_are_handed_over_without_copy() { + let big = format!("INSERT INTO t VALUES('{}');", "x".repeat(LARGE_FRAME_BYTES)); + let mut framer = StatementFramer::new(1 << 30); + let mut frames = Vec::new(); + for chunk in big.as_bytes().chunks(64 * 1024) { + frames.extend(framer.push(chunk).unwrap()); } - assert!(complete("SELECT 1;")); - assert!(complete(";")); - assert!(complete(" \n ;")); - assert!(complete("PRAGMA foreign_keys=OFF;")); - assert!(complete("EXPLAIN SELECT 1;")); - assert!(complete("SELECT 'a;b';")); - assert!(complete("/* a; */ SELECT 1;")); - assert!(complete("-- c;\nSELECT 1;")); - assert!(complete( - "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1; END;" - )); - assert!(!complete("SELECT ';")); - assert!(!complete("-- c;")); - assert!(!complete( - "CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;" - )); - assert!(!complete( - "CREATE TEMP TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;" - )); - - // the oracle only looks at `buf[start..=end]` - let mut buf = b"SELECT 1;SELECT ';".to_vec(); - assert!(is_complete_statement(&mut buf, 0, 8)); - assert!(!is_complete_statement(&mut buf, 9, 17)); - assert_eq!(buf, b"SELECT 1;SELECT ';"); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].sql, big.as_bytes()); + // the pending buffer was replaced, not drained in place + assert!(framer.buf.capacity() < LARGE_FRAME_BYTES); + // and framing continues correctly afterwards + assert_eq!(framer.push(b"SELECT 1;").unwrap().len(), 1); + assert_eq!(framer.push(b"-- tail").unwrap().len(), 0); + assert_eq!(framer.finish().unwrap().sql, b"-- tail"); } } diff --git a/libsql-server/src/namespace/dump_import/mod.rs b/libsql-server/src/namespace/dump_import/mod.rs index 3ba9b7674d..16dbc2e261 100644 --- a/libsql-server/src/namespace/dump_import/mod.rs +++ b/libsql-server/src/namespace/dump_import/mod.rs @@ -22,11 +22,15 @@ use crate::error::LoadDumpError; use crate::namespace::{DumpStream, NamespaceName}; mod buffered; +mod complete; mod framer; mod streaming; /// Which implementation loads a dump into a fresh namespace. -#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +/// +/// Deserializes leniently (case-insensitive, surrounding whitespace ignored), like the CLI flag, +/// so `"Streaming"` means the same thing in a request body and in `SQLD_DUMP_IMPORTER`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize)] #[serde(rename_all = "snake_case")] pub enum DumpImporterKind { /// Read the entire dump into memory before executing it. @@ -64,6 +68,13 @@ impl FromStr for DumpImporterKind { } } +impl<'de> serde::Deserialize<'de> for DumpImporterKind { + fn deserialize>(deserializer: D) -> Result { + let s = >::deserialize(deserializer)?; + s.parse().map_err(serde::de::Error::custom) + } +} + /// Server-wide dump import settings. #[derive(Debug, Clone)] pub struct DumpImportConfig { @@ -133,6 +144,21 @@ pub struct DumpImportStats { pub elapsed: Duration, } +/// A failed import, together with how far it got before failing. +pub(super) struct ImportFailure { + pub error: LoadDumpError, + pub partial: DumpImportStats, +} + +impl From for ImportFailure { + fn from(error: LoadDumpError) -> Self { + Self { + error, + partial: DumpImportStats::default(), + } + } +} + /// Load `stream` into the fresh database behind `conn` using the requested importer. /// /// On error the dump transaction has been rolled back (or was never started). @@ -147,7 +173,9 @@ pub(crate) async fn load_dump( tracing::info!(namespace = %namespace, importer = %kind, "loading dump"); let res = match kind { - DumpImporterKind::Buffered => buffered::load_dump_buffered(stream, conn).await, + DumpImporterKind::Buffered => buffered::load_dump_buffered(stream, conn) + .await + .map_err(ImportFailure::from), DumpImporterKind::Streaming => streaming::load_dump_streaming(stream, conn, cfg).await, }; @@ -188,20 +216,25 @@ pub(crate) async fn load_dump( ); Ok(stats) } - Err(e) => { + Err(ImportFailure { error, partial }) => { + let kind_label = failure_kind(&error); metrics::increment_counter!( "libsql_server_dump_import_failures", "importer" => kind.as_str(), - "kind" => failure_kind(&e) + "kind" => kind_label ); + // The error itself (which may quote dump SQL) is logged by the HTTP layer when the + // response is built; here we record the category and how far the import got. tracing::warn!( namespace = %namespace, importer = %kind, - error = %e, + kind = kind_label, + statements = partial.statements_executed, + bytes = partial.bytes, elapsed_ms = elapsed.as_millis() as u64, "dump load failed; transaction rolled back" ); - Err(e) + Err(error) } } } @@ -298,7 +331,17 @@ mod test { serde_json::from_str::("\"streaming\"").unwrap(), DumpImporterKind::Streaming ); - assert!(serde_json::from_str::("\"Streaming\"").is_err()); + // same leniency as the CLI flag + assert_eq!( + serde_json::from_str::("\" Buffered \"").unwrap(), + DumpImporterKind::Buffered + ); + assert!(serde_json::from_str::("\"turbo\"").is_err()); + assert!(serde_json::from_str::("1").is_err()); + assert_eq!( + serde_json::to_string(&DumpImporterKind::Streaming).unwrap(), + "\"streaming\"" + ); } #[test] diff --git a/libsql-server/src/namespace/dump_import/streaming.rs b/libsql-server/src/namespace/dump_import/streaming.rs index 325cf99326..f6ca7845a0 100644 --- a/libsql-server/src/namespace/dump_import/streaming.rs +++ b/libsql-server/src/namespace/dump_import/streaming.rs @@ -12,6 +12,7 @@ //! See `docs/STREAMING_DUMP_IMPORT_DESIGN.md` §8–9. use std::sync::Arc; +use std::time::{Duration, Instant}; use fallible_iterator::FallibleIterator; use futures::StreamExt; @@ -27,7 +28,7 @@ use crate::namespace::DumpStream; use crate::BLOCKING_RT; use super::framer::{advance_position, Frame, FrameError, StatementFramer}; -use super::{map_exec_error, map_parse_error, DumpImportConfig, DumpImportStats}; +use super::{map_exec_error, map_parse_error, DumpImportConfig, DumpImportStats, ImportFailure}; enum Msg { Stmt { @@ -54,9 +55,15 @@ impl From for LoadDumpError { FrameError::NulByte { line, column } => LoadDumpError::InvalidSqlInput(format!( "dump contains a NUL byte at line {line}, column {column}" )), - FrameError::StatementTooLarge { line, limit, .. } => { - LoadDumpError::StatementTooLarge { line, limit } - } + FrameError::StatementTooLarge { + line, + column, + limit, + } => LoadDumpError::StatementTooLarge { + line, + column, + limit, + }, } } } @@ -65,10 +72,13 @@ pub(super) async fn load_dump_streaming( mut stream: DumpStream, conn: PrimaryConnection, cfg: &DumpImportConfig, -) -> Result { - let (tx, rx) = mpsc::channel::(cfg.queue_depth); - let budget = Arc::new(Semaphore::new(cfg.queue_bytes)); - let queue_bytes = cfg.queue_bytes; +) -> Result { + // `DumpImportConfig::validate` runs for CLI-built configs only; never panic on a + // programmatically built one (mpsc::channel(0) and clamp(1, 0) both panic). + let queue_depth = cfg.queue_depth.max(1); + let queue_bytes = cfg.queue_bytes.clamp(1, u32::MAX as usize); + let (tx, rx) = mpsc::channel::(queue_depth); + let budget = Arc::new(Semaphore::new(queue_bytes)); let executor = BLOCKING_RT.spawn_blocking(move || run_executor(conn, rx)); let mut framer = StatementFramer::new(cfg.max_statement_bytes); @@ -96,23 +106,28 @@ pub(super) async fn load_dump_streaming( // Guarantees the executor observes channel closure if we bailed out before sending `End`. drop(tx); - let exec = executor + let (exec, mut stats) = executor .await .map_err(|e| LoadDumpError::Internal(format!("dump executor task failed: {e}")))?; + stats.bytes = framer.bytes_seen(); - match (feed, exec) { - (Ok(()), exec) => exec.map(|mut stats| { - stats.bytes = framer.bytes_seen(); - stats - }), + let res = match (feed, exec) { + (Ok(()), exec) => exec, // The executor failed first and closed the channel; report its error. (Err(FeedError::ExecutorGone), Err(e)) => Err(e), // Can't happen: the executor only returns Ok after receiving `End`. - (Err(FeedError::ExecutorGone), Ok(_)) => Err(LoadDumpError::Internal( + (Err(FeedError::ExecutorGone), Ok(())) => Err(LoadDumpError::Internal( "dump executor finished before the dump was fully read".to_string(), )), // A stream or framing error wins over the executor's generic "ended before completion". (Err(FeedError::Source(e)), _) => Err(e), + }; + match res { + Ok(()) => Ok(stats), + Err(error) => Err(ImportFailure { + error, + partial: stats, + }), } } @@ -139,24 +154,37 @@ async fn send_frame( .map_err(|_| FeedError::ExecutorGone) } +/// How often the executor logs progress while an import is running. +const PROGRESS_LOG_INTERVAL: Duration = Duration::from_secs(10); + struct Executor { conn: PrimaryConnection, n_stmt: u64, skipped_wasm_table: bool, stats: DumpImportStats, + started: Instant, + last_progress_log: Instant, } /// Runs on a blocking thread for the whole import. Never cancelled by tokio: it always reaches /// its own COMMIT-or-ROLLBACK decision, even if the reader (and the admin request) go away. +/// (Should it panic instead, unwinding drops the connection, and SQLite rolls back an open +/// transaction when a connection is closed.) +/// +/// Returns the outcome together with the statistics accumulated so far, so a failure can report +/// how far the import got. fn run_executor( conn: PrimaryConnection, mut rx: mpsc::Receiver, -) -> Result { +) -> (Result<(), LoadDumpError>, DumpImportStats) { + let now = Instant::now(); let mut ex = Executor { conn, n_stmt: 0, skipped_wasm_table: false, stats: DumpImportStats::default(), + started: now, + last_progress_log: now, }; ex.conn.with_raw(|c| { @@ -173,16 +201,19 @@ fn run_executor( } ex.conn .with_raw(|c| c.authorizer(None::) -> Authorization>)); - res + (res, std::mem::take(&mut ex.stats)) } impl Executor { - fn run(&mut self, rx: &mut mpsc::Receiver) -> Result { + fn run(&mut self, rx: &mut mpsc::Receiver) -> Result<(), LoadDumpError> { loop { match rx.blocking_recv() { Some(Msg::Stmt { sql, line, column, .. - }) => self.handle_statement(&sql, line, column)?, + }) => { + self.handle_statement(&sql, line, column)?; + self.maybe_log_progress(); + } Some(Msg::End) => break, None => { // Reader dropped the sender without `End`: stream error, framing error or @@ -199,7 +230,18 @@ impl Executor { return Err(LoadDumpError::NoCommit); } - Ok(std::mem::take(&mut self.stats)) + Ok(()) + } + + fn maybe_log_progress(&mut self) { + if self.last_progress_log.elapsed() >= PROGRESS_LOG_INTERVAL { + self.last_progress_log = Instant::now(); + tracing::info!( + statements = self.stats.statements_executed, + elapsed_ms = self.started.elapsed().as_millis() as u64, + "dump import in progress" + ); + } } fn is_autocommit(&self) -> bool { diff --git a/libsql-server/tests/namespaces/dumps.rs b/libsql-server/tests/namespaces/dumps.rs index 8a759cef39..958d24daab 100644 --- a/libsql-server/tests/namespaces/dumps.rs +++ b/libsql-server/tests/namespaces/dumps.rs @@ -65,6 +65,15 @@ async fn count_rows(ns: &str, table: &str) -> anyhow::Result { /// Serve `chunks` as one HTTP response body on `dump-store:8080`, each chunk as its own body /// frame. A trailing `Err` chunk aborts the body mid-way. fn make_dump_store(sim: &mut Sim, chunks: Vec>) { + make_dump_store_paced(sim, chunks, Duration::from_millis(1)) +} + +/// Like [`make_dump_store`], pausing `pause` (simulated time) before each chunk. +fn make_dump_store_paced( + sim: &mut Sim, + chunks: Vec>, + pause: Duration, +) { // turmoil may call the host closure more than once; the chunks are cloned per call. let chunks: Vec> = chunks .into_iter() @@ -83,10 +92,11 @@ fn make_dump_store(sim: &mut Sim, chunks: Vec>) { async move { // Yield between chunks so hyper flushes each one before the // next (or an error) is produced. - let stream = futures::stream::iter(chunks).then(|c| async move { - tokio::time::sleep(Duration::from_millis(1)).await; - c.map_err(|(kind, msg)| std::io::Error::new(kind, msg)) - }); + let stream = + futures::stream::iter(chunks).then(move |c| async move { + tokio::time::sleep(pause).await; + c.map_err(|(kind, msg)| std::io::Error::new(kind, msg)) + }); Ok::<_, Infallible>(HyperResponse::new(Body::wrap_stream(stream))) } })) @@ -897,7 +907,7 @@ fn streaming_statement_too_large() { assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE); let body = resp.body_string().await?; assert!( - body.contains("starting at line 3 exceeds the maximum allowed size"), + body.contains("starting at line 3, column 1 exceeds the maximum allowed size"), "unexpected body: {body}" ); assert!(count_rows("foo", "test").await.is_err()); @@ -1198,3 +1208,297 @@ fn streaming_large_dump() { sim.run().unwrap(); } + +/// The first `CREATE TABLE libsql_wasm_func_table` of a dump is skipped by both importers and +/// does not disturb the "statement 3+ must be in a transaction" rule. +fn load_dump_skips_wasm_table_with(importer: Option<&'static str>) { + const DUMP: &str = r#" + PRAGMA foreign_keys=OFF; + BEGIN TRANSACTION; + CREATE TABLE libsql_wasm_func_table (name text PRIMARY KEY, body text) WITHOUT ROWID; + CREATE TABLE test (x); + INSERT INTO test VALUES(1); + COMMIT;"#; + + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + std::fs::write(tmp_path.join("dump.sql"), DUMP).unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + importer, + ) + .await?; + assert_eq!( + resp.status(), + StatusCode::OK, + "{}", + resp.body_string().await.unwrap_or_default() + ); + assert_eq!(count_rows("foo", "test").await?, 1); + // the wasm table itself was not created + assert!(count_rows("foo", "libsql_wasm_func_table").await.is_err()); + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn load_dump_skips_wasm_table() { + load_dump_skips_wasm_table_with(BUFFERED); +} + +#[test] +fn load_dump_skips_wasm_table_streaming() { + load_dump_skips_wasm_table_with(STREAMING); +} + +/// An empty (or comment-only) dump creates an empty namespace with both importers. +fn load_empty_dump_with(importer: Option<&'static str>) { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + std::fs::write(tmp_path.join("empty.sql"), "").unwrap(); + std::fs::write(tmp_path.join("comment.sql"), "-- nothing to see here\n").unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + for (ns, file) in [("empty", "empty.sql"), ("comment", "comment.sql")] { + let resp = + create_from_dump(&client, ns, &file_url(&tmp_path.join(file)), importer).await?; + assert_eq!( + resp.status(), + StatusCode::OK, + "{ns}: {}", + resp.body_string().await.unwrap_or_default() + ); + let db = Database::open_remote_with_connector( + &format!("http://{ns}.primary:8080"), + "", + TurmoilConnector, + )?; + let conn = db.connect()?; + let mut rows = conn.query("select count(*) from sqlite_schema", ()).await?; + assert_eq!(rows.next().await?.unwrap().get::(0)?, 0); + } + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn load_empty_dump() { + load_empty_dump_with(BUFFERED); +} + +#[test] +fn load_empty_dump_streaming() { + load_empty_dump_with(STREAMING); +} + +/// A dump whose final `COMMIT` has no trailing `;` (the framer's `finish()` tail) commits, and +/// a file that ends in the middle of a statement is rejected cleanly. +#[test] +fn streaming_eof_without_semicolon() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + std::fs::write( + tmp_path.join("ok.sql"), + "BEGIN TRANSACTION;\nCREATE TABLE test (x);\nINSERT INTO test VALUES(1);\nCOMMIT", + ) + .unwrap(); + std::fs::write( + tmp_path.join("cut.sql"), + "BEGIN TRANSACTION;\nCREATE TABLE test (x);\nINSERT INTO test VALUES(1);\nINSERT INTO test VAL", + ) + .unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "ok", + &file_url(&tmp_path.join("ok.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!(count_rows("ok", "test").await?, 1); + + let resp = create_from_dump( + &client, + "cut", + &file_url(&tmp_path.join("cut.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = resp.body_string().await?; + assert!( + body.contains("syntax error") && body.contains("line 4"), + "unexpected body: {body}" + ); + assert!(count_rows("cut", "test").await.is_err()); + Ok(()) + }); + + sim.run().unwrap(); +} + +/// Executor failure while the reader is blocked on a saturated queue: the executor must drop +/// its receiver so the queued permits are released and the reader observes the failure instead +/// of hanging. +#[test] +fn streaming_failure_under_backpressure() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + let mut dump = String::from("BEGIN TRANSACTION;\nCREATE TABLE test (x);\n"); + for i in 0..20_000 { + dump.push_str(&format!("INSERT INTO test VALUES({i});\n")); + } + // an oversized statement deep inside the dump + dump.push_str(&format!( + "INSERT INTO test VALUES('{}');\n", + "x".repeat(DumpImportConfig::MIN_MAX_STATEMENT_BYTES) + )); + for i in 0..20_000 { + dump.push_str(&format!("INSERT INTO test VALUES({i});\n")); + } + dump.push_str("COMMIT;\n"); + std::fs::write(tmp_path.join("dump.sql"), dump).unwrap(); + + make_primary_with_db_config( + &mut sim, + tmp.path().to_path_buf(), + DbConfig { + dump_import: DumpImportConfig { + max_statement_bytes: DumpImportConfig::MIN_MAX_STATEMENT_BYTES, + queue_bytes: DumpImportConfig::MIN_QUEUE_BYTES, + queue_depth: 4, + ..Default::default() + }, + ..Default::default() + }, + ); + + sim.client("client", async move { + let client = Client::new(); + let resp = create_from_dump( + &client, + "foo", + &file_url(&tmp_path.join("dump.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE); + assert!(count_rows("foo", "test").await.is_err()); + + // a statement-level failure (not a framing one) behind a full queue behaves the same + let mut dump = String::from("BEGIN TRANSACTION;\nCREATE TABLE test (x);\n"); + for i in 0..20_000 { + dump.push_str(&format!("INSERT INTO test VALUES({i});\n")); + } + dump.push_str("INSERT INTO nope VALUES(1);\n"); + for i in 0..20_000 { + dump.push_str(&format!("INSERT INTO test VALUES({i});\n")); + } + dump.push_str("COMMIT;\n"); + std::fs::write(tmp_path.join("dump2.sql"), dump).unwrap(); + let resp = create_from_dump( + &client, + "bar", + &file_url(&tmp_path.join("dump2.sql")), + STREAMING, + ) + .await?; + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + let body = resp.body_string().await?; + assert!( + body.contains("no such table: nope"), + "unexpected body: {body}" + ); + assert!(count_rows("bar", "test").await.is_err()); + Ok(()) + }); + + sim.run().unwrap(); +} + +/// The admin request is abandoned while the import is still streaming in. The executor thread +/// must notice the closed channel, roll back and release the connection, so the server stays +/// healthy and the namespace can be used afterwards instead of being wedged by a half-open +/// transaction. +#[test] +fn streaming_cancelled_request_rolls_back() { + let mut sim = sim(); + let tmp = tempdir().unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + // Deliver the dump slowly (64-byte chunks, 50ms apart) so the request is still in flight + // when it is abandoned. Few, large chunks also keep the number of unread segments below + // turmoil's simulated socket buffer once nobody reads them anymore. + make_dump_store_paced( + &mut sim, + CHUNKY_DUMP + .as_bytes() + .chunks(64) + .map(|c| Ok(Bytes::copy_from_slice(c))) + .collect(), + Duration::from_millis(50), + ); + + sim.client("client", async move { + let client = Client::new(); + let aborted = tokio::time::timeout( + Duration::from_millis(100), + create_from_dump(&client, "foo", "http://dump-store:8080/", STREAMING), + ) + .await; + assert!(aborted.is_err(), "the request should still be in flight"); + // Let the server notice the closed connection (simulated time) and the executor thread + // roll back and drop its connection (real time: the sim clock doesn't wait for it). + tokio::time::sleep(Duration::from_secs(2)).await; + std::thread::sleep(Duration::from_millis(300)); + + // Depending on where the request was cut, the namespace was registered (pre-existing + // behavior: the metastore row survives a failed create) or not. Either way it must be + // usable now: no lingering write transaction, no partial data. + let resp = client + .post_raw("http://primary:9090/v1/namespaces/foo/create", json!({})) + .await?; + assert!( + resp.status() == StatusCode::OK || resp.status() == StatusCode::BAD_REQUEST, + "unexpected status {}", + resp.status() + ); + assert!(count_rows("foo", "test").await.is_err()); + let db = + Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; + let conn = db.connect()?; + conn.execute("create table after_cancel (x)", ()).await?; + conn.execute("insert into after_cancel values (1)", ()) + .await?; + assert_eq!(count_rows("foo", "after_cancel").await?, 1); + + // and other namespaces are unaffected + let resp = create_from_dump(&client, "bar", "http://dump-store:8080/", STREAMING).await?; + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!(count_rows("bar", "test").await?, 3); + Ok(()) + }); + + sim.run().unwrap(); +} diff --git a/scripts/bench-dump-import.sh b/scripts/bench-dump-import.sh index 4e05577bfd..d71079dd78 100755 --- a/scripts/bench-dump-import.sh +++ b/scripts/bench-dump-import.sh @@ -17,10 +17,14 @@ # Start the server with e.g.: # sqld --enable-namespaces --admin-listen-addr 127.0.0.1:9090 --http-listen-addr 127.0.0.1:8080 # -# Linux reads VmRSS from /proc; macOS falls back to `ps -o rss`. +# Linux reads VmRSS from /proc; macOS falls back to `ps -o rss`. Requires curl and python3. set -euo pipefail +for tool in curl python3; do + command -v "$tool" >/dev/null 2>&1 || { echo "$tool is required" >&2; exit 2; } +done + DUMP=${1:?usage: $0 [admin_url] [user_host:port] [sqld_pid]} ADMIN=${2:-http://127.0.0.1:9090} USER_HOST=${3:-127.0.0.1:8080} @@ -34,18 +38,29 @@ case "$DUMP" in *) echo "dump path must be absolute: $DUMP" >&2; exit 2 ;; esac +if [ -n "$PID" ]; then + # Refuse to sample an unrelated process (e.g. a container-local PID passed from the host). + comm=$(ps -o comm= -p "$PID" 2>/dev/null | tr -d ' ' || true) + case "$comm" in + *sqld*) ;; + *) echo "PID $PID is not a sqld process (comm=${comm:-?}); RSS sampling disabled" >&2; PID= ;; + esac +fi + +now_ms() { python3 -c 'import time; print(int(time.time() * 1000))'; } + rss_kb() { if [ -z "$PID" ]; then echo 0; return; fi if [ -r "/proc/$PID/status" ]; then awk '/^VmRSS:/ {print $2}' "/proc/$PID/status" else - ps -o rss= -p "$PID" | tr -d ' ' + ps -o rss= -p "$PID" 2>/dev/null | tr -d ' ' || echo 0 fi } sample_rss() { # $1 = output csv; samples every 200ms until killed while :; do - printf '%s,%s\n' "$(date +%s%3N 2>/dev/null || python3 -c 'import time;print(int(time.time()*1000))')" "$(rss_kb)" + printf '%s,%s\n' "$(now_ms)" "$(rss_kb)" sleep 0.2 done >"$1" } @@ -55,18 +70,20 @@ import_with() { # $1 = importer, $2 = namespace local csv="$OUT/rss_$importer.csv" local baseline; baseline=$(rss_kb) if [ -n "$PID" ]; then sample_rss "$csv" & sampler=$!; fi - local start; start=$(date +%s.%N) + local start; start=$(now_ms) local code code=$(curl -sS -o "$OUT/create_$importer.json" -w '%{http_code}' \ -X POST "$ADMIN/v1/namespaces/$ns/create" \ -H 'content-type: application/json' \ - -d "{\"dump_url\":\"file://$DUMP\",\"dump_importer\":\"$importer\"}") - local end; end=$(date +%s.%N) + -d "{\"dump_url\":\"file://$DUMP\",\"dump_importer\":\"$importer\"}") \ + || { echo "request to $ADMIN failed (is the admin API reachable?)" >&2; exit 2; } + local end; end=$(now_ms) if [ -n "$sampler" ]; then kill "$sampler" 2>/dev/null || true; wait "$sampler" 2>/dev/null || true; fi local peak=0 if [ -s "$csv" ]; then peak=$(cut -d, -f2 "$csv" | sort -n | tail -n1); fi - printf '%s\t%s\t%s\t%.2f\t%s\t%s\t%s\n' \ - "$importer" "$ns" "$code" "$(echo "$end - $start" | bc)" "$baseline" "$peak" "$(( (peak - baseline) / 1024 ))" \ + printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ + "$importer" "$ns" "$code" "$(awk -v s="$start" -v e="$end" 'BEGIN { printf "%.2f", (e - s) / 1000 }')" \ + "$baseline" "$peak" "$(( (peak - baseline) / 1024 ))" \ >>"$OUT/results.tsv" [ "$code" = 200 ] || { echo "import with $importer failed ($code): $(cat "$OUT/create_$importer.json")" >&2; exit 1; } } @@ -107,7 +124,8 @@ for importer in buffered streaming; do integrity=$(curl -sS -H "x-namespace: $ns" -H 'content-type: application/json' \ -X POST "http://$USER_HOST/v2/pipeline" \ -d '{"requests":[{"type":"execute","stmt":{"sql":"PRAGMA integrity_check"}},{"type":"close"}]}' \ - | python3 -c 'import json,sys; r=json.load(sys.stdin)["results"][0]; print(r["response"]["result"]["rows"][0][0]["value"] if r["type"]=="ok" else r)') + | python3 -c 'import json,sys; r=json.load(sys.stdin)["results"][0]; print(r["response"]["result"]["rows"][0][0]["value"] if r["type"]=="ok" else r)') \ + || { echo "integrity_check request for $ns failed" >&2; exit 2; } echo "$importer: integrity_check=$integrity rows=$(wc -l <"$OUT/data_$importer.sql") size=$(wc -c <"$OUT/dump_$importer.sql")" done if cmp -s "$OUT/data_buffered.sql" "$OUT/data_streaming.sql"; then From ecb94314aa27e798ea3b41527d431b40707e027c Mon Sep 17 00:00:00 2001 From: Tomasz Szymczyszyn Date: Thu, 8 Oct 2026 15:33:37 +0200 Subject: [PATCH 6/6] docs: describe the linear dump statement scanner --- docs/ADMIN_API.md | 8 +- docs/STREAMING_DUMP_IMPORT_DESIGN.md | 160 ++++++++---------- .../src/namespace/dump_import/mod.rs | 7 +- 3 files changed, 77 insertions(+), 98 deletions(-) diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index c606effcf0..651f43ebae 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -34,10 +34,10 @@ rejected. - `buffered` (historical): the whole dump is read into memory, parsed, then executed. Memory usage is proportional to the dump size. -- `streaming`: statements are framed with `sqlite3_complete()` and executed while the dump is - still being read. Memory usage is bounded by the server's queue settings plus the largest - single statement (see `--dump-import-*` flags); a statement larger than - `--dump-import-max-statement-size` is rejected with `413`. +- `streaming`: statements are framed with a resumable port of SQLite's `sqlite3_complete()` state + machine and executed while the dump is still being read. Memory usage is bounded by the + server's queue settings plus the largest single statement (see `--dump-import-*` flags); a + statement larger than `--dump-import-max-statement-size` is rejected with `413`. When omitted, the server's `--dump-importer` setting (`SQLD_DUMP_IMPORTER`, default `buffered`) applies. Values are matched case-insensitively; an unknown value is rejected with `422`. diff --git a/docs/STREAMING_DUMP_IMPORT_DESIGN.md b/docs/STREAMING_DUMP_IMPORT_DESIGN.md index a5777e6175..525f947071 100644 --- a/docs/STREAMING_DUMP_IMPORT_DESIGN.md +++ b/docs/STREAMING_DUMP_IMPORT_DESIGN.md @@ -58,14 +58,14 @@ Where the WAL bytes go during the import transaction: `ReplicationLoggerWalWrapp ``` async (tokio runtime) blocking thread (BLOCKING_RT) DumpStream ──chunks──▶ StatementFramer ──frames──▶ mpsc(depth) + byte budget ──▶ StatementExecutor ──▶ PrimaryConnection -(hyper body / memchr(';') + (Semaphore permits UTF-8 check → sqlite3_parser → - tokio file) sqlite3_complete() travel with each frame) policy checks → execute original SQL +(hyper body / resumable complete.c (Semaphore permits UTF-8 check → sqlite3_parser → + tokio file) state machine travel with each frame) policy checks → execute original SQL → ROLLBACK on any failure ``` Three components, each independently testable: -- **`StatementFramer`** (sync, pure): accumulates bytes, emits complete SQL statements with their 1-based `(line, column)` in the original dump. Uses SQLite's own `sqlite3_complete()` so semicolons inside strings, comments and `CREATE TRIGGER … END` bodies are handled exactly as the `sqlite3` shell does. +- **`StatementFramer`** (sync, pure): accumulates bytes, emits complete SQL statements with their 1-based `(line, column)` in the original dump. Uses a resumable Rust port of SQLite's `complete.c` tokenizer and state machine so semicolons inside strings, comments and `CREATE TRIGGER … END` bodies are handled like `sqlite3_complete()`, but in one linear pass. A differential test checks the port against SQLite's function. - **Reader task** (async): drives the stream, feeds the framer, pushes frames into a bounded channel, sends an explicit `End` marker. - **`StatementExecutor`** (blocking, one thread for the whole import): owns the `PrimaryConnection`, validates/parses/executes each frame in order inside the dump's own transaction, enforces the transaction rules, rolls back on abort. @@ -156,7 +156,8 @@ libsql-server/src/namespace/dump_import/ ├── mod.rs DumpImporterKind, DumpImportConfig, DumpImportStats, pub(crate) async fn load_dump(...) dispatcher, │ shared error-mapping helpers (map_parse_error, map_exec_error), metrics/logging ├── buffered.rs legacy load_dump moved verbatim (renamed load_dump_buffered), returns DumpImportStats -├── framer.rs StatementFramer + is_complete_statement() + position helpers + unit tests +├── complete.rs resumable port of SQLite's complete.c tokenizer/state machine +├── framer.rs StatementFramer + position helpers + differential/unit tests └── streaming.rs load_dump_streaming (reader task) + run_executor (blocking) + Msg type ``` @@ -226,29 +227,18 @@ pub struct DumpImportStats { Turn an arbitrary sequence of byte chunks into complete SQL statements, each ending at the `;` that terminates it, without ever holding more than one unfinished statement in memory. -### 7.2 Completeness oracle +### 7.2 Completeness scanner -> *As built:* the FFI call below was the first implementation. Because `sqlite3_complete` has no resumable form, calling it for every candidate `;` costs O(statement length) each time, which is quadratic for statements with many interior semicolons (measured: a 1 MiB text value with 50k semicolons took 20 s of CPU on a tokio worker). The shipped framer instead uses `complete.rs`, a resumable Rust port of complete.c's tokenizer and 8×8 state machine (`CompletionScanner::find_statement_end`), so a dump is scanned exactly once. `sqlite3_complete` is kept only as the reference in a differential unit test (`framing_matches_sqlite3_complete`: random token soups × chunkings must frame identically). The rest of this section describes the semantics both implementations share. +`CompletionScanner` in `complete.rs` is a resumable Rust port of SQLite's `complete.c`. It carries two pieces of state across chunks: -```rust -use rusqlite::ffi::sqlite3_complete; // re-exported libsql_ffi binding: fn(*const c_char) -> c_int - -/// `buf[start..=end]` is a candidate statement whose last byte is b';'. -/// sqlite3_complete needs a NUL-terminated string, so a terminator is temporarily placed at end+1. -fn is_complete_statement(buf: &mut Vec, start: usize, end: usize) -> bool { - debug_assert_eq!(buf[end], b';'); - let pushed = if end + 1 == buf.len() { buf.push(0); true } else { false }; - let saved = buf[end + 1]; - buf[end + 1] = 0; - // SAFETY: buf[start..] is a NUL-terminated byte string; sqlite3_complete only reads it. - let complete = unsafe { sqlite3_complete(buf[start..].as_ptr() as *const _) } != 0; - buf[end + 1] = saved; - if pushed { buf.pop(); } - complete -} -``` +- lexical state (`Normal`, comments, bracket/quote, partial `/` or `-`, or an identifier spanning a chunk boundary); +- SQLite's 8×8 statement-completion state machine for `EXPLAIN`, `CREATE [TEMP] TRIGGER`, interior semicolons and `END;`. -Semantics of `sqlite3_complete` (sqlite3.h:2770–2805): returns non-zero iff the string ends with a semicolon token that is not inside a string/identifier/comment and the text is not an unfinished `CREATE TRIGGER … BEGIN … END`. It does not parse; it tokenizes only. Verified against the bundled SQLite (probe run on this branch): +`find_statement_end(bytes)` consumes each byte once and returns the first `;` that takes the state machine back to `START`. The caller resumes immediately after that byte for the next statement. The input therefore costs O(dump bytes), including statements with many semicolons in strings, comments or trigger bodies. + +The first implementation called SQLite's non-resumable `sqlite3_complete()` FFI function for every candidate `;`, rescanning from the statement start and becoming O(k·len) for *k* interior semicolons (measured: 20 s for 1 MiB/50k `;` on a tokio worker). The FFI remains only as the reference in `framing_matches_sqlite3_complete`: 2,000 deterministic random token soups under five chunkings must frame identically. + +The scanner matches these `sqlite3_complete` semantics (sqlite3.h:2770–2805): a statement ends with a semicolon token outside strings/identifiers/comments and not inside an unfinished `CREATE TRIGGER … BEGIN … END`. It tokenizes; it does not parse. | input | result | |---|---| @@ -264,8 +254,9 @@ Embedded NUL bytes would truncate the view, hence §7.4 rule 1. ```rust pub struct StatementFramer { - buf: Vec, // bytes after the last emitted frame; starts with the next statement's leading whitespace/comments - scan_from: usize, // offset in buf from which to look for the next ';' (avoids re-testing rejected candidates) + buf: Vec, // bytes after the last emitted frame + fed: usize, // leading bytes already consumed by scanner + scanner: CompletionScanner, line: u64, // 1-based line of buf[0] in the whole dump column: usize, // 1-based byte column of buf[0] max_statement_bytes: usize, @@ -274,32 +265,28 @@ pub struct StatementFramer { pub struct Frame { pub sql: Vec, pub line: u64, pub column: usize } -#[derive(Debug)] pub enum FrameError { NulByte { line: u64, column: usize }, StatementTooLarge { line: u64, column: usize, limit: usize }, } ``` -`new(max_statement_bytes)` → `line = 1, column = 1`, empty buffer. +`new(max_statement_bytes)` starts at `(line=1, column=1)` with an empty buffer and a fresh scanner. ### 7.4 `push(&mut self, chunk: &[u8]) -> Result, FrameError>` -1. If `memchr(0, chunk)` finds a NUL at offset `k`: compute its position (advance a copy of `(line, column)` over `buf[..]` then `chunk[..k]`) and return `NulByte`. Nothing is appended. -2. `bytes_seen += chunk.len()`; `buf.extend_from_slice(chunk)`. -3. `let mut start = 0; let mut frames = Vec::new();` -4. Loop: `let Some(rel) = memchr(b';', &buf[scan_from..]) else break; let end = scan_from + rel;` - - If `is_complete_statement(&mut buf, start, end)`: - - *as built:* if `end + 1 - start > max_statement_bytes` → `StatementTooLarge` (position = first non-whitespace byte of the statement, via `statement_start`) - - `frames.push(Frame { sql: buf[start..=end].to_vec(), line: self.line, column: self.column })` - - advance position over `buf[start..=end]` (§7.6) - - `start = end + 1; scan_from = start;` - - else `scan_from = end + 1;` -5. After the loop: `buf.drain(..start)` (one memmove per push, not per frame); `scan_from -= start`. -6. If `buf.len() > max_statement_bytes` → `StatementTooLarge { line, column, limit }` where `(line, column)` is the first non-whitespace byte of the pending statement (*as built*: `statement_start`, so the message points at the statement rather than at the end of the previous line). -7. Return `frames`. - -Complexity: each candidate `;` costs one `sqlite3_complete` scan from the statement start, so a statement containing *k* interior semicolons (trigger bodies, string literals) costs O(k·len). Normal dumps have k ≤ a few. A frame is emitted at the first terminating `;`, so a frame contains exactly one statement plus any leading whitespace/comments. +1. Reject the first NUL in the chunk with its exact absolute position. Nothing is appended. +2. Add the chunk length to `bytes_seen` and append it to `buf`. +3. Starting at `fed`, repeatedly ask `scanner.find_statement_end(...)` for the next terminating `;`. The scanner consumes every examined byte exactly once, including across calls to `push`. +4. For each complete frame: + - reject it if its length exceeds `max_statement_bytes`, reporting the first non-whitespace byte; + - record its absolute start position and advance `(line, column)` over it; + - copy normal frames; when a frame starts at buffer offset zero and is at least 1 MiB, hand over the existing allocation with `split_off`/`mem::replace` so peak memory remains one large-statement copy. +5. Drain all emitted small frames from `buf` in one operation and adjust `fed`. +6. If the pending unterminated bytes exceed the limit, return `StatementTooLarge` at the statement start. +7. Return the frames in input order. + +A frame contains exactly one statement plus any leading whitespace/comments. The framing pass is O(dump bytes); copying emitted frames adds O(dump bytes). ### 7.5 `finish(&mut self) -> Option` @@ -577,19 +564,22 @@ Disk: the single dump transaction means the SQLite WAL and the replication log e ## 12. Observability -Logging (in the dispatcher, `tracing`): +Logging (`tracing`): - start: `info!(namespace, importer, "loading dump")` +- progress every 10 s from the executor: `info!(statements, elapsed_ms, "dump import in progress")` - success: `info!(namespace, importer, statements, skipped, bytes, max_statement_bytes, elapsed_ms, "dump loaded")` -- failure: `warn!(namespace, importer, error = %e, elapsed_ms, "dump load failed; transaction rolled back")` +- failure: `warn!(namespace, importer, kind, statements, bytes, elapsed_ms, "dump load failed; transaction rolled back")` -Metrics (`libsql-server/src/metrics.rs` style; `metrics` 0.21 macros with labels): +The dispatcher intentionally omits the error text from the WARN because parser/execution errors may quote dump SQL and the HTTP layer already logs the response error. Failures retain partial streaming statistics. -- `libsql_server_dump_import_duration_seconds{importer}` histogram -- `libsql_server_dump_import_bytes{importer}` counter -- `libsql_server_dump_import_statements{importer}` counter -- `libsql_server_dump_import_failures{importer, kind}` counter, `kind ∈ {stream, parse, exec, txn, limit}` -- `libsql_server_dump_import_max_statement_bytes{importer}` histogram +Metrics (`metrics` 0.21 macros with labels): + +- `libsql_server_dump_import_duration_seconds{importer}` histogram (success or failure) +- `libsql_server_dump_import_bytes{importer}` counter (success) +- `libsql_server_dump_import_statements{importer}` counter (success) +- `libsql_server_dump_import_failures{importer, kind}` counter, `kind ∈ {txn, parse, exec, limit, other}` +- `libsql_server_dump_import_max_statement_bytes{importer}` histogram (success) --- @@ -606,50 +596,37 @@ These are tracked by the DB Mover quarantine/fence work (Retail #35846/#35848). ## 14. Testing -### 14.1 Unit tests — `framer.rs` +### 14.1 Unit tests — `complete.rs`, `framer.rs`, `mod.rs` -Use a helper that feeds a dump in every chunk size from 1 to N (`for cs in 1..=input.len()`) and asserts identical frames. Cases: +20 dump-import unit tests cover: -1. Two simple statements; `;` at the very end of a chunk; `;` as the first byte of a chunk. -2. `;` inside a string literal `'a;b'`, inside `"quoted;ident"`, inside `-- comment;\n`, inside `/* block ; comment */`. -3. `CREATE TRIGGER … BEGIN INSERT …; UPDATE …; END;` → one frame. -4. Multibyte UTF-8 (`'żółć'`, emoji) split across chunk boundaries → frames are valid UTF-8. -5. Empty statements `;;` and `;\n;` → frames emitted; concatenation reproduces input. -6. `finish()` returns `None` after a trailing `;`, returns the tail for `COMMIT` without `;` and for `-- trailing comment\n`. -7. NUL byte → `NulByte` with correct line/column. -8. Oversized pending statement → `StatementTooLarge` at the correct position; a statement exactly at the limit passes. -9. Position tracking: construct a multi-line dump, assert each frame's `(line, column)`; assert `absolute_position` on the §7.6 worked example. -10. `is_complete_statement` directly: `"SELECT 1;"` true, `"SELECT ';"` false, `"CREATE TRIGGER t AFTER INSERT ON x BEGIN SELECT 1;"` false, `"… END;"` true, `";"` true. +- completion-state behavior for ordinary statements, comments, quotes, brackets and `CREATE [TEMP] TRIGGER … END;`; +- chunk invariance (including one-byte chunks), multibyte UTF-8 splits, exact positions, NUL rejection, exact/over statement limits, empty statements and EOF tails; +- a differential oracle: 2,000 deterministic random token soups, each tested under five chunkings, must frame identically to SQLite's `sqlite3_complete()`; +- a semicolon-dense 1 MiB statement to guard the linear scanner path; +- allocation identity for the ≥1 MiB zero-copy frame hand-over; +- importer parsing and config validation. ### 14.2 Integration tests — `libsql-server/tests/namespaces/dumps.rs` -Add helper: - -```rust -async fn create_from_dump(client: &Client, ns: &str, dump_url: String, importer: Option<&str>) -> Response { - let mut body = json!({ "dump_url": dump_url }); - if let Some(i) = importer { body["dump_importer"] = json!(i); } - client.post(&format!("http://primary:9090/v1/namespaces/{ns}/create"), body).await.unwrap() -} -``` - -Refactor each existing test body into `fn _with(importer: Option<&str>)` and keep the existing `#[test] fn ()` calling `_with(None)` (snapshots untouched), plus `#[test] fn _streaming()` calling `_with(Some("streaming"))`. Streaming variants that need a snapshot use `insta::assert_snapshot!("", value)` (named snapshot) when the message is identical, otherwise a new snapshot. +39 dump integration tests run the historical cases against the buffered control and matching streaming variants where behavior should agree. Additional coverage includes: -New tests (streaming unless noted): +- HTTP delivery in 1-byte, 7-byte and whole-body chunks, plus a body error half-way through a transaction; +- trigger, CASE and nested-CASE bodies; syntax positions; `ATTACH` rejection; invalid UTF-8 and NUL input; +- configurable server default, `dump_importer` without `dump_url` (400), unknown importer (422), and the statement-size limit (413); +- equivalence of imported data across both implementations, including triggers, views, indexes, blobs, REAL values, rowids and `WITHOUT ROWID` tables; +- a dump much larger than the queue budget, executor failure behind a saturated queue, and cancellation of the admin request (rollback and connection release); +- empty/comment-only dumps, skipped `libsql_wasm_func_table`, and final `COMMIT` without a semicolon. -- `streaming_chunked_http_delivery`: the turmoil `dump-store` host serves the dump as `hyper::Body::wrap_stream(futures::stream::iter(chunks))` with 1-, 3- and 7-byte chunks; assert 200 and row count. End-to-end framing test. -- `streaming_truncated_http_body`: body stream yields half the dump then `Err(io::Error)` → 500; `select count(*) from test` fails. -- `streaming_empty_statements`, `streaming_commit_without_semicolon`, `streaming_semicolon_in_string_and_comment`. -- `streaming_attach_statement_rejected` (`ATTACH 'x.db' AS x;`) → 400 with the legacy message; `streaming_word_attach_in_data_accepted`. -- `streaming_statement_too_large` → 413, requires a server with `DbConfig { dump_import: DumpImportConfig { max_statement_bytes: 1024, .. } }` — add `make_primary_with_db_config(sim, path, DbConfig)` beside `make_primary`. -- `dump_importer_without_dump_url` → 400; `dump_importer_unknown_value` → 400. -- `server_default_streaming`: server started with `default_importer: Streaming`, request omits `dump_importer`, dump whose data contains the word `'attachment'` succeeds (buffered would reject it with 400 — proves streaming was used). -- `importers_produce_identical_databases`: same dump (triggers, views, indexes, `sqlite_sequence`, text with quotes/newlines, blobs, REAL values like `1.0e10`, a `WITHOUT ROWID` table, a table whose rowid must be preserved) into `ns_buffered` and `ns_streaming`; `GET /dump?preserve_row_ids=true` on both; assert identical data lines, assert the streaming output contains the source DDL verbatim, snapshot the streaming output (*as built*; see §10 for why not byte-equal). -- `streaming_large_dump`: generate 200k single-row INSERTs into a temp file; assert count. Guards against accidental O(n²) in framing. +### 14.3 Validation commands -### 14.3 Lints +- `cargo fmt --check -p libsql-server` +- `RUSTFLAGS="-D warnings --cfg tokio_unstable" cargo check -p libsql-server --all-targets` +- `cargo test -p libsql-server --lib dump_import` +- `cargo test -p libsql-server --test tests namespaces::dumps` +- clippy clean on the new importer files -`cargo clippy -p libsql-server -- -D warnings` and `cargo fmt` (toolchain 1.98.1, workspace enforces `-D warnings`). +CI uses nextest process isolation. A few unrelated turmoil tests also flake under in-process parallel `cargo test` on the base commit; they pass alone and are not changed here. --- @@ -665,7 +642,7 @@ New tests (streaming unless noted): - `curl -H 'x-namespace: ns_buffered' localhost:8080/dump?preserve_row_ids=true > a.sql`; same for `ns_streaming` → the `INSERT`/`DELETE` lines must be identical (DDL text legitimately differs, §10); - `PRAGMA integrity_check` via hrana (`POST /v2/pipeline`) on both → `ok`; - compare `libsql_server_dump_import_*` metrics from `/metrics`. -5. Acceptance: streaming peak RSS delta plateaus (< 64 MiB) across the three sizes while buffered scales ≈ 2× dump; streaming wall time ≤ buffered wall time; dumps byte-identical. +5. Acceptance: streaming peak RSS delta plateaus (< 64 MiB) across the three sizes while buffered scales ≈ 2× dump; streaming wall time ≤ buffered wall time; data lines are byte-identical and the streaming schema text matches the input (buffered normalizes DDL). The script is `scripts/bench-dump-import.sh`; it implements steps 3–4 against a running server and prints a results table. @@ -689,12 +666,13 @@ After the review fixes (§18), a 10 MB dump of 50 rows holding 200 KB CSS-like v - Add `DumpImporterKind`, `DumpImportConfig`, `DumpImportStats`, dispatcher `load_dump` (streaming arm temporarily `unimplemented!()`-free: return `Internal("streaming importer not available")`). - `DumpSource`, `RestoreOption::Dump(DumpSource)`; update `helpers.rs` match; `admin/mod.rs` builds `DumpSource { stream, importer: req.dump_importer }`. - `DbConfig.dump_import`, `BaseNamespaceConfig.dump_import`, `lib.rs` plumbing, `scheduler.rs` test constructors, CLI flags in `main.rs`, `make_db_config` validation. -- New `LoadDumpError` variants: `ImporterWithoutDumpUrl` (400), `StatementTooLarge { line: u64, limit: usize }` (413, `StatusCode::PAYLOAD_TOO_LARGE`). Update `IntoResponse for &LoadDumpError`. +- New `LoadDumpError` variants: `ImporterWithoutDumpUrl` (400), `StatementTooLarge { line: u64, column: usize, limit: usize }` (413, `StatusCode::PAYLOAD_TOO_LARGE`). Update `IntoResponse for &LoadDumpError`. - `ReaderStream::with_capacity(f, 64 * 1024)`. - Run the existing dump tests: all green, snapshots unchanged. **Step 2 — framer** - `framer.rs` per §7 with the unit tests of §14.1. Add `memchr` dependency. +- *As reviewed:* add `complete.rs`, the resumable linear port of SQLite's completion state machine, and keep the FFI only as the differential-test oracle. **Step 3 — streaming importer** - `streaming.rs` per §8–9; wire the dispatcher arm; `From for LoadDumpError`. @@ -710,7 +688,7 @@ File change list: | File | Change | |---|---| | `libsql-server/Cargo.toml` | `memchr = "2"` | -| `libsql-server/src/namespace/dump_import/{mod,buffered,framer,streaming}.rs` | new | +| `libsql-server/src/namespace/dump_import/{mod,buffered,complete,framer,streaming}.rs` | new | | `libsql-server/src/namespace/mod.rs` | `pub(crate) mod dump_import;`, `DumpSource`, `RestoreOption::Dump(DumpSource)` | | `libsql-server/src/namespace/configurator/helpers.rs` | remove `load_dump` + its imports; new match arm | | `libsql-server/src/namespace/configurator/mod.rs` | `BaseNamespaceConfig.dump_import` | @@ -719,7 +697,7 @@ File change list: | `libsql-server/src/main.rs` | 4 flags, `make_db_config`, validation | | `libsql-server/src/http/admin/mod.rs` | `dump_importer` field, check, `DumpSource`, `ReaderStream::with_capacity` | | `libsql-server/src/error.rs` | 2 variants + response mapping | -| `libsql-server/src/metrics.rs` | 5 metrics | +| `libsql-server/src/namespace/dump_import/mod.rs` | importer dispatch, stats/logs, 5 metrics, shared error mapping | | `libsql-server/src/schema/scheduler.rs` | test `BaseNamespaceConfig` literals | | `libsql-server/tests/namespaces/{mod,dumps}.rs` + snapshots | helpers, variants, new tests | | `docs/ADMIN_API.md`, `docs/USER_GUIDE.md` | docs | @@ -728,7 +706,7 @@ File change list: ## 17. Risks and open questions -- **Oracle/parser disagreement.** `sqlite3_complete` and `sqlite3_parser` are two tokenizers. If they ever disagree on where a statement ends, the result is a parse error (fail closed) or the §8.5 step 2 guard, never silent corruption. Known shared rules: strings, quoted identifiers, `--`/`/* */` comments, `CREATE [TEMP] TRIGGER … END`. +- **Completion-scanner/parser disagreement.** The resumable `complete.c` port and `sqlite3_parser` are two tokenizers. If they disagree on where a statement ends, the result is a parse error (fail closed) or the §8.5 step 2 guard, never silent corruption. The differential test checks framing against SQLite's own `sqlite3_complete()` across strings, quoted identifiers, comments, triggers and randomized token soups. - **Per-statement `Parser` allocation.** `Parser::new` allocates a lemon stack per call. If profiling shows it matters, reuse via `Parser::reset` behind a small wrapper; not needed for correctness. - **Original-text execution vs AST text.** Deliberate (§8.5). If an A/B shows a divergence, the `/dump` diff in §15 will surface it; the fix belongs in the parser, not in the importer. - **`EXPLAIN`/row-returning statements** are executed and their rows discarded; the buffered importer fails on them. Acceptable and documented. diff --git a/libsql-server/src/namespace/dump_import/mod.rs b/libsql-server/src/namespace/dump_import/mod.rs index 16dbc2e261..3e13cd5ae4 100644 --- a/libsql-server/src/namespace/dump_import/mod.rs +++ b/libsql-server/src/namespace/dump_import/mod.rs @@ -5,9 +5,10 @@ //! //! - [`DumpImporterKind::Buffered`]: the historical importer. Reads the whole dump into memory, //! parses it, and executes each statement. Memory usage is proportional to the dump size. -//! - [`DumpImporterKind::Streaming`]: frames complete statements incrementally with -//! `sqlite3_complete()` and executes them on a dedicated blocking thread while the dump is -//! still being read. Memory usage is bounded by [`DumpImportConfig`] plus the largest statement. +//! - [`DumpImporterKind::Streaming`]: frames complete statements incrementally with a resumable +//! port of SQLite's `sqlite3_complete()` state machine and executes them on a dedicated blocking +//! thread while the dump is still being read. Memory usage is bounded by [`DumpImportConfig`] +//! plus the largest statement. //! //! See `docs/STREAMING_DUMP_IMPORT_DESIGN.md` for the full design.