diff --git a/README.md b/README.md index a94930b0df..da827825b0 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 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. ## Documentation 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/docs/ADMIN_API.md b/docs/ADMIN_API.md index d5a7fe2478..303bfef7e8 100644 --- a/docs/ADMIN_API.md +++ b/docs/ADMIN_API.md @@ -9,13 +9,91 @@ 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. + +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/`; 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 ```HTTP 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-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-server/src/admin_shell.rs b/libsql-server/src/admin_shell.rs index e97b272d72..117ae5bfed 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()) @@ -107,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>>; @@ -115,20 +168,9 @@ 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())?; - match self.with_namespace(ns_bytes, request.into_inner()).await { + 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/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/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/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 70b419ebe9..7390505ab9 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,13 @@ 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>>, + // 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, @@ -190,6 +203,8 @@ impl MetaStoreInner { let mut this = MetaStoreInner { configs: Default::default(), + generations: Default::default(), + reset_pins: Default::default(), conn: conn.into(), wal_manager, db_kind, @@ -220,15 +235,53 @@ 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; + } + }; + // 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 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,27 +318,32 @@ 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 config = match metadata::DatabaseConfig::decode(&v[..]) { - Ok(c) => Arc::new(DatabaseConfig::from(&c)), - Err(e) => { - tracing::warn!("unable to convert config: {}", e); - continue; + let ns = NamespaceName::from_string(k.clone()).map_err(|e| { + Error::InvalidPersistedNamespaceConfig { + namespace: k, + reason: format!("invalid persisted namespace name: {e}"), } - }; + })?; + + // 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 // 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); } @@ -306,41 +364,59 @@ 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(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 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 _ = ret_chan.send(ret); + 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(result); } } @@ -348,10 +424,23 @@ 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(); + 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) { + 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() { @@ -367,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)", @@ -417,6 +506,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"); @@ -499,16 +593,208 @@ 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. + #[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, + Arc::new(AtomicBool::new(true)), + )) + .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>> { + 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( + &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 @@ -520,6 +806,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(); @@ -546,6 +842,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); @@ -618,6 +919,373 @@ impl MetaStore { } } +#[cfg(test)] +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(); + 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(); + 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(); + 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, + ..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 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], + ) + .unwrap(); + } + let store = MetaStore::new( + config, + tmp.path(), + maker().unwrap(), + wal, + DatabaseKind::Primary, + ) + .await + .unwrap(); + assert_eq!( + store + .handle(NamespaceName::from("tenant")) + .await + .get() + .shared_schema_name + .as_ref() + .unwrap() + .as_str(), + "schema" + ); + } + + #[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 +1301,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())))), } } @@ -655,21 +1323,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(); @@ -698,7 +1366,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. @@ -708,7 +1379,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/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/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/namespace/store.rs b/libsql-server/src/namespace/store.rs index 86e9438ccd..91da8e9924 100644 --- a/libsql-server/src/namespace/store.rs +++ b/libsql-server/src/namespace/store.rs @@ -1,11 +1,16 @@ +use std::collections::HashMap; +use std::io::Write; +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; @@ -15,11 +20,12 @@ 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; 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 +33,2187 @@ 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. +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 + ); + NAMESPACE_QUARANTINE_COUNT.increment(1); + 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}"); + } + } +} + +// 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; + } + #[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 prost::Message; + 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 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) { + 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) + } + + #[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; + 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; + 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, None) + .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 reset_setup_failure_fences_old_data_until_restart() { + 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").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 + .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 + .store(changed) + .await + .unwrap(); + 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 cancelled_reset_waiter_cannot_release_name_lock_during_setup() { + 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").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 + .reset(name, RestoreOption::Dump(Box::new(Box::pin(dump)))) + .await + } + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .unwrap(); + waiter.abort(); + assert!(waiter.await.unwrap_err().is_cancelled()); + let name_lock = store + .inner + .name_operations + .lock() + .unwrap() + .get(&name) + .unwrap() + .upgrade() + .unwrap(); + assert!( + timeout(Duration::from_millis(50), name_lock.clone().lock_owned()) + .await + .is_err() + ); + 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 + .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 restart_reconciles_reset_at_each_boundary() { + let name = NamespaceName::from("victim"); + 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 bytes = serde_json::to_vec(&ResetIntent { + old_identity: expected, + old_config, + }) + .unwrap(); + store + .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 pending_reset_refuses_schema_switch_and_keeps_migration_membership() { + let (tmp, store) = primary_fixture().await; + let tenant = NamespaceName::from("tenant"); + store + .create( + tenant.clone(), + RestoreOption::Latest, + DatabaseConfig::default(), + ) + .await + .unwrap(); + 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(); + store + .inner + .metadata + .handle(b.clone()) + .await + .store(schema_config) + .await + .unwrap(); + let mut linked = DatabaseConfig::default(); + linked.shared_schema_name = Some(a.clone()); + store + .inner + .metadata + .handle(tenant.clone()) + .await + .store(linked.clone()) + .await + .unwrap(); + 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 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; + 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 + .detach_owned_directory( + &name, + identity, + false, + Some(store.destroy_quarantine_path(&name)), + ) + .await + .unwrap(); + } + // 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(); + let restarted = NamespaceStore::new( + false, + false, + 10, + metadata, + NamespaceConfigurators::empty(), + DatabaseKind::Primary, + tmp.path(), + ) + .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()); + } + } + } + + #[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(); + 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 incomplete_destroy_intent_prevents_reuse_until_recovered() { + let (tmp, store) = primary_fixture().await; + let name = NamespaceName::from("victim"); + 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(); + store.persist_destroy_intent(&destination, None).unwrap(); + assert!(store + .fork( + source.clone(), + destination.clone(), + DatabaseConfig::default(), + None + ) + .await + .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 + .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); + } + + #[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(); + // 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 + .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 +2237,16 @@ pub struct NamespaceStoreInner { broadcasters: BroadcasterRegistry, 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, } impl NamespaceStore { @@ -60,6 +2257,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() @@ -84,7 +2282,7 @@ impl NamespaceStore { .time_to_idle(Duration::from_secs(86400)) .build(); - Ok(Self { + let this = Self { inner: Arc::new(NamespaceStoreInner { store, metadata, @@ -95,35 +2293,866 @@ 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, }), + }; + 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(()) + } + + // 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(()) + } + + 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(()) + } + + 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) + }) + .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)?; + } + // 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(()) + } + + 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 + // 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?; + self.check_no_destroy_intent(namespace)?; + 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; + // 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(_)) => 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" + )) }) } - pub async fn exists(&self, namespace: &NamespaceName) -> bool { - self.inner.metadata.exists(namespace).await + // 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)?; + // 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::>>()?; + 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(()) } - pub async fn destroy(&self, namespace: NamespaceName, prune_all: bool) -> crate::Result<()> { - if self.inner.has_shutdown.load(Ordering::Relaxed) { - return Err(Error::NamespaceStoreShutdown); + 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()), } + } - // 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())) + // 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, + destroy_target: Option, + ) -> 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 durable_destroy = destroy_target.is_some(); + 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 + ))); } - }) - .await??; + 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 + }; + // 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<()> { + 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()), @@ -132,12 +3161,107 @@ impl NamespaceStore { } } - self.cleanup(&namespace, &db_config, prune_all, bottomless_db_id_init) - .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" + ))); + } + // 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(); + let removed = tokio::task::spawn_blocking(move || { + metadata + .remove_if_generation(name.clone(), Some(&generation))? + .ok_or_else(|| Error::NamespaceDoesntExist(name.to_string())) + }) + .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, + 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(()) + }); + if let Some(ready) = after_commit_started { + ready.notify_one(); + } + task.await? } pub async fn checkpoint(&self, namespace: NamespaceName) -> crate::Result<()> { @@ -153,16 +3277,69 @@ 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<()> { - // The process for reseting is as follow: - // - 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_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, + 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 @@ -172,22 +3349,148 @@ impl NamespaceStore { if let Some(ns) = lock.take() { ns.destroy().await?; } - - let db_config = self.inner.metadata.handle(namespace.clone()).await; - // destroy on-disk database - self.cleanup( - &namespace, - &db_config.get(), - false, - NamespaceBottomlessDbIdInit::FetchFromConfig, - ) - .await?; - let ns = self - .make_namespace(&namespace, db_config, restore_option) + // 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; + match self.reset_migration_job_check(&schema).await { + Ok(true) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(Error::PendingMigrationOnSchema(schema).into()); + } + Ok(false) => Some(guard), + Err(e) => { + self.release_unpublished_reset_pin(&namespace).await; + return Err(e); + } + } + } 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 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()); + } + }; + // 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(()) } @@ -216,6 +3519,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 +3532,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 +3562,74 @@ impl NamespaceStore { return Err(crate::error::Error::NamespaceDoesntExist(from.to_string())); }; - struct Bomb { - store: MetaStore, - ns: NamespaceName, - should_delete: bool, - } + // 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())); + } + self.reserve_directory(&to).await? + }; + self.inner.metadata.activate_for_create(&to); + 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?; - 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}"); - } + // 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); } - } - - let mut bomb = Bomb { - store: self.inner.metadata.clone(), - ns: to.clone(), - should_delete: true, + Ok(to_ns) }; - - 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(()) + 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( @@ -321,6 +3654,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 @@ -329,6 +3679,7 @@ impl NamespaceStore { return Err(Error::NamespaceDoesntExist(namespace.to_string())); } + after_check.await; let f = { let name = namespace.clone(); move |ns: NamespaceEntry| async move { @@ -341,11 +3692,69 @@ 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; + // 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() { + 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 + }; + 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; - 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 +3799,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 +3826,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 +3888,110 @@ 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? + }; + self.inner.metadata.activate_for_create(&namespace); + 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; @@ -510,6 +4047,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 => { @@ -520,7 +4105,7 @@ impl NamespaceStore { } } - async fn cleanup( + async fn prepare_cleanup( &self, namespace: &NamespaceName, db_config: &DatabaseConfig, @@ -528,7 +4113,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/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-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..a07335f7d6 100644 --- a/libsql-server/src/schema/scheduler.rs +++ b/libsql-server/src/schema/scheduler.rs @@ -81,6 +81,14 @@ impl Scheduler { tracing::info!("all scheduler handles dropped: exiting."); break; } + Err(e @ Error::InvalidPersistedNamespace { .. }) => { + // 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}" + ); + break; + } Err(e) => { if tries >= MAX_ERROR_RETRIES { tracing::error!("scheduler could not make progress after {MAX_ERROR_RETRIES}, exiting: {e}"); @@ -410,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) }) @@ -852,10 +864,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(); @@ -984,10 +1003,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(); @@ -1065,10 +1091,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| { @@ -1099,10 +1132,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(); @@ -1179,10 +1219,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..b3166956aa 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,362 @@ 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?; + // 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. + 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 37b373b76e..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() }; @@ -105,6 +117,85 @@ fn fork_namespace() { sim.run().unwrap(); } +#[test] +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"); + + 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(client_dbs.join("sentinel"))?; + std::fs::write(client_dbs.join("sentinel/marker"), b"keep me too")?; + + for name in [ + "%2e%2e".to_string(), + "%2e%2e%2foutside".to_string(), + absolute_victim, + "bad%5cname".to_string(), + ] { + let create = format!("http://primary:9090/v1/namespaces/{name}/create"); + assert!( + client.post(&create, json!({})).await?.status() == hyper::StatusCode::BAD_REQUEST, + "create {name}" + ); + + let delete = format!("http://primary:9090/v1/namespaces/{name}"); + assert!( + client.delete(&delete, json!({})).await?.status() == hyper::StatusCode::BAD_REQUEST, + "delete {name}" + ); + + 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}" + ); + } + // 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 + ); + 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() 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"); +} 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 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/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" 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 {