From 3f0c00efc3705b6b1bddf09bae9096d147b37117 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Wed, 25 Mar 2026 10:15:03 +0200 Subject: [PATCH 01/11] Update README.md --- README.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a94930b0df..f0c0936a62 100644 --- a/README.md +++ b/README.md @@ -33,8 +33,14 @@ --- -> [!NOTE] -> This repository contains libSQL, a fork of SQLite developed by Turso. For the full SQLite rewriten in Rust (also by Turso), please visit [tursodatabase/turso](https://github.com/tursodatabase/turso). +> [!IMPORTANT] +> **Turso database and libSQL are two different projects from the same team.** +> +> **libSQL** (this repository) is an open-source fork of SQLite. It extends SQLite with features like embedded replicas and remote access, but inherits SQLite's fundamental limitations such as the single-writer model. +> +> **[Turso](https://github.com/tursodatabase/turso) database** is a SQLite-compatible database rewritten from scratch in Rust. It is **not** a fork of SQLite — it is a completely new implementation that goes beyond what any SQLite fork can offer, including concurrent writes and bi-directional sync with offline support. Turso is currently in beta. +> +> **If you're starting a new project, you probably want to look into [Turso](https://github.com/tursodatabase/turso).** libSQL is actively maintained, but new features are being developed in Turso. ## Documentation From e4beacaa266fba930b637515e2082b42c2d6a817 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Wed, 25 Mar 2026 10:17:01 +0200 Subject: [PATCH 02/11] Fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f0c0936a62..da827825b0 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ > > **libSQL** (this repository) is an open-source fork of SQLite. It extends SQLite with features like embedded replicas and remote access, but inherits SQLite's fundamental limitations such as the single-writer model. > -> **[Turso](https://github.com/tursodatabase/turso) database** is a SQLite-compatible database rewritten from scratch in Rust. It is **not** a fork of SQLite — it is a completely new implementation that goes beyond what any SQLite fork can offer, including concurrent writes and bi-directional sync with offline support. Turso is currently in beta. +> **[Turso database](https://github.com/tursodatabase/turso)** is a SQLite-compatible database rewritten from scratch in Rust. It is **not** a fork of SQLite — it is a completely new implementation that goes beyond what any SQLite fork can offer, including concurrent writes and bi-directional sync with offline support. Turso is currently in beta. > > **If you're starting a new project, you probably want to look into [Turso](https://github.com/tursodatabase/turso).** libSQL is actively maintained, but new features are being developed in Turso. From 553cb83a9c1349eaff48d41f6b9866cb60c3469e Mon Sep 17 00:00:00 2001 From: Aryan Suvarna Date: Mon, 28 Sep 2026 15:52:09 -0400 Subject: [PATCH 03/11] Fix out-of-bounds read when formatting overlong vector text elements vectorParseSqliteText stores each vector element in a 1025-byte stack buffer whose final byte must remain NUL. The length guard used `>`, so a 1025-character element overwrote that terminator before the guard fired; the following error path then formatted the buffer with `%s`, reading past the end of the stack buffer. Reject the element once it reaches MAX_FLOAT_CHAR_SZ characters so the terminator is preserved, regenerate the bundled amalgamations, and add boundary regression tests for every vector function that parses TEXT. --- .../SQLite3MultipleCiphers/src/sqlite3.c | 2 +- libsql-ffi/bundled/src/sqlite3.c | 2 +- libsql-sqlite3/src/vector.c | 2 +- libsql-sqlite3/test/libsql_vector.test | 52 +++++++++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/libsql-ffi/bundled/SQLite3MultipleCiphers/src/sqlite3.c b/libsql-ffi/bundled/SQLite3MultipleCiphers/src/sqlite3.c index 8dc3ca8e72..4147f83b82 100644 --- a/libsql-ffi/bundled/SQLite3MultipleCiphers/src/sqlite3.c +++ b/libsql-ffi/bundled/SQLite3MultipleCiphers/src/sqlite3.c @@ -211731,7 +211731,7 @@ static int vectorParseSqliteText( continue; } if( this != ',' && this != ']' ){ - if( iBuf > MAX_FLOAT_CHAR_SZ ){ + if( iBuf >= MAX_FLOAT_CHAR_SZ ){ *pzErrMsg = sqlite3_mprintf("vector: float string length exceeded %d characters: '%s'", MAX_FLOAT_CHAR_SZ, valueBuf); goto error; } diff --git a/libsql-ffi/bundled/src/sqlite3.c b/libsql-ffi/bundled/src/sqlite3.c index 8dc3ca8e72..4147f83b82 100644 --- a/libsql-ffi/bundled/src/sqlite3.c +++ b/libsql-ffi/bundled/src/sqlite3.c @@ -211731,7 +211731,7 @@ static int vectorParseSqliteText( continue; } if( this != ',' && this != ']' ){ - if( iBuf > MAX_FLOAT_CHAR_SZ ){ + if( iBuf >= MAX_FLOAT_CHAR_SZ ){ *pzErrMsg = sqlite3_mprintf("vector: float string length exceeded %d characters: '%s'", MAX_FLOAT_CHAR_SZ, valueBuf); goto error; } diff --git a/libsql-sqlite3/src/vector.c b/libsql-sqlite3/src/vector.c index 51f8af5d05..ee1c025528 100644 --- a/libsql-sqlite3/src/vector.c +++ b/libsql-sqlite3/src/vector.c @@ -217,7 +217,7 @@ static int vectorParseSqliteText( continue; } if( this != ',' && this != ']' ){ - if( iBuf > MAX_FLOAT_CHAR_SZ ){ + if( iBuf >= MAX_FLOAT_CHAR_SZ ){ *pzErrMsg = sqlite3_mprintf("vector: float string length exceeded %d characters: '%s'", MAX_FLOAT_CHAR_SZ, valueBuf); goto error; } diff --git a/libsql-sqlite3/test/libsql_vector.test b/libsql-sqlite3/test/libsql_vector.test index 793358e068..3cfe69fedc 100644 --- a/libsql-sqlite3/test/libsql_vector.test +++ b/libsql-sqlite3/test/libsql_vector.test @@ -201,6 +201,56 @@ do_execsql_test vector-1-conversion-f8 { {[-20,-35.25,1.0625,1.63281,2.20313,2.76563,10.1875,99.5,104.5,110]} A0C10DC2883FD13F0D4031402341C742D142DC4206 } +foreach {name sql} { + vector {SELECT vector_extract(vector($input)) = vector_extract(vector('[1]'))} + vector32 {SELECT vector_extract(vector32($input)) = vector_extract(vector32('[1]'))} + vector64 {SELECT vector_extract(vector64($input)) = vector_extract(vector64('[1]'))} + vector8 {SELECT vector_extract(vector8($input)) = vector_extract(vector8('[1]'))} + vector16 {SELECT vector_extract(vector16($input)) = vector_extract(vector16('[1]'))} + vectorb16 {SELECT vector_extract(vectorb16($input)) = vector_extract(vectorb16('[1]'))} + vector1bit {SELECT vector_extract(vector1bit($input)) = vector_extract(vector1bit('[1]'))} + extract {SELECT vector_extract($input) = '[1]'} + cos-left {SELECT vector_distance_cos($input, '[1]') = 0} + cos-right {SELECT vector_distance_cos('[1]', $input) = 0} + l2-left {SELECT vector_distance_l2($input, '[1]') = 0} + l2-right {SELECT vector_distance_l2('[1]', $input) = 0} +} { + foreach length {1023 1024} { + set element "[string repeat 0 [expr {$length - 1}]]1" + set input [format {[%s]} $element] + do_execsql_test vector-1-text-length-$name-$length $sql {1} + } + + foreach length {1025 1026 5000} { + set input [format {[%s]} [string repeat 1 $length]] + do_catchsql_test vector-1-text-length-$name-$length $sql [list 1 \ + "vector: float string length exceeded 1024 characters: '[string repeat 1 1024]'" + ] + } + + set element "1[string repeat x 1023]" + set input [format {[%s]} $element] + do_catchsql_test vector-1-text-length-$name-invalid-1024 $sql [list 1 \ + "vector: invalid float at position 0: '$element'" + ] + + set input [format {[%sx]} $element] + do_catchsql_test vector-1-text-length-$name-invalid-1025 $sql [list 1 \ + "vector: float string length exceeded 1024 characters: '$element'" + ] +} + +set element "[string repeat 0 1023]1" +set input [format {[%s,%s]} $element $element] +do_execsql_test vector-1-text-length-multiple-elements { + SELECT vector_extract(vector($input)); +} {{[1,1]}} + +set input [format {[1,%sx]} $element] +do_catchsql_test vector-1-text-length-invalid-second-element { + SELECT vector($input); +} [list 1 "vector: float string length exceeded 1024 characters: '$element'"] + proc error_messages {sql} { set ret "" set stmt [sqlite3_prepare db $sql -1 dummy] @@ -239,3 +289,5 @@ do_test vector-1-func-errors { {vector_distance: vectors must have the same type: 1 != 2} {vector_distance: l2 distance is not supported for float1bit vectors} }] + +finish_test From e48d754dbe05818f15c58f2cb794eef842250cde Mon Sep 17 00:00:00 2001 From: Aryan Suvarna Date: Tue, 29 Sep 2026 19:55:44 -0400 Subject: [PATCH 04/11] Bump pinned Rust toolchain to 1.98.1 libsql-sqlite3/test/rust_suite has no committed Cargo.lock, so CI resolves its transitive dependencies fresh on every run. Several of those now require a newer compiler than the pinned 1.85.0 (icu_* and wasm-encoder/wast declare rust-version 1.88; yoke-derive 0.8.3 declares none but uses str::from_utf8 as an inherent method, stabilized in 1.87). This breaks the Extensions Tests job and the rusttestwasm step of make-sqlite3 on main. Pin current stable (1.98.1) rather than the minimum that compiles today (1.88.0, verified green in CI), so crate MSRV bumps do not break CI again in the near term. Document the constraint next to the unlocked test crate. --- rust-toolchain.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain.toml b/rust-toolchain.toml index c2324b9b48..2449619493 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] profile = "default" -channel = "1.85.0" +channel = "1.98.1" From c22cc620e8378bced5c3c0f4fa24d95a14756387 Mon Sep 17 00:00:00 2001 From: Aryan Suvarna Date: Wed, 30 Sep 2026 10:33:25 -0400 Subject: [PATCH 05/11] Fix lints surfaced by Rust 1.98.1 under -D warnings CI compiles with RUSTFLAGS="-D warnings", so lints added since 1.85.0 fail the build: - mismatched_lifetime_syntaxes (new in 1.89): eleven signatures elide a lifetime on the input side (&self / &str) but hide it on the output type (Vec, PageHdrIter, CursorStep, Cow). Spell the output lifetime as '_ as the compiler suggests. No semantic change; this is the lifetime rustc already inferred. - unused_assignments: `frameno` in bottomless-cli's restore loop was only ever copied into BatchReader::new and then incremented, never read. BatchReader tracks its own next_frame_no and the function returns the separate last_received_frame_no, so the local was dead since it was introduced in 4a71b2072a. Remove it and pass first_frame_no directly. Verified locally on 1.98.1 with the same flags as CI: cargo check --all-targets --all-features, cargo fmt --check, and cargo check -p libsql --no-default-features for core/replication/remote. --- bottomless-cli/src/replicator_extras.rs | 4 +--- libsql-server/src/query_analysis.rs | 2 +- libsql-sys/src/wal/mod.rs | 2 +- libsql/src/hrana/cursor.rs | 2 +- libsql/src/hrana/hyper.rs | 2 +- libsql/src/local/impls.rs | 2 +- libsql/src/local/statement.rs | 2 +- libsql/src/replication/connection.rs | 2 +- libsql/src/statement.rs | 4 ++-- libsql/src/sync/statement.rs | 2 +- vendored/rusqlite/src/column.rs | 2 +- 11 files changed, 12 insertions(+), 14 deletions(-) diff --git a/bottomless-cli/src/replicator_extras.rs b/bottomless-cli/src/replicator_extras.rs index 94670bd07f..9586020c54 100644 --- a/bottomless-cli/src/replicator_extras.rs +++ b/bottomless-cli/src/replicator_extras.rs @@ -382,9 +382,8 @@ impl Replicator { let frame = tokio::fs::File::open(&obj).await?; let frame_buf_reader = BufReader::new(frame); - let mut frameno = first_frame_no; let mut reader = bottomless::read::BatchReader::new( - frameno, + first_frame_no, frame_buf_reader, page_size as usize, compression_kind, @@ -411,7 +410,6 @@ impl Replicator { ); pending_pages.flush(db).await?; } - frameno += 1; last_received_frame_no += 1; } db.flush().await?; diff --git a/libsql-server/src/query_analysis.rs b/libsql-server/src/query_analysis.rs index 5762b86d4c..5478047eba 100644 --- a/libsql-server/src/query_analysis.rs +++ b/libsql-server/src/query_analysis.rs @@ -233,7 +233,7 @@ impl StmtKind { } } -fn to_ascii_lower(s: &str) -> Cow { +fn to_ascii_lower(s: &str) -> Cow<'_, str> { if s.chars().all(|c| char::is_ascii_lowercase(&c)) { Cow::Borrowed(s) } else { diff --git a/libsql-sys/src/wal/mod.rs b/libsql-sys/src/wal/mod.rs index 71e0c21ee3..1a35d7cc73 100644 --- a/libsql-sys/src/wal/mod.rs +++ b/libsql-sys/src/wal/mod.rs @@ -131,7 +131,7 @@ impl PageHeaders { Self { inner } } - pub fn iter(&self) -> PageHdrIter { + pub fn iter(&self) -> PageHdrIter<'_> { // TODO: move LIBSQL_PAGE_SIZE PageHdrIter::new(self.as_ptr(), 4096) } diff --git a/libsql/src/hrana/cursor.rs b/libsql/src/hrana/cursor.rs index aa0b1191b1..4aa62c9832 100644 --- a/libsql/src/hrana/cursor.rs +++ b/libsql/src/hrana/cursor.rs @@ -147,7 +147,7 @@ where }) } - pub async fn next_step(&mut self) -> Result> { + pub async fn next_step(&mut self) -> Result> { CursorStep::new(self).await } diff --git a/libsql/src/hrana/hyper.rs b/libsql/src/hrana/hyper.rs index 300602c27e..3ea1ad631e 100644 --- a/libsql/src/hrana/hyper.rs +++ b/libsql/src/hrana/hyper.rs @@ -276,7 +276,7 @@ impl crate::statement::Stmt for crate::hrana::Statement { self.cols.len() } - fn columns(&self) -> Vec { + fn columns(&self) -> Vec> { //FIXME: there are several blockers here: // 1. We cannot know the column types before sending a query, so this method will never return results right // away. diff --git a/libsql/src/local/impls.rs b/libsql/src/local/impls.rs index b86405610e..5714d4761a 100644 --- a/libsql/src/local/impls.rs +++ b/libsql/src/local/impls.rs @@ -160,7 +160,7 @@ impl Stmt for LibsqlStmt { self.0.column_count() } - fn columns(&self) -> Vec { + fn columns(&self) -> Vec> { self.0.columns() } } diff --git a/libsql/src/local/statement.rs b/libsql/src/local/statement.rs index c31e751734..e8e0b415e1 100644 --- a/libsql/src/local/statement.rs +++ b/libsql/src/local/statement.rs @@ -339,7 +339,7 @@ impl Statement { /// If associated DB schema can be altered concurrently, you should make /// sure that current statement has already been stepped once before /// calling this method. - pub fn columns(&self) -> Vec { + pub fn columns(&self) -> Vec> { let n = self.column_count(); let mut cols = Vec::with_capacity(n); for i in 0..n { diff --git a/libsql/src/replication/connection.rs b/libsql/src/replication/connection.rs index 418ae03465..e21d01511e 100644 --- a/libsql/src/replication/connection.rs +++ b/libsql/src/replication/connection.rs @@ -797,7 +797,7 @@ impl Stmt for RemoteStatement { } } - fn columns(&self) -> Vec { + fn columns(&self) -> Vec> { if let Some(stmt) = self.local_statement.as_ref() { return stmt.columns(); } diff --git a/libsql/src/statement.rs b/libsql/src/statement.rs index 861fdf8023..e5704f42fe 100644 --- a/libsql/src/statement.rs +++ b/libsql/src/statement.rs @@ -24,7 +24,7 @@ pub(crate) trait Stmt { fn column_count(&self) -> usize; - fn columns(&self) -> Vec; + fn columns(&self) -> Vec>; } /// A cached prepared statement. @@ -103,7 +103,7 @@ impl Statement { } /// Fetch the list of columns for the prepared statement. - pub fn columns(&self) -> Vec { + pub fn columns(&self) -> Vec> { self.inner.columns() } } diff --git a/libsql/src/sync/statement.rs b/libsql/src/sync/statement.rs index b3de1338ef..7b31577672 100644 --- a/libsql/src/sync/statement.rs +++ b/libsql/src/sync/statement.rs @@ -67,7 +67,7 @@ impl Stmt for SyncedStatement { self.inner.column_count() } - fn columns(&self) -> Vec { + fn columns(&self) -> Vec> { self.inner.columns() } } diff --git a/vendored/rusqlite/src/column.rs b/vendored/rusqlite/src/column.rs index 4413a62bcb..64c1cac9c0 100644 --- a/vendored/rusqlite/src/column.rs +++ b/vendored/rusqlite/src/column.rs @@ -136,7 +136,7 @@ impl Statement<'_> { /// calling this method. #[cfg(feature = "column_decltype")] #[cfg_attr(docsrs, doc(cfg(feature = "column_decltype")))] - pub fn columns(&self) -> Vec { + pub fn columns(&self) -> Vec> { let n = self.column_count(); let mut cols = Vec::with_capacity(n); for i in 0..n { From fbd6921e4856d1aa95059892fd6c14511d18815f Mon Sep 17 00:00:00 2001 From: Raphael Lopes Feitoza Date: Wed, 30 Sep 2026 17:31:23 -0400 Subject: [PATCH 06/11] Reject namespace path syntax across all constructors and persisted metadata --- docs/ADMIN_API.md | 25 ++ libsql-server/src/admin_shell.rs | 8 +- libsql-server/src/connection/config.rs | 61 ++++- libsql-server/src/error.rs | 5 + libsql-server/src/namespace/meta_store.rs | 257 ++++++++++++++++-- libsql-server/src/namespace/name.rs | 179 +++++++++++- .../src/replication/replicator_client.rs | 2 +- .../src/replication/script_backup_manager.rs | 18 +- libsql-server/src/schema/db.rs | 233 ++++++++++++---- libsql-server/src/schema/error.rs | 6 + libsql-server/src/schema/scheduler.rs | 7 + libsql-server/tests/namespaces/mod.rs | 90 ++++++ 12 files changed, 794 insertions(+), 97 deletions(-) diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index d5a7fe2478..0688d755d1 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -9,6 +9,31 @@ To enable the admin API, and manage namespaces, two extra flags need to be passe - `--admin-listen-addr :`: the address and port on which the admin API should listen. It must be different from the user API listen address (which defaults to port 8080). - `--enable-namespaces`: enable namespaces for the instance. By default namespaces are disabled. +## Namespace names + +Namespace names must be non-empty single filesystem components. `.` and `..`, +forward/backward slashes, and NUL are rejected, including percent-encoded path +parameters after HTTP decoding. Safe existing names with spaces, punctuation, +and Unicode remain supported. On Windows, invalid Win32 characters, trailing +ASCII dots/spaces, and reserved device names are also rejected. These rules +also apply to fork source/destination and shared-schema names. Invalid names +return `400 Bad Request` on the admin API. + +On upgrade, invalid namespace names or configs in the metastore (including +invalid shared-schema names) prevent startup instead of being treated as absent +or allowing their rows to be overwritten; this holds even +with `--meta-store-destroy-on-error`. Filesystem recovery skips invalid and +symlinked directory entries without deleting them. An invalid persisted +migration job/task stops its scheduler without marking that work complete. +Back up and inspect +metastore and namespace files before repairing these entries explicitly. + +This validation prevents path traversal *through a namespace string*. It does +not establish ownership of existing directories or protect against symlinks, +case/normalization aliases, or filesystem replacement races. Only trusted +operators should have write access to the data directory; additional directory +reservation and ownership protection is addressed separately. + ## Routes ```HTTP diff --git a/libsql-server/src/admin_shell.rs b/libsql-server/src/admin_shell.rs index e97b272d72..97a7d6941c 100644 --- a/libsql-server/src/admin_shell.rs +++ b/libsql-server/src/admin_shell.rs @@ -2,7 +2,6 @@ use std::fmt::Display; use std::pin::Pin; use std::str::FromStr; -use bytes::Bytes; use dialoguer::BasicHistory; use rusqlite::types::ValueRef; use tokio_stream::{Stream, StreamExt as _}; @@ -37,10 +36,9 @@ impl AdminShell { async fn with_namespace( &self, - ns: Bytes, + namespace: NamespaceName, queries: impl Stream>, ) -> anyhow::Result>> { - let namespace = NamespaceName::from_bytes(ns).unwrap(); let connection_maker = self .namespace_store .with(namespace, |ns| ns.db.connection_maker()) @@ -128,7 +126,9 @@ impl AdminShellService for AdminShell { )); }; - match self.with_namespace(ns_bytes, request.into_inner()).await { + let namespace = NamespaceName::from_bytes(ns_bytes) + .map_err(|_| tonic::Status::invalid_argument("invalid namespace"))?; + match self.with_namespace(namespace, request.into_inner()).await { Ok(s) => Ok(tonic::Response::new(Box::pin(s))), Err(e) => Err(tonic::Status::new( tonic::Code::FailedPrecondition, diff --git a/libsql-server/src/connection/config.rs b/libsql-server/src/connection/config.rs index 970014415d..08ed21031e 100644 --- a/libsql-server/src/connection/config.rs +++ b/libsql-server/src/connection/config.rs @@ -65,30 +65,36 @@ impl Default for DatabaseConfig { } } -impl From<&metadata::DatabaseConfig> for DatabaseConfig { - fn from(value: &metadata::DatabaseConfig) -> Self { - DatabaseConfig { +impl TryFrom<&metadata::DatabaseConfig> for DatabaseConfig { + type Error = crate::Error; + + fn try_from(value: &metadata::DatabaseConfig) -> Result { + Ok(DatabaseConfig { block_reads: value.block_reads, block_writes: value.block_writes, block_reason: value.block_reason.clone(), max_db_pages: value.max_db_pages, - heartbeat_url: value.heartbeat_url.as_ref().map(|s| Url::parse(s).unwrap()), + heartbeat_url: value + .heartbeat_url + .as_ref() + .map(|s| Url::parse(s)) + .transpose()?, bottomless_db_id: value.bottomless_db_id.clone(), jwt_key: value.jwt_key.clone(), txn_timeout: value.txn_timeout_s.map(Duration::from_secs), allow_attach: value.allow_attach, max_row_size: value.max_row_size.unwrap_or_else(default_max_row_size), is_shared_schema: value.shared_schema.unwrap_or(false), - // namespace name is coming from primary, we assume it's valid shared_schema_name: value .shared_schema_name .clone() - .map(NamespaceName::new_unchecked), + .map(NamespaceName::from_string) + .transpose()?, durability_mode: match value.durability_mode { None => DurabilityMode::default(), Some(m) => DurabilityMode::from(metadata::DurabilityMode::try_from(m)), }, - } + }) } } @@ -112,6 +118,47 @@ impl From<&DatabaseConfig> for metadata::DatabaseConfig { } } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn replicated_config_rejects_unsafe_shared_schema_names() { + for name in ["", ".", "..", "../schema", "/schema", "a\\b", "a\0b"] { + let config = metadata::DatabaseConfig { + shared_schema_name: Some(name.into()), + ..Default::default() + }; + assert!( + matches!( + DatabaseConfig::try_from(&config), + Err(crate::Error::InvalidNamespace) + ), + "{name:?}" + ); + } + } + + #[test] + fn replicated_config_preserves_safe_shared_schema_names() { + for name in [None, Some("schema-1.example"), Some("tenant café")] { + let config = metadata::DatabaseConfig { + shared_schema_name: name.map(str::to_owned), + ..Default::default() + }; + let decoded = DatabaseConfig::try_from(&config).unwrap(); + assert_eq!( + decoded.shared_schema_name.as_ref().map(|n| n.as_str()), + name + ); + assert_eq!( + metadata::DatabaseConfig::from(&decoded).shared_schema_name, + config.shared_schema_name + ); + } + } +} + /// Durability mode specifies the `PRAGMA SYNCHRONOUS` setting for the connection #[derive(PartialEq, Clone, Copy, Debug, Deserialize, Serialize, Default)] #[serde(rename_all = "lowercase")] diff --git a/libsql-server/src/error.rs b/libsql-server/src/error.rs index bfe67f47c7..692aad5411 100644 --- a/libsql-server/src/error.rs +++ b/libsql-server/src/error.rs @@ -70,6 +70,8 @@ pub enum Error { NamespaceAlreadyExist(String), #[error("Invalid namespace")] InvalidNamespace, + #[error("Invalid persisted namespace config for `{namespace}`: {reason}. Repair the persisted config before restarting; no data was removed")] + InvalidPersistedNamespaceConfig { namespace: String, reason: String }, #[error("Invalid namespace bytes: `{0}`")] InvalidNamespaceBytes(Box), #[error("Replica meta error: {0}")] @@ -192,6 +194,9 @@ impl IntoResponse for &Error { PrimaryConnectionTimeout => self.format_err(StatusCode::INTERNAL_SERVER_ERROR), NamespaceAlreadyExist(_) => self.format_err(StatusCode::BAD_REQUEST), InvalidNamespace => self.format_err(StatusCode::BAD_REQUEST), + InvalidPersistedNamespaceConfig { .. } => { + self.format_err(StatusCode::INTERNAL_SERVER_ERROR) + } InvalidNamespaceBytes(_) => self.format_err(StatusCode::BAD_REQUEST), LoadDumpError(e) => e.into_response(), InvalidMetadataBytes(_) => self.format_err(StatusCode::INTERNAL_SERVER_ERROR), diff --git a/libsql-server/src/namespace/meta_store.rs b/libsql-server/src/namespace/meta_store.rs index 70b419ebe9..b0e19b833d 100644 --- a/libsql-server/src/namespace/meta_store.rs +++ b/libsql-server/src/namespace/meta_store.rs @@ -220,15 +220,30 @@ impl MetaStoreInner { let db_dir = read_dir(&dbs_dir_path)?; for entry in db_dir { let entry = entry?; - if !entry.path().is_dir() { + // Do not follow symlinked directories during filesystem recovery. + if !entry.file_type()?.is_dir() { continue; } + let Some(file_name) = entry.file_name().to_str().map(str::to_owned) else { + tracing::warn!("skipping namespace directory with non-UTF-8 name"); + continue; + }; + let name = match NamespaceName::from_string(file_name) { + Ok(name) => name, + Err(_) => { + tracing::warn!("skipping invalid namespace directory during recovery"); + continue; + } + }; let config_path = entry.path().join("config.json"); - let name = - NamespaceName::from_string(entry.file_name().to_str().unwrap().to_string())?; let config = if config_path.try_exists()? { let config_bytes = std::fs::read(&config_path)?; - serde_json::from_slice(&config_bytes)? + serde_json::from_slice(&config_bytes).map_err(|e| { + Error::InvalidPersistedNamespaceConfig { + namespace: name.to_string(), + reason: format!("invalid filesystem config.json: {e}"), + } + })? } else { DatabaseConfig::default() }; @@ -265,21 +280,23 @@ impl MetaStoreInner { for row in rows { match row { Ok((k, v)) => { - let ns = match NamespaceName::from_string(k) { - Ok(ns) => ns, - Err(e) => { - tracing::warn!("unable to convert namespace name: {}", e); - continue; + let ns = NamespaceName::from_string(k.clone()).map_err(|e| { + Error::InvalidPersistedNamespaceConfig { + namespace: k, + reason: format!("invalid persisted namespace name: {e}"), } - }; - - let config = match metadata::DatabaseConfig::decode(&v[..]) { - Ok(c) => Arc::new(DatabaseConfig::from(&c)), - Err(e) => { - tracing::warn!("unable to convert config: {}", e); - continue; - } - }; + })?; + + // Retained invalid configs must not be treated as missing: a + // later create could overwrite the row and its schema links. + let config = metadata::DatabaseConfig::decode(&v[..]) + .map_err(Error::from) + .and_then(|c| DatabaseConfig::try_from(&c)) + .map_err(|e| Error::InvalidPersistedNamespaceConfig { + namespace: ns.to_string(), + reason: e.to_string(), + })?; + let config = Arc::new(config); // We don't store the version in the sqlitedb due to the session token // changed each time we start the primary, this will cause the replica to @@ -417,6 +434,11 @@ impl MetaStore { let inner = match maybe_inner { Ok(inner) => inner, Err(e) => { + // An invalid persisted name/config requires operator repair; do + // not erase otherwise healthy metastore links, jobs, or data. + if matches!(e, Error::InvalidPersistedNamespaceConfig { .. }) { + return Err(e); + } if destroy_on_error { let db_path = base_path.join("metastore"); @@ -618,6 +640,199 @@ impl MetaStore { } } +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[tokio::test] + async fn invalid_shared_schema_refuses_startup_without_destroying_metastore() { + let tmp = tempdir().unwrap(); + let namespace_dir = tmp.path().join("dbs/tenant"); + std::fs::create_dir_all(&namespace_dir).unwrap(); + std::fs::write(namespace_dir.join("sentinel"), b"namespace intact").unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metastore_sentinel = tmp.path().join("metastore/sentinel"); + let invalid = metadata::DatabaseConfig { + shared_schema_name: Some("../schema".into()), + ..metadata::DatabaseConfig::from(&DatabaseConfig::default()) + } + .encode_to_vec(); + { + let conn = maker().unwrap(); + setup_connection(&conn).unwrap(); + std::fs::write(&metastore_sentinel, b"metastore intact").unwrap(); + conn.execute( + "INSERT INTO namespace_configs VALUES (?1, ?2)", + rusqlite::params!["tenant", invalid.clone()], + ) + .unwrap(); + } + let config = MetaStoreConfig { + destroy_on_error: true, + ..Default::default() + }; + let err = match MetaStore::new( + config.clone(), + tmp.path(), + maker().unwrap(), + wal.clone(), + DatabaseKind::Primary, + ) + .await + { + Ok(_) => panic!("invalid persisted config should fail startup"), + Err(e) => e, + }; + assert!( + matches!(err, Error::InvalidPersistedNamespaceConfig { namespace, .. } if namespace == "tenant") + ); + assert_eq!( + std::fs::read(&metastore_sentinel).unwrap(), + b"metastore intact" + ); + assert_eq!( + std::fs::read(namespace_dir.join("sentinel")).unwrap(), + b"namespace intact" + ); + { + let conn = maker().unwrap(); + let stored: Vec = conn + .query_row( + "SELECT config FROM namespace_configs WHERE namespace = 'tenant'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(stored, invalid); + let repaired = + metadata::DatabaseConfig::from(&DatabaseConfig::default()).encode_to_vec(); + conn.execute( + "UPDATE namespace_configs SET config = ?1 WHERE namespace = 'tenant'", + [repaired], + ) + .unwrap(); + } + let store = MetaStore::new( + config, + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + assert!(store.exists(&NamespaceName::from("tenant")).await); + } + + #[tokio::test] + async fn invalid_persisted_name_refuses_startup_without_erasing_other_rows() { + let tmp = tempdir().unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metastore_sentinel = tmp.path().join("metastore/sentinel"); + { + let conn = maker().unwrap(); + setup_connection(&conn).unwrap(); + std::fs::write(&metastore_sentinel, b"keep").unwrap(); + let valid = metadata::DatabaseConfig::from(&DatabaseConfig::default()).encode_to_vec(); + conn.execute( + "INSERT INTO namespace_configs VALUES (?1, ?2)", + rusqlite::params!["valid", valid.clone()], + ) + .unwrap(); + conn.execute( + "INSERT INTO namespace_configs VALUES (?1, ?2)", + rusqlite::params!["../bad", valid], + ) + .unwrap(); + } + let config = MetaStoreConfig { + destroy_on_error: true, + ..Default::default() + }; + let err = match MetaStore::new( + config.clone(), + tmp.path(), + maker().unwrap(), + wal.clone(), + DatabaseKind::Primary, + ) + .await + { + Ok(_) => panic!("invalid persisted name should fail startup"), + Err(e) => e, + }; + assert!( + matches!(err, Error::InvalidPersistedNamespaceConfig { namespace, .. } if namespace == "../bad") + ); + assert_eq!(std::fs::read(&metastore_sentinel).unwrap(), b"keep"); + { + let conn = maker().unwrap(); + let count: i64 = conn + .query_row("SELECT count(*) FROM namespace_configs", [], |row| { + row.get(0) + }) + .unwrap(); + assert_eq!(count, 2); + conn.execute( + "UPDATE namespace_configs SET namespace = 'repaired' WHERE namespace = '../bad'", + [], + ) + .unwrap(); + } + let store = MetaStore::new( + config, + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + assert!(store.exists(&NamespaceName::from("valid")).await); + assert!(store.exists(&NamespaceName::from("repaired")).await); + } + + #[cfg(unix)] + #[tokio::test] + async fn fs_recovery_skips_invalid_names_without_deleting_dirs() { + let tmp = tempdir().unwrap(); + let dbs = tmp.path().join("dbs"); + std::fs::create_dir_all(dbs.join("valid")).unwrap(); + std::fs::create_dir_all(dbs.join("bad\\name")).unwrap(); + std::fs::write(dbs.join("bad\\name/sentinel"), b"keep").unwrap(); + let outside = tmp.path().join("outside"); + std::fs::create_dir(&outside).unwrap(); + std::fs::write(outside.join("sentinel"), b"outside intact").unwrap(); + std::os::unix::fs::symlink(&outside, dbs.join("alias")).unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let config = MetaStoreConfig { + allow_recover_from_fs: true, + destroy_on_error: true, + ..Default::default() + }; + let store = MetaStore::new( + config, + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + assert!(store.exists(&NamespaceName::from("valid")).await); + assert_eq!( + std::fs::read(dbs.join("bad\\name/sentinel")).unwrap(), + b"keep" + ); + assert!(!store.exists(&NamespaceName::from("alias")).await); + assert_eq!( + std::fs::read(outside.join("sentinel")).unwrap(), + b"outside intact" + ); + } +} + impl MetaStoreHandle { #[cfg(test)] pub fn new_test() -> Self { @@ -633,21 +848,21 @@ impl MetaStoreHandle { let config = match fs::read(config_path) { Ok(data) => { let c = metadata::DatabaseConfig::decode(&data[..])?; - DatabaseConfig::from(&c) + DatabaseConfig::try_from(&c)? } Err(err) if err.kind() == io::ErrorKind::NotFound => DatabaseConfig::default(), Err(err) => return Err(Error::IOError(err)), }; Ok(Self { - namespace: NamespaceName::new_unchecked("testmetastore"), + namespace: NamespaceName::from("testmetastore"), inner: HandleState::Internal(Arc::new(Mutex::new(Arc::new(config)))), }) } pub fn internal() -> Self { MetaStoreHandle { - namespace: NamespaceName::new_unchecked("testmetastore"), + namespace: NamespaceName::from("testmetastore"), inner: HandleState::Internal(Arc::new(Mutex::new(Arc::new(DatabaseConfig::default())))), } } diff --git a/libsql-server/src/namespace/name.rs b/libsql-server/src/namespace/name.rs index 51a3eb4905..105be75534 100644 --- a/libsql-server/src/namespace/name.rs +++ b/libsql-server/src/namespace/name.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::path::{Component, Path}; use bytes::Bytes; use serde::{de::Visitor, Deserialize}; @@ -45,9 +46,19 @@ impl NamespaceName { } fn validate(s: &str) -> crate::Result<()> { - if s.is_empty() { - tracing::warn!("invalid namespace: empty namespace"); - return Err(crate::error::Error::InvalidNamespace); + // Names become one directory component under `dbs`, including at + // cleanup/fork sinks. Preserve existing safe names (including spaces and + // Unicode), but never allow path syntax on Unix or Windows. On Windows, + // a colon can denote a drive prefix or alternate data stream. + let mut components = Path::new(s).components(); + if s.is_empty() + || s.chars().any(|c| matches!(c, '/' | '\\' | '\0')) + || (cfg!(windows) && invalid_windows_component(s)) + || !matches!(components.next(), Some(Component::Normal(_))) + || components.next().is_some() + { + tracing::warn!("invalid namespace name"); + return Err(Error::InvalidNamespace); } Ok(()) @@ -67,10 +78,37 @@ impl NamespaceName { pub fn as_slice(&self) -> &[u8] { &self.0 } +} - pub(crate) fn new_unchecked(s: impl AsRef) -> Self { - Self(Bytes::copy_from_slice(s.as_ref().as_bytes())) +// Test this policy on Unix too: server tests are not run on Windows by CI. +fn invalid_windows_component(s: &str) -> bool { + if s.ends_with(['.', ' ']) + || s.chars() + .any(|c| c <= '\u{1f}' || matches!(c, ':' | '<' | '>' | '"' | '|' | '?' | '*')) + { + return true; + } + + // Win32 treats these device names as special even when followed by an + // extension. Superscript 1, 2 and 3 are also recognized as COM/LPT digits. + let stem = s.split('.').next().unwrap_or("").trim_end_matches(' '); + let stem = stem.to_ascii_uppercase(); + if matches!( + stem.as_str(), + "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$" + ) { + return true; + } + if let Some(suffix) = stem + .strip_prefix("COM") + .or_else(|| stem.strip_prefix("LPT")) + { + return matches!( + suffix, + "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "¹" | "²" | "³" + ); } + false } impl fmt::Display for NamespaceName { @@ -112,6 +150,137 @@ impl<'de> Deserialize<'de> for NamespaceName { } } +#[cfg(test)] +mod tests { + use super::{invalid_windows_component, NamespaceName}; + use bytes::Bytes; + + #[test] + fn accepts_single_component_names() { + for name in [ + "default", + "a_B-09", + "tenant.example", + ".hidden", + "name..part", + "tenant east", + "tenant@corp", + "a+b", + "a~b", + "café", + ] { + assert_eq!( + NamespaceName::from_string(name.into()).unwrap().as_str(), + name + ); + assert_eq!( + NamespaceName::from_bytes(Bytes::copy_from_slice(name.as_bytes())) + .unwrap() + .as_str(), + name + ); + assert_eq!( + serde_json::from_str::(&format!("\"{name}\"")) + .unwrap() + .as_str(), + name + ); + } + #[cfg(not(windows))] + assert!(NamespaceName::from_string("tenant:1".into()).is_ok()); + } + + #[test] + fn windows_component_policy() { + for name in [ + "tenant.", + "tenant ", + "tenant..", + "CON", + "con.txt", + "prn.log", + "AuX", + "nul.tar.gz", + "COM1", + "com9.db", + "Lpt1", + "lpt9.txt", + "COM¹", + "com².txt", + "COM³", + "lpt¹", + "LPT².db", + "lpt³", + "CONIN$", + "conout$.txt", + "COM1 .txt", + "a:b", + "a?b", + "a*b", + "ab", + "a|b", + "a\"b", + "a\u{1f}b", + ] { + assert!(invalid_windows_component(name), "{name:?}"); + #[cfg(windows)] + assert!(NamespaceName::from_string(name.into()).is_err(), "{name:?}"); + } + for name in [ + "tenant east", + "tenant.example", + ".hidden", + "name..part", + "café", + "COM0", + "COM10", + "LPT0", + "acorn.txt", + "community", + "xCON.txt", + ] { + assert!(!invalid_windows_component(name), "{name:?}"); + assert!(NamespaceName::from_string(name.into()).is_ok(), "{name:?}"); + } + #[cfg(not(windows))] + for name in ["tenant.", "tenant ", "con.txt", "COM¹", "a:b"] { + assert!(NamespaceName::from_string(name.into()).is_ok(), "{name:?}"); + } + } + + #[test] + fn rejects_unsafe_names_on_all_checked_constructors() { + for name in [ + "", + ".", + "..", + "../outside", + "a/../b", + "/absolute", + "a\\b", + "C:\\db", + "a\0b", + ] { + assert!(NamespaceName::from_string(name.into()).is_err(), "{name:?}"); + assert!( + NamespaceName::from_bytes(Bytes::copy_from_slice(name.as_bytes())).is_err(), + "{name:?}" + ); + assert!( + serde_json::from_str::(&serde_json::to_string(name).unwrap()) + .is_err(), + "{name:?}" + ); + } + assert!(NamespaceName::from_bytes(Bytes::from_static(b"\xff")).is_err()); + #[cfg(windows)] + for name in ["C:relative", "file:stream"] { + assert!(NamespaceName::from_string(name.into()).is_err(), "{name:?}"); + } + } +} + impl serde::Serialize for NamespaceName { fn serialize(&self, serializer: S) -> Result where diff --git a/libsql-server/src/replication/replicator_client.rs b/libsql-server/src/replication/replicator_client.rs index fb8154824d..02a43ecc4a 100644 --- a/libsql-server/src/replication/replicator_client.rs +++ b/libsql-server/src/replication/replicator_client.rs @@ -185,7 +185,7 @@ impl ReplicatorClient for Client { } self.meta_store_handle - .store(DatabaseConfig::from(config)) + .store(DatabaseConfig::try_from(config).map_err(|e| Error::Internal(e.into()))?) .await .map_err(|e| Error::Internal(e.into()))?; diff --git a/libsql-server/src/replication/script_backup_manager.rs b/libsql-server/src/replication/script_backup_manager.rs index ab78b49dd7..4980e3eb80 100644 --- a/libsql-server/src/replication/script_backup_manager.rs +++ b/libsql-server/src/replication/script_backup_manager.rs @@ -211,13 +211,16 @@ fn parse_snapshot_path(path: PathBuf) -> Result { return Err(Error::InvalidSnapshotPath(path.clone())); }; - let start_frame_no = FrameNo::from_str_radix(start_str, 16).unwrap(); - let end_frame_no = FrameNo::from_str_radix(end_str, 16).unwrap(); + let start_frame_no = FrameNo::from_str_radix(start_str, 16) + .map_err(|_| Error::InvalidSnapshotPath(path.clone()))?; + let end_frame_no = FrameNo::from_str_radix(end_str, 16) + .map_err(|_| Error::InvalidSnapshotPath(path.clone()))?; let Ok(log_id) = Uuid::from_str(log_id) else { return Err(Error::InvalidSnapshotPath(path.clone())); }; - let namespace = NamespaceName::from_string(namespace.to_string()).unwrap(); + let namespace = NamespaceName::from_string(namespace.to_string()) + .map_err(|_| Error::InvalidSnapshotPath(path.clone()))?; Ok(SnapshotEntry { namespace, @@ -299,6 +302,15 @@ mod test { use tempfile::tempdir; use uuid::Uuid; + #[test] + fn invalid_legacy_snapshot_namespace_does_not_panic() { + let path = PathBuf::from("script_backup/bad\\name:550e8400-e29b-41d4-a716-446655440000:00000000000000000001-00000000000000000002.snap"); + assert!(matches!( + parse_snapshot_path(path), + Err(Error::InvalidSnapshotPath(_)) + )); + } + proptest! { #[test] fn parse_rountrip_snapshot_path( diff --git a/libsql-server/src/schema/db.rs b/libsql-server/src/schema/db.rs index ec8dcad840..e377795204 100644 --- a/libsql-server/src/schema/db.rs +++ b/libsql-server/src/schema/db.rs @@ -125,9 +125,14 @@ pub(super) fn register_schema_migration_job( let Some(row) = rows.next()? else { return Err(Error::SchemaDoesntExist(schema.clone())); }; - let config_bytes = row.get_ref(1)?.as_blob().unwrap(); - // TODO: handle corrupted meta - let config = DatabaseConfig::from(&metadata::DatabaseConfig::decode(config_bytes).unwrap()); + let config_bytes = row + .get_ref(1)? + .as_blob() + .map_err(|e| Error::Registration(Box::new(e)))?; + let metadata = metadata::DatabaseConfig::decode(config_bytes) + .map_err(|e| Error::Registration(Box::new(e)))?; + let config = + DatabaseConfig::try_from(&metadata).map_err(|e| Error::Registration(Box::new(e)))?; if !config.is_shared_schema { return Err(Error::NotASchema(schema.clone())); } @@ -184,27 +189,16 @@ pub(super) fn get_next_pending_migration_tasks_batch( limit: usize, ) -> Result, Error> { let txn = conn.transaction_with_behavior(rusqlite::TransactionBehavior::Immediate)?; - let tasks = txn - .prepare( - "SELECT task_id, target_namespace, status, job_id - FROM pending_tasks + let tasks = { + let mut stmt = txn.prepare( + "SELECT task_id, target_namespace, status, job_id + FROM pending_tasks WHERE job_id = ? AND status = ? AND task_id NOT IN (select * from enqueued_tasks) LIMIT ?", - )? - .query_map((job_id, status as u64, limit), |row| { - let task_id = row.get::<_, i64>(0)?; - let namespace = NamespaceName::from_string(row.get::<_, String>(1)?).unwrap(); - let status = MigrationTaskStatus::from_int(row.get::<_, u64>(2)?); - let job_id = row.get::<_, i64>(3)?; - Ok(MigrationTask { - namespace, - status, - job_id, - task_id, - }) - })? - .map(|r| r.map_err(Into::into)) - .collect::, Error>>()?; + )?; + let mut rows = stmt.query((job_id, status as u64, limit))?; + read_migration_tasks(&mut rows)? + }; for task in tasks.iter() { txn.execute("INSERT INTO enqueued_tasks VALUES (?)", [task.task_id])?; @@ -221,27 +215,16 @@ pub(super) fn get_unfinished_task_batch( limit: usize, ) -> Result, Error> { let txn = conn.transaction_with_behavior(rusqlite::TransactionBehavior::Immediate)?; - let tasks = txn - .prepare( - "SELECT task_id, target_namespace, status, job_id - FROM pending_tasks + let tasks = { + let mut stmt = txn.prepare( + "SELECT task_id, target_namespace, status, job_id + FROM pending_tasks WHERE job_id = ? AND finished = false AND task_id NOT IN (select * from enqueued_tasks) LIMIT ?", - )? - .query_map((job_id, limit), |row| { - let task_id = row.get::<_, i64>(0)?; - let namespace = NamespaceName::from_string(row.get::<_, String>(1)?).unwrap(); - let status = MigrationTaskStatus::from_int(row.get::<_, u64>(2)?); - let job_id = row.get::<_, i64>(3)?; - Ok(MigrationTask { - namespace, - status, - job_id, - task_id, - }) - })? - .map(|r| r.map_err(Into::into)) - .collect::, Error>>()?; + )?; + let mut rows = stmt.query((job_id, limit))?; + read_migration_tasks(&mut rows)? + }; for task in tasks.iter() { txn.execute("INSERT INTO enqueued_tasks VALUES (?)", [task.task_id])?; @@ -251,6 +234,30 @@ pub(super) fn get_unfinished_task_batch( Ok(tasks) } +// Reject corrupt persisted names before enqueuing any tasks from the batch. An error +// rolls back the transaction, so the scheduler cannot silently lose unfinished work. +fn read_migration_tasks(rows: &mut rusqlite::Rows<'_>) -> Result, Error> { + let mut tasks = Vec::new(); + while let Some(row) = rows.next()? { + let task_id = row.get::<_, i64>(0)?; + let name = row.get::<_, String>(1)?; + let namespace = NamespaceName::from_string(name.clone()).map_err(|_| { + Error::InvalidPersistedNamespace { + kind: "task", + id: task_id, + name, + } + })?; + tasks.push(MigrationTask { + namespace, + status: MigrationTaskStatus::from_int(row.get::<_, u64>(2)?), + job_id: row.get::<_, i64>(3)?, + task_id, + }); + } + Ok(tasks) +} + pub(super) fn update_meta_task_status( conn: &mut rusqlite::Connection, task: &MigrationTask, @@ -316,7 +323,7 @@ pub(super) fn get_next_pending_migration_job( conn: &mut rusqlite::Connection, ) -> Result, Error> { let txn = conn.transaction()?; - let mut job = txn + let row = txn .query_row( "SELECT job_id, status, migration, schema FROM jobs @@ -327,23 +334,37 @@ pub(super) fn get_next_pending_migration_job( MigrationJobStatus::RunFailure as u64, ), |row| { - let job_id = row.get::<_, i64>(0)?; - let status = MigrationJobStatus::from_int(row.get::<_, u64>(1)?); - let mut migration = serde_json::from_str(row.get_ref(2)?.as_str()?).unwrap(); - let schema = NamespaceName::from_string(row.get::<_, String>(3)?).unwrap(); - let disable_foreign_key = validate_migration(&mut migration).unwrap(); - Ok(MigrationJob { - schema, - job_id, - status, - progress: Default::default(), - task_error: None, - disable_foreign_key, - migration: migration.into(), - }) + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, u64>(1)?, + row.get::<_, String>(2)?, + row.get::<_, String>(3)?, + )) }, ) .optional()?; + let mut job = if let Some((job_id, status, migration, name)) = row { + let schema = NamespaceName::from_string(name.clone()).map_err(|_| { + Error::InvalidPersistedNamespace { + kind: "job", + id: job_id, + name, + } + })?; + let mut migration = serde_json::from_str(&migration).unwrap(); + let disable_foreign_key = validate_migration(&mut migration).unwrap(); + Some(MigrationJob { + schema, + job_id, + status: MigrationJobStatus::from_int(status), + progress: Default::default(), + task_error: None, + disable_foreign_key, + migration: migration.into(), + }) + } else { + None + }; if let Some(ref mut job) = job { txn.prepare( @@ -482,6 +503,106 @@ mod test { use super::*; + #[test] + fn invalid_persisted_job_schema_can_be_repaired() { + let mut conn = rusqlite::Connection::open_in_memory().unwrap(); + setup_schema(&mut conn).unwrap(); + let migration = serde_json::to_string(&Program::seq(&["select 1"])).unwrap(); + conn.execute( + "INSERT INTO jobs (schema, migration, status) VALUES (?1, ?2, ?3)", + ( + "../schema", + migration, + MigrationJobStatus::WaitingDryRun as u64, + ), + ) + .unwrap(); + let job_id = conn.last_insert_rowid(); + + assert!(matches!( + get_next_pending_migration_job(&mut conn), + Err(Error::InvalidPersistedNamespace { kind: "job", id, .. }) if id == job_id + )); + conn.execute( + "UPDATE jobs SET schema = 'schema' WHERE job_id = ?", + [job_id], + ) + .unwrap(); + assert_eq!( + get_next_pending_migration_job(&mut conn) + .unwrap() + .unwrap() + .job_id(), + job_id + ); + } + + #[test] + fn invalid_persisted_task_namespace_does_not_enqueue_partial_batch() { + let mut conn = rusqlite::Connection::open_in_memory().unwrap(); + setup_schema(&mut conn).unwrap(); + let migration = serde_json::to_string(&Program::seq(&["select 1"])).unwrap(); + conn.execute( + "INSERT INTO jobs (schema, migration, status) VALUES (?1, ?2, ?3)", + ( + "schema", + migration, + MigrationJobStatus::WaitingDryRun as u64, + ), + ) + .unwrap(); + let job_id = conn.last_insert_rowid(); + conn.execute( + "INSERT INTO pending_tasks (job_id, target_namespace, status) VALUES (?1, 'valid', ?2)", + (job_id, MigrationTaskStatus::Enqueued as u64), + ) + .unwrap(); + conn.execute( + "INSERT INTO pending_tasks (job_id, target_namespace, status) VALUES (?1, '../escape', ?2)", + (job_id, MigrationTaskStatus::Enqueued as u64), + ) + .unwrap(); + let bad_task_id = conn.last_insert_rowid(); + + for fetch in [false, true] { + let result = if fetch { + get_unfinished_task_batch(&mut conn, job_id, 10) + } else { + get_next_pending_migration_tasks_batch( + &mut conn, + job_id, + MigrationTaskStatus::Enqueued, + 10, + ) + }; + assert!(matches!( + result, + Err(Error::InvalidPersistedNamespace { kind: "task", id, .. }) if id == bad_task_id + )); + let queued: i64 = conn + .query_row("SELECT count(*) FROM enqueued_tasks", [], |row| row.get(0)) + .unwrap(); + assert_eq!(queued, 0); + } + + conn.execute( + "UPDATE pending_tasks SET target_namespace = 'repaired' WHERE task_id = ?", + [bad_task_id], + ) + .unwrap(); + assert_eq!( + get_next_pending_migration_tasks_batch( + &mut conn, + job_id, + MigrationTaskStatus::Enqueued, + 10, + ) + .unwrap() + .len(), + 2 + ); + } + async fn register_schema(meta_store: &MetaStore, schema: &'static str) { meta_store .handle(schema.into()) diff --git a/libsql-server/src/schema/error.rs b/libsql-server/src/schema/error.rs index 13f21f3c15..3afbe4debb 100644 --- a/libsql-server/src/schema/error.rs +++ b/libsql-server/src/schema/error.rs @@ -15,6 +15,12 @@ pub enum Error { CorruptedJobStatus(serde_json::Error), #[error("sqlite error: {0}")] Sqlite(#[from] rusqlite::Error), + #[error("invalid persisted namespace in migration {kind} {id}: {name:?}; repair the metastore entry before restarting the scheduler")] + InvalidPersistedNamespace { + kind: &'static str, + id: i64, + name: String, + }, #[error("`{0}` is not a schema database")] NotASchema(NamespaceName), #[error("schema `{0}` doesn't exist")] diff --git a/libsql-server/src/schema/scheduler.rs b/libsql-server/src/schema/scheduler.rs index d9431b2d86..ea28078339 100644 --- a/libsql-server/src/schema/scheduler.rs +++ b/libsql-server/src/schema/scheduler.rs @@ -81,6 +81,13 @@ impl Scheduler { tracing::info!("all scheduler handles dropped: exiting."); break; } + Err(e @ Error::InvalidPersistedNamespace { .. }) => { + // Keep unfinished work intact for explicit operator repair. + tracing::error!( + "migration scheduler stopped on invalid persisted namespace: {e}" + ); + break; + } Err(e) => { if tries >= MAX_ERROR_RETRIES { tracing::error!("scheduler could not make progress after {MAX_ERROR_RETRIES}, exiting: {e}"); diff --git a/libsql-server/tests/namespaces/mod.rs b/libsql-server/tests/namespaces/mod.rs index 37b373b76e..1bc14d2bba 100644 --- a/libsql-server/tests/namespaces/mod.rs +++ b/libsql-server/tests/namespaces/mod.rs @@ -105,6 +105,96 @@ fn fork_namespace() { sim.run().unwrap(); } +#[test] +fn admin_rejects_namespace_path_traversal_without_touching_outside_files() { + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + let tmp = tempdir().unwrap(); + make_primary(&mut sim, tmp.path().to_path_buf()); + + let outside = tmp.path().join("outside"); + std::fs::create_dir(&outside).unwrap(); + std::fs::write(outside.join("sentinel"), b"keep me").unwrap(); + let dbs = tmp.path().join("dbs"); + let absolute_victim = + url::form_urlencoded::byte_serialize(outside.to_str().unwrap().as_bytes()) + .collect::() + .replace('+', "%20"); + let dbs_for_client = dbs.clone(); + + sim.client("client", async move { + let client = Client::new(); + assert!(client + .post( + "http://primary:9090/v1/namespaces/safe-1.example/create", + json!({}) + ) + .await? + .status() + .is_success()); + std::fs::create_dir_all(dbs_for_client.join("sentinel"))?; + std::fs::write(dbs_for_client.join("sentinel/marker"), b"keep me too")?; + for name in [ + "%2e%2e".to_owned(), + "%2e%2e%2foutside".to_owned(), + absolute_victim, + "bad%5cname".to_owned(), + ] { + assert_eq!( + client + .post( + &format!("http://primary:9090/v1/namespaces/{name}/create"), + json!({}) + ) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST, + "create {name}" + ); + assert_eq!( + client + .delete( + &format!("http://primary:9090/v1/namespaces/{name}"), + json!({}) + ) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST, + "delete {name}" + ); + assert_eq!( + client + .post( + &format!("http://primary:9090/v1/namespaces/safe-1.example/fork/{name}"), + () + ) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST, + "fork {name}" + ); + } + assert_eq!( + client + .post("http://primary:9090/v1/namespaces/%2e%2e/fork/target", ()) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + Ok(()) + }); + + sim.run().unwrap(); + assert_eq!(std::fs::read(outside.join("sentinel")).unwrap(), b"keep me"); + assert_eq!( + std::fs::read(dbs.join("sentinel/marker")).unwrap(), + b"keep me too" + ); + assert!(dbs.join("safe-1.example").exists()); + assert!(!dbs.join("target").exists()); +} + #[test] fn delete_namespace() { let mut sim = Builder::new() From 2498227e69d96a88ec103bdcd637d7b0fe92bdbd Mon Sep 17 00:00:00 2001 From: Raphael Lopes Feitoza Date: Wed, 30 Sep 2026 17:41:23 -0400 Subject: [PATCH 07/11] Reserve namespace directories and coordinate lifecycle cleanup --- docs/ADMIN_API.md | 36 +- libsql-server/src/admin_shell.rs | 70 +- libsql-server/src/lib.rs | 18 +- libsql-server/src/main.rs | 1 + libsql-server/src/namespace/cleanup_guard.rs | 170 ++ .../src/namespace/configurator/fork.rs | 58 +- .../src/namespace/configurator/helpers.rs | 7 +- .../src/namespace/configurator/mod.rs | 4 +- .../src/namespace/configurator/primary.rs | 45 +- .../src/namespace/configurator/replica.rs | 96 +- .../src/namespace/configurator/schema.rs | 6 +- libsql-server/src/namespace/meta_store.rs | 64 +- libsql-server/src/namespace/mod.rs | 1 + libsql-server/src/namespace/store.rs | 1545 ++++++++++++++++- libsql-server/src/schema/scheduler.rs | 78 +- libsql-server/tests/cluster/mod.rs | 409 ++++- .../tests/namespaces/availability.rs | 131 ++ .../tests/namespaces/default_create.rs | 194 +++ libsql-server/tests/namespaces/mod.rs | 79 +- libsql-server/tests/namespaces/ownership.rs | 403 +++++ 20 files changed, 3118 insertions(+), 297 deletions(-) create mode 100644 libsql-server/src/namespace/cleanup_guard.rs create mode 100644 libsql-server/tests/namespaces/availability.rs create mode 100644 libsql-server/tests/namespaces/default_create.rs create mode 100644 libsql-server/tests/namespaces/ownership.rs diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index 0688d755d1..110de71230 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -25,14 +25,30 @@ or allowing their rows to be overwritten; this holds even with `--meta-store-destroy-on-error`. Filesystem recovery skips invalid and symlinked directory entries without deleting them. An invalid persisted migration job/task stops its scheduler without marking that work complete. -Back up and inspect -metastore and namespace files before repairing these entries explicitly. - -This validation prevents path traversal *through a namespace string*. It does -not establish ownership of existing directories or protect against symlinks, -case/normalization aliases, or filesystem replacement races. Only trusted -operators should have write access to the data directory; additional directory -reservation and ownership protection is addressed separately. +Back up and inspect metastore and namespace files before repairing these entries +explicitly. + +Validation prevents path traversal *through a namespace string*. Directory +ownership checks additionally reserve new namespace/fork directories atomically, +reject any existing entry (including aliases and orphan directories), and +require an unloaded persisted namespace's actual directory entry to match its +stored name. A legacy alias or symlink is refused rather than opened or +deleted. These checks assume the data directory is trusted; they do not make +filesystem operations atomic against a privileged external process replacing +paths or symlinks outside the server's coordination locks. + +A cancelled create/fork or failed cleanup can retain a newly reserved directory +as quarantine after metadata has been removed, so delayed writes cannot reach +a retry. Inspect it and the metastore after the work stops before repairing or +retrying. Per-name operation locks allow unrelated namespace administration to +continue during slow restores. Shutdown signals in-flight create/fork work to +stop and permits a bounded drain before reporting an error. A replica detecting +an incompatible log moves its old files to `replica-log-quarantine/` outside +`dbs/`, retaining the namespace directory identity, then retries once. Inspect +quarantined files before removal. Destroy/reset confirms remote backups before +moving a directory to `namespace-teardown-quarantine/` for local removal; +backup failure/cancellation before confirmation leaves the old directory in +place. Inspect remaining quarantine files after interrupted teardown. ## Routes @@ -40,7 +56,9 @@ reservation and ownership protection is addressed separately. POST /v1/namespaces/:namespace/create ``` -Create a namespace named `:namespace`. +Create a namespace named `:namespace`. Explicit creation rejects an existing +namespace, including `default`; internal startup/lazy loading of `default` +reuses its persisted config rather than replacing it. body: ```json diff --git a/libsql-server/src/admin_shell.rs b/libsql-server/src/admin_shell.rs index 97a7d6941c..117ae5bfed 100644 --- a/libsql-server/src/admin_shell.rs +++ b/libsql-server/src/admin_shell.rs @@ -105,6 +105,61 @@ fn try_run_one(conn: &mut rusqlite::Connection, q: String) -> anyhow::Result Result { + let namespace = metadata + .get_bin("x-namespace-bin") + .ok_or_else(|| tonic::Status::invalid_argument("missing namespace"))?; + let bytes = namespace + .to_bytes() + .map_err(|_| tonic::Status::invalid_argument("bad namespace encoding"))?; + NamespaceName::from_bytes(bytes) + .map_err(|_| tonic::Status::invalid_argument("invalid namespace name")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn rejects_invalid_namespace_metadata() { + let mut metadata = tonic::metadata::MetadataMap::new(); + assert_eq!( + namespace_from_metadata(&metadata).unwrap_err().code(), + tonic::Code::InvalidArgument + ); + for name in [ + b"".as_slice(), + b"..", + b"../outside", + b"/outside", + b"a\\b", + b"a\0b", + b"\xff", + ] { + metadata.insert_bin("x-namespace-bin", BinaryMetadataValue::from_bytes(name)); + assert_eq!( + namespace_from_metadata(&metadata).unwrap_err().code(), + tonic::Code::InvalidArgument, + "{name:?}" + ); + } + } + + #[test] + fn accepts_safe_namespace_metadata() { + let mut metadata = tonic::metadata::MetadataMap::new(); + for name in ["default", "tenant-1.example", "tenant café"] { + metadata.insert_bin( + "x-namespace-bin", + BinaryMetadataValue::from_bytes(name.as_bytes()), + ); + assert_eq!(namespace_from_metadata(&metadata).unwrap().as_str(), name); + } + } +} + #[async_trait::async_trait] impl AdminShellService for AdminShell { type ShellStream = Pin> + Send>>; @@ -113,21 +168,8 @@ impl AdminShellService for AdminShell { &self, request: tonic::Request>, ) -> std::result::Result, tonic::Status> { - let Some(namespace) = request.metadata().get_bin("x-namespace-bin") else { - return Err(tonic::Status::new( - tonic::Code::InvalidArgument, - "missing namespace", - )); - }; - let Ok(ns_bytes) = namespace.to_bytes() else { - return Err(tonic::Status::new( - tonic::Code::InvalidArgument, - "bad namespace encoding", - )); - }; + let namespace = namespace_from_metadata(request.metadata())?; - let namespace = NamespaceName::from_bytes(ns_bytes) - .map_err(|_| tonic::Status::invalid_argument("invalid namespace"))?; match self.with_namespace(namespace, request.into_inner()).await { Ok(s) => Ok(tonic::Response::new(Box::pin(s))), Err(e) => Err(tonic::Status::new( diff --git a/libsql-server/src/lib.rs b/libsql-server/src/lib.rs index 1642ad951a..52b41a38ed 100644 --- a/libsql-server/src/lib.rs +++ b/libsql-server/src/lib.rs @@ -57,7 +57,7 @@ use self::namespace::configurator::{ BaseNamespaceConfig, NamespaceConfigurators, PrimaryConfig, PrimaryConfigurator, ReplicaConfigurator, SchemaConfigurator, }; -use self::namespace::NamespaceStore; +pub use self::namespace::{NamespaceStore, RestoreOption}; use self::net::AddrIncoming; use self::replication::script_backup_manager::{CommandHandler, ScriptBackupManager}; use self::schema::SchedulerHandle; @@ -139,6 +139,9 @@ pub struct Server, pub disable_namespaces: bool, pub shutdown: Arc, + /// Optional in-process lifecycle hook. The caller can observe and + /// coordinate the namespace store on the server's runtime (e.g. tests). + pub namespace_store_ready: Option>, pub max_active_namespaces: usize, pub meta_store_config: MetaStoreConfig, pub max_concurrent_connections: usize, @@ -168,6 +171,7 @@ impl Default for Server { heartbeat_config: Default::default(), disable_namespaces: true, shutdown: Default::default(), + namespace_store_ready: None, max_active_namespaces: 100, meta_store_config: Default::default(), max_concurrent_connections: 128, @@ -634,8 +638,12 @@ where meta_store, configurators, db_kind, + &self.path, ) .await?; + if let Some(ready) = self.namespace_store_ready.take() { + let _ = ready.send(namespace_store.clone()); + } self.spawn_monitoring_tasks(&mut task_manager, stats_receiver)?; @@ -695,13 +703,7 @@ where }); if self.disable_namespaces { - namespace_store - .create( - NamespaceName::default(), - namespace::RestoreOption::Latest, - Default::default(), - ) - .await?; + namespace_store.ensure_default_namespace().await?; } let replication_svc = make_replication_svc( diff --git a/libsql-server/src/main.rs b/libsql-server/src/main.rs index 307d5482fe..d106d39888 100644 --- a/libsql-server/src/main.rs +++ b/libsql-server/src/main.rs @@ -709,6 +709,7 @@ async fn build_server( disable_default_namespace: config.disable_default_namespace, disable_namespaces: !config.enable_namespaces, shutdown, + namespace_store_ready: None, max_active_namespaces: config.max_active_namespaces, meta_store_config, max_concurrent_connections: config.max_concurrent_connections, diff --git a/libsql-server/src/namespace/cleanup_guard.rs b/libsql-server/src/namespace/cleanup_guard.rs new file mode 100644 index 0000000000..5b12bbb4f0 --- /dev/null +++ b/libsql-server/src/namespace/cleanup_guard.rs @@ -0,0 +1,170 @@ +//! Cancellation-safe, runtime-neutral cleanup scheduling. This module has no +//! database dependencies so its current-thread tests can run independently of +//! the server's native SQLite linkage. +use std::future::Future; +use std::pin::Pin; + +use tokio::task::JoinHandle; + +pub(crate) type CleanupFuture = Pin + Send>>; + +pub(crate) struct DeferredCleanup { + state: Option, + cleanup: fn(State, bool) -> CleanupFuture, +} + +impl DeferredCleanup { + pub(crate) fn new(state: State, cleanup: fn(State, bool) -> CleanupFuture) -> Self { + Self { + state: Some(state), + cleanup, + } + } + + pub(crate) fn disarm(&mut self) -> Option { + self.state.take() + } + + fn start(&mut self, remove_directory: bool) -> Option> { + let state = self.state.take()?; + match tokio::runtime::Handle::try_current() { + Ok(runtime) => Some(runtime.spawn((self.cleanup)(state, remove_directory))), + Err(e) => { + // Dropping state must be non-destructive. The caller retains + // metadata/files for repair if no runtime can run the barrier. + tracing::error!("namespace cleanup quarantined without runtime: {e}"); + None + } + } + } + + pub(crate) async fn finish(mut self) { + if let Some(task) = self.start(true) { + if let Err(e) = task.await { + tracing::error!("namespace cleanup task failed: {e}"); + } + } + } +} + +impl Drop for DeferredCleanup { + fn drop(&mut self) { + // Never block a current-thread runtime. Dropping a request after + // scheduling cleanup detaches its waiter, not the cleanup worker. + let _ = self.start(false); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }; + use std::time::Duration; + use tokio::sync::{Mutex, Notify, OwnedMutexGuard}; + + struct State { + operation: OwnedMutexGuard<()>, + proceed: Arc, + started: Arc, + removed: Arc, + } + + fn cleanup(state: State, remove_directory: bool) -> CleanupFuture { + Box::pin(async move { + state.started.notify_one(); + state.proceed.notified().await; + state.removed.store(remove_directory, Ordering::SeqCst); + drop(state.operation); + }) + } + + async fn async_fixture() -> ( + DeferredCleanup, + Arc>, + Arc, + Arc, + Arc, + ) { + let operation = Arc::new(Mutex::new(())); + let proceed = Arc::new(Notify::new()); + let started = Arc::new(Notify::new()); + let removed = Arc::new(AtomicBool::new(false)); + let guard = operation.clone().lock_owned().await; + let cleanup = DeferredCleanup::new( + State { + operation: guard, + proceed: proceed.clone(), + started: started.clone(), + removed: removed.clone(), + }, + cleanup, + ); + (cleanup, operation, proceed, started, removed) + } + + #[tokio::test(flavor = "current_thread")] + async fn error_cleanup_awaits_on_current_thread_without_blocking() { + let (cleanup, operation, proceed, _started, removed) = async_fixture().await; + proceed.notify_one(); + cleanup.finish().await; + assert!(removed.load(Ordering::SeqCst)); + assert!(operation.try_lock().is_ok()); + } + + #[tokio::test(flavor = "current_thread")] + async fn disarmed_success_never_runs_cleanup() { + let (mut cleanup, operation, _proceed, _started, removed) = async_fixture().await; + let committed = cleanup.disarm().unwrap(); + drop(cleanup); + drop(committed); + assert!(operation.try_lock().is_ok()); + assert!(!removed.load(Ordering::SeqCst)); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_operation_keeps_reservation_until_worker_finishes() { + let (cleanup, operation, proceed, _started, removed) = async_fixture().await; + drop(cleanup); + assert!( + tokio::time::timeout(Duration::from_millis(20), operation.lock()) + .await + .is_err() + ); + proceed.notify_one(); + let _new_owner = tokio::time::timeout(Duration::from_secs(2), operation.lock()) + .await + .unwrap(); + assert!(!removed.load(Ordering::SeqCst)); + } + + async fn cancelled_waiter_keeps_reservation() { + let (cleanup, operation, proceed, started, removed) = async_fixture().await; + let waiting = tokio::spawn(cleanup.finish()); + started.notified().await; + waiting.abort(); + assert!(waiting.await.is_err()); + assert!( + tokio::time::timeout(Duration::from_millis(20), operation.lock()) + .await + .is_err() + ); + proceed.notify_one(); + let _new_owner = tokio::time::timeout(Duration::from_secs(2), operation.lock()) + .await + .unwrap(); + assert!(removed.load(Ordering::SeqCst)); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_cleanup_waiter_on_current_thread_is_ordered() { + cancelled_waiter_keeps_reservation().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn cancelled_cleanup_waiter_on_multithread_is_ordered() { + cancelled_waiter_keeps_reservation().await; + } +} diff --git a/libsql-server/src/namespace/configurator/fork.rs b/libsql-server/src/namespace/configurator/fork.rs index 80de9ab19c..5f08971007 100644 --- a/libsql-server/src/namespace/configurator/fork.rs +++ b/libsql-server/src/namespace/configurator/fork.rs @@ -17,7 +17,7 @@ use crate::namespace::meta_store::MetaStoreHandle; use crate::namespace::{Namespace, NamespaceBottomlessDbId}; use crate::replication::primary::frame_stream::FrameStream; use crate::replication::{LogReadError, ReplicationLogger}; -use crate::{BLOCKING_RT, LIBSQL_PAGE_SIZE}; +use crate::LIBSQL_PAGE_SIZE; use super::helpers::make_bottomless_options; use super::{NamespaceName, NamespaceStore, PrimaryConfig, RestoreOption}; @@ -126,26 +126,13 @@ pub struct PointInTimeRestore { impl ForkTask { pub async fn fork(self) -> Result { - let base_path = self.base_path.clone(); - let dest_namespace = self.to_namespace.clone(); - match self.try_fork().await { - Err(e) => { - let _ = - tokio::fs::remove_dir_all(base_path.join("dbs").join(dest_namespace.as_str())) - .await; - Err(e) - } - Ok(ns) => Ok(ns), - } - } - - async fn try_fork(self) -> Result { - // until what index to replicate - let base_path = self.base_path.clone(); - let temp_dir = BLOCKING_RT - .spawn_blocking(move || tempfile::tempdir_in(base_path)) - .await??; - let db_path = temp_dir.path().join("data"); + // The caller atomically reserved this directory and exclusively owns + // cleanup until we return. Never delete a destination by name here. + let db_path = self + .base_path + .join("dbs") + .join(self.to_namespace.as_str()) + .join("data"); if let Some(restore) = self.restore_to { Self::restore_from_backup(restore, db_path) @@ -155,9 +142,6 @@ impl ForkTask { Self::restore_from_log_file(&self.logger, db_path).await?; } - let dest_path = self.base_path.join("dbs").join(self.to_namespace.as_str()); - tokio::fs::rename(temp_dir.path(), dest_path).await?; - self.store .make_namespace(&self.to_namespace, self.to_config, RestoreOption::Latest) .await @@ -185,18 +169,22 @@ impl ForkTask { write_frame(&frame, &mut data_file).await?; } Err(LogReadError::SnapshotRequired) => { - let snapshot = loop { - if let Some(snap) = logger - .get_snapshot_file(next_frame_no) - .await - .map_err(ForkError::Internal)? - { - break snap; + // Bound the wait for each missing snapshot, not the + // total restore (large valid forks may take longer). + let snapshot = tokio::time::timeout(Duration::from_secs(600), async { + loop { + if let Some(snap) = logger + .get_snapshot_file(next_frame_no) + .await + .map_err(ForkError::Internal)? + { + break Ok::<_, ForkError>(snap); + } + tokio::time::sleep(Duration::from_millis(100)).await; } - - // the snapshot must exist, it is just not yet available. - tokio::time::sleep(Duration::from_millis(100)).await; - }; + }) + .await + .map_err(|_| ForkError::LogRead(anyhow!("timed out waiting for replication snapshot at frame {next_frame_no}")))??; let frames = snapshot.into_stream_mut_from(next_frame_no); tokio::pin!(frames); diff --git a/libsql-server/src/namespace/configurator/helpers.rs b/libsql-server/src/namespace/configurator/helpers.rs index 599320783d..de396aa796 100644 --- a/libsql-server/src/namespace/configurator/helpers.rs +++ b/libsql-server/src/namespace/configurator/helpers.rs @@ -470,7 +470,7 @@ pub(crate) async fn run_storage_monitor( } } -pub(super) async fn cleanup_primary( +pub(super) async fn prepare_primary_cleanup( base: &BaseNamespaceConfig, primary_config: &PrimaryConfig, namespace: &NamespaceName, @@ -502,10 +502,5 @@ pub(super) async fn cleanup_primary( } } - if ns_path.try_exists()? { - tracing::debug!("removing database directory: {}", ns_path.display()); - tokio::fs::remove_dir_all(ns_path).await?; - } - Ok(()) } diff --git a/libsql-server/src/namespace/configurator/mod.rs b/libsql-server/src/namespace/configurator/mod.rs index 517b21ca5a..c73e8c5aa4 100644 --- a/libsql-server/src/namespace/configurator/mod.rs +++ b/libsql-server/src/namespace/configurator/mod.rs @@ -121,7 +121,9 @@ pub trait ConfigureNamespace { broadcaster: BroadcasterHandle, ) -> Pin> + Send + 'a>>; - fn cleanup<'a>( + // Remote backup/pruning only. Local directory detachment is owned by + // NamespaceStore under its filesystem identity lock. + fn prepare_cleanup<'a>( &'a self, namespace: &'a NamespaceName, db_config: &'a DatabaseConfig, diff --git a/libsql-server/src/namespace/configurator/primary.rs b/libsql-server/src/namespace/configurator/primary.rs index f68405fad6..4392aebdca 100644 --- a/libsql-server/src/namespace/configurator/primary.rs +++ b/libsql-server/src/namespace/configurator/primary.rs @@ -21,7 +21,7 @@ use crate::namespace::{ use crate::run_periodic_checkpoint; use crate::schema::{has_pending_migration_task, setup_migration_table}; -use super::helpers::cleanup_primary; +use super::helpers::prepare_primary_cleanup; use super::{BaseNamespaceConfig, ConfigureNamespace, PrimaryConfig}; pub struct PrimaryConfigurator { @@ -129,36 +129,23 @@ impl ConfigureNamespace for PrimaryConfigurator { ) -> Pin> + Send + 'a>> { Box::pin(async move { let db_path: Arc = self.base.base_path.join("dbs").join(name.as_str()).into(); - let fresh_namespace = !db_path.try_exists()?; - // FIXME: make that truly atomic. explore the idea of using temp directories, and it's implications - match self - .try_new_primary( - name.clone(), - meta_store_handle, - restore_option, - resolve_attach_path, - db_path.clone(), - broadcaster, - self.base.encryption_config.clone(), - ) - .await - { - Ok(this) => Ok(this), - Err(e) if fresh_namespace => { - tracing::error!( - "an error occured while deleting creating namespace, cleaning..." - ); - if let Err(e) = tokio::fs::remove_dir_all(&db_path).await { - tracing::error!("failed to remove dirty namespace directory: {e}") - } - Err(e) - } - Err(e) => Err(e), - } + // A failed setup must not delete by path: a concurrent alias or + // replacement could own it. The store's reservation/cleanup guard + // decides whether the directory can safely be removed. + self.try_new_primary( + name.clone(), + meta_store_handle, + restore_option, + resolve_attach_path, + db_path, + broadcaster, + self.base.encryption_config.clone(), + ) + .await }) } - fn cleanup<'a>( + fn prepare_cleanup<'a>( &'a self, namespace: &'a NamespaceName, db_config: &'a DatabaseConfig, @@ -166,7 +153,7 @@ impl ConfigureNamespace for PrimaryConfigurator { bottomless_db_id_init: NamespaceBottomlessDbIdInit, ) -> Pin> + Send + 'a>> { Box::pin(async move { - cleanup_primary( + prepare_primary_cleanup( &self.base, &self.primary_config, namespace, diff --git a/libsql-server/src/namespace/configurator/replica.rs b/libsql-server/src/namespace/configurator/replica.rs index b1a108af73..b4245fc731 100644 --- a/libsql-server/src/namespace/configurator/replica.rs +++ b/libsql-server/src/namespace/configurator/replica.rs @@ -54,7 +54,7 @@ impl ConfigureNamespace for ReplicaConfigurator { fn setup<'a>( &'a self, meta_store_handle: MetaStoreHandle, - restore_option: RestoreOption, + _restore_option: RestoreOption, name: &'a NamespaceName, reset: ResetCb, resolve_attach_path: ResolveNamespacePathFn, @@ -67,49 +67,48 @@ impl ConfigureNamespace for ReplicaConfigurator { let channel = self.channel.clone(); let uri = self.uri.clone(); - let (new_frame_sender, new_frame_receiver) = watch::channel(None); - let rpc_client = ReplicationLogClient::with_origin(channel.clone(), uri.clone()); - let client = crate::replication::replicator_client::Client::new( - name.clone(), - rpc_client, - meta_store_handle.clone(), - store.clone(), - WalImpl::new_sqlite(&db_path, new_frame_sender).await?, - ) - .await?; - let mut replicator = libsql_replication::replicator::Replicator::new_sqlite( - client, - db_path.join("data"), - DEFAULT_AUTO_CHECKPOINT, - None, - ) - .await?; + // Capture the directory inode before opening the WAL. The global + // identity lock must not cover handshake: a linked tenant can + // load an uncached schema through store.with during that call. + let directory_identity = store.replica_directory_identity(name).await?; + let mut retried_incompatible_log = false; + let (mut replicator, new_frame_receiver) = loop { + let (new_frame_sender, new_frame_receiver) = watch::channel(None); + let rpc_client = ReplicationLogClient::with_origin(channel.clone(), uri.clone()); + let client = crate::replication::replicator_client::Client::new( + name.clone(), + rpc_client, + meta_store_handle.clone(), + store.clone(), + WalImpl::new_sqlite(&db_path, new_frame_sender).await?, + ) + .await?; + let mut replicator = libsql_replication::replicator::Replicator::new_sqlite( + client, + db_path.join("data"), + DEFAULT_AUTO_CHECKPOINT, + None, + ) + .await?; - tracing::debug!("try perform handshake"); - // force a handshake now, to retrieve the primary's current replication index - match replicator.try_perform_handshake().await { - Err(libsql_replication::replicator::Error::Meta( - libsql_replication::meta::Error::LogIncompatible, - )) => { - tracing::error!( - "trying to replicate incompatible logs, reseting replica and nuking db dir" - ); - std::fs::remove_dir_all(&db_path).unwrap(); - return self - .setup( - meta_store_handle, - restore_option, - name, - reset, - resolve_attach_path, - store, - broadcaster, - ) - .await; + tracing::debug!("try perform handshake"); + match replicator.try_perform_handshake().await { + Err(libsql_replication::replicator::Error::Meta( + libsql_replication::meta::Error::LogIncompatible, + )) if !retried_incompatible_log => { + // Close the WAL before moving its files. Retain the + // directory inode and quarantine its old contents so + // reservations stay valid and a failed retry is safe. + drop(replicator); + store + .quarantine_incompatible_replica_log(name, directory_identity) + .await?; + retried_incompatible_log = true; + } + Err(e) => return Err(e.into()), + Ok(_) => break (replicator, new_frame_receiver), } - Err(e) => Err(e)?, - Ok(_) => (), - } + }; tracing::debug!("done performing handshake"); @@ -278,21 +277,14 @@ impl ConfigureNamespace for ReplicaConfigurator { }) } - fn cleanup<'a>( + fn prepare_cleanup<'a>( &'a self, - namespace: &'a NamespaceName, + _namespace: &'a NamespaceName, _db_config: &DatabaseConfig, _prune_all: bool, _bottomless_db_id_init: NamespaceBottomlessDbIdInit, ) -> Pin> + Send + 'a>> { - Box::pin(async move { - let ns_path = self.base.base_path.join("dbs").join(namespace.as_str()); - if ns_path.try_exists()? { - tracing::debug!("removing database directory: {}", ns_path.display()); - tokio::fs::remove_dir_all(ns_path).await?; - } - Ok(()) - }) + Box::pin(async move { Ok(()) }) } fn fork<'a>( diff --git a/libsql-server/src/namespace/configurator/schema.rs b/libsql-server/src/namespace/configurator/schema.rs index 275fd71e93..97b23a67f8 100644 --- a/libsql-server/src/namespace/configurator/schema.rs +++ b/libsql-server/src/namespace/configurator/schema.rs @@ -13,7 +13,7 @@ use crate::namespace::{ }; use crate::schema::SchedulerHandle; -use super::helpers::{cleanup_primary, make_primary_connection_maker}; +use super::helpers::{make_primary_connection_maker, prepare_primary_cleanup}; use super::{BaseNamespaceConfig, ConfigureNamespace, PrimaryConfig}; pub struct SchemaConfigurator { @@ -94,7 +94,7 @@ impl ConfigureNamespace for SchemaConfigurator { }) } - fn cleanup<'a>( + fn prepare_cleanup<'a>( &'a self, namespace: &'a NamespaceName, db_config: &'a DatabaseConfig, @@ -102,7 +102,7 @@ impl ConfigureNamespace for SchemaConfigurator { bottomless_db_id_init: crate::namespace::NamespaceBottomlessDbIdInit, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { - cleanup_primary( + prepare_primary_cleanup( &self.base, &self.primary_config, namespace, diff --git a/libsql-server/src/namespace/meta_store.rs b/libsql-server/src/namespace/meta_store.rs index b0e19b833d..27eadb7237 100644 --- a/libsql-server/src/namespace/meta_store.rs +++ b/libsql-server/src/namespace/meta_store.rs @@ -530,6 +530,32 @@ impl MetaStore { } } + // A cancelled config update may already be queued when its caller drops. + // Place a barrier after it before removing that caller's metadata, so the + // background worker cannot reinsert a row after cleanup. + #[cfg(test)] + pub(crate) fn pending_change_count_for_test(&self) -> usize { + self.changes_tx.max_capacity() - self.changes_tx.capacity() + } + + #[cfg(test)] + pub(crate) async fn hold_connection_for_test( + &self, + ) -> tokio::sync::MutexGuard<'_, MetaStoreConnection> { + self.inner.conn.lock().await + } + + pub(crate) async fn wait_for_pending_changes(&self) -> Result<()> { + let (send, recv) = oneshot::channel(); + self.changes_tx + .send((NamespaceName::default(), None, send, false)) + .await + .map_err(|e| Error::MetaStoreUpdateFailure(e.into()))?; + recv.await + .map_err(|e| Error::MetaStoreUpdateFailure(e.into()))??; + Ok(()) + } + pub fn remove(&self, namespace: NamespaceName) -> Result>> { tracing::debug!("removing namespace `{}` from meta store", namespace); @@ -662,11 +688,22 @@ mod tests { let conn = maker().unwrap(); setup_connection(&conn).unwrap(); std::fs::write(&metastore_sentinel, b"metastore intact").unwrap(); + let schema = metadata::DatabaseConfig::from(&DatabaseConfig::default()).encode_to_vec(); + conn.execute( + "INSERT INTO namespace_configs VALUES (?1, ?2)", + rusqlite::params!["schema", schema], + ) + .unwrap(); conn.execute( "INSERT INTO namespace_configs VALUES (?1, ?2)", rusqlite::params!["tenant", invalid.clone()], ) .unwrap(); + conn.execute( + "INSERT INTO shared_schema_links VALUES ('schema', 'tenant')", + [], + ) + .unwrap(); } let config = MetaStoreConfig { destroy_on_error: true, @@ -705,8 +742,19 @@ mod tests { ) .unwrap(); assert_eq!(stored, invalid); - let repaired = - metadata::DatabaseConfig::from(&DatabaseConfig::default()).encode_to_vec(); + let links: i64 = conn + .query_row( + "SELECT count(*) FROM shared_schema_links WHERE shared_schema_name = 'schema' AND namespace = 'tenant'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(links, 1); + let repaired = metadata::DatabaseConfig { + shared_schema_name: Some("schema".into()), + ..metadata::DatabaseConfig::from(&DatabaseConfig::default()) + } + .encode_to_vec(); conn.execute( "UPDATE namespace_configs SET config = ?1 WHERE namespace = 'tenant'", [repaired], @@ -722,7 +770,17 @@ mod tests { ) .await .unwrap(); - assert!(store.exists(&NamespaceName::from("tenant")).await); + assert_eq!( + store + .handle(NamespaceName::from("tenant")) + .await + .get() + .shared_schema_name + .as_ref() + .unwrap() + .as_str(), + "schema" + ); } #[tokio::test] diff --git a/libsql-server/src/namespace/mod.rs b/libsql-server/src/namespace/mod.rs index ec45b50445..1af32f895c 100644 --- a/libsql-server/src/namespace/mod.rs +++ b/libsql-server/src/namespace/mod.rs @@ -19,6 +19,7 @@ pub use self::name::NamespaceName; pub use self::store::NamespaceStore; pub mod broadcasters; +mod cleanup_guard; pub(crate) mod configurator; pub mod meta_store; mod name; diff --git a/libsql-server/src/namespace/store.rs b/libsql-server/src/namespace/store.rs index 86e9438ccd..9021fdfc30 100644 --- a/libsql-server/src/namespace/store.rs +++ b/libsql-server/src/namespace/store.rs @@ -1,11 +1,15 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; +use std::sync::{Mutex as StdMutex, Weak}; use async_lock::RwLock; use chrono::NaiveDateTime; use futures::TryFutureExt; use moka::future::Cache; use once_cell::sync::OnceCell; +use tokio::sync::OwnedMutexGuard; use tokio::task::JoinSet; use tokio::time::{Duration, Instant}; use tokio_stream::wrappers::BroadcastStream; @@ -20,6 +24,7 @@ use crate::namespace::{NamespaceBottomlessDbId, NamespaceBottomlessDbIdInit, Nam use crate::stats::Stats; use super::broadcasters::{BroadcasterHandle, BroadcasterRegistry}; +use super::cleanup_guard::{CleanupFuture, DeferredCleanup}; use super::configurator::{DynConfigurator, NamespaceConfigurators}; use super::meta_store::{MetaStore, MetaStoreHandle}; use super::schema_lock::SchemaLocksRegistry; @@ -27,6 +32,889 @@ use super::{Namespace, ResetCb, ResetOp, ResolveNamespacePathFn, RestoreOption}; type NamespaceEntry = Arc>>; +// A new directory is owned only after create_dir succeeds. Dropping a +// reservation never deletes by path: cancellation without a cleanup worker +// must leave a safe, explicit orphan instead of racing a later creator. +struct DirectoryReservation { + path: PathBuf, + identity: Option<(u64, u64)>, + owned: bool, +} + +struct PendingCleanup { + metadata: MetaStore, + namespace: NamespaceName, + directory: DirectoryReservation, + // Held across the queued-write barrier, metadata removal, and disk cleanup. + _operation: Vec>, +} + +type CleanupGuard = DeferredCleanup; + +fn pending_cleanup( + metadata: MetaStore, + namespace: NamespaceName, + directory: DirectoryReservation, + operation: Vec>, +) -> CleanupGuard { + fn run(pending: PendingCleanup, remove_directory: bool) -> CleanupFuture { + Box::pin(pending.run(remove_directory)) + } + CleanupGuard::new( + PendingCleanup { + metadata, + namespace, + directory, + _operation: operation, + }, + run, + ) +} + +impl PendingCleanup { + async fn run(mut self, remove_directory: bool) { + if !remove_directory { + // A cancelled filesystem future may still run in Tokio's blocking + // pool. Retain its exact path so late writes cannot hit a retry. + tracing::warn!( + "quarantining cancelled namespace directory {:?}", + self.directory.path + ); + self.directory.disarm(); + } + if let Err(e) = self.metadata.wait_for_pending_changes().await { + tracing::error!("namespace cleanup quarantined after config barrier failure: {e}"); + return; + } + // Once started, the blocking closure owns the operation guard, so + // cancellation of this async task cannot release it while removal is + // running (or let an older cleanup delete a later reservation). + let task = tokio::task::spawn_blocking(move || { + if let Err(e) = self.metadata.remove(self.namespace) { + tracing::error!("namespace cleanup quarantined after metadata failure: {e}"); + return; + } + self.directory.remove_if_owned(); + }); + if let Err(e) = task.await { + tracing::error!("namespace cleanup worker failed: {e}"); + } + } +} + +fn directory_identity(metadata: &std::fs::Metadata) -> Option<(u64, u64)> { + if !metadata.is_dir() || metadata.file_type().is_symlink() { + return None; + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + Some((metadata.dev(), metadata.ino())) + } + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt; + Some(( + metadata.volume_serial_number()? as u64, + metadata.file_index()?, + )) + } + #[cfg(not(any(unix, windows)))] + { + None + } +} + +impl Drop for DirectoryReservation { + fn drop(&mut self) { + if self.owned { + tracing::warn!( + "leaving reserved namespace directory {:?} for repair", + self.path + ); + } + } +} + +impl DirectoryReservation { + fn disarm(&mut self) { + self.owned = false; + } + + fn remove_if_owned(mut self) { + if !self.owned { + return; + } + // A replaced entry (or an unavailable identity) is not ours to + // remove. This is run on a blocking worker, never in Drop. + let still_owned = self.identity.is_some_and(|identity| { + std::fs::symlink_metadata(&self.path) + .ok() + .and_then(|m| directory_identity(&m)) + == Some(identity) + }); + if still_owned { + if let Err(e) = std::fs::remove_dir_all(&self.path) { + tracing::error!( + "failed to clean reserved namespace directory {:?}: {e}", + self.path + ); + } + } else { + tracing::warn!( + "leaving replaced or unidentified namespace directory {:?}", + self.path + ); + } + self.disarm(); + } +} + +#[cfg(test)] +mod directory_tests { + use super::*; + use crate::namespace::configurator::{ + BaseNamespaceConfig, ConfigureNamespace, PrimaryConfig, PrimaryConfigurator, + }; + use crate::namespace::meta_store::metastore_connection_maker; + use libsql_sys::wal::Sqlite3WalManager; + use tokio::sync::{Notify, Semaphore}; + use tokio::time::timeout; + + struct StalledCleanupConfigurator { + primary: PrimaryConfigurator, + entered: Arc, + release: Arc, + fail_backup: bool, + } + + impl ConfigureNamespace for StalledCleanupConfigurator { + fn setup<'a>( + &'a self, + db_config: MetaStoreHandle, + restore: RestoreOption, + name: &'a NamespaceName, + reset: ResetCb, + resolve: ResolveNamespacePathFn, + store: NamespaceStore, + broadcaster: BroadcasterHandle, + ) -> std::pin::Pin> + Send + 'a>> + { + self.primary + .setup(db_config, restore, name, reset, resolve, store, broadcaster) + } + + fn prepare_cleanup<'a>( + &'a self, + namespace: &'a NamespaceName, + config: &'a DatabaseConfig, + prune_all: bool, + init: NamespaceBottomlessDbIdInit, + ) -> std::pin::Pin> + Send + 'a>> + { + Box::pin(async move { + self.entered.notify_one(); + self.release.notified().await; + if self.fail_backup { + return Err(Error::InvalidPath( + "injected backup confirmation failure".into(), + )); + } + self.primary + .prepare_cleanup(namespace, config, prune_all, init) + .await + }) + } + + fn fork<'a>( + &'a self, + source: &'a Namespace, + source_config: MetaStoreHandle, + destination: NamespaceName, + dest_config: MetaStoreHandle, + timestamp: Option, + store: NamespaceStore, + ) -> std::pin::Pin> + Send + 'a>> + { + self.primary.fork( + source, + source_config, + destination, + dest_config, + timestamp, + store, + ) + } + } + + async fn cleanup_fixture() -> ( + tempfile::TempDir, + MetaStore, + Arc>, + CleanupGuard, + ) { + let tmp = tempfile::tempdir().unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + Default::default(), + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + let dbs = tmp.path().join("dbs"); + std::fs::create_dir(&dbs).unwrap(); + let path = dbs.join("failed"); + std::fs::create_dir(&path).unwrap(); + let reservation = DirectoryReservation { + identity: directory_identity(&std::fs::symlink_metadata(&path).unwrap()), + path, + owned: true, + }; + let lock = Arc::new(tokio::sync::Mutex::new(())); + let operation = lock.clone().lock_owned().await; + let cleanup = pending_cleanup( + metadata.clone(), + NamespaceName::from("failed"), + reservation, + vec![operation], + ); + (tmp, metadata, lock, cleanup) + } + + async fn primary_fixture() -> (tempfile::TempDir, NamespaceStore) { + primary_fixture_with_cleanup_gate(None).await + } + + async fn primary_fixture_with_cleanup_gate( + gate: Option<(Arc, Arc, bool)>, + ) -> (tempfile::TempDir, NamespaceStore) { + let tmp = tempfile::tempdir().unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + Default::default(), + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + let base = BaseNamespaceConfig { + base_path: tmp.path().to_path_buf().into(), + extensions: Arc::new([]), + stats_sender: tokio::sync::mpsc::channel(1).0, + max_response_size: 100000000000000, + max_total_response_size: 100000000000, + max_concurrent_connections: Arc::new(Semaphore::new(10)), + max_concurrent_requests: 10000, + encryption_config: None, + connection_creation_timeout: None, + disable_intelligent_throttling: false, + }; + let primary = PrimaryConfig { + max_log_size: 1000000000, + max_log_duration: None, + bottomless_replication: None, + scripted_backup: None, + checkpoint_interval: None, + }; + let mut configurators = NamespaceConfigurators::empty(); + let primary = + PrimaryConfigurator::new(base, primary, Arc::new(|| Sqlite3WalManager::default())); + if let Some((entered, release, fail_backup)) = gate { + configurators.with_primary(StalledCleanupConfigurator { + primary, + entered, + release, + fail_backup, + }); + } else { + configurators.with_primary(primary); + } + let store = NamespaceStore::new( + false, + false, + 10, + metadata, + configurators, + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); + (tmp, store) + } + + #[tokio::test(flavor = "current_thread")] + async fn teardown_refuses_replaced_directory_and_preserves_both_inodes() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + let path = tmp.path().join("dbs/victim"); + std::fs::create_dir_all(&path).unwrap(); + let expected = store.cleanup_directory_identity(&name).await.unwrap(); + std::fs::rename(&path, tmp.path().join("original")).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("sentinel"), b"new owner").unwrap(); + assert!(store + .detach_owned_directory(&name, expected, false) + .await + .is_err()); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"new owner"); + assert!(tmp.path().join("original").exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn stalled_destroy_backup_does_not_serialize_unrelated_namespaces() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let victim = NamespaceName::from("victim"); + store + .create( + victim.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let victim_path = tmp.path().join("dbs/victim"); + std::fs::write(victim_path.join("sentinel"), b"old data").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let victim = victim.clone(); + async move { store.destroy(victim, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + assert_eq!( + std::fs::read(victim_path.join("sentinel")).unwrap(), + b"old data" + ); + // This is an actual store/configurator cleanup halted at the point + // where bottomless savepoint().confirmed() would await remote I/O. + timeout( + Duration::from_secs(5), + store.create( + NamespaceName::from("unrelated"), + RestoreOption::Latest, + DatabaseConfig::default(), + ), + ) + .await + .unwrap() + .unwrap(); + let retry = tokio::spawn({ + let store = store.clone(); + let victim = victim.clone(); + async move { + store + .create(victim, RestoreOption::Latest, DatabaseConfig::default()) + .await + } + }); + tokio::task::yield_now().await; + assert!(!retry.is_finished()); + assert!(victim_path.join("sentinel").exists()); + release.notify_one(); + timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .unwrap(); + timeout(Duration::from_secs(5), retry) + .await + .unwrap() + .unwrap() + .unwrap(); + assert!(!victim_path.join("sentinel").exists()); + assert!(store.exists(&victim).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn stalled_reset_backup_keeps_old_inode_until_confirmed() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old data").unwrap(); + let reset = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.reset(name, RestoreOption::Latest).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old data"); + timeout( + Duration::from_secs(5), + store.create( + NamespaceName::from("unrelated"), + RestoreOption::Latest, + DatabaseConfig::default(), + ), + ) + .await + .unwrap() + .unwrap(); + let same_name = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.with(name, |ns| ns.path.clone()).await } + }); + tokio::task::yield_now().await; + assert!(!same_name.is_finished()); + release.notify_one(); + timeout(Duration::from_secs(5), reset) + .await + .unwrap() + .unwrap() + .unwrap(); + timeout(Duration::from_secs(5), same_name) + .await + .unwrap() + .unwrap() + .unwrap(); + assert!(!path.join("sentinel").exists()); + assert!(store.exists(&name).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn failed_backup_keeps_old_directory_and_rejects_same_name_retry() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), true))).await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"not backed up").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + release.notify_one(); + assert!(timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .is_err()); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"not backed up" + ); + assert!(store + .create(name, RestoreOption::Latest, DatabaseConfig::default()) + .await + .is_err()); + } + + #[tokio::test(flavor = "current_thread")] + async fn shutdown_reports_failed_drain_while_backup_confirmation_is_stalled() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (_tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + let result = timeout( + Duration::from_secs(2), + store + .clone() + .shutdown_with_timeout(Duration::from_millis(40)), + ) + .await + .unwrap(); + assert!(matches!(result, Err(Error::Blocked(_)))); + release.notify_one(); + timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .unwrap(); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_backup_keeps_old_directory() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release, false))).await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"not confirmed").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + destroy.abort(); + assert!(destroy.await.unwrap_err().is_cancelled()); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"not confirmed" + ); + assert!(store + .create(name, RestoreOption::Latest, DatabaseConfig::default()) + .await + .is_err()); + } + + #[tokio::test(flavor = "current_thread")] + async fn pending_dump_does_not_block_unrelated_operations_and_shutdown() { + let (tmp, store) = primary_fixture().await; + store + .create( + NamespaceName::from("victim"), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let slow = tokio::spawn({ + let store = store.clone(); + async move { + let pending = futures::stream::pending::>(); + store + .create( + NamespaceName::from("slow"), + RestoreOption::Dump(Box::new(pending)), + DatabaseConfig::default(), + ) + .await + } + }); + timeout(Duration::from_secs(5), async { + while !tmp.path().join("dbs/slow").exists() { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + timeout(Duration::from_secs(5), async { + store + .create( + NamespaceName::from("fast"), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await?; + store.destroy(NamespaceName::from("victim"), false).await?; + store.ensure_default_namespace().await?; + Ok::<_, crate::Error>(()) + }) + .await + .unwrap() + .unwrap(); + assert!(!slow.is_finished()); + let duplicate = tokio::spawn({ + let store = store.clone(); + async move { + store + .create( + NamespaceName::from("slow"), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + } + }); + // Shutdown signals slow restore to cancel, and drains its detached + // quarantine cleanup before metastore backup; it must not wait 20s. + timeout(Duration::from_secs(5), store.clone().shutdown()) + .await + .unwrap() + .unwrap(); + assert!(slow.await.unwrap().is_err()); + assert!(duplicate.await.unwrap().is_err()); + assert!(tmp.path().join("dbs/slow").exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn incompatible_replica_log_quarantines_contents_without_replacing_directory() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("tenant"); + let path = tmp.path().join("dbs/tenant"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("old-log"), b"preserved").unwrap(); + let identity = store.replica_directory_identity(&name).await.unwrap(); + store + .quarantine_incompatible_replica_log(&name, identity) + .await + .unwrap(); + assert_eq!( + store.replica_directory_identity(&name).await.unwrap(), + identity + ); + assert!(!path.join("old-log").exists()); + let root = tmp.path().join("replica-log-quarantine"); + let quarantined = std::fs::read_dir(root) + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + assert_eq!( + std::fs::read(quarantined.join("old-log")).unwrap(), + b"preserved" + ); + + std::fs::rename(&path, tmp.path().join("moved")).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("new-log"), b"untouched").unwrap(); + assert!(store + .quarantine_incompatible_replica_log(&name, identity) + .await + .is_err()); + assert_eq!(std::fs::read(path.join("new-log")).unwrap(), b"untouched"); + } + + #[cfg(unix)] + #[tokio::test(flavor = "current_thread")] + async fn incompatible_replica_log_refuses_alias_directory() { + use std::os::unix::fs::symlink; + let (tmp, store) = primary_fixture().await; + let path = tmp.path().join("dbs/actual"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("sentinel"), b"untouched").unwrap(); + symlink(&path, tmp.path().join("dbs/alias")).unwrap(); + assert!(store + .replica_directory_identity(&NamespaceName::from("alias")) + .await + .is_err()); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"untouched"); + } + + #[tokio::test(flavor = "current_thread")] + async fn name_lock_registry_prunes_idle_names() { + let (_tmp, store) = primary_fixture().await; + for id in 0..128 { + let namespace = NamespaceName::from_string(format!("test-{id}")).unwrap(); + drop(store.lock_names(&[namespace]).await.unwrap()); + } + assert!(store.inner.name_operations.lock().unwrap().len() <= 1); + } + + #[tokio::test(flavor = "current_thread")] + async fn shutdown_returns_bounded_error_if_operation_cannot_drain() { + let (_tmp, store) = primary_fixture().await; + let held = store + .lock_names(&[NamespaceName::from("stuck")]) + .await + .unwrap(); + let result = timeout( + Duration::from_secs(2), + store + .clone() + .shutdown_with_timeout(Duration::from_millis(40)), + ) + .await + .unwrap(); + assert!(matches!(result, Err(Error::Blocked(_)))); + drop(held); + } + + #[tokio::test(flavor = "current_thread")] + async fn current_thread_error_cleanup_awaits_barrier_and_removes_owned_directory() { + let (tmp, metadata, _lock, cleanup) = cleanup_fixture().await; + let namespace = NamespaceName::from("failed"); + metadata + .handle(namespace.clone()) + .await + .store(Arc::new(DatabaseConfig::default())) + .await + .unwrap(); + cleanup.finish().await; + assert!(!metadata.exists(&namespace).await); + assert!(!tmp.path().join("dbs/failed").exists()); + } + + async fn cancelled_cleanup_keeps_lock_until_queued_write_and_removal_finish() { + let (tmp, metadata, lock, cleanup) = cleanup_fixture().await; + let namespace = NamespaceName::from("failed"); + let handle = metadata.handle(namespace.clone()).await; + let held_conn = metadata.hold_connection_for_test().await; + let mut queued_write = Box::pin(handle.store(Arc::new(DatabaseConfig::default()))); + // Poll once while the connection is locked: the write is enqueued but + // cannot complete. Cancel its caller, as with a cancelled fork/create. + assert!(matches!( + futures::poll!(queued_write.as_mut()), + std::task::Poll::Pending + )); + drop(queued_write); + // Wait until the worker has consumed the blocked write, then observe + // the barrier enqueued by cleanup before cancelling its waiter. + timeout(Duration::from_secs(2), async { + while metadata.pending_change_count_for_test() != 0 { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + let waiter = tokio::spawn(cleanup.finish()); + timeout(Duration::from_secs(2), async { + while metadata.pending_change_count_for_test() == 0 { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + waiter.abort(); + assert!(waiter.await.is_err()); + assert!(timeout(Duration::from_millis(30), lock.lock()) + .await + .is_err()); + drop(held_conn); + let _next_operation = timeout(Duration::from_secs(5), lock.lock()).await.unwrap(); + assert!(!metadata.exists(&namespace).await); + assert!(!tmp.path().join("dbs/failed").exists()); + // A later reservation must not be removed or reinserted by the old + // detached cleanup, even after it has had another scheduling turn. + std::fs::create_dir(tmp.path().join("dbs/failed")).unwrap(); + std::fs::write(tmp.path().join("dbs/failed/sentinel"), b"new owner").unwrap(); + metadata + .handle(namespace.clone()) + .await + .store(Arc::new(DatabaseConfig::default())) + .await + .unwrap(); + tokio::task::yield_now().await; + assert_eq!( + std::fs::read(tmp.path().join("dbs/failed/sentinel")).unwrap(), + b"new owner" + ); + assert!(metadata.exists(&namespace).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_operation_quarantines_directory_and_queued_write() { + let (tmp, metadata, lock, cleanup) = cleanup_fixture().await; + let namespace = NamespaceName::from("failed"); + let other = tmp.path().join("dbs/other"); + std::fs::create_dir(&other).unwrap(); + std::fs::write(other.join("sentinel"), b"unrelated").unwrap(); + let held_conn = metadata.hold_connection_for_test().await; + let handle = metadata.handle(namespace.clone()).await; + let mut queued_write = Box::pin(handle.store(Arc::new(DatabaseConfig::default()))); + assert!(matches!( + futures::poll!(queued_write.as_mut()), + std::task::Poll::Pending + )); + drop(queued_write); + let (ready, started) = tokio::sync::oneshot::channel(); + let cancelled = tokio::spawn(async move { + let _cleanup = cleanup; + ready.send(()).unwrap(); + futures::future::pending::<()>().await; + }); + started.await.unwrap(); + cancelled.abort(); + assert!(cancelled.await.is_err()); + assert!(timeout(Duration::from_millis(30), lock.lock()) + .await + .is_err()); + drop(held_conn); + let _next_operation = timeout(Duration::from_secs(5), lock.lock()).await.unwrap(); + assert!(!metadata.exists(&namespace).await); + assert!(tmp.path().join("dbs/failed").is_dir()); + assert_eq!( + std::fs::create_dir(tmp.path().join("dbs/failed")) + .unwrap_err() + .kind(), + std::io::ErrorKind::AlreadyExists + ); + assert_eq!(std::fs::read(other.join("sentinel")).unwrap(), b"unrelated"); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancellation_on_current_thread_is_ordered() { + cancelled_cleanup_keeps_lock_until_queued_write_and_removal_finish().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn cancellation_on_multithread_is_ordered() { + cancelled_cleanup_keeps_lock_until_queued_write_and_removal_finish().await; + } + + #[test] + fn reservation_cleanup_does_not_remove_replacement() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("dest"); + std::fs::create_dir(&path).unwrap(); + let identity = directory_identity(&std::fs::symlink_metadata(&path).unwrap()); + let reservation = DirectoryReservation { + path: path.clone(), + identity, + owned: true, + }; + std::fs::rename(&path, tmp.path().join("original")).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("sentinel"), b"do not remove replacement").unwrap(); + reservation.remove_if_owned(); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"do not remove replacement" + ); + assert!(tmp.path().join("original").is_dir()); + } +} + /// Stores and manage a set of namespaces. pub struct NamespaceStore { pub inner: Arc, @@ -50,6 +938,10 @@ pub struct NamespaceStoreInner { broadcasters: BroadcasterRegistry, configurators: NamespaceConfigurators, db_kind: DatabaseKind, + dbs_path: PathBuf, + fs_operations: Arc>, + name_operations: StdMutex>>>, + shutdown_signal: tokio::sync::watch::Sender, } impl NamespaceStore { @@ -60,6 +952,7 @@ impl NamespaceStore { metadata: MetaStore, configurators: NamespaceConfigurators, db_kind: DatabaseKind, + base_path: &Path, ) -> crate::Result { tracing::trace!("Max active namespaces: {max_active_namespaces}"); let store = Cache::::builder() @@ -95,6 +988,10 @@ impl NamespaceStore { broadcasters: Default::default(), configurators, db_kind, + dbs_path: base_path.join("dbs"), + fs_operations: Arc::new(tokio::sync::Mutex::new(())), + name_operations: StdMutex::new(HashMap::new()), + shutdown_signal: tokio::sync::watch::channel(false).0, }), }) } @@ -103,11 +1000,291 @@ impl NamespaceStore { self.inner.metadata.exists(namespace).await } - pub async fn destroy(&self, namespace: NamespaceName, prune_all: bool) -> crate::Result<()> { + /// Test/embedding hook: force a cache miss without touching persisted + /// config or namespace files. Integration tests use this to exercise a + /// replica reset whose linked schema really is unloaded at handshake. + #[doc(hidden)] + pub async fn evict_cached_namespace(&self, namespace: &NamespaceName) { + self.inner.store.invalidate(namespace).await; + } + + // Weak entries prevent an unbounded registry. All operations acquire + // multiple names in lexical order (source before/destination as sorted), + // then the short filesystem identity lock if needed. + async fn lock_names(&self, names: &[NamespaceName]) -> crate::Result>> { + let mut names = names.to_vec(); + names.sort_by(|a, b| a.as_str().cmp(b.as_str())); + names.dedup(); + let locks = { + let mut registry = self.inner.name_operations.lock().unwrap(); + registry.retain(|_, lock| lock.strong_count() > 0); + names + .iter() + .map(|name| { + if let Some(lock) = registry.get(name).and_then(Weak::upgrade) { + lock + } else { + let lock = Arc::new(tokio::sync::Mutex::new(())); + registry.insert(name.clone(), Arc::downgrade(&lock)); + lock + } + }) + .collect::>() + }; + let mut guards = Vec::with_capacity(locks.len()); + for lock in locks { + guards.push(lock.lock_owned().await); + } if self.inner.has_shutdown.load(Ordering::Relaxed) { return Err(Error::NamespaceStoreShutdown); } + Ok(guards) + } + fn directory_path(&self, namespace: &NamespaceName) -> PathBuf { + self.inner.dbs_path.join(namespace.as_str()) + } + + // Lookup by path alone is insufficient on case-insensitive/normalizing + // filesystems. The actual entry must be spelled exactly as the metastore key. + async fn check_existing_directory(&self, namespace: &NamespaceName) -> crate::Result<()> { + let path = self.directory_path(namespace); + match tokio::fs::symlink_metadata(&path).await { + Ok(_) => { + let mut entries = tokio::fs::read_dir(&self.inner.dbs_path).await?; + while let Some(entry) = entries.next_entry().await? { + if entry.file_name() == namespace.as_str() { + if entry.file_type().await?.is_dir() { + return Ok(()); + } + break; + } + } + Err(Error::InvalidPath(format!( + "namespace `{namespace}` resolves to a different or non-directory filesystem entry" + ))) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e.into()), + } + } + + async fn reserve_directory( + &self, + namespace: &NamespaceName, + ) -> crate::Result { + tokio::fs::create_dir_all(&self.inner.dbs_path).await?; + let path = self.directory_path(namespace); + match tokio::fs::create_dir(&path).await { + Ok(()) => { + let identity = tokio::fs::symlink_metadata(&path) + .await + .ok() + .and_then(|m| directory_identity(&m)); + Ok(DirectoryReservation { + path, + identity, + owned: true, + }) + } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + Err(Error::NamespaceAlreadyExist(namespace.to_string())) + } + Err(e) => Err(e.into()), + } + } + + async fn ensure_existing_directory( + &self, + namespace: &NamespaceName, + ) -> crate::Result> { + // Also serialize restoration of a missing persisted directory with + // deletion of a legacy filesystem alias, which may have another key. + let _identity = self.inner.fs_operations.lock().await; + self.check_existing_directory(namespace).await?; + match self.reserve_directory(namespace).await { + Ok(reservation) => Ok(Some(reservation)), + Err(Error::NamespaceAlreadyExist(_)) => { + self.check_existing_directory(namespace).await?; + Ok(None) + } + Err(e) => Err(e), + } + } + + // Replica setup calls this before opening its WAL. Never hold this lock + // during handshake: linked-schema resolution may load another namespace. + pub(crate) async fn replica_directory_identity( + &self, + namespace: &NamespaceName, + ) -> crate::Result<(u64, u64)> { + let _identity = self.inner.fs_operations.lock().await; + self.check_existing_directory(namespace).await?; + let path = self.directory_path(namespace); + let metadata = tokio::fs::symlink_metadata(&path).await?; + directory_identity(&metadata).ok_or_else(|| { + Error::InvalidPath(format!( + "replica directory `{namespace}` has no safe filesystem identity" + )) + }) + } + + // Keep the directory inode (and any DirectoryReservation for it) stable. + // Move incompatible files to a separate quarantine outside dbs rather + // than deleting the directory by name or recursively retrying setup. + pub(crate) async fn quarantine_incompatible_replica_log( + &self, + namespace: &NamespaceName, + expected: (u64, u64), + ) -> crate::Result<()> { + let _identity = self.inner.fs_operations.lock().await; + self.check_existing_directory(namespace).await?; + let path = self.directory_path(namespace); + let actual = std::fs::symlink_metadata(&path) + .ok() + .and_then(|metadata| directory_identity(&metadata)); + if actual != Some(expected) { + return Err(Error::InvalidPath(format!( + "replica directory `{namespace}` changed during handshake; refusing to replace its log" + ))); + } + let quarantine_root = self + .inner + .dbs_path + .parent() + .unwrap() + .join("replica-log-quarantine"); + std::fs::create_dir_all(&quarantine_root)?; + let metadata = std::fs::symlink_metadata(&quarantine_root)?; + if !metadata.is_dir() || metadata.file_type().is_symlink() { + return Err(Error::InvalidPath(format!( + "replica quarantine path {:?} is not a directory", + quarantine_root + ))); + } + let quarantine = quarantine_root.join(uuid::Uuid::new_v4().to_string()); + std::fs::create_dir(&quarantine)?; + let entries = std::fs::read_dir(&path)? + .map(|entry| entry.map(|entry| (entry.path(), entry.file_name()))) + .collect::>>()?; + for (source, name) in entries { + std::fs::rename(source, quarantine.join(name))?; + } + if std::fs::read_dir(&path)?.next().is_some() { + return Err(Error::InvalidPath(format!( + "replica directory `{namespace}` changed while quarantining its log" + ))); + } + tracing::warn!( + "quarantined incompatible replica files for `{namespace}` at {:?}", + quarantine + ); + Ok(()) + } + + async fn cleanup_directory_identity( + &self, + namespace: &NamespaceName, + ) -> crate::Result> { + let _identity = self.inner.fs_operations.lock().await; + self.check_existing_directory(namespace).await?; + match std::fs::symlink_metadata(self.directory_path(namespace)) { + Ok(metadata) => directory_identity(&metadata).map(Some).ok_or_else(|| { + Error::InvalidPath(format!( + "namespace `{namespace}` has no safe directory identity" + )) + }), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(e.into()), + } + } + + // Call only after remote backup confirmation and task teardown, while + // holding the name's operation guard. The identity lock protects the + // check/rename/reservation transaction, never backup or recursive removal. + async fn detach_owned_directory( + &self, + namespace: &NamespaceName, + expected: Option<(u64, u64)>, + replace: bool, + ) -> crate::Result<(Option, Option)> { + let _identity = self.inner.fs_operations.lock().await; + self.check_existing_directory(namespace).await?; + let path = self.directory_path(namespace); + let actual = match std::fs::symlink_metadata(&path) { + Ok(metadata) => directory_identity(&metadata), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => return Err(e.into()), + }; + if actual != expected { + return Err(Error::InvalidPath(format!( + "namespace `{namespace}` directory changed during cleanup; refusing to remove it" + ))); + } + let detached = if actual.is_some() { + let root = self + .inner + .dbs_path + .parent() + .unwrap() + .join("namespace-teardown-quarantine"); + std::fs::create_dir_all(&root)?; + let metadata = std::fs::symlink_metadata(&root)?; + if !metadata.is_dir() || metadata.file_type().is_symlink() { + return Err(Error::InvalidPath(format!( + "unsafe namespace teardown path {:?}", + root + ))); + } + let target = root.join(uuid::Uuid::new_v4().to_string()); + std::fs::rename(&path, &target)?; + Some(target) + } else { + None + }; + // There is no await between detaching the old inode and reserving + // the replacement. An alias cannot take over this path in between. + let reservation = if replace { + let result = (|| -> crate::Result { + std::fs::create_dir_all(&self.inner.dbs_path)?; + std::fs::create_dir(&path)?; + let identity = directory_identity(&std::fs::symlink_metadata(&path)?); + Ok(DirectoryReservation { + path: path.clone(), + identity, + owned: true, + }) + })(); + match result { + Ok(reservation) => Some(reservation), + Err(e) => { + if let Some(ref old) = detached { + if let Err(rollback) = std::fs::rename(old, &path) { + tracing::error!("failed to restore namespace `{namespace}` after reservation failure; old data retained at {:?}: {rollback}", old); + } + } + return Err(e); + } + } + } else { + None + }; + Ok((detached, reservation)) + } + + async fn remove_detached_directory(path: Option) -> crate::Result<()> { + if let Some(path) = path { + tokio::fs::remove_dir_all(path).await?; + } + Ok(()) + } + + pub async fn destroy(&self, namespace: NamespaceName, prune_all: bool) -> crate::Result<()> { + let _name_operation = self.lock_names(&[namespace.clone()]).await?; + // Do not remove metadata before proving this name owns its directory. + // Retain this expected inode through slow teardown and revalidate it + // under the global lock before touching the filesystem again. + let expected = self.cleanup_directory_identity(&namespace).await?; // destroy on-disk database and backups let db_config = tokio::task::spawn_blocking({ let inner = self.inner.clone(); @@ -132,8 +1309,12 @@ impl NamespaceStore { } } - self.cleanup(&namespace, &db_config, prune_all, bottomless_db_id_init) + self.prepare_cleanup(&namespace, &db_config, prune_all, bottomless_db_id_init) + .await?; + let (detached, _) = self + .detach_owned_directory(&namespace, expected, false) .await?; + Self::remove_detached_directory(detached).await?; tracing::info!("destroyed namespace: {namespace}"); @@ -158,7 +1339,9 @@ impl NamespaceStore { namespace: NamespaceName, restore_option: RestoreOption, ) -> anyhow::Result<()> { - // The process for reseting is as follow: + let _name_operation = self.lock_names(&[namespace.clone()]).await?; + let expected = self.cleanup_directory_identity(&namespace).await?; + // The process for resetting is as follows: // - get a lock on the namespace entry, if the entry exists, then it's a lock on the entry, // if it doesn't exist, insert an empty entry and take a lock on it // - destroy the old namespace @@ -174,19 +1357,30 @@ impl NamespaceStore { } let db_config = self.inner.metadata.handle(namespace.clone()).await; - // destroy on-disk database - self.cleanup( + // Confirm remote backups before detaching the old local inode. + self.prepare_cleanup( &namespace, &db_config.get(), false, NamespaceBottomlessDbIdInit::FetchFromConfig, ) .await?; - let ns = self - .make_namespace(&namespace, db_config, restore_option) + let (detached, reservation) = self + .detach_owned_directory(&namespace, expected, true) .await?; + let mut reservation = reservation.expect("reset reserves a replacement directory"); + Self::remove_detached_directory(detached).await?; + // Replica handshake may load an uncached shared schema. Never retain + // the global identity lock over setup or a callback into this store. + let mut shutdown = self.inner.shutdown_signal.subscribe(); + let ns = tokio::select! { + biased; + _ = shutdown.wait_for(|requested| *requested) => return Err(Error::NamespaceStoreShutdown.into()), + result = self.make_namespace(&namespace, db_config, restore_option) => result?, + }; lock.replace(ns); + reservation.disarm(); Ok(()) } @@ -216,6 +1410,10 @@ impl NamespaceStore { to_config: DatabaseConfig, timestamp: Option, ) -> crate::Result<()> { + if from == to { + return Err(Error::NamespaceAlreadyExist(to.to_string())); + } + let operation = self.lock_names(&[from.clone(), to.clone()]).await?; if self.inner.has_shutdown.load(Ordering::Relaxed) { return Err(Error::NamespaceStoreShutdown); } @@ -225,13 +1423,19 @@ impl NamespaceStore { return Err(crate::error::Error::NamespaceDoesntExist(from.to_string())); } + // Reject a persisted destination before inserting an empty cache + // entry: an unloaded existing namespace must remain loadable after + // the rejected fork. The name lock excludes competing create/destroy. + if self.inner.metadata.exists(&to).await { + return Err(Error::NamespaceAlreadyExist(to.to_string())); + } let to_entry = self .inner .store .get_with(to.clone(), async { Default::default() }) .await; let mut to_lock = to_entry.write().await; - if to_lock.is_some() { + if to_lock.is_some() || self.inner.metadata.exists(&to).await { return Err(crate::error::Error::NamespaceAlreadyExist(to.to_string())); } @@ -249,54 +1453,73 @@ impl NamespaceStore { return Err(crate::error::Error::NamespaceDoesntExist(from.to_string())); }; - struct Bomb { - store: MetaStore, - ns: NamespaceName, - should_delete: bool, - } - - impl Drop for Bomb { - fn drop(&mut self) { - if self.should_delete { - // we need to block in place because the inner connection may blocking, or - // unsing tokio's blocking methods (bottomless), which would cause a panic. - if let Err(e) = - tokio::task::block_in_place(|| self.store.remove(self.ns.clone())) - { - tracing::error!("failed to clean handle while forking: {e}"); - } - } + // Atomic mkdir is the filesystem's identity check: it rejects case and + // normalization aliases as well as orphan directories and symlinks. + // Reserve before creating any destination metadata. + let destination = { + let _identity = self.inner.fs_operations.lock().await; + if self.inner.metadata.exists(&to).await { + return Err(Error::NamespaceAlreadyExist(to.to_string())); } - } - - let mut bomb = Bomb { - store: self.inner.metadata.clone(), - ns: to.clone(), - should_delete: true, + self.reserve_directory(&to).await? }; + let mut cleanup = pending_cleanup( + self.inner.metadata.clone(), + to.clone(), + destination, + operation, + ); + let mut shutdown = self.inner.shutdown_signal.subscribe(); + let work = async { + let handle = self.inner.metadata.handle(to.clone()).await; + handle + .store_and_maybe_flush(Some(to_config.into()), false) + .await?; + let to_ns = self + .get_configurator(&from_config.get()) + .fork( + from_ns, + from_config, + to.clone(), + handle.clone(), + timestamp, + self.clone(), + ) + .await?; - let handle = self.inner.metadata.handle(to.clone()).await; - handle - .store_and_maybe_flush(Some(to_config.into()), false) - .await?; - let to_ns = self - .get_configurator(&from_config.get()) - .fork( - from_ns, - from_config, - to.clone(), - handle.clone(), - timestamp, - self.clone(), - ) - .await?; - - to_lock.replace(to_ns); - handle.flush().await?; - // defuse - bomb.should_delete = false; - - Ok(()) + // Persist only after the destination is ready; do not publish an + // unflushed namespace or leave it running if the flush fails. + if let Err(e) = handle.flush().await { + if let Err(shutdown_err) = to_ns.shutdown(false).await { + tracing::error!("failed to shut down uncommitted fork: {shutdown_err}"); + } + return Err(e); + } + Ok(to_ns) + }; + let result = tokio::select! { + biased; + _ = shutdown.wait_for(|requested| *requested) => { + // Cancellation keeps the new directory quarantined; the guard + // drains queued config updates without blocking this runtime. + return Err(Error::NamespaceStoreShutdown); + } + result = work => result, + }; + match result { + Ok(to_ns) => { + // No fallible or cancellable step after publishing. + to_lock.replace(to_ns); + if let Some(mut pending) = cleanup.disarm() { + pending.directory.disarm(); + } + Ok(()) + } + Err(e) => { + cleanup.finish().await; + Err(e) + } + } } pub async fn with_authenticated( @@ -341,11 +1564,56 @@ impl NamespaceStore { } }; + if self.inner.db_kind.is_primary() && namespace == NamespaceName::default() { + // The first request may race startup or another first request. + // This is internal ensure/load, not an explicit create operation. + // Recheck under the fs lock only when the namespace is not yet + // loaded, so ordinary requests do not serialize on it. + let loaded = self + .inner + .store + .get(&namespace) + .await + .is_some_and(|entry| entry.try_read().is_some_and(|ns| ns.is_some())); + if !loaded { + self.ensure_default_namespace().await?; + } + } + // A cached namespace needs only its entry read lock. For a cache miss, + // own this name through the entire load/handshake, including any + // incompatible-log quarantine. Do not hold this lock over f: callers + // may themselves resolve other namespaces. + if let Some(entry) = self.inner.store.get(&namespace).await { + if entry.try_read().is_some_and(|guard| guard.is_some()) { + return f(entry).await; + } + } + let _name_operation = self.lock_names(&[namespace.clone()]).await?; + let is_new = !self.inner.metadata.exists(&namespace).await; + // Replicas can have an exact, previously replicated directory without + // local metadata. Reserve/check it before handle() registers a name. + let mut reservation = if is_new && self.inner.db_kind.is_replica() { + let _identity = self.inner.fs_operations.lock().await; + match self.reserve_directory(&namespace).await { + Ok(reservation) => Some(reservation), + Err(Error::NamespaceAlreadyExist(_)) => { + self.check_existing_directory(&namespace).await?; + None + } + Err(e) => return Err(e), + } + } else { + None + }; let handle = self.inner.metadata.handle(namespace.to_owned()).await; - f(self + let entry = self .load_namespace(&namespace, handle, RestoreOption::Latest) - .await?) - .await + .await?; + if let Some(ref mut reservation) = reservation { + reservation.disarm(); + } + drop(_name_operation); + f(entry).await } fn resolve_attach_fn(&self) -> ResolveNamespacePathFn { @@ -390,10 +1658,20 @@ impl NamespaceStore { db_config: MetaStoreHandle, restore_option: RestoreOption, ) -> crate::Result { + // A prior failed/cancelled fork may have published a None cache entry; + // the persisted metastore row, not that placeholder, is authoritative. + self.forget_empty_cache_entry(namespace).await; let init = async { + let mut reservation = self.ensure_existing_directory(namespace).await?; + // If opening a persisted namespace whose directory was missing + // fails or is cancelled, leave its new directory for repair. It + // has no operation guard and cannot safely race a later destroy. let ns = self .make_namespace(namespace, db_config, restore_option) .await?; + if let Some(ref mut reservation) = reservation { + reservation.disarm(); + } Ok(Some(ns)) }; @@ -407,10 +1685,49 @@ impl NamespaceStore { ) .await?; NAMESPACE_LOAD_LATENCY.record(before_load.elapsed()); + if ns.read().await.is_none() { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } Ok(ns) } + async fn forget_empty_cache_entry(&self, namespace: &NamespaceName) { + // Checkpoint and failed forks can leave an empty cache entry. It must + // not prevent a legitimate persisted namespace from loading. + if let Some(entry) = self.inner.store.get(namespace).await { + let empty = entry.read().await.is_none(); + if empty { + self.inner.store.invalidate(namespace).await; + } + } + } + + /// Internal startup/lazy-load path. Unlike explicit create, an existing + /// default is loaded using its persisted config rather than replaced. + pub(crate) async fn ensure_default_namespace(&self) -> crate::Result<()> { + let namespace = NamespaceName::default(); + let operation = self.lock_names(&[namespace.clone()]).await?; + if self.inner.has_shutdown.load(Ordering::Relaxed) { + return Err(Error::NamespaceStoreShutdown); + } + self.forget_empty_cache_entry(&namespace).await; + if self.inner.metadata.exists(&namespace).await { + let handle = self.inner.metadata.handle(namespace.clone()).await; + self.load_namespace(&namespace, handle, RestoreOption::Latest) + .await?; + } else { + self.create_new_namespace( + namespace, + RestoreOption::Latest, + DatabaseConfig::default(), + operation, + ) + .await?; + } + Ok(()) + } + #[tracing::instrument(skip_all, fields(namespace))] pub async fn create( &self, @@ -430,31 +1747,109 @@ impl NamespaceStore { .await; }; - // With namespaces disabled, the default namespace can be auto-created, - // otherwise it's an error. - // FIXME: move the default namespace check out of this function. - if self.inner.allow_lazy_creation || namespace == NamespaceName::default() { - tracing::trace!("auto-creating the namespace"); - } else if self.inner.metadata.exists(&namespace).await { + let operation = self.lock_names(&[namespace.clone()]).await?; + if self.inner.has_shutdown.load(Ordering::Relaxed) { + return Err(Error::NamespaceStoreShutdown); + } + if self.inner.metadata.exists(&namespace).await { return Err(Error::NamespaceAlreadyExist(namespace.to_string())); } + self.create_new_namespace(namespace, restore_option, db_config, operation) + .await + } - let db_config = Arc::new(db_config); - let handle = self.inner.metadata.handle(namespace.clone()).await; - tracing::debug!("storing db config"); - handle.store(db_config).await?; - tracing::debug!("completed storing db config, loading namespace"); - self.load_namespace(&namespace, handle, restore_option) - .await?; - - tracing::debug!("completed loading namespace"); - - Ok(()) + // Caller holds the namespace operation lock. The filesystem identity + // lock below covers only the atomic reservation, not dump/setup/cleanup. + async fn create_new_namespace( + &self, + namespace: NamespaceName, + restore_option: RestoreOption, + db_config: DatabaseConfig, + operation: Vec>, + ) -> crate::Result<()> { + // Protect new namespace identity before publishing its metadata. + let reservation = { + let _identity = self.inner.fs_operations.lock().await; + if self.inner.metadata.exists(&namespace).await { + return Err(Error::NamespaceAlreadyExist(namespace.to_string())); + } + self.reserve_directory(&namespace).await? + }; + let mut cleanup = pending_cleanup( + self.inner.metadata.clone(), + namespace.clone(), + reservation, + operation, + ); + let mut shutdown = self.inner.shutdown_signal.subscribe(); + let work = async { + // A failed/cancelled fork may have left an empty cache entry for + // this name; it must not make a subsequent create skip setup. + self.forget_empty_cache_entry(&namespace).await; + let handle = self.inner.metadata.handle(namespace.clone()).await; + handle.store(Arc::new(db_config)).await?; + tracing::debug!("completed storing db config, loading namespace"); + self.load_namespace(&namespace, handle, restore_option) + .await?; + Ok(()) + }; + let result = tokio::select! { + biased; + _ = shutdown.wait_for(|requested| *requested) => { + return Err(Error::NamespaceStoreShutdown); + } + result = work => result, + }; + match result { + Ok(()) => { + if let Some(mut pending) = cleanup.disarm() { + pending.directory.disarm(); + } + tracing::debug!("completed loading namespace"); + Ok(()) + } + Err(e) => { + cleanup.finish().await; + Err(e) + } + } } pub async fn shutdown(self) -> crate::Result<()> { + // Existing server shutdown defaults to 30s. Reserve 10s for namespace + // and metastore backup after draining in-flight operations. + self.shutdown_with_timeout(Duration::from_secs(20)).await + } + + async fn shutdown_with_timeout(self, operation_drain_timeout: Duration) -> crate::Result<()> { let mut set = JoinSet::new(); self.inner.has_shutdown.store(true, Ordering::Relaxed); + self.inner.shutdown_signal.send_replace(true); + let locks = { + let registry = self.inner.name_operations.lock().unwrap(); + let mut locks = registry + .iter() + .filter_map(|(name, lock)| lock.upgrade().map(|lock| (name.clone(), lock))) + .collect::>(); + locks.sort_by(|(a, _), (b, _)| a.as_str().cmp(b.as_str())); + locks.into_iter().map(|(_, lock)| lock).collect::>() + }; + let identity = self.inner.fs_operations.clone(); + let (_operations, identity_guard) = tokio::time::timeout(operation_drain_timeout, async move { + let mut guards = Vec::with_capacity(locks.len()); + for lock in locks { + guards.push(lock.lock_owned().await); + } + let identity = identity.lock_owned().await; + (guards, identity) + }) + .await + .map_err(|_| Error::Blocked(Some("namespace operations did not drain before shutdown; incomplete directories remain quarantined".into())))?; + // No new operations can enter after has_shutdown. Release every + // coordination lock before checkpoint or shutdown callbacks, which + // may otherwise reenter namespace loading. + drop(identity_guard); + drop(_operations); for (_name, entry) in self.inner.store.iter() { let snapshow_at_shutdown = self.inner.snapshot_at_shutdown; @@ -520,7 +1915,7 @@ impl NamespaceStore { } } - async fn cleanup( + async fn prepare_cleanup( &self, namespace: &NamespaceName, db_config: &DatabaseConfig, @@ -528,7 +1923,7 @@ impl NamespaceStore { bottomless_db_id_init: NamespaceBottomlessDbIdInit, ) -> crate::Result<()> { self.get_configurator(db_config) - .cleanup(namespace, db_config, prune_all, bottomless_db_id_init) + .prepare_cleanup(namespace, db_config, prune_all, bottomless_db_id_init) .await } } diff --git a/libsql-server/src/schema/scheduler.rs b/libsql-server/src/schema/scheduler.rs index ea28078339..82df9b1ce6 100644 --- a/libsql-server/src/schema/scheduler.rs +++ b/libsql-server/src/schema/scheduler.rs @@ -82,7 +82,8 @@ impl Scheduler { break; } Err(e @ Error::InvalidPersistedNamespace { .. }) => { - // Keep unfinished work intact for explicit operator repair. + // Do not retry or skip corrupt work: it must be repaired before + // resuming, and must remain unfinished in the metastore. tracing::error!( "migration scheduler stopped on invalid persisted namespace: {e}" ); @@ -859,10 +860,17 @@ mod test { .unwrap(); let (sender, mut receiver) = mpsc::channel(100); let config = make_config(sender.clone().into(), tmp.path()); - let store = - NamespaceStore::new(false, false, 10, meta_store, config, DatabaseKind::Primary) - .await - .unwrap(); + let store = NamespaceStore::new( + false, + false, + 10, + meta_store, + config, + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); let mut scheduler = Scheduler::new(store.clone(), maker().unwrap()) .await .unwrap(); @@ -991,10 +999,17 @@ mod test { .unwrap(); let (sender, mut receiver) = mpsc::channel(100); let config = make_config(sender.clone().into(), tmp.path()); - let store = - NamespaceStore::new(false, false, 10, meta_store, config, DatabaseKind::Primary) - .await - .unwrap(); + let store = NamespaceStore::new( + false, + false, + 10, + meta_store, + config, + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); let mut scheduler = Scheduler::new(store.clone(), maker().unwrap()) .await .unwrap(); @@ -1072,10 +1087,17 @@ mod test { .unwrap(); let (sender, _receiver) = mpsc::channel(100); let config = make_config(sender.clone().into(), tmp.path()); - let store = - NamespaceStore::new(false, false, 10, meta_store, config, DatabaseKind::Primary) - .await - .unwrap(); + let store = NamespaceStore::new( + false, + false, + 10, + meta_store, + config, + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); store .with("ns".into(), |ns| { @@ -1106,10 +1128,17 @@ mod test { .unwrap(); let (sender, mut receiver) = mpsc::channel(100); let config = make_config(sender.clone().into(), tmp.path()); - let store = - NamespaceStore::new(false, false, 10, meta_store, config, DatabaseKind::Primary) - .await - .unwrap(); + let store = NamespaceStore::new( + false, + false, + 10, + meta_store, + config, + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); let mut scheduler = Scheduler::new(store.clone(), maker().unwrap()) .await .unwrap(); @@ -1186,10 +1215,17 @@ mod test { .unwrap(); let (sender, _receiver) = mpsc::channel(100); let config = make_config(sender.clone().into(), tmp.path()); - let store = - NamespaceStore::new(false, false, 10, meta_store, config, DatabaseKind::Primary) - .await - .unwrap(); + let store = NamespaceStore::new( + false, + false, + 10, + meta_store, + config, + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); let scheduler = Scheduler::new(store.clone(), maker().unwrap()) .await .unwrap(); diff --git a/libsql-server/tests/cluster/mod.rs b/libsql-server/tests/cluster/mod.rs index 4cfb20dccf..3f4271036e 100644 --- a/libsql-server/tests/cluster/mod.rs +++ b/libsql-server/tests/cluster/mod.rs @@ -19,7 +19,28 @@ mod replica_restart; mod replication; mod schema_dbs; +type ResetControl = ( + std::sync::Arc, + std::sync::Arc, + std::sync::Arc>>, +); + pub fn make_cluster(sim: &mut Sim, num_replica: usize, disable_namespaces: bool) { + make_cluster_with_options(sim, num_replica, disable_namespaces, 100, None, None); +} + +fn make_cluster_with_options( + sim: &mut Sim, + num_replica: usize, + disable_namespaces: bool, + replica_capacity: usize, + replica_fixture: Option<( + std::path::PathBuf, + std::sync::Arc, + std::sync::Arc, + )>, + reset_control: Option, +) { init_tracing(); let tmp = tempdir().unwrap(); sim.host("primary", move || { @@ -53,9 +74,19 @@ pub fn make_cluster(sim: &mut Sim, num_replica: usize, disable_namespaces: bool) for i in 0..num_replica { let tmp = tempdir().unwrap(); + let fixture = replica_fixture.clone(); + let reset_control = reset_control.clone(); sim.host(format!("replica{i}"), move || { - let path = tmp.path().to_path_buf(); + let path = fixture + .as_ref() + .map(|(path, _, _)| path.clone()) + .unwrap_or_else(|| tmp.path().to_path_buf()); + let shutdown = fixture.as_ref().map(|(_, shutdown, _)| shutdown.clone()); + let done = fixture.as_ref().map(|(_, _, done)| done.clone()); + let reset_control = reset_control.clone(); async move { + let (store_ready, store_rx) = tokio::sync::oneshot::channel(); + let store_hook = reset_control.as_ref().map(|_| store_ready); let server = TestServer { path: path.into(), user_api_config: UserApiConfig { @@ -74,10 +105,31 @@ pub fn make_cluster(sim: &mut Sim, num_replica: usize, disable_namespaces: bool) }), disable_namespaces, disable_default_namespace: !disable_namespaces, + max_active_namespaces: replica_capacity, + shutdown: shutdown.unwrap_or_default(), + namespace_store_ready: store_hook, ..Default::default() }; - + if let Some((request, finished, failure)) = reset_control { + tokio::spawn(async move { + let result = match store_rx.await { + Ok(store) => { + request.notified().await; + store.evict_cached_namespace(&"schema".into()).await; + store + .reset("tenant".into(), libsql_server::RestoreOption::Latest) + .await + } + Err(e) => Err(e.into()), + }; + *failure.lock().unwrap() = result.err().map(|e| e.to_string()); + finished.notify_one(); + }); + } server.start_sim(8080).await.unwrap(); + if let Some(done) = done { + done.notify_one(); + } Ok(()) } @@ -312,6 +364,359 @@ fn large_proxy_query() { sim.run().unwrap(); } +// Schema migrations to linked tenants run asynchronously. Wait for the +// specific prerequisite, not an arbitrary delay or a successful HTTP reply. +async fn wait_for_linked_test_table(uri: &str) -> anyhow::Result<()> { + let db = Database::open_remote_with_connector(uri, "", TurmoilConnector)?; + loop { + match db.connect()?.query("select * from test", ()).await { + Ok(_) => return Ok(()), + Err(err) if err.to_string().contains("no such table") => { + tokio::time::sleep(Duration::from_millis(20)).await; + } + Err(err) => return Err(err.into()), + } + } +} + +#[test] +fn replica_reset_loads_uncached_shared_schema_without_identity_lock_deadlock() { + use std::sync::Arc; + use tokio::sync::Notify; + + let replica_dir = tempdir().unwrap(); + let marker = replica_dir.path().join("dbs/tenant/reset-marker"); + let stop = Arc::new(Notify::new()); + let done = Arc::new(Notify::new()); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(100)) + .tcp_capacity(100000) + .build(); + let reset_request = Arc::new(Notify::new()); + let reset_finished = Arc::new(Notify::new()); + let reset_failure = Arc::new(std::sync::Mutex::new(None)); + make_cluster_with_options( + &mut sim, + 1, + false, + 1, + Some((replica_dir.path().to_path_buf(), stop.clone(), done.clone())), + Some(( + reset_request.clone(), + reset_finished.clone(), + reset_failure.clone(), + )), + ); + sim.client("client", async move { + tokio::time::timeout(Duration::from_secs(30), async move { + let admin = Client::new(); + assert!(admin + .post( + "http://primary:9090/v1/namespaces/schema/create", + json!({"shared_schema": true}) + ) + .await? + .status() + .is_success()); + let schema = Database::open_remote_with_connector( + "http://schema.primary:8080", + "", + TurmoilConnector, + )?; + schema + .connect()? + .execute("create table test (v integer)", ()) + .await?; + schema + .connect()? + .execute("insert into test values (42)", ()) + .await?; + assert!(admin + .post( + "http://primary:9090/v1/namespaces/tenant/create", + json!({"shared_schema_name": "schema"}) + ) + .await? + .status() + .is_success()); + wait_for_linked_test_table("http://tenant.primary:8080").await?; + assert!(admin + .post("http://primary:9090/v1/namespaces/filler/create", json!({})) + .await? + .status() + .is_success()); + wait_for_linked_test_table("http://tenant.replica0:8080").await?; + let tenant_replica = Database::open_remote_with_connector( + "http://tenant.replica0:8080", + "", + TurmoilConnector, + )?; + let filler = Database::open_remote_with_connector( + "http://filler.replica0:8080", + "", + TurmoilConnector, + )?; + tenant_replica + .connect()? + .query("select * from test", ()) + .await?; + // Capacity one and an extra namespace force the shared schema + // out of cache before an explicit replica reset/handshake. + filler.connect()?.query("select 1", ()).await?; + tenant_replica + .connect()? + .query("select * from test", ()) + .await?; + std::fs::write(&marker, b"reset must remove this")?; + let primary_tenant = Database::open_remote_with_connector( + "http://tenant.primary:8080", + "", + TurmoilConnector, + )?; + primary_tenant + .connect()? + .execute("insert into test values (19)", ()) + .await?; + // Recreating the primary is not a deterministic trigger for a + // cached replica's existing long-lived frame stream. Ask the + // replica host itself to run the real NamespaceStore::reset. + reset_request.notify_one(); + tokio::time::timeout(Duration::from_secs(20), reset_finished.notified()) + .await + .map_err(|_| { + anyhow::anyhow!("replica reset/handshake stalled while schema was uncached") + })?; + if let Some(error) = reset_failure.lock().unwrap().take() { + anyhow::bail!("replica NamespaceStore::reset failed: {error}"); + } + assert!( + !marker.exists(), + "replica reset did not remove its old directory" + ); + loop { + if let Ok(mut rows) = tenant_replica + .connect()? + .query("select v from test where v = 19", ()) + .await + { + if rows.next().await?.is_some() { + break; + } + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + let mut schema_rows = schema + .connect()? + .query("select v from test where v = 42", ()) + .await?; + assert!(schema_rows.next().await?.is_some()); + stop.notify_one(); + done.notified().await; + Ok::<_, anyhow::Error>(()) + }) + .await??; + Ok(()) + }); + sim.run().unwrap(); +} + +#[test] +fn replica_restart_quarantines_incompatible_linked_tenant_log() { + use std::sync::Arc; + use tokio::sync::Notify; + + let replica_dir = tempdir().unwrap(); + let marker = replica_dir.path().join("dbs/tenant/old-log-marker"); + let stop = Arc::new(Notify::new()); + let stopped = Arc::new(Notify::new()); + let restart = Arc::new(Notify::new()); + let shutdown = Arc::new(Notify::new()); + let done = Arc::new(Notify::new()); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(100)) + .tcp_capacity(100000) + .build(); + make_cluster_with_options(&mut sim, 0, false, 100, None, None); + sim.host("replica0", { + let path = replica_dir.path().to_path_buf(); + let (stop, stopped, restart, shutdown, done) = ( + stop.clone(), + stopped.clone(), + restart.clone(), + shutdown.clone(), + done.clone(), + ); + move || { + let path = path.clone(); + let (stop, stopped, restart, shutdown, done) = ( + stop.clone(), + stopped.clone(), + restart.clone(), + shutdown.clone(), + done.clone(), + ); + async move { + let make_server = |shutdown: std::sync::Arc| { + let path = path.clone(); + async move { + TestServer { + path: path.into(), + user_api_config: UserApiConfig::default(), + admin_api_config: Some(AdminApiConfig { + acceptor: TurmoilAcceptor::bind(([0, 0, 0, 0], 9090)) + .await + .unwrap(), + connector: TurmoilConnector, + disable_metrics: true, + auth_key: None, + }), + rpc_client_config: Some(RpcClientConfig { + remote_url: "http://primary:4567".into(), + connector: TurmoilConnector, + tls_config: None, + }), + disable_namespaces: false, + disable_default_namespace: true, + shutdown, + ..Default::default() + } + } + }; + // Shut the first replica down cleanly. Cancelling start_sim + // would leave its replication tasks alive across restart, + // racing the new handshake for the old directory inode. + make_server(stop).await.start_sim(8080).await?; + stopped.notify_one(); + restart.notified().await; + make_server(shutdown).await.start_sim(8080).await?; + done.notify_one(); + Ok(()) + } + } + }); + sim.client("client", async move { + tokio::time::timeout(Duration::from_secs(30), async move { + let admin = Client::new(); + assert!(admin + .post( + "http://primary:9090/v1/namespaces/schema/create", + json!({"shared_schema": true}) + ) + .await? + .status() + .is_success()); + let schema = Database::open_remote_with_connector( + "http://schema.primary:8080", + "", + TurmoilConnector, + )?; + schema + .connect()? + .execute("create table test (v integer)", ()) + .await?; + schema + .connect()? + .execute("insert into test values (42)", ()) + .await?; + assert!(admin + .post( + "http://primary:9090/v1/namespaces/tenant/create", + json!({"shared_schema_name": "schema"}) + ) + .await? + .status() + .is_success()); + wait_for_linked_test_table("http://tenant.primary:8080").await?; + wait_for_linked_test_table("http://tenant.replica0:8080").await?; + let replica = Database::open_remote_with_connector( + "http://tenant.replica0:8080", + "", + TurmoilConnector, + )?; + replica.connect()?.query("select * from test", ()).await?; + let tenant_primary = Database::open_remote_with_connector( + "http://tenant.primary:8080", + "", + TurmoilConnector, + )?; + tenant_primary + .connect()? + .execute("insert into test values (7)", ()) + .await?; + loop { + let mut rows = replica + .connect()? + .query("select v from test where v = 7", ()) + .await?; + if rows.next().await?.is_some() { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + let wal_index = replica_dir.path().join("dbs/tenant/client_wal_index"); + let wal_len = std::fs::metadata(&wal_index)?.len(); + assert!( + wal_len >= 32, + "replica must persist an old log identity before restart: {:?} has {wal_len} bytes", + wal_index + ); + std::fs::write(&marker, b"old log quarantined")?; + stop.notify_one(); + stopped.notified().await; + assert!(admin + .delete("http://primary:9090/v1/namespaces/tenant", json!({})) + .await? + .status() + .is_success()); + assert!(admin + .post( + "http://primary:9090/v1/namespaces/tenant/create", + json!({"shared_schema_name": "schema"}) + ) + .await? + .status() + .is_success()); + restart.notify_one(); + loop { + if replica + .connect()? + .query("select * from test", ()) + .await + .is_ok() + { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + assert!(!marker.exists(), "stale log was not quarantined"); + let quarantine_root = replica_dir.path().join("replica-log-quarantine"); + let quarantined = std::fs::read_dir(&quarantine_root) + .map_err(|e| { + anyhow::anyhow!( + "incompatible-log handshake did not quarantine old files under {:?}: {e}", + quarantine_root + ) + })? + .map(|entry| entry.map(|entry| entry.path().join("old-log-marker"))) + .collect::>>()?; + assert!(quarantined + .iter() + .any(|path| std::fs::read(path).ok().as_deref() == Some(b"old log quarantined"))); + let mut rows = schema + .connect()? + .query("select v from test where v = 42", ()) + .await?; + assert!(rows.next().await?.is_some()); + shutdown.notify_one(); + done.notified().await; + Ok::<_, anyhow::Error>(()) + }) + .await??; + Ok(()) + }); + sim.run().unwrap(); +} + #[test] fn replicate_from_shared_schema() { let mut sim = Builder::new() diff --git a/libsql-server/tests/namespaces/availability.rs b/libsql-server/tests/namespaces/availability.rs new file mode 100644 index 0000000000..91022860d2 --- /dev/null +++ b/libsql-server/tests/namespaces/availability.rs @@ -0,0 +1,131 @@ +use std::convert::Infallible; +use std::sync::Arc; +use std::time::Duration; + +use hyper::{service::make_service_fn, Body, Response, StatusCode}; +use libsql::Database; +use serde_json::json; +use tempfile::tempdir; +use tokio::sync::Notify; +use tower::service_fn; +use turmoil::Builder; + +use crate::common::http::Client; +use crate::common::net::{TurmoilAcceptor, TurmoilConnector}; + +use super::make_primary_configured; + +#[test] +fn pending_dump_does_not_block_unrelated_admin_or_default() { + let tmp = tempdir().unwrap(); + let dbs = tmp.path().join("dbs"); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary_configured(&mut sim, tmp.path().to_path_buf(), false, false); + let release = Arc::new(Notify::new()); + let host_release = release.clone(); + sim.host("slow-dump", move || { + let release = host_release.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 release = release.clone(); + async move { + Ok::<_, Infallible>(service_fn(move |_request| { + let release = release.clone(); + async move { + let (mut sender, body) = Body::channel(); + tokio::spawn(async move { + release.notified().await; + let _ = sender + .send_data( + "BEGIN TRANSACTION; CREATE TABLE slow (v); COMMIT;" + .into(), + ) + .await; + }); + Ok::<_, Infallible>(Response::new(body)) + } + })) + } + })); + server.await.unwrap(); + Ok(()) + } + }); + sim.client("client", async move { + let client = Client::new(); + assert!(client + .post("http://primary:9090/v1/namespaces/victim/create", json!({})) + .await? + .status() + .is_success()); + let slow = tokio::spawn(async { + Client::new() + .post( + "http://primary:9090/v1/namespaces/slow/create", + json!({"dump_url": "http://slow-dump:8080/"}), + ) + .await + }); + // The directory is reserved only after create entered NamespaceStore; + // waiting for that fact avoids mistaking a delayed HTTP fetch for a + // held operation lock. This timeout is a test deadlock guard. + tokio::time::timeout(Duration::from_secs(5), async { + while !dbs.join("slow").exists() { + // A continuously ready yield loop can starve Turmoil's + // virtual clock and the other simulated hosts. + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await?; + tokio::time::timeout(Duration::from_secs(5), async { + assert_eq!( + client + .post( + "http://primary:9090/v1/namespaces/unrelated/create", + json!({}) + ) + .await? + .status(), + StatusCode::OK + ); + assert!(client + .delete("http://primary:9090/v1/namespaces/victim", json!({})) + .await? + .status() + .is_success()); + let default = Database::open_remote_with_connector( + "http://default.primary:8080", + "", + TurmoilConnector, + )?; + default.connect()?.execute("select 1", ()).await?; + Ok::<_, anyhow::Error>(()) + }) + .await??; + let duplicate = tokio::spawn(async { + Client::new() + .post("http://primary:9090/v1/namespaces/slow/create", json!({})) + .await + }); + tokio::task::yield_now().await; + assert!(!slow.is_finished()); + // A duplicate must either wait or reject; it must never take over the + // reserved directory while the dump is still in flight. + if duplicate.is_finished() { + assert_eq!(duplicate.await??.status(), StatusCode::BAD_REQUEST); + } else { + release.notify_one(); + assert_eq!(slow.await??.status(), StatusCode::OK); + assert_eq!(duplicate.await??.status(), StatusCode::BAD_REQUEST); + return Ok(()); + } + release.notify_one(); + assert_eq!(slow.await??.status(), StatusCode::OK); + Ok(()) + }); + sim.run().unwrap(); +} diff --git a/libsql-server/tests/namespaces/default_create.rs b/libsql-server/tests/namespaces/default_create.rs new file mode 100644 index 0000000000..baddd269b2 --- /dev/null +++ b/libsql-server/tests/namespaces/default_create.rs @@ -0,0 +1,194 @@ +use std::path::Path; +use std::time::Duration; + +use hyper::StatusCode; +use libsql::{Database, Value}; +use libsql_replication::rpc::metadata; +use prost::Message; +use serde_json::json; +use tempfile::tempdir; +use turmoil::Builder; + +use crate::common::auth::{encode, key_pair}; +use crate::common::http::Client; +use crate::common::net::TurmoilConnector; + +use super::make_primary_configured; + +const DUMP: &str = "PRAGMA foreign_keys=OFF; BEGIN TRANSACTION; CREATE TABLE dumped (v); INSERT INTO dumped VALUES(42); COMMIT;"; + +fn persisted_default(path: &Path) -> metadata::DatabaseConfig { + let conn = rusqlite::Connection::open(path.join("metastore/data")).unwrap(); + let bytes: Vec = conn + .query_row( + "SELECT config FROM namespace_configs WHERE namespace = 'default'", + (), + |row| row.get(0), + ) + .unwrap(); + metadata::DatabaseConfig::decode(&bytes[..]).unwrap() +} + +#[test] +fn explicit_default_create_rejects_duplicate_after_lazy_access() { + let tmp = tempdir().unwrap(); + std::fs::write(tmp.path().join("dump.sql"), DUMP).unwrap(); + let dump_url = format!("file:{}", tmp.path().join("dump.sql").display()); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary_configured(&mut sim, tmp.path().to_path_buf(), false, false); + sim.client("client", async move { + let client = Client::new(); + let db = Database::open_remote_with_connector( + "http://default.primary:8080", + "", + TurmoilConnector, + )?; + let conn = db.connect()?; + conn.execute("create table original (v)", ()).await?; + conn.execute("insert into original values (17)", ()).await?; + + let (_, jwt_key) = key_pair(); + let response = client + .post( + "http://primary:9090/v1/namespaces/default/create", + json!({"jwt_key": jwt_key, "dump_url": dump_url, "allow_attach": true}), + ) + .await?; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + let mut rows = conn.query("select v from original", ()).await?; + assert!(matches!( + rows.next().await?.unwrap().get_value(0)?, + Value::Integer(17) + )); + assert!(conn.query("select * from dumped", ()).await.is_err()); + Ok(()) + }); + sim.run().unwrap(); + let config = persisted_default(tmp.path()); + assert_eq!(config.jwt_key, None); + assert!(!config.allow_attach); +} + +#[test] +fn explicit_first_default_create_applies_valid_jwt_and_dump() { + let tmp = tempdir().unwrap(); + std::fs::write(tmp.path().join("dump.sql"), DUMP).unwrap(); + let dump_url = format!("file:{}", tmp.path().join("dump.sql").display()); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary_configured(&mut sim, tmp.path().to_path_buf(), false, false); + sim.client("client", async move { + let client = Client::new(); + let (enc, jwt_key) = key_pair(); + assert_eq!( + client + .post( + "http://primary:9090/v1/namespaces/default/create", + json!({"jwt_key": jwt_key, "dump_url": dump_url, "allow_attach": true}), + ) + .await? + .status(), + StatusCode::OK + ); + let token = encode(&json!({"id": "default"}), &enc); + let db = Database::open_remote_with_connector( + "http://default.primary:8080", + &token, + TurmoilConnector, + )?; + let mut rows = db.connect()?.query("select v from dumped", ()).await?; + assert!(matches!( + rows.next().await?.unwrap().get_value(0)?, + Value::Integer(42) + )); + Ok(()) + }); + sim.run().unwrap(); + let config = persisted_default(tmp.path()); + assert!(config.jwt_key.is_some()); + assert!(config.allow_attach); +} + +#[test] +fn namespace_disabled_restart_preserves_persisted_default_config() { + let tmp = tempdir().unwrap(); + { + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary_configured(&mut sim, tmp.path().to_path_buf(), true, false); + sim.client("client", async { + let client = Client::new(); + let db = + Database::open_remote_with_connector("http://primary:8080", "", TurmoilConnector)?; + db.connect()? + .execute("create table original (v)", ()) + .await?; + assert_eq!( + client + .post( + "http://primary:9090/v1/namespaces/default/config", + json!({"block_reads": false, "block_writes": false, "allow_attach": true}), + ) + .await? + .status(), + StatusCode::OK + ); + Ok(()) + }); + sim.run().unwrap(); + } + assert!(persisted_default(tmp.path()).allow_attach); + { + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary_configured(&mut sim, tmp.path().to_path_buf(), true, false); + sim.client("client", async { + let db = + Database::open_remote_with_connector("http://primary:8080", "", TurmoilConnector)?; + db.connect()?.execute("select * from original", ()).await?; + Ok(()) + }); + sim.run().unwrap(); + } + assert!(persisted_default(tmp.path()).allow_attach); +} + +#[test] +fn simultaneous_lazy_default_accesses_both_succeed() { + let tmp = tempdir().unwrap(); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary_configured(&mut sim, tmp.path().to_path_buf(), false, false); + sim.client("client", async { + let request = || async { + let db = Database::open_remote_with_connector( + "http://default.primary:8080", + "", + TurmoilConnector, + )?; + db.connect()?.execute("select 1", ()).await?; + Ok::<_, anyhow::Error>(()) + }; + let (first, second) = tokio::join!(request(), request()); + first?; + second?; + Ok(()) + }); + sim.run().unwrap(); + let conn = rusqlite::Connection::open(tmp.path().join("metastore/data")).unwrap(); + let count: i64 = conn + .query_row( + "SELECT count(*) FROM namespace_configs WHERE namespace = 'default'", + (), + |row| row.get(0), + ) + .unwrap(); + assert_eq!(count, 1); + assert!(tmp.path().join("dbs/default/data").exists()); +} diff --git a/libsql-server/tests/namespaces/mod.rs b/libsql-server/tests/namespaces/mod.rs index 1bc14d2bba..8009da430a 100644 --- a/libsql-server/tests/namespaces/mod.rs +++ b/libsql-server/tests/namespaces/mod.rs @@ -1,7 +1,10 @@ #![allow(deprecated)] +mod availability; +mod default_create; mod dumps; mod meta; +mod ownership; mod shared_schema; use std::path::PathBuf; @@ -16,6 +19,15 @@ use tempfile::tempdir; use turmoil::{Builder, Sim}; fn make_primary(sim: &mut Sim, path: PathBuf) { + make_primary_configured(sim, path, false, true); +} + +fn make_primary_configured( + sim: &mut Sim, + path: PathBuf, + disable_namespaces: bool, + disable_default_namespace: bool, +) { init_tracing(); sim.host("primary", move || { let path = path.clone(); @@ -35,8 +47,8 @@ fn make_primary(sim: &mut Sim, path: PathBuf) { acceptor: TurmoilAcceptor::bind(([0, 0, 0, 0], 4567)).await?, tls_config: None, }), - disable_namespaces: false, - disable_default_namespace: true, + disable_namespaces, + disable_default_namespace, ..Default::default() }; @@ -106,22 +118,25 @@ fn fork_namespace() { } #[test] -fn admin_rejects_namespace_path_traversal_without_touching_outside_files() { +fn admin_rejects_namespace_path_traversal() { let mut sim = Builder::new() .simulation_duration(Duration::from_secs(1000)) .build(); let tmp = tempdir().unwrap(); make_primary(&mut sim, tmp.path().to_path_buf()); + // These paths are outside dbs/. In particular, deleting `..` + // must never recursively delete dbs and its other namespaces. let outside = tmp.path().join("outside"); std::fs::create_dir(&outside).unwrap(); std::fs::write(outside.join("sentinel"), b"keep me").unwrap(); let dbs = tmp.path().join("dbs"); + let client_dbs = dbs.clone(); + // Form encoding uses `+` for spaces, but path parameters require `%20`. let absolute_victim = url::form_urlencoded::byte_serialize(outside.to_str().unwrap().as_bytes()) .collect::() .replace('+', "%20"); - let dbs_for_client = dbs.clone(); sim.client("client", async move { let client = Client::new(); @@ -133,54 +148,40 @@ fn admin_rejects_namespace_path_traversal_without_touching_outside_files() { .await? .status() .is_success()); - std::fs::create_dir_all(dbs_for_client.join("sentinel"))?; - std::fs::write(dbs_for_client.join("sentinel/marker"), b"keep me too")?; + std::fs::create_dir_all(client_dbs.join("sentinel"))?; + std::fs::write(client_dbs.join("sentinel/marker"), b"keep me too")?; + for name in [ - "%2e%2e".to_owned(), - "%2e%2e%2foutside".to_owned(), + "%2e%2e".to_string(), + "%2e%2e%2foutside".to_string(), absolute_victim, - "bad%5cname".to_owned(), + "bad%5cname".to_string(), ] { - assert_eq!( - client - .post( - &format!("http://primary:9090/v1/namespaces/{name}/create"), - json!({}) - ) - .await? - .status(), - hyper::StatusCode::BAD_REQUEST, + let create = format!("http://primary:9090/v1/namespaces/{name}/create"); + assert!( + client.post(&create, json!({})).await?.status() == hyper::StatusCode::BAD_REQUEST, "create {name}" ); - assert_eq!( - client - .delete( - &format!("http://primary:9090/v1/namespaces/{name}"), - json!({}) - ) - .await? - .status(), - hyper::StatusCode::BAD_REQUEST, + + let delete = format!("http://primary:9090/v1/namespaces/{name}"); + assert!( + client.delete(&delete, json!({})).await?.status() == hyper::StatusCode::BAD_REQUEST, "delete {name}" ); - assert_eq!( - client - .post( - &format!("http://primary:9090/v1/namespaces/safe-1.example/fork/{name}"), - () - ) - .await? - .status(), - hyper::StatusCode::BAD_REQUEST, + + let fork = format!("http://primary:9090/v1/namespaces/safe-1.example/fork/{name}"); + assert!( + client.post(&fork, ()).await?.status() == hyper::StatusCode::BAD_REQUEST, "fork {name}" ); } - assert_eq!( + // Also reject traversal in the source namespace of a fork. + assert!( client .post("http://primary:9090/v1/namespaces/%2e%2e/fork/target", ()) .await? - .status(), - hyper::StatusCode::BAD_REQUEST + .status() + == hyper::StatusCode::BAD_REQUEST ); Ok(()) }); diff --git a/libsql-server/tests/namespaces/ownership.rs b/libsql-server/tests/namespaces/ownership.rs new file mode 100644 index 0000000000..b397adac4f --- /dev/null +++ b/libsql-server/tests/namespaces/ownership.rs @@ -0,0 +1,403 @@ +use std::time::Duration; + +use libsql::{Database, Value}; +use serde_json::json; +use tempfile::tempdir; +use turmoil::Builder; + +use crate::common::{http::Client, net::TurmoilConnector}; + +use super::make_primary; + +#[test] +fn fork_refuses_unloaded_destination_and_preserves_data_config_and_link() { + let tmp = tempdir().unwrap(); + { + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary(&mut sim, tmp.path().to_path_buf()); + sim.client("client", async { + let client = Client::new(); + for (name, body) in [ + ("source", json!({})), + ("schema", json!({"shared_schema": true})), + ("dest", json!({"shared_schema_name": "schema"})), + ] { + assert!(client + .post( + &format!("http://primary:9090/v1/namespaces/{name}/create"), + body + ) + .await? + .status() + .is_success()); + } + let source = Database::open_remote_with_connector( + "http://source.primary:8080", + "", + TurmoilConnector, + )?; + source + .connect()? + .execute("create table source_data (v)", ()) + .await?; + let schema = Database::open_remote_with_connector( + "http://schema.primary:8080", + "", + TurmoilConnector, + )?; + schema + .connect()? + .execute("create table dest_data (v)", ()) + .await?; + let dest = Database::open_remote_with_connector( + "http://dest.primary:8080", + "", + TurmoilConnector, + )?; + let conn = dest.connect()?; + // Schema migrations are asynchronous; wait for the linked tenant + // to receive the table before persisting its own row. + let mut inserted = false; + for _ in 0..100 { + if conn + .execute("insert into dest_data values (17)", ()) + .await + .is_ok() + { + inserted = true; + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + assert!(inserted, "schema migration did not reach destination"); + assert!(client + .post( + "http://primary:9090/v1/namespaces/dest/config", + json!({"block_reads": false, "block_writes": true}) + ) + .await? + .status() + .is_success()); + Ok(()) + }); + sim.run().unwrap(); + } + let sentinel = tmp.path().join("dbs/dest/sentinel"); + std::fs::write(&sentinel, b"original destination").unwrap(); + { + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary(&mut sim, tmp.path().to_path_buf()); + sim.client("client", async { + let client = Client::new(); + assert_eq!( + client + .post("http://primary:9090/v1/namespaces/source/fork/dest", ()) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + // The existing destination is still linked to the schema; removing + // that schema must be refused, not silently unlinked by fork cleanup. + assert!(!client + .delete("http://primary:9090/v1/namespaces/schema", json!({})) + .await? + .status() + .is_success()); + let dest = Database::open_remote_with_connector( + "http://dest.primary:8080", + "", + TurmoilConnector, + )?; + let conn = dest.connect()?; + let mut rows = conn.query("select v from dest_data", ()).await?; + assert!(matches!( + rows.next().await?.unwrap().get_value(0)?, + Value::Integer(17) + )); + assert!(conn + .execute("insert into dest_data values (18)", ()) + .await + .is_err()); + let source = Database::open_remote_with_connector( + "http://source.primary:8080", + "", + TurmoilConnector, + )?; + source + .connect()? + .execute("select * from source_data", ()) + .await?; + Ok(()) + }); + sim.run().unwrap(); + } + assert_eq!(std::fs::read(sentinel).unwrap(), b"original destination"); + let conn = rusqlite::Connection::open(tmp.path().join("metastore/data")).unwrap(); + let links: i64 = conn + .query_row( + "SELECT count(*) FROM shared_schema_links WHERE shared_schema_name = 'schema' AND namespace = 'dest'", + (), + |row| row.get(0), + ) + .unwrap(); + assert_eq!(links, 1); +} + +#[test] +fn new_names_reject_filesystem_aliases_and_orphan_destinations() { + let tmp = tempdir().unwrap(); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary(&mut sim, tmp.path().to_path_buf()); + let dbs = tmp.path().join("dbs"); + let check_dbs = dbs.clone(); + let malformed_dump = tmp.path().join("invalid-dump.sql"); + std::fs::write( + &malformed_dump, + "BEGIN TRANSACTION; CREATE TABLE unfinished (v);", + ) + .unwrap(); + sim.client("client", async move { + let client = Client::new(); + for name in ["tenant", "source", "unrelated"] { + assert!(client + .post( + &format!("http://primary:9090/v1/namespaces/{name}/create"), + json!({}) + ) + .await? + .status() + .is_success()); + } + // Probe the actual temporary dbs volume after creating tenant. + // On case-sensitive volumes both names are distinct. + let case_insensitive = check_dbs.join("TENANT").exists(); + std::fs::write(check_dbs.join("tenant/sentinel"), b"keep tenant")?; + std::fs::create_dir(check_dbs.join("orphan"))?; + std::fs::write(check_dbs.join("orphan/sentinel"), b"keep orphan")?; + let tenant = Database::open_remote_with_connector( + "http://tenant.primary:8080", + "", + TurmoilConnector, + )?; + tenant + .connect()? + .execute("create table private_data (v)", ()) + .await?; + let create = client + .post("http://primary:9090/v1/namespaces/TENANT/create", json!({})) + .await?; + if case_insensitive { + assert_eq!(create.status(), hyper::StatusCode::BAD_REQUEST); + assert_eq!( + client + .post("http://primary:9090/v1/namespaces/tenant/fork/TENANT", ()) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + assert_eq!( + client + .delete("http://primary:9090/v1/namespaces/TENANT", json!({})) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + } else { + assert!(create.status().is_success()); + assert!(client + .post("http://primary:9090/v1/namespaces/source/fork/SOURCE", ()) + .await? + .status() + .is_success()); + } + assert_eq!( + client + .post("http://primary:9090/v1/namespaces/source/fork/orphan", ()) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + assert_eq!( + client + .post("http://primary:9090/v1/namespaces/orphan/create", json!({})) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + assert_eq!( + client + .delete("http://primary:9090/v1/namespaces/orphan", json!({})) + .await? + .status(), + hyper::StatusCode::NOT_FOUND + ); + assert_eq!( + client + .post("http://primary:9090/v1/namespaces/source/fork/source", ()) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + // An error after reservation must release only the new directory and + // metadata, so a later create can actually initialize the namespace. + // The common Client::post converts 5xx into an error before callers + // can inspect the response; use raw Hyper to assert this exact path. + let raw = hyper::Client::builder().build::<_, hyper::Body>(TurmoilConnector); + let request = + hyper::Request::post("http://primary:9090/v1/namespaces/source/fork/failed-fork") + .header("content-type", "application/json") + .body(hyper::Body::from(serde_json::to_vec( + &json!({"timestamp": "2024-01-01T00:00:00"}), + )?))?; + let response = raw.request(request).await?; + assert_eq!(response.status(), hyper::StatusCode::INTERNAL_SERVER_ERROR); + let error_body = hyper::body::to_bytes(response.into_body()).await?; + assert!(String::from_utf8_lossy(&error_body).contains("backup service not configured")); + assert!(!check_dbs.join("failed-fork").exists()); + assert!(client + .post( + "http://primary:9090/v1/namespaces/failed-fork/create", + json!({}) + ) + .await? + .status() + .is_success()); + assert_eq!( + client + .post( + "http://primary:9090/v1/namespaces/failed-create/create", + json!({"dump_url": format!("file:{}", malformed_dump.display())}), + ) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + assert!(!check_dbs.join("failed-create").exists()); + assert!(client + .post( + "http://primary:9090/v1/namespaces/failed-create/create", + json!({}), + ) + .await? + .status() + .is_success()); + let failed = Database::open_remote_with_connector( + "http://failed-fork.primary:8080", + "", + TurmoilConnector, + )?; + failed.connect()?.execute("select 1", ()).await?; + let retried = Database::open_remote_with_connector( + "http://failed-create.primary:8080", + "", + TurmoilConnector, + )?; + retried.connect()?.execute("select 1", ()).await?; + let mut rows = tenant + .connect()? + .query("select count(*) from private_data", ()) + .await?; + assert!(matches!( + rows.next().await?.unwrap().get_value(0)?, + Value::Integer(0) + )); + Ok(()) + }); + sim.run().unwrap(); + assert_eq!( + std::fs::read(dbs.join("tenant/sentinel")).unwrap(), + b"keep tenant" + ); + assert_eq!( + std::fs::read(dbs.join("orphan/sentinel")).unwrap(), + b"keep orphan" + ); + assert!(dbs.join("unrelated").exists()); +} + +#[test] +fn legacy_alias_row_cannot_open_or_delete_other_namespace() { + let tmp = tempdir().unwrap(); + { + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary(&mut sim, tmp.path().to_path_buf()); + sim.client("client", async { + let client = Client::new(); + assert!(client + .post("http://primary:9090/v1/namespaces/tenant/create", json!({})) + .await? + .status() + .is_success()); + let tenant = Database::open_remote_with_connector( + "http://tenant.primary:8080", + "", + TurmoilConnector, + )?; + tenant + .connect()? + .execute("create table private_data (v)", ()) + .await?; + Ok(()) + }); + sim.run().unwrap(); + } + let dbs = tmp.path().join("dbs"); + if !dbs.join("TENANT").exists() { + // A case-sensitive volume has no alias to test here. + return; + } + let sentinel = dbs.join("tenant/sentinel"); + std::fs::write(&sentinel, b"keep tenant").unwrap(); + // Reproduce a pre-upgrade metastore containing both keys. The second key + // must never be used to open or remove the first key's physical directory. + let conn = rusqlite::Connection::open(tmp.path().join("metastore/data")).unwrap(); + conn.execute( + "INSERT INTO namespace_configs (namespace, config) SELECT 'TENANT', config FROM namespace_configs WHERE namespace = 'tenant'", + (), + ).unwrap(); + drop(conn); + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + make_primary(&mut sim, tmp.path().to_path_buf()); + sim.client("client", async { + let client = Client::new(); + assert_eq!( + client + .delete("http://primary:9090/v1/namespaces/TENANT", json!({})) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + assert_eq!( + client + .post( + "http://primary:9090/v1/namespaces/TENANT/config", + json!({"block_reads": false, "block_writes": false}), + ) + .await? + .status(), + hyper::StatusCode::BAD_REQUEST + ); + let tenant = Database::open_remote_with_connector( + "http://tenant.primary:8080", + "", + TurmoilConnector, + )?; + tenant + .connect()? + .execute("select * from private_data", ()) + .await?; + Ok(()) + }); + sim.run().unwrap(); + assert_eq!(std::fs::read(sentinel).unwrap(), b"keep tenant"); +} From 1de7785e6bdee315d21cb5984634a3e438f5ecad Mon Sep 17 00:00:00 2001 From: Raphael Lopes Feitoza Date: Wed, 30 Sep 2026 17:52:12 -0400 Subject: [PATCH 08/11] Fence stale namespace config writes and complete deletion after cancellation --- docs/ADMIN_API.md | 10 +- libsql-server/src/namespace/meta_store.rs | 267 +++++++++++++--- libsql-server/src/namespace/store.rs | 351 ++++++++++++++++++++-- 3 files changed, 564 insertions(+), 64 deletions(-) diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index 110de71230..4c55e064e8 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -47,8 +47,14 @@ an incompatible log moves its old files to `replica-log-quarantine/` outside `dbs/`, retaining the namespace directory identity, then retries once. Inspect quarantined files before removal. Destroy/reset confirms remote backups before moving a directory to `namespace-teardown-quarantine/` for local removal; -backup failure/cancellation before confirmation leaves the old directory in -place. Inspect remaining quarantine files after interrupted teardown. +backup failure/cancellation before confirmation leaves the old directory and +metastore row in place; after confirmation an independently draining worker +owns the name lock through metadata removal and identity-checked teardown even +if the HTTP request is cancelled. Stale config/replication handles from a +prior incarnation cannot reinsert rows or links after deletion; a fresh +create/reset uses a new generation. Unexpected identity changes are preserved +for explicit operator repair. Inspect remaining quarantine files after +interrupted teardown. ## Routes diff --git a/libsql-server/src/namespace/meta_store.rs b/libsql-server/src/namespace/meta_store.rs index 27eadb7237..9df5f0ead5 100644 --- a/libsql-server/src/namespace/meta_store.rs +++ b/libsql-server/src/namespace/meta_store.rs @@ -1,5 +1,6 @@ #![allow(clippy::mutable_key_type)] use std::path::Path; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::{collections::HashMap, fs::read_dir}; @@ -34,7 +35,8 @@ type ChangeMsg = ( NamespaceName, Option>, oneshot::Sender>, - bool, // flush + bool, // flush + Arc, // one namespace incarnation; revoked before deletion ); type MetaStoreWalManager = WalWrapper, Sqlite3WalManager>; pub type MetaStoreConnection = @@ -55,7 +57,11 @@ pub struct MetaStoreHandle { #[derive(Debug, Clone)] enum HandleState { Internal(Arc>>), - External(mpsc::Sender, Receiver), + External( + mpsc::Sender, + Receiver, + Arc, + ), } #[derive(Debug, Default, Clone)] @@ -72,6 +78,10 @@ struct MetaStoreInner { // when we are updating the config. The config si already synced via the watch // channel. configs: tokio::sync::Mutex>>, + // A deleted name remains tombstoned until explicit create/replica reserve + // installs a NEW token. Old handles and queued messages keep the revoked + // token, even if a later namespace reuses the same spelling. + generations: Mutex>>, conn: tokio::sync::Mutex, wal_manager: MetaStoreWalManager, db_kind: DatabaseKind, @@ -190,6 +200,7 @@ impl MetaStoreInner { let mut this = MetaStoreInner { configs: Default::default(), + generations: Default::default(), conn: conn.into(), wal_manager, db_kind, @@ -303,6 +314,9 @@ impl MetaStoreInner { // handshake again and get the latest config. let (tx, _) = watch::channel(InnerConfig { version: 0, config }); + self.generations + .get_mut() + .insert(ns.clone(), Arc::new(AtomicBool::new(true))); self.configs.get_mut().insert(ns, tx); } @@ -323,41 +337,52 @@ impl MetaStoreInner { /// Handles config change updates by inserting them into the database and in-memory /// cache of configs. fn process(msg: ChangeMsg, inner: Arc) { - let (namespace, config, ret_chan, flush) = msg; + let (namespace, config, ret_chan, flush, generation) = msg; if let Some(config) = config { - let ret = if flush { - try_process(&inner, &namespace, &config) + let result = if flush { + try_process(&inner, &namespace, &config, &generation) } else { Ok(()) }; let mut configs = inner.configs.blocking_lock(); - if let Some(config_watch) = configs.get_mut(&namespace) { - let new_version = config_watch.borrow().version.wrapping_add(1); - - config_watch.send_modify(|c| { - *c = InnerConfig { - version: new_version, - config, - }; - }); - } else { - let (tx, _) = watch::channel(InnerConfig { version: 0, config }); - configs.insert(namespace, tx); - } - let _ = ret_chan.send(ret); - } else { - let ret = if flush { - let mut configs = inner.configs.blocking_lock(); + // Removing a namespace takes the same lock before revoking this token. + // Do not resurrect watch state after a queued write or a failed flush. + let result = result.and_then(|()| { + if !generation.load(Ordering::Acquire) { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } if let Some(config_watch) = configs.get_mut(&namespace) { - let config = config_watch.subscribe().borrow().clone(); - try_process(&inner, &namespace, &config.config) + let new_version = config_watch.borrow().version.wrapping_add(1); + config_watch.send_modify(|c| { + *c = InnerConfig { + version: new_version, + config, + }; + }); } else { - Ok(()) + let (tx, _) = watch::channel(InnerConfig { version: 0, config }); + configs.insert(namespace.clone(), tx); } - } else { Ok(()) + }); + let _ = ret_chan.send(result); + } else { + // Do not hold configs while waiting for conn: remove locks conn first. + let config = if flush { + inner + .configs + .blocking_lock() + .get(&namespace) + .map(|watch| watch.subscribe().borrow().config.clone()) + } else { + None + }; + let result = match config { + Some(config) => try_process(&inner, &namespace, &config, &generation), + None if generation.load(Ordering::Acquire) => Ok(()), + None => Err(Error::NamespaceDoesntExist(namespace.to_string())), }; - let _ = ret_chan.send(ret); + let _ = ret_chan.send(result); } } @@ -365,10 +390,16 @@ fn try_process( inner: &MetaStoreInner, namespace: &NamespaceName, config: &DatabaseConfig, + generation: &AtomicBool, ) -> Result<()> { - let config_encoded = metadata::DatabaseConfig::from(&*config).encode_to_vec(); + let config_encoded = metadata::DatabaseConfig::from(config).encode_to_vec(); let mut conn = inner.conn.blocking_lock(); + // This check is AFTER acquiring the DB lock. A pre-delete write either + // commits before remove (and is removed), or sees the revoked generation. + if !generation.load(Ordering::Acquire) { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } if let Some(schema) = config.shared_schema_name.as_ref() { let tx = conn.transaction()?; if inner.db_kind.is_primary() { @@ -521,15 +552,36 @@ impl MetaStore { }); let rx = sender.subscribe(); + let generation = self + .inner + .generations + .lock() + .entry(namespace.clone()) + .or_insert_with(|| Arc::new(AtomicBool::new(true))) + .clone(); tracing::debug!("meta handle subscribed"); MetaStoreHandle { namespace, - inner: HandleState::External(change_tx, rx), + inner: HandleState::External(change_tx, rx, generation), + } + } + + // Called under the store's per-name lock, after a new directory has been + // reserved. Revoking old handles is distinct from checking configs: a new + // incarnation's first INSERT must be allowed even without a stored row. + pub(crate) fn activate_for_create(&self, namespace: &NamespaceName) { + let mut generations = self.inner.generations.lock(); + if let Some(old) = generations.insert(namespace.clone(), Arc::new(AtomicBool::new(true))) { + old.store(false, Ordering::Release); } } + pub(crate) fn generation(&self, namespace: &NamespaceName) -> Option> { + self.inner.generations.lock().get(namespace).cloned() + } + // A cancelled config update may already be queued when its caller drops. // Place a barrier after it before removing that caller's metadata, so the // background worker cannot reinsert a row after cleanup. @@ -548,7 +600,13 @@ impl MetaStore { pub(crate) async fn wait_for_pending_changes(&self) -> Result<()> { let (send, recv) = oneshot::channel(); self.changes_tx - .send((NamespaceName::default(), None, send, false)) + .send(( + NamespaceName::default(), + None, + send, + false, + Arc::new(AtomicBool::new(true)), + )) .await .map_err(|e| Error::MetaStoreUpdateFailure(e.into()))?; recv.await @@ -557,6 +615,16 @@ impl MetaStore { } pub fn remove(&self, namespace: NamespaceName) -> Result>> { + self.remove_if_generation(namespace, None) + } + + // For deferred destroy, an older worker may never remove a later + // incarnation. The comparison and revocation happen under the DB lock. + pub(crate) fn remove_if_generation( + &self, + namespace: NamespaceName, + expected: Option<&Arc>, + ) -> Result>> { tracing::debug!("removing namespace `{}` from meta store", namespace); // "configs" lock can be used in both async and sync contexts while "conn" lock always used @@ -568,6 +636,16 @@ impl MetaStore { let mut conn = self.inner.conn.blocking_lock(); let mut configs = self.inner.configs.blocking_lock(); + if let Some(expected) = expected { + let generations = self.inner.generations.lock(); + if !expected.load(Ordering::Acquire) + || !generations + .get(&namespace) + .is_some_and(|current| Arc::ptr_eq(current, expected)) + { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } + } let r = if let Some(sender) = configs.get(&namespace) { tracing::debug!("removed namespace `{}` from meta store", namespace); let config = sender.borrow().clone(); @@ -594,6 +672,11 @@ impl MetaStore { [namespace.as_str()], )?; tx.commit()?; + // conn + configs are still held. A worker checking its token after + // acquiring conn cannot insert the deleted row or update watches. + if let Some(token) = self.inner.generations.lock().get(&namespace) { + token.store(false, Ordering::Release); + } Ok(Some(config.config)) } else { tracing::trace!("namespace `{}` not found in meta store", namespace); @@ -671,6 +754,111 @@ mod tests { use super::*; use tempfile::tempdir; + #[tokio::test] + async fn stale_handle_cannot_recreate_config_or_schema_link_after_delete_or_recreate() { + let tmp = tempdir().unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + MetaStoreConfig::default(), + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + { + // Only the jobs columns read by the shared-schema guard matter. + let conn = maker().unwrap(); + conn.execute_batch("CREATE TABLE jobs (schema TEXT, finished BOOLEAN)") + .unwrap(); + } + let schema = NamespaceName::from("schema"); + metadata + .handle(schema.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + let tenant = NamespaceName::from("tenant"); + let old = metadata.handle(tenant.clone()).await; + let mut linked = DatabaseConfig::default(); + linked.shared_schema_name = Some(schema); + old.store(linked.clone()).await.unwrap(); + assert!(metadata.exists(&tenant).await); + let removed = tokio::task::spawn_blocking({ + let metadata = metadata.clone(); + let tenant = tenant.clone(); + move || metadata.remove(tenant) + }) + .await + .unwrap() + .unwrap(); + assert!(removed.is_some()); + assert!(!metadata.exists(&tenant).await); + // Simulate a message already accepted into the worker queue before + // deletion: the handle's early check cannot protect this path. + let stale_generation = match &old.inner { + HandleState::External(_, _, generation) => generation.clone(), + HandleState::Internal(_) => unreachable!(), + }; + let stale_name = tenant.clone(); + let stale_inner = metadata.inner.clone(); + let (send, receive) = oneshot::channel(); + tokio::task::spawn_blocking(move || { + process( + ( + stale_name, + Some(Arc::new(DatabaseConfig::default())), + send, + true, + stale_generation, + ), + stale_inner, + ); + }) + .await + .unwrap(); + assert!(matches!( + receive.await.unwrap(), + Err(Error::NamespaceDoesntExist(_)) + )); + assert!(!metadata.exists(&tenant).await); + assert!(matches!( + old.store(linked.clone()).await, + Err(Error::NamespaceDoesntExist(_)) + )); + // A stale handle acquired anew after deletion remains tombstoned. + assert!(matches!( + metadata.handle(tenant.clone()).await.store(linked).await, + Err(Error::NamespaceDoesntExist(_)) + )); + metadata.activate_for_create(&tenant); + let fresh = metadata.handle(tenant.clone()).await; + fresh.store(DatabaseConfig::default()).await.unwrap(); + assert!(matches!( + old.store(DatabaseConfig::default()).await, + Err(Error::NamespaceDoesntExist(_)) + )); + let conn = maker().unwrap(); + let rows: i64 = conn + .query_row( + "SELECT count(*) FROM namespace_configs WHERE namespace = 'tenant'", + [], + |row| row.get(0), + ) + .unwrap(); + let links: i64 = conn + .query_row( + "SELECT count(*) FROM shared_schema_links WHERE namespace = 'tenant'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(rows, 1); + assert_eq!(links, 0); + } + #[tokio::test] async fn invalid_shared_schema_refuses_startup_without_destroying_metastore() { let tmp = tempdir().unwrap(); @@ -928,21 +1116,21 @@ impl MetaStoreHandle { pub fn get(&self) -> Arc { match &self.inner { HandleState::Internal(config) => config.lock().clone(), - HandleState::External(_, config) => config.borrow().clone().config, + HandleState::External(_, config, _) => config.borrow().clone().config, } } pub fn version(&self) -> usize { match &self.inner { HandleState::Internal(_) => 0, - HandleState::External(_, config) => config.borrow().version, + HandleState::External(_, config, _) => config.borrow().version, } } pub fn changed(&self) -> impl Future { let mut rcv = match &self.inner { HandleState::Internal(_) => panic!("can't wait for change on internal handle"), - HandleState::External(_, rcv) => rcv.clone(), + HandleState::External(_, rcv, _) => rcv.clone(), }; // ack the current value. rcv.borrow_and_update(); @@ -971,7 +1159,10 @@ impl MetaStoreHandle { *config.lock() = c; } } - HandleState::External(changes_tx, config) => { + HandleState::External(changes_tx, config, generation) => { + if !generation.load(Ordering::Acquire) { + return Err(Error::NamespaceDoesntExist(self.namespace.to_string())); + } tracing::debug!(?new_config, "storing new namespace config"); let mut c = config.clone(); // ack the current value. @@ -981,7 +1172,13 @@ impl MetaStoreHandle { let (snd, rcv) = oneshot::channel(); changes_tx - .send((self.namespace.clone(), new_config, snd, flush)) + .send(( + self.namespace.clone(), + new_config, + snd, + flush, + generation.clone(), + )) .await .map_err(|e| Error::MetaStoreUpdateFailure(e.into()))?; diff --git a/libsql-server/src/namespace/store.rs b/libsql-server/src/namespace/store.rs index 9021fdfc30..b65390b320 100644 --- a/libsql-server/src/namespace/store.rs +++ b/libsql-server/src/namespace/store.rs @@ -348,6 +348,118 @@ mod directory_tests { (tmp, store) } + #[tokio::test(flavor = "current_thread")] + async fn cache_miss_does_not_resurrect_primary_after_concurrent_destroy() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + store.evict_cached_namespace(&name).await; + let entered = Arc::new(Notify::new()); + let resume = Arc::new(Notify::new()); + let read = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + let entered = entered.clone(); + let resume = resume.clone(); + async move { + store + .with_after_initial_check(name, |ns| ns.path.clone(), async move { + entered.notify_one(); + resume.notified().await; + }) + .await + } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + store.destroy(name.clone(), false).await.unwrap(); + resume.notify_one(); + assert!(matches!( + timeout(Duration::from_secs(5), read) + .await + .unwrap() + .unwrap(), + Err(Error::NamespaceDoesntExist(_)) + )); + assert!(!store.exists(&name).await); + assert!(!tmp.path().join("dbs/victim").exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn delete_fences_delayed_config_handle_and_new_create_gets_new_generation() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("tenant"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let stale = store.config_store(name.clone()).await.unwrap(); + store.destroy(name.clone(), false).await.unwrap(); + assert!(!tmp.path().join("dbs/tenant").exists()); + assert!(!store.exists(&name).await); + assert!(matches!( + stale.store(DatabaseConfig::default()).await, + Err(Error::NamespaceDoesntExist(_)) + )); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + assert!(matches!( + stale.store(DatabaseConfig::default()).await, + Err(Error::NamespaceDoesntExist(_)) + )); + assert!(store.exists(&name).await); + assert!(tmp.path().join("dbs/tenant").exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn reset_rotates_config_generation_without_dropping_new_writes() { + let (_tmp, store) = primary_fixture().await; + let name = NamespaceName::from("tenant"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let stale = store.config_store(name.clone()).await.unwrap(); + store + .reset(name.clone(), RestoreOption::Latest) + .await + .unwrap(); + assert!(matches!( + stale.store(DatabaseConfig::default()).await, + Err(Error::NamespaceDoesntExist(_)) + )); + store + .config_store(name.clone()) + .await + .unwrap() + .store(DatabaseConfig::default()) + .await + .unwrap(); + assert!(store.exists(&name).await); + } + #[tokio::test(flavor = "current_thread")] async fn teardown_refuses_replaced_directory_and_preserves_both_inodes() { let (tmp, store) = primary_fixture().await; @@ -531,6 +643,7 @@ mod directory_tests { std::fs::read(path.join("sentinel")).unwrap(), b"not backed up" ); + assert!(store.exists(&name).await); assert!(store .create(name, RestoreOption::Latest, DatabaseConfig::default()) .await @@ -608,12 +721,125 @@ mod directory_tests { std::fs::read(path.join("sentinel")).unwrap(), b"not confirmed" ); + assert!(store.exists(&name).await); assert!(store .create(name, RestoreOption::Latest, DatabaseConfig::default()) .await .is_err()); } + #[tokio::test(flavor = "current_thread")] + async fn cancelled_waiter_after_backup_confirmation_drains_owned_teardown() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let held_connection = store.inner.metadata.hold_connection_for_test().await; + let ready = Arc::new(Notify::new()); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + let ready = ready.clone(); + async move { + store + .destroy_with_commit_signal(name, false, Some(ready)) + .await + } + }); + timeout(Duration::from_secs(5), ready.notified()) + .await + .unwrap(); + destroy.abort(); + assert!(destroy.await.unwrap_err().is_cancelled()); + // The worker retains the name lock even after the caller exits. + let operation = store + .inner + .name_operations + .lock() + .unwrap() + .get(&name) + .and_then(Weak::upgrade) + .unwrap(); + assert!(timeout(Duration::from_millis(20), operation.lock()) + .await + .is_err()); + drop(held_connection); + timeout(Duration::from_secs(5), async { + loop { + if !store.exists(&name).await && !tmp.path().join("dbs/victim").exists() { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + timeout( + Duration::from_secs(5), + store.create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ), + ) + .await + .unwrap() + .unwrap(); + assert!(store.exists(&name).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn confirmed_destroy_rejects_replaced_inode_without_losing_metadata() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old inode").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + std::fs::rename(&path, tmp.path().join("original")).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("sentinel"), b"replacement").unwrap(); + release.notify_one(); + assert!(timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .is_err()); + assert!(store.exists(&name).await); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"replacement" + ); + assert_eq!( + std::fs::read(tmp.path().join("original/sentinel")).unwrap(), + b"old inode" + ); + } + #[tokio::test(flavor = "current_thread")] async fn pending_dump_does_not_block_unrelated_operations_and_shutdown() { let (tmp, store) = primary_fixture().await; @@ -1280,27 +1506,31 @@ impl NamespaceStore { } pub async fn destroy(&self, namespace: NamespaceName, prune_all: bool) -> crate::Result<()> { - let _name_operation = self.lock_names(&[namespace.clone()]).await?; - // Do not remove metadata before proving this name owns its directory. - // Retain this expected inode through slow teardown and revalidate it - // under the global lock before touching the filesystem again. - let expected = self.cleanup_directory_identity(&namespace).await?; - // destroy on-disk database and backups - let db_config = tokio::task::spawn_blocking({ - let inner = self.inner.clone(); - let namespace = namespace.clone(); - move || { - inner - .metadata - .remove(namespace.clone())? - .ok_or_else(|| crate::Error::NamespaceDoesntExist(namespace.to_string())) - } - }) - .await??; + self.destroy_with_commit_signal(namespace, prune_all, None) + .await + } + async fn destroy_with_commit_signal( + &self, + namespace: NamespaceName, + prune_all: bool, + after_commit_started: Option>, + ) -> crate::Result<()> { + let operation = self.lock_names(&[namespace.clone()]).await?; + // Until remote backup confirmation, metadata and the old inode remain + // available. Cancellation here cannot strand a metadata-less orphan. + if !self.inner.metadata.exists(&namespace).await { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } + let expected = self.cleanup_directory_identity(&namespace).await?; + let db_config = self.inner.metadata.handle(namespace.clone()).await; + let generation = self + .inner + .metadata + .generation(&namespace) + .expect("persisted namespace has a generation"); let mut bottomless_db_id_init = NamespaceBottomlessDbIdInit::FetchFromConfig; if let Some(ns) = self.inner.store.remove(&namespace).await { - // deallocate in-memory resources if let Some(ns) = ns.write().await.take() { bottomless_db_id_init = NamespaceBottomlessDbIdInit::Provided( NamespaceBottomlessDbId::from_config(&ns.db_config_store.get()), @@ -1309,16 +1539,46 @@ impl NamespaceStore { } } - self.prepare_cleanup(&namespace, &db_config, prune_all, bottomless_db_id_init) - .await?; - let (detached, _) = self - .detach_owned_directory(&namespace, expected, false) - .await?; - Self::remove_detached_directory(detached).await?; - - tracing::info!("destroyed namespace: {namespace}"); + self.prepare_cleanup( + &namespace, + &db_config.get(), + prune_all, + bottomless_db_id_init, + ) + .await?; - Ok(()) + // From this point on, cancellation of the request must not interrupt + // the metadata+directory teardown. Transfer the name lock to a worker + // BEFORE the next await; shutdown and a new create wait for this lock. + let store = self.clone(); + let task = tokio::spawn(async move { + let _operation = operation; + // If the directory changed during backup, preserve its row and + // files for operator repair instead of deleting a replacement. + if store.cleanup_directory_identity(&namespace).await? != expected { + return Err(Error::InvalidPath(format!( + "namespace `{namespace}` directory changed before confirmed teardown" + ))); + } + let metadata = store.inner.metadata.clone(); + let name = namespace.clone(); + tokio::task::spawn_blocking(move || { + metadata + .remove_if_generation(name.clone(), Some(&generation))? + .ok_or_else(|| Error::NamespaceDoesntExist(name.to_string())) + }) + .await??; + let (detached, _) = store + .detach_owned_directory(&namespace, expected, false) + .await?; + Self::remove_detached_directory(detached).await?; + tracing::info!("destroyed namespace: {namespace}"); + Ok(()) + }); + if let Some(ready) = after_commit_started { + ready.notify_one(); + } + task.await? } pub async fn checkpoint(&self, namespace: NamespaceName) -> crate::Result<()> { @@ -1370,6 +1630,10 @@ impl NamespaceStore { .await?; let mut reservation = reservation.expect("reset reserves a replacement directory"); Self::remove_detached_directory(detached).await?; + // Reset keeps the same stored row but owns a new on-disk incarnation. + // Old handles/handshakes must not overwrite its config after reset. + self.inner.metadata.activate_for_create(&namespace); + let db_config = self.inner.metadata.handle(namespace.clone()).await; // Replica handshake may load an uncached shared schema. Never retain // the global identity lock over setup or a callback into this store. let mut shutdown = self.inner.shutdown_signal.subscribe(); @@ -1463,6 +1727,7 @@ impl NamespaceStore { } self.reserve_directory(&to).await? }; + self.inner.metadata.activate_for_create(&to); let mut cleanup = pending_cleanup( self.inner.metadata.clone(), to.clone(), @@ -1544,6 +1809,23 @@ impl NamespaceStore { pub async fn with(&self, namespace: NamespaceName, f: Fun) -> crate::Result where Fun: FnOnce(&Namespace) -> R, + { + self.with_after_initial_check(namespace, f, std::future::ready(())) + .await + } + + // The hook permits deterministic regression coverage of a delete racing + // the first metadata read. Production callers pass an immediately ready + // future and add no scheduling point. + async fn with_after_initial_check( + &self, + namespace: NamespaceName, + f: Fun, + after_check: Hook, + ) -> crate::Result + where + Fun: FnOnce(&Namespace) -> R, + Hook: std::future::Future, { if namespace != NamespaceName::default() && !self.inner.metadata.exists(&namespace).await @@ -1552,6 +1834,7 @@ impl NamespaceStore { return Err(Error::NamespaceDoesntExist(namespace.to_string())); } + after_check.await; let f = { let name = namespace.clone(); move |ns: NamespaceEntry| async move { @@ -1590,6 +1873,16 @@ impl NamespaceStore { } let _name_operation = self.lock_names(&[namespace.clone()]).await?; let is_new = !self.inner.metadata.exists(&namespace).await; + // The initial check above preceded this lock. A concurrent destroy + // may have removed the persisted row in between; never resurrect a + // primary cache miss using the default config without its metadata. + if is_new + && self.inner.db_kind.is_primary() + && namespace != NamespaceName::default() + && !self.inner.allow_lazy_creation + { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } // Replicas can have an exact, previously replicated directory without // local metadata. Reserve/check it before handle() registers a name. let mut reservation = if is_new && self.inner.db_kind.is_replica() { @@ -1605,6 +1898,9 @@ impl NamespaceStore { } else { None }; + if is_new && self.inner.db_kind.is_replica() { + self.inner.metadata.activate_for_create(&namespace); + } let handle = self.inner.metadata.handle(namespace.to_owned()).await; let entry = self .load_namespace(&namespace, handle, RestoreOption::Latest) @@ -1775,6 +2071,7 @@ impl NamespaceStore { } self.reserve_directory(&namespace).await? }; + self.inner.metadata.activate_for_create(&namespace); let mut cleanup = pending_cleanup( self.inner.metadata.clone(), namespace.clone(), From 30e83ad1ec3f7d072a6e3041983f778dd1a83fb2 Mon Sep 17 00:00:00 2001 From: Raphael Lopes Feitoza Date: Wed, 30 Sep 2026 18:21:36 -0400 Subject: [PATCH 09/11] Activate new namespace incarnation in cleanup race test --- libsql-server/src/namespace/store.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libsql-server/src/namespace/store.rs b/libsql-server/src/namespace/store.rs index b65390b320..dbe6f152ca 100644 --- a/libsql-server/src/namespace/store.rs +++ b/libsql-server/src/namespace/store.rs @@ -1054,6 +1054,9 @@ mod directory_tests { // detached cleanup, even after it has had another scheduling turn. std::fs::create_dir(tmp.path().join("dbs/failed")).unwrap(); std::fs::write(tmp.path().join("dbs/failed/sentinel"), b"new owner").unwrap(); + // The real create path rotates the revoked generation only after a + // fresh directory is reserved under this name's operation lock. + metadata.activate_for_create(&namespace); metadata .handle(namespace.clone()) .await From 55c324e1e4f52ce8363e00487f5e9a4c71c3d542 Mon Sep 17 00:00:00 2001 From: Raphael Lopes Feitoza Date: Thu, 1 Oct 2026 13:19:48 -0400 Subject: [PATCH 10/11] Recover namespace destroy and reset across crashes --- docs/ADMIN_API.md | 47 +- libsql-server/src/metrics.rs | 8 + libsql-server/src/namespace/meta_store.rs | 209 +- libsql-server/src/namespace/store.rs | 2433 ++++++++++++++++++--- libsql-server/src/schema/scheduler.rs | 4 + 5 files changed, 2391 insertions(+), 310 deletions(-) diff --git a/docs/ADMIN_API.md b/docs/ADMIN_API.md index 4c55e064e8..303bfef7e8 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -46,15 +46,44 @@ stop and permits a bounded drain before reporting an error. A replica detecting an incompatible log moves its old files to `replica-log-quarantine/` outside `dbs/`, retaining the namespace directory identity, then retries once. Inspect quarantined files before removal. Destroy/reset confirms remote backups before -moving a directory to `namespace-teardown-quarantine/` for local removal; -backup failure/cancellation before confirmation leaves the old directory and -metastore row in place; after confirmation an independently draining worker -owns the name lock through metadata removal and identity-checked teardown even -if the HTTP request is cancelled. Stale config/replication handles from a -prior incarnation cannot reinsert rows or links after deletion; a fresh -create/reset uses a new generation. Unexpected identity changes are preserved -for explicit operator repair. Inspect remaining quarantine files after -interrupted teardown. +moving a directory to `namespace-teardown-quarantine/`; backup failure before +confirmation leaves the old directory and metastore row in place. After +confirmation, independently draining workers own the name lock even if the HTTP +request is cancelled. Destroy then removes metadata and the old directory. +Before deleting metadata, destroy atomically publishes a fully written `namespace-destroy-intents/` record identifying the +old directory; unpublished temporary records are discarded on startup. +Reconciliation rolls back an uncommitted intent or finishes a committed delete +before namespaces are served. A mismatched/missing live inode or unexpected +quarantine fails startup for operator repair rather than opening a blank DB. +Incomplete intents also fence create, fork, replica load, and reset until +recovered. Reset separately publishes a `namespace-reset-intents/` record with +the old inode and persisted config before detaching the old directory. The old +files remain quarantined while the replacement is set up. A pending reset +rejects changing its shared-schema membership: the original link must keep the +old schema available, and a second link would incorrectly enlist the partial +database in schema migrations. A reset waits for the schema registration lock +and refuses an existing migration; new migrations are rejected while a linked +tenant or the schema namespace itself has a pending reset intent, avoiding +scheduler retry exhaustion. Only a +durable commit marker permits release of this restriction and deletion of the +old files. Before that marker, a crash +restores the old files and config and preserves partial new files under +`namespace-reset-abandoned/` for inspection. A setup error keeps the name +fenced until process restart, when all late setup workers have stopped; +rollback during the live process could otherwise redirect late path opens into +the old database. Invalid identities or interrupted recovery fail closed. +Unix directory fsync orders the intent ahead of the SQLite commit; +sudden power-loss durability is not guaranteed on Windows (which has no +portable directory fsync here), nor is macOS physical flush guaranteed by +ordinary fsync. Process-crash recovery is supported on both. Short filesystem +identity scans and renames remain synchronous under the filesystem lock: +offloading only the syscalls would allow cancellation to release a name lock +while late writes are still running. Large directories can therefore briefly +stall a current-thread runtime pending a separately coordinated offload. Stale +config/replication handles from a prior incarnation cannot reinsert rows or +links after deletion; a fresh create/reset uses a new generation. Unexpected +identity changes are preserved for explicit operator repair. Inspect remaining +quarantine files after interrupted teardown. ## Routes diff --git a/libsql-server/src/metrics.rs b/libsql-server/src/metrics.rs index e5de216b79..44c85d2596 100644 --- a/libsql-server/src/metrics.rs +++ b/libsql-server/src/metrics.rs @@ -37,6 +37,14 @@ pub static STREAM_HANDLES_COUNT: Lazy = Lazy::new(|| { describe_gauge!(NAME, "amount of in-memory stream handles"); register_gauge!(NAME) }); +pub static NAMESPACE_QUARANTINE_COUNT: Lazy = Lazy::new(|| { + const NAME: &str = "libsql_server_namespace_quarantine_total"; + describe_counter!( + NAME, + "namespace data retained or moved to quarantine, including incomplete moves" + ); + register_counter!(NAME) +}); pub static NAMESPACE_LOAD_LATENCY: Lazy = Lazy::new(|| { const NAME: &str = "libsql_server_namespace_load_latency"; describe_histogram!(NAME, "latency is us when loading a namespace"); diff --git a/libsql-server/src/namespace/meta_store.rs b/libsql-server/src/namespace/meta_store.rs index 9df5f0ead5..7390505ab9 100644 --- a/libsql-server/src/namespace/meta_store.rs +++ b/libsql-server/src/namespace/meta_store.rs @@ -82,6 +82,9 @@ struct MetaStoreInner { // installs a NEW token. Old handles and queued messages keep the revoked // token, even if a later namespace reuses the same spelling. generations: Mutex>>, + // Pending reset preserves exactly one schema membership. A second link + // would also enqueue schema migrations against a fenced, partial DB. + reset_pins: Mutex>>, conn: tokio::sync::Mutex, wal_manager: MetaStoreWalManager, db_kind: DatabaseKind, @@ -201,6 +204,7 @@ impl MetaStoreInner { let mut this = MetaStoreInner { configs: Default::default(), generations: Default::default(), + reset_pins: Default::default(), conn: conn.into(), wal_manager, db_kind, @@ -246,6 +250,29 @@ impl MetaStoreInner { continue; } }; + // A committed destroy can crash before detaching its old + // directory. Do not resurrect it when recovering an empty + // metastore; NamespaceStore finishes the durable intent on + // startup before accepting namespace operations. + let intent_key: String = name + .as_slice() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect(); + if std::fs::symlink_metadata( + base_path + .join("namespace-destroy-intents") + .join(&intent_key), + ) + .is_ok() + || std::fs::symlink_metadata( + base_path.join("namespace-reset-intents").join(intent_key), + ) + .is_ok() + { + tracing::warn!("skipping namespace `{name}` with pending destroy intent"); + continue; + } let config_path = entry.path().join("config.json"); let config = if config_path.try_exists()? { let config_bytes = std::fs::read(&config_path)?; @@ -351,6 +378,13 @@ fn process(msg: ChangeMsg, inner: Arc) { if !generation.load(Ordering::Acquire) { return Err(Error::NamespaceDoesntExist(namespace.to_string())); } + if let Some(old_schema) = inner.reset_pins.lock().get(&namespace) { + if &config.shared_schema_name != old_schema { + return Err(Error::InvalidPath(format!( + "schema change during pending reset of `{namespace}`" + ))); + } + } if let Some(config_watch) = configs.get_mut(&namespace) { let new_version = config_watch.borrow().version.wrapping_add(1); config_watch.send_modify(|c| { @@ -395,6 +429,13 @@ fn try_process( let config_encoded = metadata::DatabaseConfig::from(config).encode_to_vec(); let mut conn = inner.conn.blocking_lock(); + if let Some(old_schema) = inner.reset_pins.lock().get(namespace) { + if &config.shared_schema_name != old_schema { + return Err(Error::InvalidPath(format!( + "schema change during pending reset of `{namespace}`" + ))); + } + } // This check is AFTER acquiring the DB lock. A pre-delete write either // commits before remove (and is removed), or sees the revoked generation. if !generation.load(Ordering::Acquire) { @@ -415,7 +456,7 @@ fn try_process( )?; tx.execute( "DELETE FROM shared_schema_links WHERE namespace = ?", - rusqlite::params![namespace.as_str()], + [namespace.as_str()], )?; tx.execute( "INSERT OR REPLACE INTO shared_schema_links (shared_schema_name, namespace) VALUES (?1, ?2)", @@ -618,6 +659,135 @@ impl MetaStore { self.remove_if_generation(namespace, None) } + /// Snapshot the old config, preserve its schema link, and revoke old + /// handles while holding the SQL connection lock. Queued old writes either + /// precede this snapshot or observe the revoked generation. + pub(crate) fn pin_reset_and_snapshot(&self, namespace: &NamespaceName) -> Result> { + let conn = self.inner.conn.blocking_lock(); + let bytes: Vec = conn.query_row( + "SELECT config FROM namespace_configs WHERE namespace = ?1", + [namespace.as_str()], + |row| row.get(0), + )?; + let config = DatabaseConfig::try_from(&metadata::DatabaseConfig::decode(&bytes[..])?)?; + // Discard unflushed watch-only config from the old incarnation. + if let Some(watch) = self.inner.configs.blocking_lock().get_mut(namespace) { + let version = watch.borrow().version.wrapping_add(1); + watch.send_modify(|value| { + *value = InnerConfig { + version, + config: Arc::new(config.clone()), + }; + }); + } + self.inner + .reset_pins + .lock() + .insert(namespace.clone(), config.shared_schema_name.clone()); + if let Some(old) = self + .inner + .generations + .lock() + .insert(namespace.clone(), Arc::new(AtomicBool::new(true))) + { + old.store(false, Ordering::Release); + } + Ok(bytes) + } + + pub(crate) fn reset_snapshot_schema_lock( + namespace: &NamespaceName, + bytes: &[u8], + ) -> Result> { + let config = DatabaseConfig::try_from(&metadata::DatabaseConfig::decode(bytes)?)?; + // A schema reset must exclude registration on its OWN namespace, + // although shared_schema_name is None on the schema's config. + Ok(if config.is_shared_schema { + Some(namespace.clone()) + } else { + config.shared_schema_name + }) + } + + /// Call only after the reset commit marker is durable. The ordinary + /// metadata path may then change schema membership normally. + pub(crate) fn release_reset_pin(&self, namespace: &NamespaceName) { + let _conn = self.inner.conn.blocking_lock(); + self.inner.reset_pins.lock().remove(namespace); + } + + /// Restore both the row and shared-schema links before a pending reset + /// intent is cleared. This also updates the in-memory watch on live repair. + pub(crate) fn restore_reset_config( + &self, + namespace: &NamespaceName, + bytes: &[u8], + ) -> Result<()> { + let config = DatabaseConfig::try_from(&metadata::DatabaseConfig::decode(bytes)?)?; + let mut conn = self.inner.conn.blocking_lock(); + let mut configs = self.inner.configs.blocking_lock(); + let tx = conn.transaction()?; + if tx.execute( + "UPDATE namespace_configs SET config = ?1 WHERE namespace = ?2", + rusqlite::params![bytes, namespace.as_str()], + )? != 1 + { + return Err(Error::NamespaceDoesntExist(namespace.to_string())); + } + tx.execute( + "DELETE FROM shared_schema_links WHERE namespace = ?1", + [namespace.as_str()], + )?; + if let Some(schema) = config.shared_schema_name.as_ref() { + tx.execute( + "INSERT INTO shared_schema_links (shared_schema_name, namespace) VALUES (?1, ?2)", + (schema.as_str(), namespace.as_str()), + )?; + } + tx.commit()?; + self.inner.reset_pins.lock().remove(namespace); + if let Some(watch) = configs.get_mut(namespace) { + let version = watch.borrow().version.wrapping_add(1); + watch.send_modify(|value| { + *value = InnerConfig { + version, + config: Arc::new(config), + }; + }); + } + Ok(()) + } + + pub(crate) fn linked_namespaces(&self, schema: &NamespaceName) -> Result> { + let conn = self.inner.conn.blocking_lock(); + let mut stmt = conn + .prepare("SELECT namespace FROM shared_schema_links WHERE shared_schema_name = ?1")?; + let names = stmt + .query_map([schema.as_str()], |row| row.get::<_, String>(0))? + .map(|row| NamespaceName::from_string(row?)) + .collect(); + names + } + + pub(crate) fn schema_has_pending_jobs(&self, schema: &NamespaceName) -> Result { + let conn = self.inner.conn.blocking_lock(); + Ok(crate::schema::db::has_pending_migration_jobs( + &conn, schema, + )?) + } + + /// Query the committed row under the same connection lock as deletion. + /// The in-memory watch map cannot resolve an ambiguous commit failure. + pub(crate) fn persisted_namespace_exists(&self, namespace: &NamespaceName) -> Result { + let conn = self.inner.conn.blocking_lock(); + let exists: i64 = conn.query_row( + "SELECT EXISTS(SELECT 1 FROM namespace_configs WHERE namespace = ?1)", + [namespace.as_str()], + |row| row.get(0), + )?; + Ok(exists != 0) + } + // For deferred destroy, an older worker may never remove a later // incarnation. The comparison and revocation happen under the DB lock. pub(crate) fn remove_if_generation( @@ -754,6 +924,43 @@ mod tests { use super::*; use tempfile::tempdir; + #[tokio::test] + async fn committed_row_check_does_not_trust_stale_in_memory_config() { + let tmp = tempdir().unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + MetaStoreConfig::default(), + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + let name = NamespaceName::from("victim"); + metadata + .handle(name.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + { + let conn = metadata.hold_connection_for_test().await; + conn.execute( + "DELETE FROM namespace_configs WHERE namespace = 'victim'", + [], + ) + .unwrap(); + } + assert!(metadata.exists(&name).await); + let persisted = + tokio::task::spawn_blocking(move || metadata.persisted_namespace_exists(&name)) + .await + .unwrap() + .unwrap(); + assert!(!persisted); + } + #[tokio::test] async fn stale_handle_cannot_recreate_config_or_schema_link_after_delete_or_recreate() { let tmp = tempdir().unwrap(); diff --git a/libsql-server/src/namespace/store.rs b/libsql-server/src/namespace/store.rs index dbe6f152ca..bb42077b96 100644 --- a/libsql-server/src/namespace/store.rs +++ b/libsql-server/src/namespace/store.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -19,7 +20,7 @@ use crate::broadcaster::BroadcastMsg; use crate::connection::config::DatabaseConfig; use crate::database::DatabaseKind; use crate::error::Error; -use crate::metrics::NAMESPACE_LOAD_LATENCY; +use crate::metrics::{NAMESPACE_LOAD_LATENCY, NAMESPACE_QUARANTINE_COUNT}; use crate::namespace::{NamespaceBottomlessDbId, NamespaceBottomlessDbIdInit, NamespaceName}; use crate::stats::Stats; @@ -32,6 +33,12 @@ use super::{Namespace, ResetCb, ResetOp, ResolveNamespacePathFn, RestoreOption}; type NamespaceEntry = Arc>>; +#[derive(serde::Serialize, serde::Deserialize)] +struct ResetIntent { + old_identity: (u64, u64), + old_config: Vec, +} + // A new directory is owned only after create_dir succeeds. Dropping a // reservation never deletes by path: cancellation without a cleanup worker // must leave a safe, explicit orphan instead of racing a later creator. @@ -80,6 +87,7 @@ impl PendingCleanup { "quarantining cancelled namespace directory {:?}", self.directory.path ); + NAMESPACE_QUARANTINE_COUNT.increment(1); self.directory.disarm(); } if let Err(e) = self.metadata.wait_for_pending_changes().await { @@ -102,6 +110,18 @@ impl PendingCleanup { } } +// Unix directory fsync orders the intent's publication ahead of SQLite's +// delete commit. Windows does not expose a portable directory fsync here: +// process-crash recovery works, but sudden-power-loss durability is NOT +// promised on Windows (nor does macOS fsync imply hardware F_FULLFSYNC). +fn sync_directory(path: &Path) -> std::io::Result<()> { + #[cfg(unix)] + std::fs::File::open(path)?.sync_all()?; + #[cfg(not(unix))] + let _ = path; + Ok(()) +} + fn directory_identity(metadata: &std::fs::Metadata) -> Option<(u64, u64)> { if !metadata.is_dir() || metadata.file_type().is_symlink() { return None; @@ -178,6 +198,7 @@ mod directory_tests { }; use crate::namespace::meta_store::metastore_connection_maker; use libsql_sys::wal::Sqlite3WalManager; + use prost::Message; use tokio::sync::{Notify, Semaphore}; use tokio::time::timeout; @@ -288,6 +309,33 @@ mod directory_tests { primary_fixture_with_cleanup_gate(None).await } + async fn reopened_store(tmp: &tempfile::TempDir) -> NamespaceStore { + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + crate::config::MetaStoreConfig { + allow_recover_from_fs: true, + ..Default::default() + }, + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + NamespaceStore::new( + false, + false, + 10, + metadata, + NamespaceConfigurators::empty(), + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap() + } + async fn primary_fixture_with_cleanup_gate( gate: Option<(Arc, Arc, bool)>, ) -> (tempfile::TempDir, NamespaceStore) { @@ -348,6 +396,27 @@ mod directory_tests { (tmp, store) } + #[cfg(unix)] + #[tokio::test(flavor = "current_thread")] + async fn ensure_existing_directory_single_scan_rejects_symlink_alias() { + use std::os::unix::fs::symlink; + let (tmp, store) = primary_fixture().await; + let real = tmp.path().join("dbs/real"); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("sentinel"), b"real").unwrap(); + symlink(&real, tmp.path().join("dbs/alias")).unwrap(); + assert!(store + .ensure_existing_directory(&NamespaceName::from("alias")) + .await + .is_err()); + assert!(store + .ensure_existing_directory(&NamespaceName::from("real")) + .await + .unwrap() + .is_none()); + assert_eq!(std::fs::read(real.join("sentinel")).unwrap(), b"real"); + } + #[tokio::test(flavor = "current_thread")] async fn cache_miss_does_not_resurrect_primary_after_concurrent_destroy() { let (tmp, store) = primary_fixture().await; @@ -471,7 +540,7 @@ mod directory_tests { std::fs::create_dir(&path).unwrap(); std::fs::write(path.join("sentinel"), b"new owner").unwrap(); assert!(store - .detach_owned_directory(&name, expected, false) + .detach_owned_directory(&name, expected, false, None) .await .is_err()); assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"new owner"); @@ -549,12 +618,8 @@ mod directory_tests { } #[tokio::test(flavor = "current_thread")] - async fn stalled_reset_backup_keeps_old_inode_until_confirmed() { - let entered = Arc::new(Notify::new()); - let release = Arc::new(Notify::new()); - let (tmp, store) = - primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) - .await; + async fn reset_setup_failure_fences_old_data_until_restart() { + let (tmp, store) = primary_fixture().await; let name = NamespaceName::from("victim"); store .create( @@ -565,55 +630,53 @@ mod directory_tests { .await .unwrap(); let path = tmp.path().join("dbs/victim"); - std::fs::write(path.join("sentinel"), b"old data").unwrap(); - let reset = tokio::spawn({ - let store = store.clone(); - let name = name.clone(); - async move { store.reset(name, RestoreOption::Latest).await } - }); - timeout(Duration::from_secs(5), entered.notified()) + std::fs::write(path.join("sentinel"), b"old").unwrap(); + let old = store.cleanup_directory_identity(&name).await.unwrap(); + let dump = futures::stream::iter(vec![Ok(bytes::Bytes::from_static(b"not valid SQL;"))]); + assert!(store + .reset(name.clone(), RestoreOption::Dump(Box::new(dump))) .await - .unwrap(); - assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old data"); - timeout( - Duration::from_secs(5), - store.create( - NamespaceName::from("unrelated"), - RestoreOption::Latest, - DatabaseConfig::default(), - ), - ) - .await - .unwrap() - .unwrap(); - let same_name = tokio::spawn({ - let store = store.clone(); - let name = name.clone(); - async move { store.with(name, |ns| ns.path.clone()).await } - }); - tokio::task::yield_now().await; - assert!(!same_name.is_finished()); - release.notify_one(); - timeout(Duration::from_secs(5), reset) + .is_err()); + assert_eq!( + std::fs::read(store.reset_quarantine_path(&name).join("sentinel")).unwrap(), + b"old" + ); + assert!(store.reset_intent_path(&name).exists()); + assert!(store.lock_names(&[name.clone()]).await.is_err()); + // Simulate config changes from setup that must not remain paired with + // the restored old database after process restart. + let mut changed = DatabaseConfig::default(); + changed.block_reads = true; + store + .inner + .metadata + .handle(name.clone()) .await - .unwrap() - .unwrap() - .unwrap(); - timeout(Duration::from_secs(5), same_name) + .store(changed) .await - .unwrap() - .unwrap() .unwrap(); - assert!(!path.join("sentinel").exists()); - assert!(store.exists(&name).await); + drop(store); + let restarted = reopened_store(&tmp).await; + assert_eq!( + restarted.cleanup_directory_identity(&name).await.unwrap(), + old + ); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old"); + assert!( + !restarted + .inner + .metadata + .handle(name.clone()) + .await + .get() + .block_reads + ); + assert!(!restarted.reset_intent_path(&name).exists()); } #[tokio::test(flavor = "current_thread")] - async fn failed_backup_keeps_old_directory_and_rejects_same_name_retry() { - let entered = Arc::new(Notify::new()); - let release = Arc::new(Notify::new()); - let (tmp, store) = - primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), true))).await; + async fn cancelled_reset_waiter_cannot_release_name_lock_during_setup() { + let (tmp, store) = primary_fixture().await; let name = NamespaceName::from("victim"); store .create( @@ -624,220 +687,1175 @@ mod directory_tests { .await .unwrap(); let path = tmp.path().join("dbs/victim"); - std::fs::write(path.join("sentinel"), b"not backed up").unwrap(); - let destroy = tokio::spawn({ + std::fs::write(path.join("sentinel"), b"old").unwrap(); + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let dump = futures::stream::once({ + let entered = entered.clone(); + let release = release.clone(); + async move { + entered.notify_one(); + release.notified().await; + Ok(bytes::Bytes::from_static(b"not valid SQL;")) + } + }); + let waiter = tokio::spawn({ let store = store.clone(); let name = name.clone(); - async move { store.destroy(name, false).await } + async move { + store + .reset(name, RestoreOption::Dump(Box::new(Box::pin(dump)))) + .await + } }); timeout(Duration::from_secs(5), entered.notified()) .await .unwrap(); - release.notify_one(); - assert!(timeout(Duration::from_secs(5), destroy) - .await + waiter.abort(); + assert!(waiter.await.unwrap_err().is_cancelled()); + let name_lock = store + .inner + .name_operations + .lock() .unwrap() + .get(&name) .unwrap() - .is_err()); - assert_eq!( - std::fs::read(path.join("sentinel")).unwrap(), - b"not backed up" + .upgrade() + .unwrap(); + assert!( + timeout(Duration::from_millis(50), name_lock.clone().lock_owned()) + .await + .is_err() ); - assert!(store.exists(&name).await); - assert!(store - .create(name, RestoreOption::Latest, DatabaseConfig::default()) + release.notify_one(); + // The detached owner finishes (or safely fences on setup failure) + // before a new same-name operation can proceed. + let guard = timeout(Duration::from_secs(5), name_lock.lock_owned()) .await - .is_err()); + .unwrap(); + assert!(store.reset_intent_path(&name).exists()); + assert_eq!( + std::fs::read(store.reset_quarantine_path(&name).join("sentinel")).unwrap(), + b"old" + ); + drop(guard); + drop(store); + let restarted = reopened_store(&tmp).await; + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old"); + assert!(!restarted.reset_intent_path(&name).exists()); } #[tokio::test(flavor = "current_thread")] - async fn shutdown_reports_failed_drain_while_backup_confirmation_is_stalled() { - let entered = Arc::new(Notify::new()); - let release = Arc::new(Notify::new()); - let (_tmp, store) = - primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) - .await; + async fn restart_reconciles_reset_at_each_boundary() { let name = NamespaceName::from("victim"); - store - .create( - name.clone(), - RestoreOption::Latest, - DatabaseConfig::default(), - ) + for stage in 0..7 { + let (tmp, store) = primary_fixture().await; + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + store.evict_cached_namespace(&name).await; + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old").unwrap(); + let expected = store + .cleanup_directory_identity(&name) + .await + .unwrap() + .unwrap(); + let old_config = tokio::task::spawn_blocking({ + let metadata = store.inner.metadata.clone(); + let name = name.clone(); + move || metadata.pin_reset_and_snapshot(&name).unwrap() + }) .await .unwrap(); - let destroy = tokio::spawn({ - let store = store.clone(); - async move { store.destroy(name, false).await } - }); - timeout(Duration::from_secs(5), entered.notified()) - .await + let bytes = serde_json::to_vec(&ResetIntent { + old_identity: expected, + old_config, + }) .unwrap(); - let result = timeout( - Duration::from_secs(2), store - .clone() - .shutdown_with_timeout(Duration::from_millis(40)), - ) - .await - .unwrap(); - assert!(matches!(result, Err(Error::Blocked(_)))); - release.notify_one(); - timeout(Duration::from_secs(5), destroy) - .await - .unwrap() - .unwrap() - .unwrap(); + .publish_reset_file(&store.reset_intent_path(&name), &bytes) + .unwrap(); + if stage >= 1 { + store + .detach_owned_directory( + &name, + Some(expected), + true, + Some(store.reset_quarantine_path(&name)), + ) + .await + .unwrap(); + if stage >= 2 { + std::fs::write(path.join("partial"), b"new").unwrap(); + } + let mut changed = DatabaseConfig::default(); + changed.block_reads = true; + store + .inner + .metadata + .handle(name.clone()) + .await + .store(changed) + .await + .unwrap(); + } + if stage >= 3 { + let fresh = store + .cleanup_directory_identity(&name) + .await + .unwrap() + .unwrap(); + store + .publish_reset_file( + &store.reset_committed_path(&name), + format!("committed {} {}\n", fresh.0, fresh.1).as_bytes(), + ) + .unwrap(); + } + if stage >= 4 { + std::fs::remove_dir_all(store.reset_quarantine_path(&name)).unwrap(); + } + if stage >= 5 { + std::fs::remove_file(store.reset_intent_path(&name)).unwrap(); + } + if stage == 6 { + std::fs::remove_file(store.reset_committed_path(&name)).unwrap(); + } + drop(store); + let restarted = reopened_store(&tmp).await; + assert!(!restarted.reset_intent_path(&name).exists()); + assert!(!restarted.reset_committed_path(&name).exists()); + if stage < 3 { + assert_eq!( + restarted.cleanup_directory_identity(&name).await.unwrap(), + Some(expected) + ); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old"); + assert!( + !restarted + .inner + .metadata + .handle(name.clone()) + .await + .get() + .block_reads + ); + if stage >= 1 { + assert!( + std::fs::read_dir(tmp.path().join("namespace-reset-abandoned")) + .unwrap() + .next() + .is_some() + ); + } + } else { + assert_ne!( + restarted.cleanup_directory_identity(&name).await.unwrap(), + Some(expected) + ); + assert_eq!(std::fs::read(path.join("partial")).unwrap(), b"new"); + assert!( + restarted + .inner + .metadata + .handle(name.clone()) + .await + .get() + .block_reads + ); + assert!(!restarted.reset_quarantine_path(&name).exists()); + } + } } #[tokio::test(flavor = "current_thread")] - async fn cancelled_backup_keeps_old_directory() { - let entered = Arc::new(Notify::new()); - let release = Arc::new(Notify::new()); - let (tmp, store) = - primary_fixture_with_cleanup_gate(Some((entered.clone(), release, false))).await; - let name = NamespaceName::from("victim"); + async fn pending_reset_refuses_schema_switch_and_keeps_migration_membership() { + let (tmp, store) = primary_fixture().await; + let tenant = NamespaceName::from("tenant"); store .create( - name.clone(), + tenant.clone(), RestoreOption::Latest, DatabaseConfig::default(), ) .await .unwrap(); - let path = tmp.path().join("dbs/victim"); - std::fs::write(path.join("sentinel"), b"not confirmed").unwrap(); - let destroy = tokio::spawn({ - let store = store.clone(); - let name = name.clone(); - async move { store.destroy(name, false).await } - }); - timeout(Duration::from_secs(5), entered.notified()) + store.evict_cached_namespace(&tenant).await; + std::fs::write(tmp.path().join("dbs/tenant/sentinel"), b"old tenant").unwrap(); + { + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute_batch("CREATE TABLE jobs (schema TEXT, finished BOOLEAN)") + .unwrap(); + } + let a = NamespaceName::from("schema-a"); + let b = NamespaceName::from("schema-b"); + let mut schema_config = DatabaseConfig::default(); + schema_config.is_shared_schema = true; + store + .inner + .metadata + .handle(a.clone()) + .await + .store(schema_config.clone()) .await .unwrap(); - destroy.abort(); - assert!(destroy.await.unwrap_err().is_cancelled()); - assert_eq!( - std::fs::read(path.join("sentinel")).unwrap(), - b"not confirmed" - ); - assert!(store.exists(&name).await); - assert!(store - .create(name, RestoreOption::Latest, DatabaseConfig::default()) + store + .inner + .metadata + .handle(b.clone()) .await - .is_err()); - } - - #[tokio::test(flavor = "current_thread")] - async fn cancelled_waiter_after_backup_confirmation_drains_owned_teardown() { - let (tmp, store) = primary_fixture().await; - let name = NamespaceName::from("victim"); + .store(schema_config) + .await + .unwrap(); + let mut linked = DatabaseConfig::default(); + linked.shared_schema_name = Some(a.clone()); store - .create( - name.clone(), - RestoreOption::Latest, - DatabaseConfig::default(), - ) + .inner + .metadata + .handle(tenant.clone()) + .await + .store(linked.clone()) .await .unwrap(); - let held_connection = store.inner.metadata.hold_connection_for_test().await; - let ready = Arc::new(Notify::new()); - let destroy = tokio::spawn({ - let store = store.clone(); - let name = name.clone(); - let ready = ready.clone(); - async move { + let old_identity = store + .cleanup_directory_identity(&tenant) + .await + .unwrap() + .unwrap(); + let old_config = tokio::task::spawn_blocking({ + let metadata = store.inner.metadata.clone(); + let tenant = tenant.clone(); + move || metadata.pin_reset_and_snapshot(&tenant).unwrap() + }) + .await + .unwrap(); + store + .publish_reset_file( + &store.reset_intent_path(&tenant), + &serde_json::to_vec(&ResetIntent { + old_identity, + old_config, + }) + .unwrap(), + ) + .unwrap(); + store + .detach_owned_directory( + &tenant, + Some(old_identity), + true, + Some(store.reset_quarantine_path(&tenant)), + ) + .await + .unwrap(); + assert!(store.ensure_schema_has_no_pending_resets(&a).await.is_err()); + linked.shared_schema_name = Some(b.clone()); + // Refuse the switch rather than create a second A+B link: links are + // also the schema migration worklist, not only a deletion guard. + assert!(store + .inner + .metadata + .handle(tenant.clone()) + .await + .store(linked) + .await + .is_err()); + let remove_a = tokio::task::spawn_blocking({ + let metadata = store.inner.metadata.clone(); + let a = a.clone(); + move || metadata.remove(a) + }) + .await + .unwrap(); + assert!(matches!(remove_a, Err(crate::Error::HasLinkedDbs(_)))); + drop(store); + let restarted = reopened_store(&tmp).await; + assert!(restarted + .ensure_schema_has_no_pending_resets(&a) + .await + .is_ok()); + assert_eq!( + restarted.cleanup_directory_identity(&tenant).await.unwrap(), + Some(old_identity) + ); + assert_eq!( + std::fs::read(tmp.path().join("dbs/tenant/sentinel")).unwrap(), + b"old tenant" + ); + assert_eq!( + restarted + .inner + .metadata + .handle(tenant.clone()) + .await + .get() + .shared_schema_name + .as_ref(), + Some(&a) + ); + let conn = restarted.inner.metadata.hold_connection_for_test().await; + let links_a: i64 = conn.query_row( + "SELECT count(*) FROM shared_schema_links WHERE shared_schema_name = 'schema-a' AND namespace = 'tenant'", + [], |row| row.get(0)).unwrap(); + let links_b: i64 = conn.query_row( + "SELECT count(*) FROM shared_schema_links WHERE shared_schema_name = 'schema-b' AND namespace = 'tenant'", + [], |row| row.get(0)).unwrap(); + assert_eq!((links_a, links_b), (1, 0)); + } + + #[tokio::test(flavor = "current_thread")] + async fn schema_own_reset_blocks_migration_registration_until_recovered() { + let (tmp, store) = primary_fixture().await; + let schema = NamespaceName::from("schema-a"); + let mut config = DatabaseConfig::default(); + config.is_shared_schema = true; + store + .inner + .metadata + .handle(schema.clone()) + .await + .store(config) + .await + .unwrap(); + std::fs::create_dir_all(tmp.path().join("dbs/schema-a")).unwrap(); + let old = store + .cleanup_directory_identity(&schema) + .await + .unwrap() + .unwrap(); + let snapshot = tokio::task::spawn_blocking({ + let metadata = store.inner.metadata.clone(); + let schema = schema.clone(); + move || metadata.pin_reset_and_snapshot(&schema).unwrap() + }) + .await + .unwrap(); + assert_eq!( + MetaStore::reset_snapshot_schema_lock(&schema, &snapshot).unwrap(), + Some(schema.clone()) + ); + let shared = store.schema_locks().acquire_shared(schema.clone()).await; + let registration = tokio::spawn({ + let store = store.clone(); + let schema = schema.clone(); + async move { + let _exclusive = store.schema_locks().acquire_exlusive(schema.clone()).await; + store.ensure_schema_has_no_pending_resets(&schema).await + } + }); + tokio::task::yield_now().await; + assert!(!registration.is_finished()); + let intent = serde_json::to_vec(&ResetIntent { + old_identity: old, + old_config: snapshot, + }) + .unwrap(); + store + .publish_reset_file(&store.reset_intent_path(&schema), &intent) + .unwrap(); + drop(shared); + assert!(timeout(Duration::from_secs(5), registration) + .await + .unwrap() + .unwrap() + .is_err()); + drop(store); + let restarted = reopened_store(&tmp).await; + assert!(!restarted.reset_intent_path(&schema).exists()); + assert_eq!( + restarted.cleanup_directory_identity(&schema).await.unwrap(), + Some(old) + ); + assert!(restarted + .ensure_schema_has_no_pending_resets(&schema) + .await + .is_ok()); + } + + #[tokio::test(flavor = "current_thread")] + async fn reset_refuses_pending_schema_migration_before_detaching_old_data() { + let (_tmp, store) = primary_fixture().await; + let name = NamespaceName::from("tenant"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let a = NamespaceName::from("schema-a"); + { + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute_batch("CREATE TABLE jobs (schema TEXT, finished BOOLEAN)") + .unwrap(); + } + store + .inner + .metadata + .handle(a.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + let mut linked = DatabaseConfig::default(); + linked.shared_schema_name = Some(a); + store + .inner + .metadata + .handle(name.clone()) + .await + .store(linked) + .await + .unwrap(); + { + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute("INSERT INTO jobs VALUES ('schema-a', false)", []) + .unwrap(); + } + let old = store.cleanup_directory_identity(&name).await.unwrap(); + assert!(store + .reset(name.clone(), RestoreOption::Latest) + .await + .is_err()); + assert_eq!(store.cleanup_directory_identity(&name).await.unwrap(), old); + assert!(!store.reset_intent_path(&name).exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn reset_schema_lock_uses_persisted_snapshot_not_stale_watch() { + let (_tmp, store) = primary_fixture().await; + let tenant = NamespaceName::from("tenant"); + store + .create( + tenant.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let a = NamespaceName::from("schema-a"); + let b = NamespaceName::from("schema-b"); + { + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute_batch("CREATE TABLE jobs (schema TEXT, finished BOOLEAN)") + .unwrap(); + } + store + .inner + .metadata + .handle(a.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + store + .inner + .metadata + .handle(b.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + let mut linked = DatabaseConfig::default(); + linked.shared_schema_name = Some(b); + store + .inner + .metadata + .handle(tenant.clone()) + .await + .store(linked.clone()) + .await + .unwrap(); + // Reproduce the worker gap after SQL committed A but before it updates + // the watch: the cache still says B while the old row/link says A. + linked.shared_schema_name = Some(a.clone()); + let encoded = + libsql_replication::rpc::metadata::DatabaseConfig::from(&linked).encode_to_vec(); + { + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute( + "UPDATE namespace_configs SET config = ?1 WHERE namespace = 'tenant'", + [encoded], + ) + .unwrap(); + conn.execute( + "DELETE FROM shared_schema_links WHERE namespace = 'tenant'", + [], + ) + .unwrap(); + conn.execute( + "INSERT INTO shared_schema_links VALUES ('schema-a', 'tenant')", + [], + ) + .unwrap(); + conn.execute("INSERT INTO jobs VALUES ('schema-a', false)", []) + .unwrap(); + } + let old = store.cleanup_directory_identity(&tenant).await.unwrap(); + assert!(store + .reset(tenant.clone(), RestoreOption::Latest) + .await + .is_err()); + assert_eq!( + store.cleanup_directory_identity(&tenant).await.unwrap(), + old + ); + assert!(!store.reset_intent_path(&tenant).exists()); + assert_eq!( + store + .inner + .metadata + .handle(tenant) + .await + .get() + .shared_schema_name, + Some(a) + ); + } + + #[tokio::test(flavor = "current_thread")] + async fn failed_reset_journal_publication_releases_ephemeral_schema_pin() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("tenant"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let old = store.cleanup_directory_identity(&name).await.unwrap(); + std::fs::write( + tmp.path().join("namespace-reset-intents"), + b"not a directory", + ) + .unwrap(); + assert!(store + .reset(name.clone(), RestoreOption::Latest) + .await + .is_err()); + assert_eq!(store.cleanup_directory_identity(&name).await.unwrap(), old); + // The failed temporary journal write must not leave an invisible + // in-memory pin blocking otherwise valid config updates. + { + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute_batch("CREATE TABLE jobs (schema TEXT, finished BOOLEAN)") + .unwrap(); + } + let schema = NamespaceName::from("schema-b"); + store + .inner + .metadata + .handle(schema.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + let mut changed = DatabaseConfig::default(); + changed.shared_schema_name = Some(schema); + store + .inner + .metadata + .handle(name.clone()) + .await + .store(changed) + .await + .unwrap(); + assert!(!store.reset_intent_path(&name).exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn failed_reset_backup_preserves_old_inode_without_journal() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), true))).await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old data").unwrap(); + let old = store.cleanup_directory_identity(&name).await.unwrap(); + let reset = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.reset(name, RestoreOption::Latest).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + assert_eq!(store.cleanup_directory_identity(&name).await.unwrap(), old); + release.notify_one(); + assert!(timeout(Duration::from_secs(5), reset) + .await + .unwrap() + .unwrap() + .is_err()); + assert_eq!(store.cleanup_directory_identity(&name).await.unwrap(), old); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old data"); + assert!(!store.reset_intent_path(&name).exists()); + assert!(!store.reset_quarantine_path(&name).exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn stalled_reset_backup_keeps_old_inode_until_confirmed() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old data").unwrap(); + let reset = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.reset(name, RestoreOption::Latest).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old data"); + timeout( + Duration::from_secs(5), + store.create( + NamespaceName::from("unrelated"), + RestoreOption::Latest, + DatabaseConfig::default(), + ), + ) + .await + .unwrap() + .unwrap(); + let same_name = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.with(name, |ns| ns.path.clone()).await } + }); + tokio::task::yield_now().await; + assert!(!same_name.is_finished()); + release.notify_one(); + timeout(Duration::from_secs(5), reset) + .await + .unwrap() + .unwrap() + .unwrap(); + timeout(Duration::from_secs(5), same_name) + .await + .unwrap() + .unwrap() + .unwrap(); + assert!(!path.join("sentinel").exists()); + assert!(store.exists(&name).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn failed_backup_keeps_old_directory_and_rejects_same_name_retry() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), true))).await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"not backed up").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + release.notify_one(); + assert!(timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .is_err()); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"not backed up" + ); + assert!(store.exists(&name).await); + assert!(store + .create(name, RestoreOption::Latest, DatabaseConfig::default()) + .await + .is_err()); + } + + #[tokio::test(flavor = "current_thread")] + async fn shutdown_reports_failed_drain_while_backup_confirmation_is_stalled() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (_tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + let result = timeout( + Duration::from_secs(2), + store + .clone() + .shutdown_with_timeout(Duration::from_millis(40)), + ) + .await + .unwrap(); + assert!(matches!(result, Err(Error::Blocked(_)))); + release.notify_one(); + timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .unwrap(); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_backup_keeps_old_directory() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release, false))).await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"not confirmed").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + destroy.abort(); + assert!(destroy.await.unwrap_err().is_cancelled()); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"not confirmed" + ); + assert!(store.exists(&name).await); + assert!(store + .create(name, RestoreOption::Latest, DatabaseConfig::default()) + .await + .is_err()); + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_waiter_after_backup_confirmation_drains_owned_teardown() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let held_connection = store.inner.metadata.hold_connection_for_test().await; + let ready = Arc::new(Notify::new()); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + let ready = ready.clone(); + async move { + store + .destroy_with_commit_signal(name, false, Some(ready)) + .await + } + }); + timeout(Duration::from_secs(5), ready.notified()) + .await + .unwrap(); + destroy.abort(); + assert!(destroy.await.unwrap_err().is_cancelled()); + // The worker retains the name lock even after the caller exits. + let operation = store + .inner + .name_operations + .lock() + .unwrap() + .get(&name) + .and_then(Weak::upgrade) + .unwrap(); + assert!(timeout(Duration::from_millis(20), operation.lock()) + .await + .is_err()); + drop(held_connection); + timeout(Duration::from_secs(5), async { + loop { + if !store.exists(&name).await && !tmp.path().join("dbs/victim").exists() { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + timeout( + Duration::from_secs(5), + store.create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ), + ) + .await + .unwrap() + .unwrap(); + assert!(store.exists(&name).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn confirmed_destroy_rejects_replaced_inode_without_losing_metadata() { + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let (tmp, store) = + primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) + .await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old inode").unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + std::fs::rename(&path, tmp.path().join("original")).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("sentinel"), b"replacement").unwrap(); + release.notify_one(); + assert!(timeout(Duration::from_secs(5), destroy) + .await + .unwrap() + .unwrap() + .is_err()); + assert!(store.exists(&name).await); + assert_eq!( + std::fs::read(path.join("sentinel")).unwrap(), + b"replacement" + ); + assert_eq!( + std::fs::read(tmp.path().join("original/sentinel")).unwrap(), + b"old inode" + ); + } + + #[tokio::test(flavor = "current_thread")] + async fn sql_failure_after_intent_publication_preserves_original_directory() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"original").unwrap(); + let conn = store.inner.metadata.hold_connection_for_test().await; + conn.execute_batch("CREATE TRIGGER reject_delete BEFORE DELETE ON namespace_configs BEGIN SELECT RAISE(ABORT, 'injected failure'); END;") + .unwrap(); + let destroy = tokio::spawn({ + let store = store.clone(); + let name = name.clone(); + async move { store.destroy(name, false).await } + }); + // Prove the durable intent is published *before* entering SQL. This + // assertion distinguishes this protocol from the old trigger test. + tokio::time::timeout(Duration::from_secs(5), async { + while !store.destroy_intent_path(&name).exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + assert!(store.exists(&name).await); + drop(conn); + assert!(destroy.await.unwrap().is_err()); + assert!(!store.destroy_intent_path(&name).exists()); + assert!(store.exists(&name).await); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"original"); + } + + #[tokio::test(flavor = "current_thread")] + async fn restart_recovers_destroy_at_each_durable_boundary() { + for stage in 0..3 { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"old data").unwrap(); + store.evict_cached_namespace(&name).await; + let identity = store.cleanup_directory_identity(&name).await.unwrap(); + store.persist_destroy_intent(&name, identity).unwrap(); + if stage >= 1 { + tokio::task::spawn_blocking({ + let metadata = store.inner.metadata.clone(); + let name = name.clone(); + move || metadata.remove(name).unwrap() + }) + .await + .unwrap(); + } + if stage == 2 { + let identity = store.cleanup_directory_identity(&name).await.unwrap(); store - .destroy_with_commit_signal(name, false, Some(ready)) + .detach_owned_directory( + &name, + identity, + false, + Some(store.destroy_quarantine_path(&name)), + ) .await + .unwrap(); } - }); - timeout(Duration::from_secs(5), ready.notified()) + // Open a fresh SQLite-backed metastore as on process restart. + drop(store); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + crate::config::MetaStoreConfig { + allow_recover_from_fs: true, + ..Default::default() + }, + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) .await .unwrap(); - destroy.abort(); - assert!(destroy.await.unwrap_err().is_cancelled()); - // The worker retains the name lock even after the caller exits. - let operation = store - .inner - .name_operations - .lock() - .unwrap() - .get(&name) - .and_then(Weak::upgrade) - .unwrap(); - assert!(timeout(Duration::from_millis(20), operation.lock()) + let restarted = NamespaceStore::new( + false, + false, + 10, + metadata, + NamespaceConfigurators::empty(), + DatabaseKind::Primary, + tmp.path(), + ) .await - .is_err()); - drop(held_connection); - timeout(Duration::from_secs(5), async { - loop { - if !store.exists(&name).await && !tmp.path().join("dbs/victim").exists() { - break; - } - tokio::task::yield_now().await; + .unwrap(); + assert!(!restarted.destroy_intent_path(&name).exists()); + assert!(!restarted.destroy_quarantine_path(&name).exists()); + if stage == 0 { + assert!(restarted.exists(&name).await); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"old data"); + } else { + assert!(!restarted.exists(&name).await); + assert!(!path.exists()); } - }) - .await - .unwrap(); - timeout( - Duration::from_secs(5), - store.create( + } + } + + #[tokio::test(flavor = "current_thread")] + async fn unpublished_truncated_intent_is_discarded_without_hiding_live_row() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( name.clone(), RestoreOption::Latest, DatabaseConfig::default(), - ), + ) + .await + .unwrap(); + store.evict_cached_namespace(&name).await; + let path = tmp.path().join("dbs/victim"); + std::fs::write(path.join("sentinel"), b"original").unwrap(); + let root = store.destroy_intent_root(); + std::fs::create_dir_all(&root).unwrap(); + let temp = root.join(".tmp-interrupted-write"); + std::fs::write(&temp, b"destroy 123").unwrap(); + drop(store); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + Default::default(), + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, ) .await - .unwrap() .unwrap(); - assert!(store.exists(&name).await); + let restarted = NamespaceStore::new( + false, + false, + 10, + metadata, + NamespaceConfigurators::empty(), + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); + assert!(!temp.exists()); + assert!(restarted.exists(&name).await); + assert_eq!(std::fs::read(path.join("sentinel")).unwrap(), b"original"); } #[tokio::test(flavor = "current_thread")] - async fn confirmed_destroy_rejects_replaced_inode_without_losing_metadata() { - let entered = Arc::new(Notify::new()); - let release = Arc::new(Notify::new()); - let (tmp, store) = - primary_fixture_with_cleanup_gate(Some((entered.clone(), release.clone(), false))) - .await; + async fn incomplete_destroy_intent_prevents_reuse_until_recovered() { + let (tmp, store) = primary_fixture().await; let name = NamespaceName::from("victim"); - store + store.persist_destroy_intent(&name, None).unwrap(); + assert!(store.ensure_existing_directory(&name).await.is_err()); + assert!(store .create( name.clone(), RestoreOption::Latest, + DatabaseConfig::default() + ) + .await + .is_err()); + let restarted = NamespaceStore::new( + false, + false, + 10, + store.inner.metadata.clone(), + NamespaceConfigurators::empty(), + DatabaseKind::Primary, + tmp.path(), + ) + .await + .unwrap(); + assert!(!restarted.destroy_intent_path(&name).exists()); + } + + #[tokio::test(flavor = "current_thread")] + async fn pending_destroy_fences_fork_destination_and_reset() { + let (tmp, store) = primary_fixture().await; + let source = NamespaceName::from("source"); + let destination = NamespaceName::from("destination"); + store + .create( + source.clone(), + RestoreOption::Latest, DatabaseConfig::default(), ) .await .unwrap(); - let path = tmp.path().join("dbs/victim"); - std::fs::write(path.join("sentinel"), b"old inode").unwrap(); - let destroy = tokio::spawn({ - let store = store.clone(); - let name = name.clone(); - async move { store.destroy(name, false).await } - }); - timeout(Duration::from_secs(5), entered.notified()) + store.persist_destroy_intent(&destination, None).unwrap(); + assert!(store + .fork( + source.clone(), + destination.clone(), + DatabaseConfig::default(), + None + ) .await - .unwrap(); - std::fs::rename(&path, tmp.path().join("original")).unwrap(); - std::fs::create_dir(&path).unwrap(); - std::fs::write(path.join("sentinel"), b"replacement").unwrap(); - release.notify_one(); - assert!(timeout(Duration::from_secs(5), destroy) + .is_err()); + assert!(!store.exists(&destination).await); + assert!(!tmp.path().join("dbs/destination").exists()); + let identity = store.cleanup_directory_identity(&source).await.unwrap(); + store.persist_destroy_intent(&source, identity).unwrap(); + assert!(store + .reset(source.clone(), RestoreOption::Latest) .await - .unwrap() - .unwrap() .is_err()); + assert!(store.exists(&source).await); + } + + #[tokio::test(flavor = "current_thread")] + async fn row_with_missing_original_and_destroy_intent_fails_closed() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + store + .create( + name.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + store.evict_cached_namespace(&name).await; + let identity = store.cleanup_directory_identity(&name).await.unwrap(); + store.persist_destroy_intent(&name, identity).unwrap(); + std::fs::rename(tmp.path().join("dbs/victim"), tmp.path().join("saved")).unwrap(); + let result = NamespaceStore::new( + false, + false, + 10, + store.inner.metadata.clone(), + NamespaceConfigurators::empty(), + DatabaseKind::Primary, + tmp.path(), + ) + .await; + assert!(result.is_err()); + assert!(tmp.path().join("saved").is_dir()); assert!(store.exists(&name).await); - assert_eq!( - std::fs::read(path.join("sentinel")).unwrap(), - b"replacement" - ); - assert_eq!( - std::fs::read(tmp.path().join("original/sentinel")).unwrap(), - b"old inode" - ); } #[tokio::test(flavor = "current_thread")] @@ -1168,6 +2186,12 @@ pub struct NamespaceStoreInner { configurators: NamespaceConfigurators, db_kind: DatabaseKind, dbs_path: PathBuf, + // The short exact-name scan and rename remain synchronous while this + // lock is held. Naively awaiting spawn_blocking would release a cancelled + // caller's name lock while a late filesystem mutation is still running. + // Large directory scans can stall a current-thread executor; offloading + // them requires moving BOTH the name and identity guards to a detached, + // cancellation-independent worker (not just the filesystem syscall). fs_operations: Arc>, name_operations: StdMutex>>>, shutdown_signal: tokio::sync::watch::Sender, @@ -1206,72 +2230,598 @@ impl NamespaceStore { .time_to_idle(Duration::from_secs(86400)) .build(); - Ok(Self { - inner: Arc::new(NamespaceStoreInner { - store, - metadata, - allow_lazy_creation, - has_shutdown: AtomicBool::new(false), - snapshot_at_shutdown, - schema_locks: Default::default(), - broadcasters: Default::default(), - configurators, - db_kind, - dbs_path: base_path.join("dbs"), - fs_operations: Arc::new(tokio::sync::Mutex::new(())), - name_operations: StdMutex::new(HashMap::new()), - shutdown_signal: tokio::sync::watch::channel(false).0, - }), - }) + let this = Self { + inner: Arc::new(NamespaceStoreInner { + store, + metadata, + allow_lazy_creation, + has_shutdown: AtomicBool::new(false), + snapshot_at_shutdown, + schema_locks: Default::default(), + broadcasters: Default::default(), + configurators, + db_kind, + dbs_path: base_path.join("dbs"), + fs_operations: Arc::new(tokio::sync::Mutex::new(())), + name_operations: StdMutex::new(HashMap::new()), + shutdown_signal: tokio::sync::watch::channel(false).0, + }), + }; + this.recover_reset_intents().await?; + this.recover_destroy_intents().await?; + Ok(this) + } + + pub async fn exists(&self, namespace: &NamespaceName) -> bool { + self.inner.metadata.exists(namespace).await + } + + /// Test/embedding hook: force a cache miss without touching persisted + /// config or namespace files. Integration tests use this to exercise a + /// replica reset whose linked schema really is unloaded at handshake. + #[doc(hidden)] + pub async fn evict_cached_namespace(&self, namespace: &NamespaceName) { + self.inner.store.invalidate(namespace).await; + } + + // Weak entries prevent an unbounded registry. All operations acquire + // multiple names in lexical order (source before/destination as sorted), + // then the short filesystem identity lock if needed. + async fn lock_names(&self, names: &[NamespaceName]) -> crate::Result>> { + let mut names = names.to_vec(); + names.sort_by(|a, b| a.as_str().cmp(b.as_str())); + names.dedup(); + let locks = { + let mut registry = self.inner.name_operations.lock().unwrap(); + registry.retain(|_, lock| lock.strong_count() > 0); + names + .iter() + .map(|name| { + if let Some(lock) = registry.get(name).and_then(Weak::upgrade) { + lock + } else { + let lock = Arc::new(tokio::sync::Mutex::new(())); + registry.insert(name.clone(), Arc::downgrade(&lock)); + lock + } + }) + .collect::>() + }; + let mut guards = Vec::with_capacity(locks.len()); + for lock in locks { + guards.push(lock.lock_owned().await); + } + if self.inner.has_shutdown.load(Ordering::Relaxed) { + return Err(Error::NamespaceStoreShutdown); + } + for name in &names { + self.check_no_destroy_intent(name)?; + } + Ok(guards) + } + + fn directory_path(&self, namespace: &NamespaceName) -> PathBuf { + self.inner.dbs_path.join(namespace.as_str()) + } + + fn reset_intent_root(&self) -> PathBuf { + self.inner + .dbs_path + .parent() + .unwrap() + .join("namespace-reset-intents") + } + + fn reset_intent_path(&self, namespace: &NamespaceName) -> PathBuf { + self.reset_intent_root().join(Self::destroy_key(namespace)) + } + + fn reset_committed_path(&self, namespace: &NamespaceName) -> PathBuf { + self.reset_intent_root() + .join(format!("{}.committed", Self::destroy_key(namespace))) + } + + fn reset_quarantine_path(&self, namespace: &NamespaceName) -> PathBuf { + self.inner + .dbs_path + .parent() + .unwrap() + .join("namespace-teardown-quarantine") + .join(format!("reset-{}", Self::destroy_key(namespace))) + } + + fn destroy_intent_root(&self) -> PathBuf { + self.inner + .dbs_path + .parent() + .unwrap() + .join("namespace-destroy-intents") + } + + fn destroy_key(namespace: &NamespaceName) -> String { + namespace + .as_slice() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect() + } + + fn destroy_intent_path(&self, namespace: &NamespaceName) -> PathBuf { + self.destroy_intent_root() + .join(Self::destroy_key(namespace)) + } + + fn check_no_destroy_intent(&self, namespace: &NamespaceName) -> crate::Result<()> { + for path in [ + self.destroy_intent_path(namespace), + self.reset_intent_path(namespace), + self.reset_committed_path(namespace), + ] { + match std::fs::symlink_metadata(&path) { + Ok(_) => { + return Err(Error::InvalidPath(format!( + "unfinished namespace operation for `{namespace}` requires recovery" + ))) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + } + Ok(()) + } + + fn destroy_quarantine_path(&self, namespace: &NamespaceName) -> PathBuf { + self.inner + .dbs_path + .parent() + .unwrap() + .join("namespace-teardown-quarantine") + .join(format!("destroy-{}", Self::destroy_key(namespace))) + } + + // The intent must reach disk before SQLite commits deletion. Without it, + // restart could mistake the old directory for a new, unowned namespace. + fn persist_destroy_intent( + &self, + namespace: &NamespaceName, + expected: Option<(u64, u64)>, + ) -> crate::Result<()> { + let root = self.destroy_intent_root(); + std::fs::create_dir_all(&root)?; + if !std::fs::symlink_metadata(&root)?.file_type().is_dir() { + return Err(Error::InvalidPath(format!( + "unsafe destroy intent root {:?}", + root + ))); + } + let path = self.destroy_intent_path(namespace); + match std::fs::symlink_metadata(&path) { + Ok(_) => { + return Err(Error::InvalidPath(format!( + "destroy intent already exists: {:?}", + path + ))) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + // An incomplete temp file never authorizes metadata deletion. Publish + // only the fully written, synced record with an atomic rename. + let temp = root.join(format!(".tmp-{}", uuid::Uuid::new_v4())); + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temp)?; + let record = match expected { + Some((device, inode)) => format!("destroy {device} {inode}\n"), + None => "destroy none\n".to_owned(), + }; + file.write_all(record.as_bytes())?; + file.sync_all()?; + drop(file); + std::fs::rename(&temp, &path)?; + sync_directory(&root)?; + sync_directory(root.parent().unwrap())?; + Ok(()) + } + + fn clear_destroy_intent(&self, namespace: &NamespaceName) -> crate::Result<()> { + std::fs::remove_file(self.destroy_intent_path(namespace))?; + sync_directory(&self.destroy_intent_root())?; + Ok(()) } - pub async fn exists(&self, namespace: &NamespaceName) -> bool { - self.inner.metadata.exists(namespace).await + // Reset's pending record is immutable; a separate atomic commit marker + // decides which incarnation wins after a process crash. Both are published + // through a synced temporary file so a truncated record is never visible. + fn publish_reset_file(&self, path: &Path, content: &[u8]) -> crate::Result<()> { + let root = self.reset_intent_root(); + std::fs::create_dir_all(&root)?; + if !std::fs::symlink_metadata(&root)?.file_type().is_dir() { + return Err(Error::InvalidPath(format!( + "unsafe reset intent root {:?}", + root + ))); + } + match std::fs::symlink_metadata(path) { + Ok(_) => { + return Err(Error::InvalidPath(format!( + "reset intent already exists: {:?}", + path + ))) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + let temp = root.join(format!(".tmp-{}", uuid::Uuid::new_v4())); + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temp)?; + file.write_all(content)?; + file.sync_all()?; + drop(file); + std::fs::rename(&temp, path)?; + sync_directory(&root)?; + sync_directory(root.parent().unwrap())?; + Ok(()) } - /// Test/embedding hook: force a cache miss without touching persisted - /// config or namespace files. Integration tests use this to exercise a - /// replica reset whose linked schema really is unloaded at handshake. - #[doc(hidden)] - pub async fn evict_cached_namespace(&self, namespace: &NamespaceName) { - self.inner.store.invalidate(namespace).await; + fn read_reset_commit(&self, namespace: &NamespaceName) -> crate::Result> { + let path = self.reset_committed_path(namespace); + let metadata = match std::fs::symlink_metadata(&path) { + Ok(metadata) => metadata, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(e.into()), + }; + if !metadata.file_type().is_file() { + return Err(Error::InvalidPath(format!( + "invalid reset commit marker {:?}", + path + ))); + } + let content = std::fs::read_to_string(&path)?; + let parts = content + .strip_suffix('\n') + .unwrap_or("") + .split(' ') + .collect::>(); + if parts.len() != 3 || parts[0] != "committed" { + return Err(Error::InvalidPath(format!( + "invalid reset commit marker {:?}", + path + ))); + } + Ok(Some(( + parts[1] + .parse() + .map_err(|_| Error::InvalidPath("invalid reset commit inode".into()))?, + parts[2] + .parse() + .map_err(|_| Error::InvalidPath("invalid reset commit inode".into()))?, + ))) + } + + fn clear_reset_intent(&self, namespace: &NamespaceName) -> crate::Result<()> { + let root = self.reset_intent_root(); + std::fs::remove_file(self.reset_intent_path(namespace))?; + sync_directory(&root)?; + match std::fs::remove_file(self.reset_committed_path(namespace)) { + Ok(()) => sync_directory(&root)?, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + Ok(()) } - // Weak entries prevent an unbounded registry. All operations acquire - // multiple names in lexical order (source before/destination as sorted), - // then the short filesystem identity lock if needed. - async fn lock_names(&self, names: &[NamespaceName]) -> crate::Result>> { - let mut names = names.to_vec(); - names.sort_by(|a, b| a.as_str().cmp(b.as_str())); - names.dedup(); - let locks = { - let mut registry = self.inner.name_operations.lock().unwrap(); - registry.retain(|_, lock| lock.strong_count() > 0); - names - .iter() - .map(|name| { - if let Some(lock) = registry.get(name).and_then(Weak::upgrade) { - lock - } else { - let lock = Arc::new(tokio::sync::Mutex::new(())); - registry.insert(name.clone(), Arc::downgrade(&lock)); - lock + async fn recover_reset_intents(&self) -> crate::Result<()> { + let root = self.reset_intent_root(); + match std::fs::symlink_metadata(&root) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e.into()), + Ok(metadata) if !metadata.file_type().is_dir() => { + return Err(Error::InvalidPath(format!( + "unsafe reset intent root {:?}", + root + ))) + } + Ok(_) => {} + } + for entry in std::fs::read_dir(&root)? { + let entry = entry?; + let key = entry + .file_name() + .into_string() + .map_err(|_| Error::InvalidPath("invalid reset intent name".into()))?; + if key.starts_with(".tmp-") { + if !entry.file_type()?.is_file() { + return Err(Error::InvalidPath(format!( + "unsafe reset temp {:?}", + entry.path() + ))); + } + std::fs::remove_file(entry.path())?; + sync_directory(&root)?; + continue; + } + if key.ends_with(".committed") { + continue; + } + if key.is_empty() + || key.len() % 2 != 0 + || !key.is_ascii() + || !entry.file_type()?.is_file() + { + return Err(Error::InvalidPath(format!( + "invalid reset intent {:?}", + entry.path() + ))); + } + let bytes = (0..key.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&key[i..i + 2], 16)) + .collect::, _>>() + .map_err(|_| { + Error::InvalidPath(format!("invalid reset intent {:?}", entry.path())) + })?; + let namespace = NamespaceName::from_bytes(bytes.into())?; + if key != Self::destroy_key(&namespace) || !self.inner.metadata.exists(&namespace).await + { + return Err(Error::InvalidPath(format!( + "reset of `{namespace}` has no persisted row" + ))); + } + let intent: ResetIntent = serde_json::from_slice(&std::fs::read(entry.path())?) + .map_err(|e| Error::InvalidPath(format!("invalid reset intent: {e}")))?; + let old = self.reset_quarantine_path(&namespace); + let old_at_quarantine = match std::fs::symlink_metadata(&old) { + Ok(meta) => { + if directory_identity(&meta) != Some(intent.old_identity) { + return Err(Error::InvalidPath(format!( + "reset old inode changed for `{namespace}`" + ))); } + true + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => false, + Err(e) => return Err(e.into()), + }; + let committed = self.read_reset_commit(&namespace)?; + if let Some(new_identity) = committed { + let replacement = self.cleanup_directory_identity(&namespace).await?; + if replacement != Some(new_identity) || replacement == Some(intent.old_identity) { + return Err(Error::InvalidPath(format!( + "missing committed reset directory for `{namespace}`" + ))); + } + tokio::task::spawn_blocking({ + let metadata = self.inner.metadata.clone(); + let namespace = namespace.clone(); + move || metadata.release_reset_pin(&namespace) }) - .collect::>() - }; - let mut guards = Vec::with_capacity(locks.len()); - for lock in locks { - guards.push(lock.lock_owned().await); + .await?; + if old_at_quarantine { + Self::remove_detached_directory(Some(old.clone())).await?; + sync_directory(old.parent().unwrap())?; + } + } else if old_at_quarantine { + // Only process restart can guarantee all canceled setup and + // blocking/path-open workers are gone. Retain partial new data + // for inspection; never recursively delete by namespace path. + let _identity = self.inner.fs_operations.lock().await; + self.check_existing_directory(&namespace).await?; + let path = self.directory_path(&namespace); + match std::fs::symlink_metadata(&path) { + Ok(meta) => { + if directory_identity(&meta).is_none() { + return Err(Error::InvalidPath(format!( + "unsafe reset replacement for `{namespace}`" + ))); + } + let abandoned_root = self + .inner + .dbs_path + .parent() + .unwrap() + .join("namespace-reset-abandoned"); + std::fs::create_dir_all(&abandoned_root)?; + if !std::fs::symlink_metadata(&abandoned_root)? + .file_type() + .is_dir() + { + return Err(Error::InvalidPath(format!( + "unsafe abandoned reset root {:?}", + abandoned_root + ))); + } + let abandoned = abandoned_root.join(uuid::Uuid::new_v4().to_string()); + std::fs::rename(&path, &abandoned)?; + NAMESPACE_QUARANTINE_COUNT.increment(1); + sync_directory(&self.inner.dbs_path)?; + sync_directory(&abandoned_root)?; + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + std::fs::rename(&old, &path)?; + sync_directory(&self.inner.dbs_path)?; + sync_directory(old.parent().unwrap())?; + } else if self.cleanup_directory_identity(&namespace).await? + != Some(intent.old_identity) + { + return Err(Error::InvalidPath(format!( + "pending reset lost old directory for `{namespace}`" + ))); + } + if committed.is_none() { + tokio::task::spawn_blocking({ + let metadata = self.inner.metadata.clone(); + let namespace = namespace.clone(); + move || metadata.restore_reset_config(&namespace, &intent.old_config) + }) + .await??; + } + self.clear_reset_intent(&namespace)?; } - if self.inner.has_shutdown.load(Ordering::Relaxed) { - return Err(Error::NamespaceStoreShutdown); + // A crash after deleting the pending record, before deleting the + // commit marker, leaves only a harmless committed marker. + for entry in std::fs::read_dir(&root)? { + let entry = entry?; + let Some(key) = entry.file_name().to_str().map(str::to_owned) else { + return Err(Error::InvalidPath("invalid reset marker name".into())); + }; + let Some(hex) = key.strip_suffix(".committed") else { + continue; + }; + if hex.len() % 2 != 0 || hex.is_empty() || !hex.is_ascii() { + return Err(Error::InvalidPath(format!( + "invalid reset commit marker {:?}", + entry.path() + ))); + } + let bytes = (0..hex.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&hex[i..i + 2], 16)) + .collect::, _>>() + .map_err(|_| Error::InvalidPath("invalid reset commit marker".into()))?; + let namespace = NamespaceName::from_bytes(bytes.into())?; + if hex != Self::destroy_key(&namespace) + || !entry.file_type()?.is_file() + || !self.inner.metadata.exists(&namespace).await + || self.cleanup_directory_identity(&namespace).await? + != self.read_reset_commit(&namespace)? + || std::fs::symlink_metadata(self.reset_quarantine_path(&namespace)).is_ok() + { + return Err(Error::InvalidPath(format!( + "orphan reset marker for `{namespace}`" + ))); + } + std::fs::remove_file(entry.path())?; + sync_directory(&root)?; } - Ok(guards) + Ok(()) } - fn directory_path(&self, namespace: &NamespaceName) -> PathBuf { - self.inner.dbs_path.join(namespace.as_str()) + async fn recover_destroy_intents(&self) -> crate::Result<()> { + let root = self.destroy_intent_root(); + match std::fs::symlink_metadata(&root) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e.into()), + Ok(metadata) if !metadata.file_type().is_dir() => { + return Err(Error::InvalidPath(format!( + "unsafe destroy intent root {:?}", + root + ))) + } + Ok(_) => {} + } + for entry in std::fs::read_dir(&root)? { + let entry = entry?; + let key = entry + .file_name() + .into_string() + .map_err(|_| Error::InvalidPath("invalid destroy intent name".into()))?; + if key.starts_with(".tmp-") { + // The metadata DELETE is never attempted before publication. + // Truncated/unpublished files cannot be treated as intents. + if !entry.file_type()?.is_file() { + return Err(Error::InvalidPath(format!( + "unsafe destroy temp {:?}", + entry.path() + ))); + } + std::fs::remove_file(entry.path())?; + sync_directory(&root)?; + continue; + } + if key.len() % 2 != 0 || key.is_empty() || !key.is_ascii() { + return Err(Error::InvalidPath(format!( + "invalid destroy intent {:?}", + entry.path() + ))); + } + let bytes = (0..key.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&key[i..i + 2], 16)) + .collect::, _>>() + .map_err(|_| { + Error::InvalidPath(format!("invalid destroy intent {:?}", entry.path())) + })?; + let namespace = NamespaceName::from_bytes(bytes.into())?; + if key != Self::destroy_key(&namespace) || !entry.file_type()?.is_file() { + return Err(Error::InvalidPath(format!( + "invalid destroy intent {:?}", + entry.path() + ))); + } + let record = std::fs::read_to_string(entry.path())?; + let expected = if record == "destroy none\n" { + None + } else { + let parts = record.trim_end_matches('\n').split(' ').collect::>(); + if parts.len() != 3 || parts[0] != "destroy" || !record.ends_with('\n') { + return Err(Error::InvalidPath(format!( + "invalid destroy intent {:?}", + entry.path() + ))); + } + Some(( + parts[1] + .parse::() + .map_err(|_| Error::InvalidPath("invalid destroy identity".into()))?, + parts[2] + .parse::() + .map_err(|_| Error::InvalidPath("invalid destroy identity".into()))?, + )) + }; + let quarantine = self.destroy_quarantine_path(&namespace); + if self.inner.metadata.exists(&namespace).await { + // Commit did not happen. Do not discard the old database or + // allow a missing directory to be lazily replaced with a blank one. + if std::fs::symlink_metadata(&quarantine).is_ok() + || self.cleanup_directory_identity(&namespace).await? != expected + || expected.is_none() + { + // In particular, never let the live row reopen a blank + // directory if the original files went missing. + return Err(Error::InvalidPath(format!( + "incomplete destroy of `{namespace}` requires repair" + ))); + } + self.check_existing_directory(&namespace).await?; + } else { + let actual = self.cleanup_directory_identity(&namespace).await?; + if actual.is_some() { + if actual != expected { + return Err(Error::InvalidPath(format!( + "destroy of `{namespace}` found a replaced directory" + ))); + } + let (detached, _) = self + .detach_owned_directory( + &namespace, + expected, + false, + Some(quarantine.clone()), + ) + .await?; + Self::remove_detached_directory(detached).await?; + sync_directory(quarantine.parent().unwrap())?; + } else if std::fs::symlink_metadata(&quarantine).is_ok() { + let metadata = std::fs::symlink_metadata(&quarantine)?; + if directory_identity(&metadata) != expected { + return Err(Error::InvalidPath(format!( + "replaced destroy quarantine {:?}", + quarantine + ))); + } + Self::remove_detached_directory(Some(quarantine.clone())).await?; + sync_directory(quarantine.parent().unwrap())?; + } + } + self.clear_destroy_intent(&namespace)?; + } + Ok(()) } // Lookup by path alone is insufficient on case-insensitive/normalizing @@ -1303,6 +2853,7 @@ impl NamespaceStore { namespace: &NamespaceName, ) -> crate::Result { tokio::fs::create_dir_all(&self.inner.dbs_path).await?; + self.check_no_destroy_intent(namespace)?; let path = self.directory_path(namespace); match tokio::fs::create_dir(&path).await { Ok(()) => { @@ -1330,13 +2881,13 @@ impl NamespaceStore { // Also serialize restoration of a missing persisted directory with // deletion of a legacy filesystem alias, which may have another key. let _identity = self.inner.fs_operations.lock().await; + // The exact-name scan rejects aliases and symlinks. No store operation + // can replace the entry before mkdir while this lock is held; another + // scan on AlreadyExists would repeat the same directory traversal. self.check_existing_directory(namespace).await?; match self.reserve_directory(namespace).await { Ok(reservation) => Ok(Some(reservation)), - Err(Error::NamespaceAlreadyExist(_)) => { - self.check_existing_directory(namespace).await?; - Ok(None) - } + Err(Error::NamespaceAlreadyExist(_)) => Ok(None), Err(e) => Err(e), } } @@ -1393,6 +2944,9 @@ impl NamespaceStore { } let quarantine = quarantine_root.join(uuid::Uuid::new_v4().to_string()); std::fs::create_dir(&quarantine)?; + // Count the newly retained directory before any fallible read/rename: + // partial moves and an empty quarantine still require inspection. + NAMESPACE_QUARANTINE_COUNT.increment(1); let entries = std::fs::read_dir(&path)? .map(|entry| entry.map(|entry| (entry.path(), entry.file_name()))) .collect::>>()?; @@ -1436,6 +2990,7 @@ impl NamespaceStore { namespace: &NamespaceName, expected: Option<(u64, u64)>, replace: bool, + destroy_target: Option, ) -> crate::Result<(Option, Option)> { let _identity = self.inner.fs_operations.lock().await; self.check_existing_directory(namespace).await?; @@ -1450,6 +3005,7 @@ impl NamespaceStore { "namespace `{namespace}` directory changed during cleanup; refusing to remove it" ))); } + let durable_destroy = destroy_target.is_some(); let detached = if actual.is_some() { let root = self .inner @@ -1465,8 +3021,19 @@ impl NamespaceStore { root ))); } - let target = root.join(uuid::Uuid::new_v4().to_string()); + let target = + destroy_target.unwrap_or_else(|| root.join(uuid::Uuid::new_v4().to_string())); + if std::fs::symlink_metadata(&target).is_ok() { + return Err(Error::InvalidPath(format!( + "namespace teardown target already exists: {:?}", + target + ))); + } std::fs::rename(&path, &target)?; + if durable_destroy { + sync_directory(&self.inner.dbs_path)?; + sync_directory(&root)?; + } Some(target) } else { None @@ -1503,7 +3070,7 @@ impl NamespaceStore { async fn remove_detached_directory(path: Option) -> crate::Result<()> { if let Some(path) = path { - tokio::fs::remove_dir_all(path).await?; + tokio::fs::remove_dir_all(&path).await?; } Ok(()) } @@ -1563,18 +3130,79 @@ impl NamespaceStore { "namespace `{namespace}` directory changed before confirmed teardown" ))); } + // Persist the intent before the database transaction. Recovery + // rolls it back if the row survived, and finishes teardown if not. + tokio::task::spawn_blocking({ + let store = store.clone(); + let name = namespace.clone(); + move || store.persist_destroy_intent(&name, expected) + }) + .await??; let metadata = store.inner.metadata.clone(); let name = namespace.clone(); - tokio::task::spawn_blocking(move || { + let removed = tokio::task::spawn_blocking(move || { metadata .remove_if_generation(name.clone(), Some(&generation))? .ok_or_else(|| Error::NamespaceDoesntExist(name.to_string())) }) - .await??; + .await?; + if let Err(e) = removed { + // A commit error can be ambiguous. The in-memory watch map + // may still contain a row that SQLite has already deleted. + // Check the committed SQL state before discarding the intent; + // on any uncertainty leave it to startup reconciliation. + let persisted = tokio::task::spawn_blocking({ + let metadata = store.inner.metadata.clone(); + let name = namespace.clone(); + move || metadata.persisted_namespace_exists(&name) + }) + .await; + match persisted { + Ok(Ok(true)) + if expected.is_some() + && store.cleanup_directory_identity(&namespace).await? == expected => + { + tokio::task::spawn_blocking({ + let store = store.clone(); + let name = namespace.clone(); + move || store.clear_destroy_intent(&name) + }) + .await??; + } + Ok(Err(ref check_error)) => tracing::warn!( + "retaining destroy intent after failed persisted-row check: {check_error}" + ), + Err(ref check_error) => tracing::warn!( + "retaining destroy intent after failed persisted-row worker: {check_error}" + ), + _ => {} + } + return Err(e); + } let (detached, _) = store - .detach_owned_directory(&namespace, expected, false) + .detach_owned_directory( + &namespace, + expected, + false, + Some(store.destroy_quarantine_path(&namespace)), + ) .await?; Self::remove_detached_directory(detached).await?; + // Durable completion of quarantine removal precedes clearing the + // intent, so restart cannot lose the record for stranded files. + let quarantine_root = store.destroy_quarantine_path(&namespace); + if expected.is_some() { + tokio::task::spawn_blocking(move || { + sync_directory(quarantine_root.parent().unwrap()) + }) + .await??; + } + tokio::task::spawn_blocking({ + let store = store.clone(); + let name = namespace.clone(); + move || store.clear_destroy_intent(&name) + }) + .await??; tracing::info!("destroyed namespace: {namespace}"); Ok(()) }); @@ -1597,18 +3225,54 @@ impl NamespaceStore { Ok(()) } + async fn release_unpublished_reset_pin(&self, namespace: &NamespaceName) { + // A sync failure after atomic publication leaves the valid final + // intent in place; keep the pin and fail closed. A failed temp write + // left no intent and must not constrain ordinary future updates. + if matches!(std::fs::symlink_metadata(self.reset_intent_path(namespace)), + Err(ref e) if e.kind() == std::io::ErrorKind::NotFound) + { + let metadata = self.inner.metadata.clone(); + let name = namespace.clone(); + let _ = tokio::task::spawn_blocking(move || metadata.release_reset_pin(&name)).await; + } + } + pub async fn reset( &self, namespace: NamespaceName, restore_option: RestoreOption, ) -> anyhow::Result<()> { - let _name_operation = self.lock_names(&[namespace.clone()]).await?; - let expected = self.cleanup_directory_identity(&namespace).await?; - // The process for resetting is as follows: - // - get a lock on the namespace entry, if the entry exists, then it's a lock on the entry, - // if it doesn't exist, insert an empty entry and take a lock on it - // - destroy the old namespace - // - create a new namespace and insert it in the held lock + let operation = self.lock_names(&[namespace.clone()]).await?; + // The task owns the name lock before the request can be cancelled. + // Never cancel setup after detaching the old inode: blocking/path-open + // work can survive cancellation and corrupt a live rollback by name. + let store = self.clone(); + tokio::spawn(async move { + store + .reset_owned(namespace, restore_option, operation) + .await + }) + .await? + } + + async fn reset_owned( + &self, + namespace: NamespaceName, + restore_option: RestoreOption, + _operation: Vec>, + ) -> anyhow::Result<()> { + if !self.inner.metadata.exists(&namespace).await { + return Err(Error::NamespaceDoesntExist(namespace.to_string()).into()); + } + let old_identity = self + .cleanup_directory_identity(&namespace) + .await? + .ok_or_else(|| { + Error::InvalidPath(format!( + "reset of `{namespace}` needs an existing directory" + )) + })?; let entry = self .inner .store @@ -1618,37 +3282,158 @@ impl NamespaceStore { if let Some(ns) = lock.take() { ns.destroy().await?; } - - let db_config = self.inner.metadata.handle(namespace.clone()).await; - // Confirm remote backups before detaching the old local inode. - self.prepare_cleanup( - &namespace, - &db_config.get(), - false, - NamespaceBottomlessDbIdInit::FetchFromConfig, - ) - .await?; - let (detached, reservation) = self - .detach_owned_directory(&namespace, expected, true) + // Revoke old handles and pin the original schema membership while + // taking the SQL snapshot. A pending reset cannot switch schemas: + // shared_schema_links is also the migration task worklist. + let pinned = tokio::task::spawn_blocking({ + let metadata = self.inner.metadata.clone(); + let name = namespace.clone(); + move || metadata.pin_reset_and_snapshot(&name) + }) + .await; + let old_config = match pinned { + Ok(Ok(bytes)) => bytes, + Ok(Err(e)) => return Err(e.into()), + Err(e) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + }; + // Registration takes the exclusive schema lock. Hold this shared + // guard across the worker; after an error the journal fences future + // registrations without starving the migration scheduler. + // Use the exact committed snapshot, never the watch (which can lag a + // SQL commit between the config worker's DB and watch updates). + let old_schema = MetaStore::reset_snapshot_schema_lock(&namespace, &old_config)?; + let _schema_guard = if let Some(schema) = old_schema { + let guard = self.inner.schema_locks.acquire_shared(schema.clone()).await; + let pending = tokio::task::spawn_blocking({ + let metadata = self.inner.metadata.clone(); + let schema = schema.clone(); + move || metadata.schema_has_pending_jobs(&schema) + }) + .await; + match pending { + Ok(Ok(true)) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(Error::PendingMigrationOnSchema(schema).into()); + } + Ok(Ok(false)) => Some(guard), + Ok(Err(e)) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + Err(e) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + } + } else { + None + }; + // Backup confirmation must use the same persisted config that will + // be journaled; a lagging watch can have different backup/schema IDs. + let pinned_config = self.inner.metadata.handle(namespace.clone()).await.get(); + if let Err(e) = self + .prepare_cleanup( + &namespace, + &pinned_config, + false, + NamespaceBottomlessDbIdInit::FetchFromConfig, + ) + .await + { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + let intent = ResetIntent { + old_identity, + old_config, + }; + let bytes = match serde_json::to_vec(&intent) { + Ok(bytes) => bytes, + Err(e) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + }; + let published = tokio::task::spawn_blocking({ + let store = self.clone(); + let name = namespace.clone(); + move || store.publish_reset_file(&store.reset_intent_path(&name), &bytes) + }) + .await; + match published { + Ok(Ok(())) => {} + Ok(Err(e)) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + Err(e) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e.into()); + } + } + let (old, reservation) = self + .detach_owned_directory( + &namespace, + Some(old_identity), + true, + Some(self.reset_quarantine_path(&namespace)), + ) .await?; - let mut reservation = reservation.expect("reset reserves a replacement directory"); - Self::remove_detached_directory(detached).await?; - // Reset keeps the same stored row but owns a new on-disk incarnation. - // Old handles/handshakes must not overwrite its config after reset. - self.inner.metadata.activate_for_create(&namespace); - let db_config = self.inner.metadata.handle(namespace.clone()).await; - // Replica handshake may load an uncached shared schema. Never retain - // the global identity lock over setup or a callback into this store. - let mut shutdown = self.inner.shutdown_signal.subscribe(); - let ns = tokio::select! { - biased; - _ = shutdown.wait_for(|requested| *requested) => return Err(Error::NamespaceStoreShutdown.into()), - result = self.make_namespace(&namespace, db_config, restore_option) => result?, + let mut reservation = reservation.expect("reset reserves replacement directory"); + let fresh = self.inner.metadata.handle(namespace.clone()).await; + let ns = match self.make_namespace(&namespace, fresh, restore_option).await { + Ok(ns) => ns, + Err(e) => { + // No live rollback: an already queued path-open/blocking setup + // write could target the old inode after rename-back. Leave the + // intent and old data fenced until all workers die on restart. + tracing::error!( + "reset setup failed for `{namespace}`; old data retained until restart: {e}" + ); + NAMESPACE_QUARANTINE_COUNT.increment(1); + reservation.disarm(); + return Err(e.into()); + } }; - - lock.replace(ns); + // Committing the marker chooses the new incarnation on restart. The + // original cannot be deleted before this publication is durable. + let new_identity = reservation.identity.ok_or_else(|| { + Error::InvalidPath(format!( + "reset of `{namespace}` has no new directory identity" + )) + })?; + if self.cleanup_directory_identity(&namespace).await? != Some(new_identity) { + return Err(Error::InvalidPath(format!( + "reset of `{namespace}` replaced its new directory" + )) + .into()); + } + tokio::task::spawn_blocking({ + let store = self.clone(); + let name = namespace.clone(); + let marker = format!("committed {} {}\n", new_identity.0, new_identity.1); + move || store.publish_reset_file(&store.reset_committed_path(&name), marker.as_bytes()) + }) + .await??; reservation.disarm(); - + lock.replace(ns); + tokio::task::spawn_blocking({ + let metadata = self.inner.metadata.clone(); + let name = namespace.clone(); + move || metadata.release_reset_pin(&name) + }) + .await?; + Self::remove_detached_directory(old).await?; + let root = self.reset_quarantine_path(&namespace); + tokio::task::spawn_blocking(move || sync_directory(root.parent().unwrap())).await??; + tokio::task::spawn_blocking({ + let store = self.clone(); + move || store.clear_reset_intent(&namespace) + }) + .await??; Ok(()) } @@ -2205,6 +3990,54 @@ impl NamespaceStore { &self.inner.schema_locks } + /// Called under the scheduler's exclusive schema lock before registration. + /// An in-progress reset holds the shared lock; a failed reset leaves an + /// intent that prevents enqueuing a task targeting its fenced namespace. + pub(crate) async fn ensure_schema_has_no_pending_resets( + &self, + schema: &NamespaceName, + ) -> crate::Result<()> { + // Also fence reset of the schema namespace itself, not just tenants + // linked to it. The caller holds this schema's exclusive lock. + for path in [ + self.reset_intent_path(schema), + self.reset_committed_path(schema), + ] { + match std::fs::symlink_metadata(&path) { + Ok(_) => { + return Err(Error::InvalidPath(format!( + "migration on `{schema}` blocked by its own pending reset" + ))) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + } + let namespaces = tokio::task::spawn_blocking({ + let metadata = self.inner.metadata.clone(); + let schema = schema.clone(); + move || metadata.linked_namespaces(&schema) + }) + .await??; + for namespace in namespaces { + for path in [ + self.reset_intent_path(&namespace), + self.reset_committed_path(&namespace), + ] { + match std::fs::symlink_metadata(&path) { + Ok(_) => { + return Err(Error::InvalidPath(format!( + "migration on `{schema}` blocked by pending reset of `{namespace}`" + ))) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + } + } + Ok(()) + } + fn get_configurator(&self, db_config: &DatabaseConfig) -> &DynConfigurator { match self.inner.db_kind { DatabaseKind::Primary if db_config.is_shared_schema => { diff --git a/libsql-server/src/schema/scheduler.rs b/libsql-server/src/schema/scheduler.rs index 82df9b1ce6..a07335f7d6 100644 --- a/libsql-server/src/schema/scheduler.rs +++ b/libsql-server/src/schema/scheduler.rs @@ -418,6 +418,10 @@ impl Scheduler { .schema_locks() .acquire_exlusive(schema.clone()) .await; + self.namespace_store + .ensure_schema_has_no_pending_resets(&schema) + .await + .map_err(|e| Error::Registration(Box::new(e)))?; with_conn_async(self.migration_db.clone(), move |conn| { register_schema_migration_job(conn, &schema, &migration) }) From 09e4e6aa0030e3ed31ea4fb6abb3190d7b37275a Mon Sep 17 00:00:00 2001 From: Raphael Lopes Feitoza Date: Thu, 1 Oct 2026 13:42:35 -0400 Subject: [PATCH 11/11] Skip primary migration job preflight on replica reset --- libsql-server/src/namespace/store.rs | 85 +++++++++++++++++++++++----- libsql-server/tests/cluster/mod.rs | 3 + 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/libsql-server/src/namespace/store.rs b/libsql-server/src/namespace/store.rs index bb42077b96..91da8e9924 100644 --- a/libsql-server/src/namespace/store.rs +++ b/libsql-server/src/namespace/store.rs @@ -1078,6 +1078,58 @@ mod directory_tests { .is_ok()); } + #[tokio::test(flavor = "current_thread")] + async fn replica_linked_reset_preflight_skips_missing_scheduler_jobs_table() { + let tmp = tempfile::tempdir().unwrap(); + let (maker, wal) = metastore_connection_maker(None, tmp.path()).await.unwrap(); + let metadata = MetaStore::new( + Default::default(), + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Replica, + ) + .await + .unwrap(); + let schema = NamespaceName::from("schema"); + metadata + .handle(schema.clone()) + .await + .store(DatabaseConfig::default()) + .await + .unwrap(); + let mut linked = DatabaseConfig::default(); + linked.shared_schema_name = Some(schema.clone()); + metadata + .handle(NamespaceName::from("tenant")) + .await + .store(linked) + .await + .unwrap(); + // A replica has neither scheduler nor jobs table. The old unguarded + // preflight returned a SQLite "no such table: jobs" error here. + let missing = tokio::task::spawn_blocking({ + let metadata = metadata.clone(); + let schema = schema.clone(); + move || metadata.schema_has_pending_jobs(&schema) + }) + .await + .unwrap(); + assert!(missing.is_err()); + let store = NamespaceStore::new( + false, + false, + 10, + metadata, + NamespaceConfigurators::empty(), + DatabaseKind::Replica, + tmp.path(), + ) + .await + .unwrap(); + assert!(!store.reset_migration_job_check(&schema).await.unwrap()); + } + #[tokio::test(flavor = "current_thread")] async fn reset_refuses_pending_schema_migration_before_detaching_old_data() { let (_tmp, store) = primary_fixture().await; @@ -3256,6 +3308,21 @@ impl NamespaceStore { .await? } + async fn reset_migration_job_check(&self, schema: &NamespaceName) -> anyhow::Result { + // Only primaries run the migration scheduler/create its jobs table. + // Replicas still take the shared schema lock and retain the reset + // intent, but must not query a table that does not exist locally. + if !self.inner.db_kind.is_primary() { + return Ok(false); + } + let metadata = self.inner.metadata.clone(); + let schema = schema.clone(); + Ok( + tokio::task::spawn_blocking(move || metadata.schema_has_pending_jobs(&schema)) + .await??, + ) + } + async fn reset_owned( &self, namespace: NamespaceName, @@ -3307,25 +3374,15 @@ impl NamespaceStore { let old_schema = MetaStore::reset_snapshot_schema_lock(&namespace, &old_config)?; let _schema_guard = if let Some(schema) = old_schema { let guard = self.inner.schema_locks.acquire_shared(schema.clone()).await; - let pending = tokio::task::spawn_blocking({ - let metadata = self.inner.metadata.clone(); - let schema = schema.clone(); - move || metadata.schema_has_pending_jobs(&schema) - }) - .await; - match pending { - Ok(Ok(true)) => { + match self.reset_migration_job_check(&schema).await { + Ok(true) => { self.release_unpublished_reset_pin(&namespace).await; return Err(Error::PendingMigrationOnSchema(schema).into()); } - Ok(Ok(false)) => Some(guard), - Ok(Err(e)) => { - self.release_unpublished_reset_pin(&namespace).await; - return Err(e.into()); - } + Ok(false) => Some(guard), Err(e) => { self.release_unpublished_reset_pin(&namespace).await; - return Err(e.into()); + return Err(e); } } } else { diff --git a/libsql-server/tests/cluster/mod.rs b/libsql-server/tests/cluster/mod.rs index 3f4271036e..b3166956aa 100644 --- a/libsql-server/tests/cluster/mod.rs +++ b/libsql-server/tests/cluster/mod.rs @@ -477,6 +477,9 @@ fn replica_reset_loads_uncached_shared_schema_without_identity_lock_deadlock() { .connect()? .execute("insert into test values (19)", ()) .await?; + // The replica has no schema scheduler `jobs` table. This real + // linked-tenant reset also guards against querying that primary- + // only table during the replica reset preflight. // Recreating the primary is not a deterministic trigger for a // cached replica's existing long-lived frame stream. Ask the // replica host itself to run the real NamespaceStore::reset.