From 5629dcd7d0a3209fcf7add148eee08a2f64a8b3a Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Sun, 23 Aug 2026 13:20:50 -0400 Subject: [PATCH] Make snapshot and backup publication durable across power loss Neither the pre-activation backup nor the pre-migration snapshot fsynced the staged file contents or the parent directory around the publishing rename. A crash after the import committed could lose the retained '.pre-blob-activation.bak' (whose directory entry was never made durable), and the next activation would then write a new backup that already contains the imported blobs, silently breaking the documented pre-activation rollback boundary; a torn staging file could likewise wedge later boots in backup validation. write_snapshot_to_staging now syncs the staged file before handing it to the caller, and both publishers sync the destination's parent directory after their rename (fabro-db on a blocking task, activation inside its existing blocking publication task). Co-Authored-By: Claude Fable 5 --- lib/apps/fabro-server/src/blob_activation.rs | 8 ++- lib/foundation/fabro-db/src/lib.rs | 52 ++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/apps/fabro-server/src/blob_activation.rs b/lib/apps/fabro-server/src/blob_activation.rs index 66d22dc24..2cc05da5f 100644 --- a/lib/apps/fabro-server/src/blob_activation.rs +++ b/lib/apps/fabro-server/src/blob_activation.rs @@ -292,7 +292,13 @@ async fn create_backup( let already_exists = spawn_blocking(move || { let staging = tempfile::TempPath::from_path(publish_staging); match staging.persist_noclobber(&publish_backup) { - Ok(()) => Ok(false), + Ok(()) => { + // Make the rename's directory entry durable: the retained + // backup is the documented rollback artifact, so it must not + // vanish in a crash after the import has already committed. + fabro_db::sync_parent_directory(&publish_backup)?; + Ok(false) + } Err(error) if error.error.kind() == std::io::ErrorKind::AlreadyExists => Ok(true), Err(error) => Err(error.error), } diff --git a/lib/foundation/fabro-db/src/lib.rs b/lib/foundation/fabro-db/src/lib.rs index abb1d482d..f51be6158 100644 --- a/lib/foundation/fabro-db/src/lib.rs +++ b/lib/foundation/fabro-db/src/lib.rs @@ -114,6 +114,19 @@ impl Database { snapshot_path.display() ) })?; + #[cfg(unix)] + { + let published_path = snapshot_path.clone(); + spawn_blocking(move || sync_parent_directory(&published_path)) + .await + .context("joining the snapshot directory sync task")? + .with_context(|| { + format!( + "syncing the directory of pre-migration snapshot {}", + snapshot_path.display() + ) + })?; + } info!( database = %database_path.display(), @@ -228,6 +241,12 @@ pub enum SnapshotStagingError { #[source] source: std::io::Error, }, + #[error("flushing snapshot staging file {path} to disk")] + Sync { + path: PathBuf, + #[source] + source: std::io::Error, + }, } /// Writes a consistent single-file copy of the live pool to `staging_path`. @@ -266,6 +285,39 @@ pub async fn write_snapshot_to_staging( path: staging_path.to_path_buf(), source, })?; + // The staging file must be durable before the caller renames it into a + // path that later recovery logic treats as a complete snapshot. + let sync_result = match fs::File::open(staging_path).await { + Ok(file) => file.sync_all().await, + Err(source) => Err(source), + }; + sync_result.map_err(|source| SnapshotStagingError::Sync { + path: staging_path.to_path_buf(), + source, + })?; + Ok(()) +} + +/// Flushes the directory entry metadata for `path`'s parent so a rename into +/// that directory survives power loss. No-op off Unix, where a directory +/// cannot be opened for syncing. +#[cfg(unix)] +#[expect( + clippy::disallowed_methods, + reason = "directory fds have no async open; callers run this on a blocking thread" +)] +pub fn sync_parent_directory(path: &Path) -> std::io::Result<()> { + let Some(parent) = path.parent() else { + return Ok(()); + }; + std::fs::File::open(parent)?.sync_all() +} + +/// Flushes the directory entry metadata for `path`'s parent so a rename into +/// that directory survives power loss. No-op off Unix, where a directory +/// cannot be opened for syncing. +#[cfg(not(unix))] +pub fn sync_parent_directory(_path: &Path) -> std::io::Result<()> { Ok(()) }