From 7858928e7d19c3c90038f662a5781d40b3a93f0d Mon Sep 17 00:00:00 2001 From: Scott Robinson Date: Tue, 6 Oct 2026 07:40:34 +0000 Subject: [PATCH] fix(sqlite): one extenddb process per database file Nothing stopped two `extenddb serve` processes from opening the same SQLite file. The backend serializes writers with a lock inside the server process, so two servers write concurrently and can hit SQLITE_BUSY mid-transaction; and startup recovery (control-plane transitions, GSI and vector index rebuilds, and, with #384, abandoned restores) assumes no other server is running, so a second server starting up would undo the first one's work in progress. Nothing stopped `destroy` from unlinking the file under a running server either, which left that server serving an unlinked inode while a later `init` created a fresh database at the same path. `extenddb serve` on a file database now takes an exclusive flock(2) on `.lock` before opening the database and holds it for the life of the process (the lock lives in the engine, so it is held while any clone is). `init` and `migrate` take the same lock for their duration through the bootstrapper's migration-lock hook, and `destroy` takes it before it removes the file; each refuses, rather than waits, when a server holds it. `init` holds it through the encryption key, default account, and admin user, not only the schema, so a server cannot start in the window where the schema exists but the key does not. Read-only commands (`settings`, `manage`, `verify`, `status`) take no lock. A second holder fails with: another extenddb process is already using (lock held on .lock); stop it first, or point this command at a different database The lock file is named after the file SQLite actually opens, not the configured string: the location is parsed with sqlx's own SqliteConnectOptions (so `sqlite:` URL forms, percent-encoding such as `a%20b.sqlite`, and `file:` URIs resolve as the engine resolves them), and the path is canonicalized, so two spellings of one file, a `..` path, or a symlink and its target all share one lock. A dangling symlink (the target not created yet) is followed by hand, since canonicalize refuses it. A disk file whose name happens to contain `mode=memory` is a file. In-memory databases take no lock. The lock file is created 0600 and an existing one is tightened to 0600 on open, matching the database and its sidecars: flock needs only a read handle, so a world-readable lock file would let any local user hold the lock and keep the server from starting. The kernel releases the lock when the process exits, including on kill -9, so there is no stale-lock cleanup. On non-Unix platforms no lock is taken and a warning is logged. flock is advisory and its behaviour on network filesystems depends on the server and mount; the docs say to keep SQLite databases on local disk. The directory holding the database must be writable so the lock file can be created. flock through libc rather than std's File::try_lock, which needs Rust 1.89; the workspace MSRV is 1.88. libc is already a dependency of the workspace and listed in every license notices file. Docs: troubleshooting entry for the error with its scope; README and the deployment and design guides now state that one instance per catalog is the supported deployment, that SQLite enforces it, and that PostgreSQL and MongoDB do not (per-instance caches without cross-instance invalidation, every worker on every instance, a liveness-only /health). The deployment guide previously said multiple PostgreSQL instances were consistent because there was no in-process cache, which was not true. Tests: database_file resolution for plain paths, sqlite: URLs, percent-encoding, file: URIs, in-memory forms, and a disk file named like a memory parameter; ServeLock refused on the same file, independent across files, shared across `..` spellings, across file and directory symlinks, and across a dangling symlink and its future target (one and two links); the lock file created 0600 and an existing 0644 one tightened; the server factory refusing a second server on one file; destroy and migrate refused while a server holds the file and a server refused while migrate holds it. Signed-off-by: Scott Robinson --- Cargo.lock | 1 + README.md | 4 + crates/app/src/cmd_init.rs | 54 ++- crates/storage-sqlite/Cargo.toml | 1 + crates/storage-sqlite/src/bootstrapper.rs | 113 ++++- crates/storage-sqlite/src/lib.rs | 50 ++- crates/storage-sqlite/src/serve_lock.rs | 489 ++++++++++++++++++++++ crates/storage-sqlite/src/store.rs | 5 + docs/manuals/02-design-guide.md | 2 +- docs/manuals/11-deployment-guide.md | 13 +- docs/troubleshooting.md | 8 + 11 files changed, 711 insertions(+), 29 deletions(-) create mode 100644 crates/storage-sqlite/src/serve_lock.rs diff --git a/Cargo.lock b/Cargo.lock index 4d78099e6..c24f1fa07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1353,6 +1353,7 @@ dependencies = [ "extenddb-core", "extenddb-storage", "futures", + "libc", "rand 0.9.4", "serde", "serde_json", diff --git a/README.md b/README.md index bb8785bb6..a6e432e21 100755 --- a/README.md +++ b/README.md @@ -136,6 +136,10 @@ extenddb healthcheck --endpoint https://127.0.0.1:18443 # explicit target This is a liveness check, which is what a container `HEALTHCHECK` wants: `/health` does not query the storage backend, so it reports healthy even if PostgreSQL becomes unreachable after startup. That is deliberate, since a liveness probe that failed on a database outage would restart every replica at once. A backend that is unreachable at startup does stop the server from listening, so that case is caught. There is no separate readiness endpoint yet. +### One instance per database + +Run one `extenddb serve` per catalog. On SQLite this is enforced: the server takes an exclusive lock on `.lock` next to the database file and a second server on the same file refuses to start (`init`, `migrate`, and `destroy` take the same lock while they run). On PostgreSQL and MongoDB nothing prevents a second instance, but running more than one against the same catalog is not supported in this release: every instance runs every background worker, credential and policy caches are per instance with no cross-instance invalidation (a revoked key stays valid on the other instance for up to the cache TTL), and `/health` does not check the database. See the [deployment guide](docs/manuals/11-deployment-guide.md#multi-instance-considerations). + To make the generated self-signed certificate valid for the name clients use — an in-cluster service DNS name, for example — pass `--tls-san` to `init` (repeatable): ```bash diff --git a/crates/app/src/cmd_init.rs b/crates/app/src/cmd_init.rs index 6cbe339f6..891ea0204 100755 --- a/crates/app/src/cmd_init.rs +++ b/crates/app/src/cmd_init.rs @@ -230,33 +230,20 @@ pub async fn run(args: InitArgs) -> anyhow::Result { // Two concurrent inits cannot get this far: the second aborts earlier, at // `create_catalog_db`, because the database already exists. The catalog // database does exist by this point, so the lock connection can be opened. + // + // The lock is held through the bootstrap rows as well, not just the + // schema: a server that starts after the schema exists but before the + // encryption key is written fails with MissingEncryptionKey, and on + // SQLite this same lock is what keeps `serve` out until init is done. bootstrapper .acquire_migration_lock() .await .map_err(|e| anyhow::anyhow!("{e:?}"))?; - let migration_result = run_init_migrations(bootstrapper.as_ref()).await; + let bootstrap_result = run_init_bootstrap(bootstrapper.as_ref()).await; if let Err(e) = bootstrapper.release_migration_lock().await { tracing::warn!("Failed to release migration lock: {e:?}"); } - migration_result?; - - bootstrapper - .bootstrap_encryption_key() - .await - .map_err(|e| anyhow::anyhow!("{e:?}"))?; // REQ-AUTH-010 - - bootstrapper - .bootstrap_default_account() - .await - .map_err(|e| anyhow::anyhow!("{e:?}"))?; - - // REQ-AUTH-003 - let env_user = std::env::var("EXTENDDB_ADMIN_USER").ok(); - let env_pass = std::env::var("EXTENDDB_ADMIN_PASSWORD").ok(); - let admin_result = bootstrapper - .bootstrap_admin_user(env_user.as_deref(), env_pass.as_deref()) - .await - .map_err(|e| anyhow::anyhow!("{e:?}"))?; + let admin_result = bootstrap_result?; if admin_result.already_existed { // Already printed by the bootstrap store. @@ -312,6 +299,33 @@ pub async fn run(args: InitArgs) -> anyhow::Result { /// Apply the catalog and data schema while the migration lock is held. Split out /// of `run` so that the lock is released on every path, including errors. +/// Everything `init` does under the migration lock: the schema, then the +/// rows a server needs before it can start (encryption key, default account, +/// admin user). +async fn run_init_bootstrap( + bootstrapper: &dyn extenddb_storage::bootstrapper::Bootstrapper, +) -> anyhow::Result { + run_init_migrations(bootstrapper).await?; + + bootstrapper + .bootstrap_encryption_key() + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; // REQ-AUTH-010 + + bootstrapper + .bootstrap_default_account() + .await + .map_err(|e| anyhow::anyhow!("{e:?}"))?; + + // REQ-AUTH-003 + let env_user = std::env::var("EXTENDDB_ADMIN_USER").ok(); + let env_pass = std::env::var("EXTENDDB_ADMIN_PASSWORD").ok(); + bootstrapper + .bootstrap_admin_user(env_user.as_deref(), env_pass.as_deref()) + .await + .map_err(|e| anyhow::anyhow!("{e:?}")) +} + async fn run_init_migrations( bootstrapper: &dyn extenddb_storage::bootstrapper::Bootstrapper, ) -> anyhow::Result<()> { diff --git a/crates/storage-sqlite/Cargo.toml b/crates/storage-sqlite/Cargo.toml index 85559bde8..a750bf4b8 100644 --- a/crates/storage-sqlite/Cargo.toml +++ b/crates/storage-sqlite/Cargo.toml @@ -35,3 +35,4 @@ rand = { workspace = true } bigdecimal = { workspace = true } crc32fast = { workspace = true } zeroize = { workspace = true } +libc = { workspace = true } diff --git a/crates/storage-sqlite/src/bootstrapper.rs b/crates/storage-sqlite/src/bootstrapper.rs index 87503b41f..2f793c68b 100644 --- a/crates/storage-sqlite/src/bootstrapper.rs +++ b/crates/storage-sqlite/src/bootstrapper.rs @@ -20,6 +20,7 @@ use sqlx::SqlitePool; use sqlx::sqlite::SqlitePoolOptions; use crate::schema::{self, CATALOG_VERSION}; +use crate::serve_lock::ServeLock; use crate::sqlite_util::sqlite_url; /// SQLite backend bootstrapper. @@ -29,11 +30,41 @@ use crate::sqlite_util::sqlite_url; /// paths, not the hot serving path. pub struct SqliteBootstrapper { path: String, + /// The database-file lock held between [`Bootstrapper::acquire_migration_lock`] + /// and [`Bootstrapper::release_migration_lock`]; see `serve_lock`. + held_lock: std::sync::Mutex>, } impl SqliteBootstrapper { pub fn new(path: impl Into) -> Self { - Self { path: path.into() } + Self { + path: path.into(), + held_lock: std::sync::Mutex::new(None), + } + } + + /// Take the exclusive lock a running `extenddb serve` holds on this + /// database, or explain which process holds it. `Ok(None)` for an + /// in-memory database. + fn exclusive_lock(&self, what: &str) -> OpResult> { + // The lock file lives next to the database; `init` may be the one + // creating that directory (see `pool`). + if !self.is_memory() + && let Some(parent) = std::path::Path::new(&self.path).parent() + && !parent.as_os_str().is_empty() + { + std::fs::create_dir_all(parent).map_err(|e| { + OpError::Internal(format!( + "create parent directory for SQLite database '{}': {e}", + self.path + )) + })?; + } + ServeLock::acquire_for_location(&self.path).map_err(|e| { + OpError::Internal(format!( + "{what} needs exclusive use of the SQLite database and cannot have it: {e}" + )) + }) } /// Build a `SqliteBootstrapper` from the config file and CLI args. @@ -444,10 +475,37 @@ impl Bootstrapper for SqliteBootstrapper { Ok(row.map(|(v,)| v)) } + /// `init` and `migrate` rewrite the schema, so they take the same + /// exclusive lock `extenddb serve` holds on the database file. Unlike the + /// trait's default contract this does not wait for the lock: the holder is + /// a server that runs until stopped, so waiting would hang, and the + /// operator is told to stop it instead. + async fn acquire_migration_lock(&self) -> OpResult<()> { + let lock = self.exclusive_lock("this command")?; + *self + .held_lock + .lock() + .map_err(|_| OpError::Internal("lock state poisoned".to_owned()))? = lock; + Ok(()) + } + + async fn release_migration_lock(&self) -> OpResult<()> { + self.held_lock + .lock() + .map_err(|_| OpError::Internal("lock state poisoned".to_owned()))? + .take(); + Ok(()) + } + async fn drop_databases(&self, _data_db: &str) -> OpResult<()> { if self.is_memory() { return Ok(()); } + // A server that still has the file open would keep serving an unlinked + // database, and a later `init` would create a second one at the same + // path. Refuse while any other process holds the file, and hold it + // ourselves until the files are gone. + let _lock = self.exclusive_lock("destroy")?; if std::path::Path::new(&self.path).exists() { std::fs::remove_file(&self.path) .map_err(|e| OpError::Internal(format!("remove database file: {e}")))?; @@ -498,3 +556,56 @@ impl Bootstrapper for SqliteBootstrapper { ) } } + +#[cfg(all(test, unix))] +mod lock_tests { + use super::SqliteBootstrapper; + use crate::serve_lock::ServeLock; + use extenddb_storage::bootstrapper::Bootstrapper; + + /// `destroy` and `migrate` are refused while a server holds the database, + /// and a server is refused while `migrate` holds it. + #[tokio::test] + async fn destroy_and_migrate_are_refused_while_a_server_holds_the_file() { + let dir = std::env::temp_dir().join(format!( + "extenddb-bootstrap-lock-{}", + uuid::Uuid::new_v4().simple() + )); + std::fs::create_dir_all(&dir).expect("dir"); + let db = dir.join("db.sqlite"); + std::fs::write(&db, b"").expect("db file"); + let bootstrap = SqliteBootstrapper::new(db.to_string_lossy().into_owned()); + + let server = ServeLock::acquire(&db).expect("server lock"); + let err = bootstrap + .drop_databases("unused") + .await + .expect_err("destroy under a server"); + assert!( + format!("{err:?}").contains("another extenddb process"), + "{err:?}" + ); + assert!( + db.exists(), + "destroy must not unlink a file a server has open" + ); + let err = bootstrap + .acquire_migration_lock() + .await + .expect_err("migrate under a server"); + assert!( + format!("{err:?}").contains("another extenddb process"), + "{err:?}" + ); + drop(server); + + bootstrap.acquire_migration_lock().await.expect("free now"); + assert!(ServeLock::acquire(&db).is_err(), "serve during migrate"); + bootstrap.release_migration_lock().await.expect("release"); + ServeLock::acquire(&db).expect("free after release"); + + bootstrap.drop_databases("unused").await.expect("destroy"); + assert!(!db.exists()); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/storage-sqlite/src/lib.rs b/crates/storage-sqlite/src/lib.rs index 54ffc0aa6..87427b883 100644 --- a/crates/storage-sqlite/src/lib.rs +++ b/crates/storage-sqlite/src/lib.rs @@ -36,6 +36,7 @@ mod metadata; mod number_key; mod operations; mod schema; +mod serve_lock; mod sqlite_util; mod store; mod stream; @@ -188,7 +189,16 @@ fn sqlite_server_components_factory( let pool_size = config.max_connections(); let region = region.to_owned(); Box::pin(async move { - let engine = SqliteEngine::new(&db_path, pool_size, ®ion, MAX_ITEM_SIZE_BYTES) + // One server per database file, taken before the database is opened: + // startup recovery below assumes no other server is using it. A file + // database only; an in-memory one belongs to this process alone. + let serve_lock = serve_lock::ServeLock::acquire_for_location(&db_path) + .map_err(BackendError::InitializationFailed)? + .map(|lock| { + tracing::debug!("holding serve lock {}", lock.path().display()); + std::sync::Arc::new(lock) + }); + let mut engine = SqliteEngine::new(&db_path, pool_size, ®ion, MAX_ITEM_SIZE_BYTES) .await .map_err(|e| BackendError::ConnectionFailed { backend: "sqlite".to_owned(), @@ -259,6 +269,7 @@ fn sqlite_server_components_factory( Err(e) => tracing::error!("Failed to reconcile incomplete vector indexes: {e}"), } + engine.serve_lock = serve_lock; let control_plane_notify = engine.control_plane_notify(); let engine = Arc::new(engine); @@ -304,3 +315,40 @@ fn sqlite_server_components_factory( }) }) } + +#[cfg(all(test, unix))] +mod serve_lock_tests { + /// The server factory itself takes the lock: a second server on the same + /// file fails to start while the first is up, and starts once it is gone. + #[tokio::test] + async fn a_second_server_on_one_file_is_refused() { + let dir = std::env::temp_dir().join(format!( + "extenddb-serve-lock-factory-{}", + uuid::Uuid::new_v4().simple() + )); + std::fs::create_dir_all(&dir).expect("dir"); + let config = crate::config::SqliteConfig { + path: dir.join("db.sqlite").to_string_lossy().into_owned(), + pool_size: 2, + }; + let mut options = extenddb_storage::server_components::ServerComponentsOptions::default(); + options.bootstrap_if_uninitialized = true; + let first = super::sqlite_server_components_factory(&config, "us-east-1", options) + .await + .expect("first server"); + let Err(err) = super::sqlite_server_components_factory(&config, "us-east-1", options).await + else { + panic!("a second server on the same file started"); + }; + assert!( + err.to_string().contains("another extenddb process"), + "{err}" + ); + drop(first); + let again = super::sqlite_server_components_factory(&config, "us-east-1", options) + .await + .expect("starts once the first is gone"); + drop(again); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/storage-sqlite/src/serve_lock.rs b/crates/storage-sqlite/src/serve_lock.rs new file mode 100644 index 000000000..0dcdf9506 --- /dev/null +++ b/crates/storage-sqlite/src/serve_lock.rs @@ -0,0 +1,489 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! One owner per SQLite database file. +//! +//! Writers on this backend are serialized by a lock inside the server process +//! (`SqliteEngine::write_lock`), and startup recovery assumes no other server +//! is using the file: it removes restore targets left CREATING and rebuilds +//! indexes left mid-build, which would destroy the work of a second server +//! that is still running. So `extenddb serve` takes an exclusive advisory lock +//! on `.lock` before touching the database, holds it for the life of +//! the process, and refuses to start if another process holds it. +//! +//! The commands that replace or rewrite the file take the same lock for their +//! duration: `init` and `migrate` (through the bootstrapper's migration lock) +//! and `destroy` (before it unlinks the file). Read-only commands (`settings`, +//! `manage`, `verify`, `status`, ...) do not, and run alongside a server. +//! +//! The lock file is named after the file SQLite actually opens, not after the +//! configured string: the location is parsed exactly as sqlx parses it +//! (`sqlite:` URL forms, percent-encoding, `file:` URIs), and the resulting +//! path is canonicalized so two spellings of one file, or a symlink and its +//! target, resolve to one lock file. +//! +//! The lock is `flock(2)` on Unix: released by the kernel when the process +//! exits, however it exits, so a crash never leaves a stale lock behind. The +//! lock file itself is left in place; its presence means nothing. On other +//! platforms no lock is taken and a warning is logged. `flock` is advisory and +//! its behaviour on network filesystems depends on the server and mount +//! options; a database on NFS or similar is outside what this check promises. + +use std::fs::File; +use std::path::{Path, PathBuf}; +use std::str::FromStr; + +use sqlx::sqlite::SqliteConnectOptions; + +use crate::sqlite_util::sqlite_url; + +/// The database file a configured SQLite location refers to, resolved the way +/// the engine resolves it, or `Ok(None)` for an in-memory database. +/// +/// # Errors +/// +/// The location does not parse as a SQLite connection string, or names a +/// `file:` URI with an authority this process cannot map to a local path. +pub(crate) fn database_file(location: &str) -> Result, String> { + let url = sqlite_url(location); + let options = SqliteConnectOptions::from_str(&url) + .map_err(|e| format!("cannot parse SQLite location {location:?}: {e}"))?; + // `mode=memory` is recorded in a private field; read it off the query the + // same way sqlx does (form-urlencoded pairs after the first `?`). + let mode_memory = url + .split_once('?') + .map(|(_, q)| { + q.split('&') + .filter_map(|kv| kv.split_once('=')) + .any(|(k, v)| k == "mode" && v == "memory") + }) + .unwrap_or(false); + let filename = options.get_filename().to_string_lossy().into_owned(); + if mode_memory + || filename == ":memory:" + || filename.starts_with("file::memory:") + || filename.starts_with("file:sqlx-in-memory-") + { + return Ok(None); + } + // sqlx opens every name with SQLITE_OPEN_URI, so a `file:` name is a + // SQLite URI: `file:/abs`, `file:rel`, `file:///abs`, `file://localhost/abs`, + // each with an optional `?query`. Anything else must be a plain path. + let path = match filename.strip_prefix("file:") { + Some(rest) => { + let rest = rest.split('?').next().unwrap_or(rest); + let rest = rest.split('#').next().unwrap_or(rest); + match rest.strip_prefix("//") { + Some(after_slashes) => { + let (authority, path) = after_slashes + .find('/') + .map_or((after_slashes, ""), |i| after_slashes.split_at(i)); + if !(authority.is_empty() || authority == "localhost") { + return Err(format!( + "cannot lock SQLite location {location:?}: `file:` URI authority \ + {authority:?} is not a local path" + )); + } + path.to_owned() + } + None => rest.to_owned(), + } + } + None => filename, + }; + if path.is_empty() { + return Err(format!( + "cannot lock SQLite location {location:?}: it names no database file" + )); + } + Ok(Some(PathBuf::from(path))) +} + +/// Holds the lock until dropped. +#[derive(Debug)] +pub(crate) struct ServeLock { + _file: File, + path: PathBuf, +} + +impl ServeLock { + /// Path of the lock file for a database file: `.lock`. + /// + /// The database file is canonicalized if it exists. Otherwise symlinks in + /// the path are followed by hand (a dangling link to a file that `init` + /// has not created yet must still name the target, or a server started + /// through the link and one started on the target would each lock a + /// different file), and then the parent directory is canonicalized, so + /// the first `serve` against a not-yet-created file and every later one + /// agree on the lock. A parent that does not exist either is left as + /// written; opening the database fails on it anyway. + pub(crate) fn lock_path(db_path: &Path) -> PathBuf { + let canonical = std::fs::canonicalize(db_path).unwrap_or_else(|_| { + let resolved = resolve_dangling_symlinks(db_path); + match (resolved.parent(), resolved.file_name()) { + (Some(parent), Some(name)) if !parent.as_os_str().is_empty() => { + std::fs::canonicalize(parent) + .map(|p| p.join(name)) + .unwrap_or_else(|_| resolved.clone()) + } + (_, Some(name)) => std::env::current_dir() + .map(|cwd| cwd.join(name)) + .unwrap_or_else(|_| resolved.clone()), + _ => resolved.clone(), + } + }); + let mut name = canonical.into_os_string(); + name.push(".lock"); + PathBuf::from(name) + } + + /// Take the lock for `db_path`, or report who holds it. + /// + /// # Errors + /// + /// A message for the operator if another process holds the lock or the + /// lock file cannot be opened (for example, the database's directory is + /// not writable; the lock file lives next to the database). + pub(crate) fn acquire(db_path: &Path) -> Result { + let path = Self::lock_path(db_path); + let file = open_options_0600() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&path) + .map_err(|e| { + format!( + "cannot open lock file {}: {e} (the directory holding a SQLite database \ + must be writable so the server can create its lock file)", + path.display() + ) + })?; + // The database and its sidecars are 0600 so other local users cannot + // read secrets out of them. The lock file must match: flock needs + // only a read handle, so a world-readable lock file lets any local + // user hold the lock and keep the server from starting. `create` + // honours the umask, and the file may predate this rule, so set the + // mode explicitly on every open. + restrict_to_owner(&file, &path)?; + try_lock_exclusive(&file).map_err(|e| { + if e.kind() == std::io::ErrorKind::WouldBlock { + format!( + "another extenddb process is already using {} (lock held on {}); \ + stop it first, or point this command at a different database", + db_path.display(), + path.display() + ) + } else { + format!("cannot lock {}: {e}", path.display()) + } + })?; + Ok(Self { _file: file, path }) + } + + /// Take the lock for a configured location, or `Ok(None)` if the location + /// is an in-memory database that belongs to this process alone. + /// + /// # Errors + /// + /// As [`ServeLock::acquire`], plus a location that cannot be resolved to + /// a file. + pub(crate) fn acquire_for_location(location: &str) -> Result, String> { + match database_file(location)? { + Some(path) => Self::acquire(&path).map(Some), + None => Ok(None), + } + } + + pub(crate) fn path(&self) -> &Path { + &self.path + } +} + +/// Follow symlinks along `path` as far as they go, by hand. `canonicalize` +/// refuses a path whose final target does not exist; this returns that +/// target instead, so a dangling link still names the file it will become. +fn resolve_dangling_symlinks(path: &Path) -> PathBuf { + let mut current = path.to_path_buf(); + // Bounded like the kernel's own symlink-loop limit. + for _ in 0..40 { + match std::fs::read_link(¤t) { + Ok(target) => { + current = if target.is_absolute() { + target + } else { + current + .parent() + .map_or_else(|| target.clone(), |p| p.join(&target)) + }; + } + Err(_) => break, + } + } + current +} + +#[cfg(unix)] +fn open_options_0600() -> std::fs::OpenOptions { + use std::os::unix::fs::OpenOptionsExt; + let mut options = std::fs::OpenOptions::new(); + options.mode(0o600); + options +} + +#[cfg(not(unix))] +fn open_options_0600() -> std::fs::OpenOptions { + std::fs::OpenOptions::new() +} + +#[cfg(unix)] +fn restrict_to_owner(file: &File, path: &Path) -> Result<(), String> { + use std::os::unix::fs::PermissionsExt; + file.set_permissions(std::fs::Permissions::from_mode(0o600)) + .map_err(|e| format!("cannot set the mode of lock file {}: {e}", path.display())) +} + +#[cfg(not(unix))] +fn restrict_to_owner(_file: &File, _path: &Path) -> Result<(), String> { + Ok(()) +} + +#[cfg(unix)] +fn try_lock_exclusive(file: &File) -> std::io::Result<()> { + use std::os::unix::io::AsRawFd; + // SAFETY: `flock` takes a file descriptor and flags and touches no memory + // the caller owns. The descriptor is valid: it belongs to `file`, which is + // borrowed for the duration of the call. + let rc = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }; + if rc == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + +#[cfg(not(unix))] +fn try_lock_exclusive(_file: &File) -> std::io::Result<()> { + tracing::warn!( + "no file lock is taken on this platform: nothing prevents a second extenddb \ + process from opening the same SQLite database" + ); + Ok(()) +} + +#[cfg(test)] +mod database_file_tests { + use super::database_file; + use std::path::PathBuf; + + fn file(location: &str) -> Option { + database_file(location).expect(location) + } + + #[test] + fn plain_paths_and_sqlite_urls() { + assert_eq!( + file("/var/lib/x.sqlite"), + Some(PathBuf::from("/var/lib/x.sqlite")) + ); + assert_eq!(file("rel.sqlite"), Some(PathBuf::from("rel.sqlite"))); + assert_eq!( + file("sqlite:///var/lib/x.sqlite?mode=rwc"), + Some(PathBuf::from("/var/lib/x.sqlite")) + ); + assert_eq!(file("sqlite:rel.sqlite"), Some(PathBuf::from("rel.sqlite"))); + } + + #[test] + fn percent_encoding_is_decoded_as_sqlx_decodes_it() { + assert_eq!( + file("sqlite:///tmp/a%20b.sqlite"), + Some(PathBuf::from("/tmp/a b.sqlite")) + ); + // An encoded `?` is part of the file name, not a query. + assert_eq!( + file("sqlite:///tmp/x%3Fmode=memory.db"), + Some(PathBuf::from("/tmp/x?mode=memory.db")) + ); + } + + #[test] + fn file_uris_resolve_to_the_path_sqlite_opens() { + assert_eq!( + file("file:/tmp/db.sqlite"), + Some(PathBuf::from("/tmp/db.sqlite")) + ); + assert_eq!( + file("file:///tmp/db.sqlite"), + Some(PathBuf::from("/tmp/db.sqlite")) + ); + assert_eq!( + file("file://localhost/tmp/db.sqlite"), + Some(PathBuf::from("/tmp/db.sqlite")) + ); + assert_eq!(file("file:rel.sqlite"), Some(PathBuf::from("rel.sqlite"))); + assert!(database_file("file://otherhost/tmp/db.sqlite").is_err()); + } + + #[test] + fn a_disk_file_named_like_a_memory_parameter_is_still_a_file() { + assert_eq!( + file("/tmp/mode=memory.db"), + Some(PathBuf::from("/tmp/mode=memory.db")) + ); + } + + #[test] + fn in_memory_forms_take_no_lock() { + for mem in [ + ":memory:", + "sqlite::memory:", + "file::memory:", + "sqlite://x?mode=memory", + ] { + assert_eq!(file(mem), None, "{mem}"); + } + // `sqlite_url` appends `?mode=rwc` to a bare path, so a path carrying + // its own query is not something the engine can open either; the lock + // reports the same parse error the engine would. + assert!(database_file("file::memory:?cache=shared").is_err()); + } +} + +#[cfg(all(test, unix))] +mod tests { + use super::ServeLock; + + fn temp_dir() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "extenddb-serve-lock-{}", + uuid::Uuid::new_v4().simple() + )); + std::fs::create_dir_all(&dir).expect("dir"); + dir + } + + #[test] + fn a_second_lock_on_the_same_database_is_refused() { + let dir = temp_dir(); + let db = dir.join("db.sqlite"); + let first = ServeLock::acquire(&db).expect("first lock"); + let err = ServeLock::acquire(&db).expect_err("second lock is refused"); + assert!(err.contains("another extenddb process"), "{err}"); + drop(first); + let again = ServeLock::acquire(&db).expect("free again once released"); + drop(again); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn different_databases_do_not_conflict() { + let dir = temp_dir(); + let la = ServeLock::acquire(&dir.join("a.sqlite")).expect("a"); + let lb = ServeLock::acquire(&dir.join("b.sqlite")).expect("b"); + drop((la, lb)); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn spellings_of_one_file_share_a_lock() { + let dir = temp_dir(); + let db = dir.join("db.sqlite"); + std::fs::write(&db, b"").expect("db file"); + let via_dots = dir.join("sub/../db.sqlite"); + std::fs::create_dir_all(dir.join("sub")).expect("sub"); + let first = ServeLock::acquire(&db).expect("first lock"); + assert!( + ServeLock::acquire(&via_dots).is_err(), + "`..` spelling bypassed" + ); + drop(first); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_lock_file_is_readable_by_its_owner_only() { + use std::os::unix::fs::PermissionsExt; + let dir = temp_dir(); + let db = dir.join("db.sqlite"); + let lock = ServeLock::acquire(&db).expect("lock"); + let mode = std::fs::metadata(lock.path()) + .expect("stat") + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600, "fresh lock file mode {mode:o}"); + drop(lock); + // A lock file left by an earlier build with a wider mode is tightened. + std::fs::set_permissions( + ServeLock::lock_path(&db), + std::fs::Permissions::from_mode(0o644), + ) + .expect("widen"); + let lock = ServeLock::acquire(&db).expect("lock again"); + let mode = std::fs::metadata(lock.path()) + .expect("stat") + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600, "existing lock file mode {mode:o}"); + drop(lock); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_dangling_symlink_and_its_future_target_share_a_lock() { + let dir = temp_dir(); + let target = dir.join("target.sqlite"); + let link = dir.join("link.sqlite"); + std::os::unix::fs::symlink(&target, &link).expect("symlink"); + assert!(!target.exists(), "the target must not exist yet"); + let via_link = ServeLock::acquire(&link).expect("lock through the link"); + assert!( + ServeLock::acquire(&target).is_err(), + "dangling symlink bypassed: target locked while the link is held" + ); + drop(via_link); + let via_target = ServeLock::acquire(&target).expect("lock on the target"); + assert!( + ServeLock::acquire(&link).is_err(), + "dangling symlink bypassed: link locked while the target is held" + ); + drop(via_target); + // A chain of links, the last one dangling. + let link2 = dir.join("link2.sqlite"); + std::os::unix::fs::symlink("link.sqlite", &link2).expect("relative symlink"); + let via_link2 = ServeLock::acquire(&link2).expect("lock through two links"); + assert!( + ServeLock::acquire(&target).is_err(), + "two-link chain bypassed" + ); + drop(via_link2); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_symlink_and_its_target_share_a_lock() { + let dir = temp_dir(); + let real = dir.join("real.sqlite"); + std::fs::write(&real, b"").expect("db file"); + let link = dir.join("link.sqlite"); + std::os::unix::fs::symlink(&real, &link).expect("symlink"); + let first = ServeLock::acquire(&real).expect("first lock"); + assert!(ServeLock::acquire(&link).is_err(), "file symlink bypassed"); + drop(first); + + let real_dir = dir.join("realdir"); + std::fs::create_dir_all(&real_dir).expect("realdir"); + let link_dir = dir.join("linkdir"); + std::os::unix::fs::symlink(&real_dir, &link_dir).expect("dir symlink"); + // The database does not exist yet: the parent is what gets canonicalized. + let first = ServeLock::acquire(&real_dir.join("new.sqlite")).expect("first lock"); + assert!( + ServeLock::acquire(&link_dir.join("new.sqlite")).is_err(), + "directory symlink bypassed" + ); + drop(first); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/storage-sqlite/src/store.rs b/crates/storage-sqlite/src/store.rs index 73584abfc..995ed7397 100644 --- a/crates/storage-sqlite/src/store.rs +++ b/crates/storage-sqlite/src/store.rs @@ -74,6 +74,10 @@ pub struct SqliteEngine { /// no entry, because nothing else ever will until a restart, and until then /// the per-table queue hold blocks every write's index maintenance. pub(crate) vector_builds_running: Arc>>, + /// The `extenddb serve` lock on the database file, held for as long as any + /// clone of the engine lives. `None` outside `serve` and for in-memory + /// databases. + pub(crate) serve_lock: Option>, } impl SqliteEngine { @@ -154,6 +158,7 @@ impl SqliteEngine { vector_builds_running: Arc::new( std::sync::Mutex::new(std::collections::HashSet::new()), ), + serve_lock: None, }) } diff --git a/docs/manuals/02-design-guide.md b/docs/manuals/02-design-guide.md index 13cd6c9d7..8a7227b2a 100755 --- a/docs/manuals/02-design-guide.md +++ b/docs/manuals/02-design-guide.md @@ -216,7 +216,7 @@ No safe TTL exists because delete-recreate can happen within milliseconds. Cross ### Multi-Instance Considerations -extenddb does not enforce single-instance-per-catalog. Multiple extenddb instances may share the same PostgreSQL catalog. Any in-process cache of catalog state would be invisible to other instances. PostgreSQL's own buffer pool provides memory-resident access to hot rows, making application-level caching unnecessary for most workloads. +On SQLite, `extenddb serve` enforces one instance per database file with an exclusive lock on `.lock`. On PostgreSQL and MongoDB nothing enforces it, and running several instances against one catalog is not supported: the credential and policy caches are per instance with no cross-instance invalidation, and every instance runs every background worker (see the deployment guide). Any in-process cache of catalog state would likewise be invisible to other instances. PostgreSQL's own buffer pool provides memory-resident access to hot rows, making application-level caching unnecessary for most workloads. ### Future Considerations diff --git a/docs/manuals/11-deployment-guide.md b/docs/manuals/11-deployment-guide.md index cd229b809..fb03c271b 100755 --- a/docs/manuals/11-deployment-guide.md +++ b/docs/manuals/11-deployment-guide.md @@ -6,7 +6,7 @@ This guide covers deploying extenddb in various environments beyond local develo ## Architecture Overview -extenddb is a single Rust binary that connects to PostgreSQL. All state lives in PostgreSQL — extenddb itself is stateless (no in-process caching). This means: +extenddb is a single Rust binary that connects to PostgreSQL. All durable state lives in PostgreSQL; extenddb keeps only short-lived credential, policy, and table-metadata caches in process. This means: - Multiple extenddb instances can share a PostgreSQL catalog (with caveats — see Multi-Instance below) - Standard PostgreSQL HA, backup, and replication tools provide durability @@ -223,12 +223,13 @@ sudo systemctl start extenddb ## Multi-Instance Considerations -Multiple extenddb instances can connect to the same PostgreSQL catalog. However: +Run one extenddb instance per catalog. Multiple instances behind a load balancer are not supported in this release: -- extenddb does not cache database state in-process — every request reads directly from PostgreSQL -- This means multiple instances see consistent data without cache invalidation -- PostgreSQL's connection pool and row-level locking handle concurrent access -- Ensure `pool_size × instance_count + 3 × instance_count ≤ PostgreSQL max_connections` +- Credentials, policies, and table metadata are cached in each process (see the cache TTLs in the admin guide). Nothing invalidates a cache on another instance, so a key revoked or a policy changed through one instance stays in force on the others until the TTL expires. +- Every instance runs every background worker (TTL expiry, GSI backfill, stream retention, control-plane transitions). The workers are safe to run twice, but the work is duplicated. +- `/health` is a liveness check that does not query the database, so a load balancer cannot use it to take an instance with a broken database connection out of rotation. + +On SQLite this limit is enforced: `extenddb serve` holds an exclusive lock on `.lock` and a second server on the same file refuses to start. On PostgreSQL and MongoDB it is not enforced; if you start a second instance anyway, size `pool_size × instance_count + 3 × instance_count ≤ PostgreSQL max_connections`. ## Performance Tuning diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 6bf083658..f47450f5e 100755 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -93,6 +93,14 @@ ss -tlnp | grep :18443 # find what's using the port extenddb serve --port 19443 --config extenddb.toml # use a different port ``` +### `another extenddb process is already using (lock held on .lock)` + +**Cause:** A SQLite database file has one owner at a time. `extenddb serve` takes an exclusive lock on `.lock` (next to the file SQLite opens, after resolving any `sqlite:` or `file:` form, relative path, or symlink) before it opens the database and holds it until it exits. `init` (through to the admin user being written), `migrate`, and `destroy` take the same lock for as long as they run, because they rewrite or remove the file. The backend serializes writers inside one process and recovers interrupted work at startup, so a second server, or a migration or destroy under a running server, would corrupt the first one's work. Read-only commands (`settings`, `manage`, `verify`, `status`) take no lock and run alongside a server. + +**Fix:** Stop the other process (`extenddb stop --config ` for a server), or point this command at a different database (`--sqlite-path` at `init` time). The lock is released by the operating system when the holder exits, including after a crash, so a leftover `.lock` file is harmless and needs no cleanup. + +**Scope:** The lock is `flock(2)`, taken on Linux and macOS. It is advisory, and on network filesystems (NFS, SMB) its behavior depends on the server and mount options; keep SQLite databases on local disk. On other platforms no lock is taken and the server logs a warning at startup. The directory holding the database must be writable so the lock file can be created; a database in a read-only directory now fails to start with `cannot open lock file`. The lock file is created mode 0600 like the database itself; a world-readable lock file would let any local user hold the lock and keep the server from starting. + ### `Failed to load TLS certificates: ` **Cause:** TLS is enabled (the default) but the server could not load the certificate or private key files. Possible causes: