From 43a9a231fac29e83d6ed84920a243990981de3af Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Mon, 9 Mar 2026 13:56:18 +0400 Subject: [PATCH 1/5] feat(accountsdb): implement external snapshot insertion --- Cargo.lock | 63 +++- magicblock-accounts-db/Cargo.toml | 4 + magicblock-accounts-db/src/lib.rs | 52 ++- magicblock-accounts-db/src/snapshot.rs | 417 +++++++++++++++---------- magicblock-accounts-db/src/tests.rs | 157 +++++++++- 5 files changed, 518 insertions(+), 175 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68548b009..e3f4e6f2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1642,6 +1642,17 @@ dependencies = [ "version_check", ] +[[package]] +name = "filetime" +version = "0.2.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f98844151eee8917efc50bd9e8318cb963ae8b297431495d3f758616ea5c57db" +dependencies = [ + "cfg-if", + "libc", + "libredox", +] + [[package]] name = "find-msvc-tools" version = "0.1.5" @@ -2728,6 +2739,18 @@ dependencies = [ "windows-link", ] +[[package]] +name = "libredox" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1744e39d1d6a9948f4f388969627434e31128196de472883b39f148769bfe30a" +dependencies = [ + "bitflags 2.10.0", + "libc", + "plain", + "redox_syscall 0.7.3", +] + [[package]] name = "librocksdb-sys" version = "0.17.1+9.9.3" @@ -2979,6 +3002,7 @@ dependencies = [ name = "magicblock-accounts-db" version = "0.8.2" dependencies = [ + "flate2", "lmdb-rkv", "magicblock-config", "memmap2 0.9.9", @@ -2986,6 +3010,7 @@ dependencies = [ "reflink-copy", "solana-account", "solana-pubkey", + "tar", "tempfile", "thiserror 1.0.69", "tracing", @@ -3992,7 +4017,7 @@ checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" dependencies = [ "cfg-if", "libc", - "redox_syscall", + "redox_syscall 0.5.18", "smallvec", "windows-link", ] @@ -4145,6 +4170,12 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" +[[package]] +name = "plain" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4596b6d070b27117e987119b4dac604f3c58cfb0b191112e24771b2faeac1a6" + [[package]] name = "polyval" version = "0.6.2" @@ -4718,6 +4749,15 @@ dependencies = [ "bitflags 2.10.0", ] +[[package]] +name = "redox_syscall" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ce70a74e890531977d37e532c34d45e9055d2409ed08ddba14529471ed0be16" +dependencies = [ + "bitflags 2.10.0", +] + [[package]] name = "ref-cast" version = "1.0.25" @@ -8319,6 +8359,17 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" +[[package]] +name = "tar" +version = "0.4.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d863878d212c87a19c1a610eb53bb01fe12951c0501cf5a0d65f724914a667a" +dependencies = [ + "filetime", + "libc", + "xattr", +] + [[package]] name = "task-local-extensions" version = "0.1.4" @@ -9660,6 +9711,16 @@ dependencies = [ "tap", ] +[[package]] +name = "xattr" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156" +dependencies = [ + "libc", + "rustix 1.1.2", +] + [[package]] name = "yansi" version = "1.0.1" diff --git a/magicblock-accounts-db/Cargo.toml b/magicblock-accounts-db/Cargo.toml index 2a854cd11..c47a3c497 100644 --- a/magicblock-accounts-db/Cargo.toml +++ b/magicblock-accounts-db/Cargo.toml @@ -13,6 +13,10 @@ memmap2 = "0.9" lmdb = { package = "lmdb-rkv", version = "0.14" } # more up to date fork of lmdb bindings by mozilla, still ancient though :( reflink = { package = "reflink-copy", version = "0.1" } +# archival +tar = "0.4" +flate2 = "1.0" + # solana solana-pubkey = { workspace = true } solana-account = { workspace = true } diff --git a/magicblock-accounts-db/src/lib.rs b/magicblock-accounts-db/src/lib.rs index f2826bc80..7662198a6 100644 --- a/magicblock-accounts-db/src/lib.rs +++ b/magicblock-accounts-db/src/lib.rs @@ -298,22 +298,36 @@ impl AccountsDb { } /// Spawns a background thread to take a snapshot. + /// The snapshot is created in two phases: + /// 1. Create snapshot directory (with write lock held) + /// 2. Archive directory to tar.gz and register (lock released) fn trigger_background_snapshot(self: &Arc, slot: u64) { let this = self.clone(); thread::spawn(move || { - // Acquire write lock to ensure consistent state capture + // Phase 1: Create snapshot directory (with write lock) let write_guard = this.write_lock.write(); this.flush(); - - // Capture the active memory map region for the snapshot let used_storage = this.storage.active_segment(); - let _ = this.snapshot_manager.create_snapshot( + let snapshot_dir = this.snapshot_manager.create_snapshot_dir( slot, used_storage, - write_guard, + write_guard, // Lock released when this returns ); + + // Phase 2: Archive directory (no lock needed) + match snapshot_dir { + Ok(dir) => { + // Take our time to archive - lock is released + if let Err(e) = + this.snapshot_manager.archive_and_register(&dir) + { + error!(error = ?e, "Failed to archive snapshot"); + } + } + Err(e) => error!(error = ?e, "Snapshot creation failed"), + } }); } @@ -376,6 +390,34 @@ impl AccountsDb { self.write_lock.clone() } + /// Inserts an external snapshot archive received over the network. + /// + /// If the snapshot slot is newer than the current DB slot, immediately + /// fast-forwards to it (bringing state forward in time). + /// + /// Returns `true` if fast-forward was performed, `false` if just registered. + pub fn insert_external_snapshot( + &mut self, + slot: u64, + archive_bytes: &[u8], + ) -> AccountsDbResult { + let current_slot = self.slot(); + let fast_forwarded = self.snapshot_manager.insert_external_snapshot( + slot, + archive_bytes, + current_slot, + )?; + + if fast_forwarded { + // Reload components to reflect new state + let path = self.snapshot_manager.database_path(); + self.storage.reload(path)?; + self.index.reload(path)?; + } + + Ok(fast_forwarded) + } + /// Computes a deterministic checksum of all active accounts. /// /// Iterates all accounts in key-sorted order (via LMDB) and hashes both diff --git a/magicblock-accounts-db/src/snapshot.rs b/magicblock-accounts-db/src/snapshot.rs index 21a17ed99..a74619e8a 100644 --- a/magicblock-accounts-db/src/snapshot.rs +++ b/magicblock-accounts-db/src/snapshot.rs @@ -2,12 +2,14 @@ use std::{ collections::VecDeque, ffi::OsStr, fs::{self, File}, - io::{self, BufWriter, Write}, + io::{self, BufWriter, Cursor, Write}, path::{Path, PathBuf}, sync::Arc, }; +use flate2::{read::GzDecoder, write::GzEncoder, Compression}; use parking_lot::{Mutex, RwLockWriteGuard}; +use tar::{Archive, Builder}; use tracing::{error, info, warn}; use crate::{ @@ -16,21 +18,23 @@ use crate::{ AccountsDbResult, }; -/// Defines the mechanism used to persist snapshots to disk. +const SNAPSHOT_PREFIX: &str = "snapshot-"; +const ARCHIVE_EXT: &str = "tar.gz"; + +/// Snapshot persistence strategy. #[derive(Debug, Clone, Copy)] enum SnapshotStrategy { - /// Utilizes filesystem CoW (Copy-on-Write) features (e.g., `ioctl_ficlonerange` on Linux). - /// This is an O(1) operation regarding data size. + /// CoW reflink - O(1) metadata operation when filesystem supports it. Reflink, - /// Fallback for standard filesystems. Performs a deep recursive copy of the directory. - /// Requires capturing the `mmap` state into RAM before writing to ensure consistency. + /// Deep copy with memory capture for consistency. LegacyCopy, } impl SnapshotStrategy { - /// Probes the filesystem at `dir` to determine if CoW operations are supported. fn detect(dir: &Path) -> AccountsDbResult { - if fs_backend::supports_reflink(dir).log_err(|| "CoW check failed")? { + let supports_cow = + fs_backend::supports_reflink(dir).log_err(|| "CoW check failed")?; + if supports_cow { info!("Snapshot Strategy: Reflink (Fast/CoW)"); Ok(Self::Reflink) } else { @@ -39,12 +43,6 @@ impl SnapshotStrategy { } } - /// Executes the snapshot operation based on the active strategy. - /// - /// # Arguments - /// * `memory_state` - Required only for `LegacyCopy`. Contains the byte-consistent view - /// of the main database file. - /// * `lock` - Write lock, which prevents any accountsdb modifications during snapshotting fn execute( &self, src: &Path, @@ -55,35 +53,29 @@ impl SnapshotStrategy { match self { Self::Reflink => fs_backend::reflink_dir(src, dst), Self::LegacyCopy => { - // Drop lock for slow copy to avoid stalling the system - drop(lock); + drop(lock); // Release lock before slow I/O fs_backend::deep_copy_dir(src, dst, &memory_state) } } } } -/// Manages the lifecycle, creation, and restoration of database snapshots. +/// Manages snapshot lifecycle: creation, archival, and restoration. /// -/// This type handles the complexity of filesystem capabilities -/// (CoW vs Copy) and ensures atomic restoration of state. +/// Snapshots are stored as compressed tar archives (`.tar.gz`). During creation, +/// a directory is first created, then archived asynchronously after releasing +/// the write lock to minimize blocking time. #[derive(Debug)] pub struct SnapshotManager { - /// Path to the active (hot) database directory. db_path: PathBuf, - /// Directory where snapshots are stored. snapshots_dir: PathBuf, - /// The persistence strategy chosen during initialization. strategy: SnapshotStrategy, - /// Ordered registry of valid snapshot paths (oldest to newest). + /// Ordered registry of archive paths (oldest to newest). registry: Mutex>, - /// Maximum number of snapshots to retain before rotating out old ones. max_snapshots: usize, } impl SnapshotManager { - /// Initializes the manager, detecting filesystem capabilities and recovering - /// existing snapshots from disk. pub fn new( db_path: PathBuf, max_snapshots: usize, @@ -96,10 +88,7 @@ impl SnapshotManager { .log_err(|| "Failed to resolve snapshots directory")? .to_path_buf(); - // 1. Determine capabilities (CoW or Deep Copy) let strategy = SnapshotStrategy::detect(&snapshots_dir)?; - - // 2. Load and sort existing snapshots from the filesystem let registry = Self::recover_registry(&snapshots_dir, max_snapshots) .log_err(|| "Failed to load snapshot registry")?; @@ -112,150 +101,235 @@ impl SnapshotManager { })) } - /// Creates a durable snapshot of the current database state for the given `slot`. - /// - /// # Locking & Consistency - /// * **CoW (Reflink)**: The `_write_lock` is held during the reflink operation. - /// Since reflink is a metadata operation, this is extremely fast. - /// * **Legacy Copy**: The `_write_lock` is dropped *after* capturing the `active_mem` - /// state to RAM, but *before* the slow disk I/O. This prevents blocking the validator - /// during the deep copy. - pub fn create_snapshot( + /// Creates a snapshot directory. Returns the path for later archiving. + pub fn create_snapshot_dir( &self, slot: u64, active_mem: &[u8], lock: RwLockWriteGuard<()>, - ) -> AccountsDbResult<()> { - let snap_path = self.generate_path(slot); - - // 1. Maintain retention policy + ) -> AccountsDbResult { self.prune_registry(); - // 2. Prepare Data Capture - // If legacy copy, we must capture state while lock is held. + let snap_path = self.slot_to_dir_path(slot); let memory_capture = matches!(self.strategy, SnapshotStrategy::LegacyCopy) .then(|| active_mem.to_vec()) .unwrap_or_default(); - // 3. Execute Snapshot self.strategy .execute(&self.db_path, &snap_path, memory_capture, lock) .log_err(|| "Snapshot failed")?; - // 4. Register success - self.registry.lock().push_back(snap_path); + Ok(snap_path) + } + + /// Archives the snapshot directory to `.tar.gz` and removes the directory. + pub fn archive_and_register( + &self, + snapshot_dir: &Path, + ) -> AccountsDbResult<()> { + let archive_path = snapshot_dir.with_extension(ARCHIVE_EXT); + + info!(archive_path = %archive_path.display(), "Archiving snapshot"); + + let file = File::create(&archive_path).log_err(|| { + format!("Failed to create archive at {}", archive_path.display()) + })?; + let enc = GzEncoder::new(file, Compression::fast()); + let mut tar = Builder::new(enc); + tar.append_dir_all(".", snapshot_dir) + .log_err(|| "Failed to append directory to tar")?; + tar.finish().log_err(|| "Failed to finalize tar archive")?; + + fs::remove_dir_all(snapshot_dir).log_err(|| { + format!( + "Failed to remove snapshot directory {}", + snapshot_dir.display() + ) + })?; + + self.register_archive(archive_path); Ok(()) } - /// Atomically restores the database to the snapshot nearest to `target_slot`. - /// - /// # Critical Operation - /// This function replaces the active database directory. - /// 1. Finds the best candidate snapshot (<= target_slot). - /// 2. Prunes all snapshots newer than the candidate (invalidated history). - /// 3. Performs an atomic swap: Active -> Backup, Snapshot -> Active. + /// Inserts an external snapshot archive received from network. /// - /// Returns the actual slot number of the restored snapshot. + /// If `slot > current_slot`, immediately fast-forwards to the snapshot. + /// Returns `true` if fast-forward was performed. + pub fn insert_external_snapshot( + &self, + slot: u64, + archive_bytes: &[u8], + current_slot: u64, + ) -> AccountsDbResult { + // Validate archive structure + Self::validate_archive(archive_bytes)?; + + let archive_path = self.slot_to_archive_path(slot); + if archive_path.exists() { + return Err(AccountsDbError::Internal(format!( + "Snapshot for slot {} already exists", + slot + ))); + } + + info!(slot, "Inserting external snapshot"); + + // Write archive to disk + let mut file = File::create(&archive_path).log_err(|| { + format!("Failed to create archive at {}", archive_path.display()) + })?; + file.write_all(archive_bytes) + .log_err(|| "Failed to write archive bytes")?; + file.sync_all() + .log_err(|| "Failed to sync archive to disk")?; + + // Fast-forward if snapshot is newer than current state + if slot > current_slot { + info!(slot, current_slot, "Fast-forwarding to external snapshot"); + self.fast_forward(slot, &archive_path)?; + return Ok(true); + } + + // Otherwise just register for later use + self.prune_registry(); + self.register_archive(archive_path); + Ok(false) + } + + /// Restores database to the snapshot nearest to `target_slot`. pub fn restore_from_snapshot( &self, target_slot: u64, ) -> AccountsDbResult { - let mut registry = self.registry.lock(); + let (chosen_archive, chosen_slot, index) = + self.find_and_remove_snapshot(target_slot)?; - // 1. Locate Snapshot (Binary Search) - let search_key = self.generate_path(target_slot); + let extracted_dir = self.extract_archive(&chosen_archive)?; + self.atomic_swap(&extracted_dir)?; + self.prune_invalidated_snapshots(index); + let _ = fs::remove_file(&chosen_archive); + + Ok(chosen_slot) + } + + /// Validates that bytes represent a valid gzip tar archive. + fn validate_archive(bytes: &[u8]) -> AccountsDbResult<()> { + let cursor = Cursor::new(bytes); + let dec = GzDecoder::new(cursor); + let mut tar = Archive::new(dec); + tar.entries() + .log_err(|| "Invalid snapshot archive: not a valid gzip tar")?; + Ok(()) + } + + /// Finds the best snapshot for target_slot, removes from registry. + /// Returns (archive_path, slot, index). + fn find_and_remove_snapshot( + &self, + target_slot: u64, + ) -> AccountsDbResult<(PathBuf, u64, usize)> { + let mut registry = self.registry.lock(); + let search_key = self.slot_to_archive_path(target_slot); let index = match registry.binary_search(&search_key) { Ok(i) => i, Err(i) if i > 0 => i - 1, _ => return Err(AccountsDbError::SnapshotMissing(target_slot)), }; - let chosen_path = registry.remove(index).unwrap(); - let chosen_slot = Self::parse_slot(&chosen_path) + let chosen_archive = registry.remove(index).unwrap(); + let chosen_slot = Self::parse_slot(&chosen_archive) .ok_or(AccountsDbError::SnapshotMissing(target_slot))?; - info!( - chosen_slot = chosen_slot, - target_slot = target_slot, - "Restoring snapshot" - ); - - // 2. Prune Invalidated Futures - // Any snapshot strictly newer than the chosen one is now on a diverging timeline. - for invalidated in registry.drain(index..) { - warn!( - invalidated_path = %invalidated.display(), - "Pruning invalidated snapshot" - ); - let _ = fs::remove_dir_all(&invalidated); - } + info!(chosen_slot, target_slot, "Restoring snapshot"); + Ok((chosen_archive, chosen_slot, index)) + } + + /// Extracts a tar.gz archive to a temporary directory. + fn extract_archive( + &self, + archive_path: &Path, + ) -> AccountsDbResult { + let extract_dir = archive_path.with_extension("extract"); + + info!(archive_path = %archive_path.display(), "Extracting snapshot archive"); + + let file = File::open(archive_path).log_err(|| { + format!("Failed to open archive {}", archive_path.display()) + })?; + let mut tar = Archive::new(GzDecoder::new(file)); + tar.unpack(&extract_dir).log_err(|| { + format!("Failed to extract archive to {}", extract_dir.display()) + })?; - // 3. Atomic Swapping via Rename + Ok(extract_dir) + } + + /// Performs atomic swap: current db -> backup, extracted -> current db. + /// On failure, rolls back to original state. + fn atomic_swap(&self, extracted_dir: &Path) -> AccountsDbResult<()> { let backup = self.db_path.with_extension("bak"); - // Stage current DB as backup if self.db_path.exists() { fs::rename(&self.db_path, &backup) .log_err(|| "Failed to stage backup")?; } - // Promote snapshot to active - if let Err(e) = fs::rename(&chosen_path, &self.db_path) { - error!( - error = ?e, - "Restore failed during promote" - ); - // Attempt to restore the backup if promotion fails + if let Err(e) = fs::rename(extracted_dir, &self.db_path) { + error!(error = ?e, "Atomic swap failed during promote"); if backup.exists() { let _ = fs::rename(&backup, &self.db_path); } + let _ = fs::remove_dir_all(extracted_dir); return Err(e.into()); } - // Success: remove backup let _ = fs::remove_dir_all(&backup); - Ok(chosen_slot) + Ok(()) } - /// Checks if a snapshot for the exact `slot` exists in the registry. - #[cfg(test)] - pub fn snapshot_exists(&self, slot: u64) -> bool { - let path = self.generate_path(slot); - self.registry.lock().binary_search(&path).is_ok() + /// Fast-forwards to a snapshot that's newer than current state. + fn fast_forward( + &self, + slot: u64, + archive_path: &Path, + ) -> AccountsDbResult<()> { + let extracted_dir = self.extract_archive(archive_path)?; + self.atomic_swap(&extracted_dir)?; + self.registry.lock().push_back(archive_path.to_path_buf()); + info!(slot, "Fast-forward complete"); + Ok(()) } - // --- Private Helpers --- - - fn generate_path(&self, slot: u64) -> PathBuf { - // Padding ensures standard string sorting aligns with numeric sorting - self.snapshots_dir.join(format!("snapshot-{:0>12}", slot)) + /// Registers an archive in the registry. + fn register_archive(&self, archive_path: PathBuf) { + info!(archive_path = %archive_path.display(), "Snapshot registered"); + self.registry.lock().push_back(archive_path); } - fn parse_slot(path: &Path) -> Option { - path.file_name() - .and_then(OsStr::to_str) - .and_then(|s| s.strip_prefix("snapshot-")) - .and_then(|s| s.parse().ok()) + /// Removes snapshots newer than the chosen one (invalidated by rollback). + fn prune_invalidated_snapshots(&self, from_index: usize) { + let mut registry = self.registry.lock(); + for invalidated in registry.drain(from_index..) { + warn!(invalidated_path = %invalidated.display(), "Pruning invalidated snapshot"); + let _ = fs::remove_file(&invalidated); + } } - /// Removes the oldest snapshots until `registry.len() < max_snapshots`. + /// Removes oldest snapshots until under the limit. fn prune_registry(&self) { let mut registry = self.registry.lock(); while registry.len() >= self.max_snapshots { - if let Some(path) = registry.pop_front() { - if let Err(e) = fs::remove_dir_all(&path) { - warn!( - path = %path.display(), - error = ?e, - "Failed to prune snapshot" - ); - } + let Some(path) = registry.pop_front() else { + break; + }; + if let Err(e) = fs::remove_file(&path) { + warn!(path = %path.display(), error = ?e, "Failed to prune snapshot archive"); } } } - /// Scans the disk for existing snapshot directories and builds the registry. + /// Scans disk for existing snapshot archives. fn recover_registry( dir: &Path, max: usize, @@ -268,75 +342,98 @@ impl SnapshotManager { let mut paths: Vec<_> = fs::read_dir(dir)? .filter_map(|e| e.ok()) .map(|e| e.path()) - .filter(|p| p.is_dir() && Self::parse_slot(p).is_some()) + .filter(|p| { + p.is_file() + && p.extension().is_some_and(|e| e == "gz") + && Self::parse_slot(p).is_some() + }) .collect(); + paths.sort(); + + // Clean orphan directories (interrupted archiving) + for entry in fs::read_dir(dir)?.filter_map(|e| e.ok()) { + let path = entry.path(); + let name = path.file_name().and_then(OsStr::to_str); + if path.is_dir() + && name.is_some_and(|n| { + n.starts_with(SNAPSHOT_PREFIX) && !n.contains('.') + }) + { + warn!(path = %path.display(), "Cleaning up orphan snapshot directory"); + let _ = fs::remove_dir_all(&path); + } + } - paths.sort(); // Sorting works due to zero-padded filenames - - // Enforce limit strictly on startup let offset = paths.len().saturating_sub(max); Ok(paths.into_iter().skip(offset).collect()) } - /// Return the path to main accountsdb directory + fn slot_to_dir_path(&self, slot: u64) -> PathBuf { + self.snapshots_dir + .join(format!("{SNAPSHOT_PREFIX}{slot:0>12}")) + } + + fn slot_to_archive_path(&self, slot: u64) -> PathBuf { + self.snapshots_dir + .join(format!("{SNAPSHOT_PREFIX}{slot:0>12}.{ARCHIVE_EXT}")) + } + + fn parse_slot(path: &Path) -> Option { + path.file_name()? + .to_str()? + .strip_prefix(SNAPSHOT_PREFIX)? + .strip_suffix(&format!(".{ARCHIVE_EXT}"))? + .parse() + .ok() + } + pub(crate) fn database_path(&self) -> &Path { &self.db_path } + + #[cfg(test)] + pub fn snapshot_exists(&self, slot: u64) -> bool { + self.registry + .lock() + .binary_search(&self.slot_to_archive_path(slot)) + .is_ok() + } } mod fs_backend { use super::*; - /// Writes a test file and attempts a reflink to determine CoW support. pub(crate) fn supports_reflink(dir: &Path) -> io::Result { let src = dir.join(".tmp_cow_probe_src"); let dst = dir.join(".tmp_cow_probe_dst"); + let _ = (fs::remove_file(&src), fs::remove_file(&dst)); - // Clean slate - let _ = fs::remove_file(&src); - let _ = fs::remove_file(&dst); - - { - let mut f = File::create(&src)?; - f.write_all(&[0u8; 64])?; - f.sync_data()?; - } - + File::create(&src)?.write_all(&[0u8; 64])?; let result = reflink::reflink(&src, &dst).is_ok(); - - let _ = fs::remove_file(src); - let _ = fs::remove_file(dst); - + let _ = (fs::remove_file(src), fs::remove_file(dst)); Ok(result) } pub(crate) fn reflink_dir(src: &Path, dst: &Path) -> io::Result<()> { - // If src is a directory, manually copy entries - if src.is_dir() { - fs::create_dir_all(dst)?; - for entry in fs::read_dir(src)? { - let entry = entry?; - let src_path = entry.path(); - let dst_path = dst.join(entry.file_name()); - - if entry.file_type()?.is_dir() { - reflink_dir(&src_path, &dst_path)?; - } else { - reflink::reflink(&src_path, &dst_path)? - } + if !src.is_dir() { + return reflink::reflink(src, dst); + } + fs::create_dir_all(dst)?; + for entry in fs::read_dir(src)? { + let entry = entry?; + let (src_path, dst_path) = + (entry.path(), dst.join(entry.file_name())); + if entry.file_type()?.is_dir() { + reflink_dir(&src_path, &dst_path)?; + } else { + reflink::reflink(&src_path, &dst_path)?; } - Ok(()) - } else { - reflink::reflink(src, dst) } + Ok(()) } - /// Recursively copies a directory. - /// - /// Special Handling: - /// If `ACCOUNTS_DB_FILENAME` is encountered, it writes the provided `mem_dump` - /// instead of copying the file from disk. This ensures we write the state - /// consistent with the capture time, ignoring any subsequent disk modifications. + /// Deep copy with special handling: writes `mem_dump` for `accounts.db` + /// instead of copying from disk to ensure consistency. pub(crate) fn deep_copy_dir( src: &Path, dst: &Path, @@ -345,11 +442,9 @@ mod fs_backend { fs::create_dir_all(dst)?; for entry in fs::read_dir(src)? { let entry = entry?; - let ty = entry.file_type()?; - let src_path = entry.path(); - let dst_path = dst.join(entry.file_name()); - - if ty.is_dir() { + let (src_path, dst_path) = + (entry.path(), dst.join(entry.file_name())); + if entry.file_type()?.is_dir() { deep_copy_dir(&src_path, &dst_path, mem_dump)?; } else if src_path.file_name().and_then(OsStr::to_str) == Some(ACCOUNTS_DB_FILENAME) @@ -362,16 +457,12 @@ mod fs_backend { Ok(()) } - /// Writes the memory capture to disk using buffered I/O. fn write_dump_file(path: &Path, data: &[u8]) -> io::Result<()> { let f = File::create(path)?; - // Pre-allocate space if OS supports it to reduce fragmentation f.set_len(data.len() as u64)?; - let mut writer = BufWriter::new(f); writer.write_all(data)?; writer.flush()?; - writer.get_mut().sync_all()?; - Ok(()) + writer.get_mut().sync_all() } } diff --git a/magicblock-accounts-db/src/tests.rs b/magicblock-accounts-db/src/tests.rs index 49d5e0303..837d06818 100644 --- a/magicblock-accounts-db/src/tests.rs +++ b/magicblock-accounts-db/src/tests.rs @@ -212,6 +212,19 @@ fn test_take_snapshot() { assert_eq!(env.slot(), env.snapshot_frequency); assert!(env.snapshot_exists(env.snapshot_frequency)); + // Verify archive file exists (not directory) + let archive_path = env + .snapshot_manager + .database_path() + .parent() + .unwrap() + .join(format!("snapshot-{:0>12}.tar.gz", env.snapshot_frequency)); + assert!(archive_path.exists(), "Archive file should exist"); + assert!( + archive_path.is_file(), + "Snapshot should be a file, not directory" + ); + // Update Account acc.account.set_data(ACCOUNT_DATA.to_vec()); env.insert_account(&acc.pubkey, &acc.account).unwrap(); @@ -221,6 +234,136 @@ fn test_take_snapshot() { assert!(env.snapshot_exists(env.snapshot_frequency * 2)); } +/// Verifies that orphan snapshot directories are cleaned up on startup. +#[test] +fn test_orphan_directory_cleanup() { + let (adb, temp_dir) = TestEnv::init_raw_db(); + + // Create an orphan directory (simulating interrupted archiving) + let orphan_dir = temp_dir.path().join("accountsdb/snapshot-000000000512"); + std::fs::create_dir_all(&orphan_dir).unwrap(); + std::fs::write(orphan_dir.join("test.txt"), "orphan data").unwrap(); + + // Drop and reopen - orphan should be cleaned up + drop(adb); + let config = AccountsDbConfig::default(); + let _adb = AccountsDb::new(&config, temp_dir.path(), 0).unwrap(); + + assert!( + !orphan_dir.exists(), + "Orphan directory should be cleaned up on startup" + ); +} + +/// Verifies external snapshot fast-forward when snapshot is newer than current state. +#[test] +fn test_external_snapshot_fast_forward() { + let env = TestEnv::new(); + + // Create an account and take a local snapshot + let acc = env.create_and_insert_account(); + let snapshot_slot = env.snapshot_frequency; + env.advance_slot(snapshot_slot); + + // Read the archive bytes + let archive_path = env + .snapshot_manager + .database_path() + .parent() + .unwrap() + .join(format!("snapshot-{:0>12}.tar.gz", snapshot_slot)); + let archive_bytes = + std::fs::read(&archive_path).expect("Failed to read archive"); + let pubkey = acc.pubkey; + + // Drop current DB and create new one at slot 0 + drop(env); + let temp_dir = tempfile::tempdir().unwrap(); + let config = AccountsDbConfig { + reset: true, + ..Default::default() + }; + let mut new_db = AccountsDb::new(&config, temp_dir.path(), 0).unwrap(); + assert_eq!(new_db.slot(), 0, "New DB should start at slot 0"); + + // Insert external snapshot (snapshot slot > current slot 0, should fast-forward) + let fast_forwarded = new_db + .insert_external_snapshot(snapshot_slot, &archive_bytes) + .unwrap(); + assert!(fast_forwarded, "Should fast-forward when snapshot is newer"); + + // Verify the account exists immediately after fast-forward + let restored = new_db.get_account(&pubkey); + assert!( + restored.is_some(), + "Account should exist after fast-forward" + ); + assert_eq!(restored.unwrap().lamports(), LAMPORTS); +} + +/// Verifies external snapshot registration without fast-forward when current state is newer. +#[test] +fn test_external_snapshot_no_fast_forward() { + let env = TestEnv::new(); + + // Create an account and take a local snapshot + let acc = env.create_and_insert_account(); + let snapshot_slot = env.snapshot_frequency; + env.advance_slot(snapshot_slot); + + // Read the archive bytes + let archive_path = env + .snapshot_manager + .database_path() + .parent() + .unwrap() + .join(format!("snapshot-{:0>12}.tar.gz", snapshot_slot)); + let archive_bytes = + std::fs::read(&archive_path).expect("Failed to read archive"); + let pubkey = acc.pubkey; + + // Drop current DB and create new one, then advance past snapshot slot + drop(env); + let temp_dir = tempfile::tempdir().unwrap(); + let config = AccountsDbConfig { + reset: true, + ..Default::default() + }; + let new_db = + Arc::new(AccountsDb::new(&config, temp_dir.path(), 0).unwrap()); + new_db.set_slot(snapshot_slot + 1000); // Advance past snapshot slot + + // Unwrap Arc to get mutable access + let mut new_db = Arc::try_unwrap(new_db).unwrap(); + + // Insert external snapshot (current slot > snapshot slot, no fast-forward) + let fast_forwarded = new_db + .insert_external_snapshot(snapshot_slot, &archive_bytes) + .unwrap(); + assert!( + !fast_forwarded, + "Should NOT fast-forward when current slot is newer" + ); + + // Account should NOT exist (we're at a newer slot, snapshot just registered) + let restored = new_db.get_account(&pubkey); + assert!( + restored.is_none(), + "Account should NOT exist without explicit restore" + ); + + // Now restore explicitly + new_db.restore_state_if_needed(snapshot_slot).unwrap(); + + // Now the account should exist + let restored = new_db.get_account(&pubkey); + assert!( + restored.is_some(), + "Account should exist after explicit restore" + ); + assert_eq!(restored.unwrap().lamports(), LAMPORTS); +} + #[test] fn test_restore_from_snapshot() { let mut env = TestEnv::new(); @@ -597,11 +740,12 @@ impl TestEnv { self.adb.set_slot(target_slot); // Simple spin-wait if we expect a snapshot trigger. - // This ensures the background thread has started and possibly finished creating the file. + // This ensures the background thread has started and finished archiving. + // Archiving takes longer than just creating a directory (compression + I/O). if target_slot.is_multiple_of(self.adb.snapshot_frequency) { let mut retries = 0; - while !self.adb.snapshot_exists(target_slot) && retries < 50 { - std::thread::sleep(std::time::Duration::from_millis(10)); + while !self.adb.snapshot_exists(target_slot) && retries < 200 { + std::thread::sleep(std::time::Duration::from_millis(50)); retries += 1; } } @@ -609,17 +753,18 @@ impl TestEnv { fn restore_to_slot(mut self, slot: u64) -> Self { // Robustly wait for background threads (snapshots) to release the Arc. + // Archiving can take several seconds for large databases. let mut retries = 0; let mut inner = loop { match Arc::try_unwrap(self.adb) { Ok(inner) => break inner, Err(adb) => { - if retries > 50 { - // Panic if still shared after ~1 second + if retries > 200 { + // Panic if still shared after ~10 seconds panic!("Cannot restore: DB is shared (background snapshot thread likely still running)"); } self.adb = adb; // Put it back to retry - std::thread::sleep(std::time::Duration::from_millis(20)); + std::thread::sleep(std::time::Duration::from_millis(50)); retries += 1; } } From 67b913fe3072b1cc57ea2488e39b366519c780e6 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Wed, 11 Mar 2026 13:27:05 +0400 Subject: [PATCH 2/5] feat(accountsdb): unite snapshot and checksum operations --- config.example.toml | 5 - magicblock-accounts-db/src/lib.rs | 76 +++++++-------- magicblock-accounts-db/src/snapshot.rs | 6 +- magicblock-accounts-db/src/tests.rs | 113 +++++++++++++---------- magicblock-config/src/config/accounts.rs | 4 - magicblock-config/src/consts.rs | 3 - magicblock-config/src/tests.rs | 3 - 7 files changed, 108 insertions(+), 102 deletions(-) diff --git a/config.example.toml b/config.example.toml index 5255258ec..398722f8e 100644 --- a/config.example.toml +++ b/config.example.toml @@ -175,11 +175,6 @@ index-size = 16_777_216 # Env: MBV_ACCOUNTSDB__MAX_SNAPSHOTS max-snapshots = 4 -# How often (in slots) to take a snapshot of the database. -# Default: 1024 -# Env: MBV_ACCOUNTSDB__SNAPSHOT_FREQUENCY -snapshot-frequency = 1024 - # If true, wipes the accounts database on every startup. # Default: false # Env: MBV_ACCOUNTSDB__RESET diff --git a/magicblock-accounts-db/src/lib.rs b/magicblock-accounts-db/src/lib.rs index 7662198a6..d0ffc0a71 100644 --- a/magicblock-accounts-db/src/lib.rs +++ b/magicblock-accounts-db/src/lib.rs @@ -1,4 +1,10 @@ -use std::{fs, hash::Hasher, path::Path, sync::Arc, thread}; +use std::{ + fs, + hash::Hasher, + path::{Path, PathBuf}, + sync::Arc, + thread::{self, JoinHandle}, +}; use error::{AccountsDbError, LogErr}; use index::{ @@ -43,8 +49,6 @@ pub struct AccountsDb { /// Note: Reads are generally wait-free/lock-free via mmap, /// unless they require index cursor stability. write_lock: GlobalSyncLock, - /// Configured interval (in slots) for creating snapshots. - snapshot_frequency: u64, } impl AccountsDb { @@ -77,18 +81,11 @@ impl AccountsDb { SnapshotManager::new(db_dir.clone(), config.max_snapshots as usize) .log_err(|| "Failed to initialize snapshot manager")?; - if config.snapshot_frequency == 0 { - return Err(AccountsDbError::Internal( - "Snapshot frequency cannot be zero".to_string(), - )); - } - let mut this = Self { storage, index, snapshot_manager, write_lock: GlobalSyncLock::default(), - snapshot_frequency: config.snapshot_frequency, }; // Recover state if the requested slot is older than our current state @@ -99,10 +96,7 @@ impl AccountsDb { /// Opens an existing database (helper for tooling/tests). pub fn open(directory: &Path) -> AccountsDbResult { - let config = AccountsDbConfig { - snapshot_frequency: u64::MAX, - ..Default::default() - }; + let config = AccountsDbConfig::default(); Self::new(&config, directory, 0) } @@ -151,6 +145,8 @@ impl AccountsDb { count: usize, ) { for acc in accounts.take(count) { + // SAFETY: + // we are rolling back committed accounts unsafe { acc.1.rollback() }; } } @@ -287,48 +283,45 @@ impl AccountsDb { self.storage.slot() } - /// Updates the current slot. Triggers a background snapshot if the schedule matches. + /// Updates the current slot #[inline(always)] pub fn set_slot(self: &Arc, slot: u64) { self.storage.update_slot(slot); - - if slot > 0 && slot.is_multiple_of(self.snapshot_frequency) { - self.trigger_background_snapshot(slot); - } } /// Spawns a background thread to take a snapshot. /// The snapshot is created in two phases: /// 1. Create snapshot directory (with write lock held) /// 2. Archive directory to tar.gz and register (lock released) - fn trigger_background_snapshot(self: &Arc, slot: u64) { + pub fn take_snapshot( + self: &Arc, + slot: u64, + ) -> JoinHandle { let this = self.clone(); thread::spawn(move || { // Phase 1: Create snapshot directory (with write lock) - let write_guard = this.write_lock.write(); + let locked = this.write_lock.write(); this.flush(); + // SAFETY: + // we have acquired the write lock above + let checksum = unsafe { this.checksum() }; let used_storage = this.storage.active_segment(); let snapshot_dir = this.snapshot_manager.create_snapshot_dir( slot, used_storage, - write_guard, // Lock released when this returns + locked, // Lock released when this returns ); // Phase 2: Archive directory (no lock needed) - match snapshot_dir { - Ok(dir) => { - // Take our time to archive - lock is released - if let Err(e) = - this.snapshot_manager.archive_and_register(&dir) - { - error!(error = ?e, "Failed to archive snapshot"); - } - } - Err(e) => error!(error = ?e, "Snapshot creation failed"), - } - }); + let archive = snapshot_dir + .and_then(|dir| { + this.snapshot_manager.archive_and_register(&dir) + }) + .log_err(|| "failed to create accountsdb snapshot"); + AccountsDbSnapshotResult { checksum, archive } + }) } /// Ensures the database state is at most `slot`. @@ -424,9 +417,10 @@ impl AccountsDb { /// pubkey and serialized account data using xxHash3. Returns a 64-bit hash /// suitable for verifying state consistency across nodes. /// - /// Acquires the write lock to ensure a consistent snapshot of the state. - pub fn checksum(&self) -> u64 { - let _locked = self.write_lock.write(); + /// # Safety + /// the caller must acquire the write lock on accountsdb, so that + /// the state doesn't change during checksum computation + pub unsafe fn checksum(&self) -> u64 { let mut hasher = xxhash3_64::Hasher::new(); for (pubkey, acc) in self.iter_all() { let Some(borrowed) = acc.as_borrowed() else { @@ -535,6 +529,14 @@ impl AccountsDb { } } +/// Result of a snapshot operation spawned via [`AccountsDb::take_snapshot`]. +pub struct AccountsDbSnapshotResult { + /// State checksum computed at snapshot time. + pub checksum: u64, + /// Path to the created archive, or error if archiving failed. + pub archive: AccountsDbResult, +} + pub mod error; mod index; mod snapshot; diff --git a/magicblock-accounts-db/src/snapshot.rs b/magicblock-accounts-db/src/snapshot.rs index a74619e8a..57d4fd1fc 100644 --- a/magicblock-accounts-db/src/snapshot.rs +++ b/magicblock-accounts-db/src/snapshot.rs @@ -127,7 +127,7 @@ impl SnapshotManager { pub fn archive_and_register( &self, snapshot_dir: &Path, - ) -> AccountsDbResult<()> { + ) -> AccountsDbResult { let archive_path = snapshot_dir.with_extension(ARCHIVE_EXT); info!(archive_path = %archive_path.display(), "Archiving snapshot"); @@ -148,8 +148,8 @@ impl SnapshotManager { ) })?; - self.register_archive(archive_path); - Ok(()) + self.register_archive(archive_path.clone()); + Ok(archive_path) } /// Inserts an external snapshot archive received from network. diff --git a/magicblock-accounts-db/src/tests.rs b/magicblock-accounts-db/src/tests.rs index 837d06818..7a572ea19 100644 --- a/magicblock-accounts-db/src/tests.rs +++ b/magicblock-accounts-db/src/tests.rs @@ -12,6 +12,8 @@ const SPACE: usize = 73; const OWNER: Pubkey = Pubkey::new_from_array([23; 32]); const ACCOUNT_DATA: &[u8] = b"hello world?"; const INIT_DATA_LEN: usize = ACCOUNT_DATA.len(); +/// Default slot used for snapshots in tests. +const SNAPSHOT_SLOT: u64 = 1024; /// Verifies basic account insertion and retrieval. #[test] @@ -208,9 +210,10 @@ fn test_take_snapshot() { assert_eq!(env.slot(), 0); // Trigger Snapshot 1 - env.advance_slot(env.snapshot_frequency); - assert_eq!(env.slot(), env.snapshot_frequency); - assert!(env.snapshot_exists(env.snapshot_frequency)); + env.set_slot(SNAPSHOT_SLOT); + env.take_snapshot_and_wait(SNAPSHOT_SLOT); + assert_eq!(env.slot(), SNAPSHOT_SLOT); + assert!(env.snapshot_exists(SNAPSHOT_SLOT)); // Verify archive file exists (not directory) let archive_path = env @@ -218,7 +221,7 @@ fn test_take_snapshot() { .database_path() .parent() .unwrap() - .join(format!("snapshot-{:0>12}.tar.gz", env.snapshot_frequency)); + .join(format!("snapshot-{:0>12}.tar.gz", SNAPSHOT_SLOT)); assert!(archive_path.exists(), "Archive file should exist"); assert!( archive_path.is_file(), @@ -230,8 +233,10 @@ fn test_take_snapshot() { env.insert_account(&acc.pubkey, &acc.account).unwrap(); // Trigger Snapshot 2 - env.advance_slot(env.snapshot_frequency * 2); - assert!(env.snapshot_exists(env.snapshot_frequency * 2)); + let slot2 = SNAPSHOT_SLOT * 2; + env.set_slot(slot2); + env.take_snapshot_and_wait(slot2); + assert!(env.snapshot_exists(slot2)); } /// Verifies that orphan snapshot directories are cleaned up on startup. @@ -262,8 +267,8 @@ fn test_external_snapshot_fast_forward() { // Create an account and take a local snapshot let acc = env.create_and_insert_account(); - let snapshot_slot = env.snapshot_frequency; - env.advance_slot(snapshot_slot); + env.set_slot(SNAPSHOT_SLOT); + env.take_snapshot_and_wait(SNAPSHOT_SLOT); // Read the archive bytes let archive_path = env @@ -271,7 +276,7 @@ fn test_external_snapshot_fast_forward() { .database_path() .parent() .unwrap() - .join(format!("snapshot-{:0>12}.tar.gz", snapshot_slot)); + .join(format!("snapshot-{:0>12}.tar.gz", SNAPSHOT_SLOT)); let archive_bytes = std::fs::read(&archive_path).expect("Failed to read archive"); let pubkey = acc.pubkey; @@ -288,7 +293,7 @@ fn test_external_snapshot_fast_forward() { // Insert external snapshot (snapshot slot > current slot 0, should fast-forward) let fast_forwarded = new_db - .insert_external_snapshot(snapshot_slot, &archive_bytes) + .insert_external_snapshot(SNAPSHOT_SLOT, &archive_bytes) .unwrap(); assert!(fast_forwarded, "Should fast-forward when snapshot is newer"); @@ -308,8 +313,8 @@ fn test_external_snapshot_no_fast_forward() { // Create an account and take a local snapshot let acc = env.create_and_insert_account(); - let snapshot_slot = env.snapshot_frequency; - env.advance_slot(snapshot_slot); + env.set_slot(SNAPSHOT_SLOT); + env.take_snapshot_and_wait(SNAPSHOT_SLOT); // Read the archive bytes let archive_path = env @@ -317,7 +322,7 @@ fn test_external_snapshot_no_fast_forward() { .database_path() .parent() .unwrap() - .join(format!("snapshot-{:0>12}.tar.gz", snapshot_slot)); + .join(format!("snapshot-{:0>12}.tar.gz", SNAPSHOT_SLOT)); let archive_bytes = std::fs::read(&archive_path).expect("Failed to read archive"); let pubkey = acc.pubkey; @@ -331,14 +336,14 @@ fn test_external_snapshot_no_fast_forward() { }; let new_db = Arc::new(AccountsDb::new(&config, temp_dir.path(), 0).unwrap()); - new_db.set_slot(snapshot_slot + 1000); // Advance past snapshot slot + new_db.set_slot(SNAPSHOT_SLOT + 1000); // Advance past snapshot slot // Unwrap Arc to get mutable access let mut new_db = Arc::try_unwrap(new_db).unwrap(); // Insert external snapshot (current slot > snapshot slot, no fast-forward) let fast_forwarded = new_db - .insert_external_snapshot(snapshot_slot, &archive_bytes) + .insert_external_snapshot(SNAPSHOT_SLOT, &archive_bytes) .unwrap(); assert!( !fast_forwarded, @@ -353,7 +358,7 @@ fn test_external_snapshot_no_fast_forward() { ); // Now restore explicitly - new_db.restore_state_if_needed(snapshot_slot).unwrap(); + new_db.restore_state_if_needed(SNAPSHOT_SLOT).unwrap(); // Now the account should exist let restored = new_db.get_account(&pubkey); @@ -368,17 +373,17 @@ fn test_external_snapshot_no_fast_forward() { fn test_restore_from_snapshot() { let mut env = TestEnv::new(); let mut acc = env.create_and_insert_account(); - let snap_freq = env.snapshot_frequency; // Create Base Snapshot - env.advance_slot(snap_freq); + env.set_slot(SNAPSHOT_SLOT); + env.take_snapshot_and_wait(SNAPSHOT_SLOT); // Make changes after snapshot - env.advance_slot(snap_freq + 3); + env.set_slot(SNAPSHOT_SLOT + 3); let new_lamports = 999; acc.account.set_lamports(new_lamports); env.insert_account(&acc.pubkey, &acc.account).unwrap(); - env.advance_slot(snap_freq + 3); + env.set_slot(SNAPSHOT_SLOT + 6); // Verify update persisted in current state assert_eq!( @@ -387,7 +392,7 @@ fn test_restore_from_snapshot() { ); // Rollback to before the update - env = env.restore_to_slot(snap_freq); + env = env.restore_to_slot(SNAPSHOT_SLOT); let restored_acc = env.get_account(&acc.pubkey).unwrap(); assert_eq!( @@ -408,14 +413,17 @@ fn test_get_all_accounts_after_rollback() { for i in 0..=ITERS { let acc = env.create_and_insert_account(); pks.push(acc.pubkey); - env.advance_slot(i); + env.set_slot(i); } + // Take a snapshot at ITERS + env.take_snapshot_and_wait(ITERS); + // Add accounts after the restore point let mut post_snap_pks = vec![]; - for i in ITERS..ITERS + env.snapshot_frequency { + for i in ITERS..ITERS + 100 { let acc = env.create_and_insert_account(); - env.advance_slot(i + 1); + env.set_slot(i + 1); post_snap_pks.push(acc.pubkey); } @@ -627,9 +635,15 @@ fn test_checksum_deterministic_across_dbs() { db2.insert_account(&pubkey, &account).unwrap(); } + // Acquire write locks before computing checksums + let lock1 = db1.write_lock(); + let lock2 = db2.write_lock(); + let _guard1 = lock1.write(); + let _guard2 = lock2.write(); + assert_eq!( - db1.checksum(), - db2.checksum(), + unsafe { db1.checksum() }, + unsafe { db2.checksum() }, "checksums must match for identical state" ); } @@ -646,28 +660,37 @@ fn test_checksum_detects_state_change() { }) .collect(); - let original_checksum = env.checksum(); + let lock = env.write_lock(); + let _guard = lock.write(); + let original_checksum = unsafe { env.checksum() }; + drop(_guard); // Modify a single account's data accounts[5].1.data_as_mut_slice()[0] ^= 0xFF; env.insert_account(&accounts[5].0, &accounts[5].1).unwrap(); - assert_ne!( - env.checksum(), - original_checksum, - "checksum must detect single account modification" - ); + { + let _guard = lock.write(); + assert_ne!( + unsafe { env.checksum() }, + original_checksum, + "checksum must detect single account modification" + ); + } // Modify lamports on a different account accounts[10].1.set_lamports(1_000_000); env.insert_account(&accounts[10].0, &accounts[10].1) .unwrap(); - assert_ne!( - env.checksum(), - original_checksum, - "checksum must detect lamport change" - ); + { + let _guard = lock.write(); + assert_ne!( + unsafe { env.checksum() }, + original_checksum, + "checksum must detect lamport change" + ); + } } // ============================================================== @@ -738,17 +761,13 @@ impl TestEnv { fn advance_slot(&self, target_slot: u64) { self.adb.set_slot(target_slot); + } - // Simple spin-wait if we expect a snapshot trigger. - // This ensures the background thread has started and finished archiving. - // Archiving takes longer than just creating a directory (compression + I/O). - if target_slot.is_multiple_of(self.adb.snapshot_frequency) { - let mut retries = 0; - while !self.adb.snapshot_exists(target_slot) && retries < 200 { - std::thread::sleep(std::time::Duration::from_millis(50)); - retries += 1; - } - } + /// Takes a snapshot and waits for it to complete. + fn take_snapshot_and_wait(&self, slot: u64) { + let handle = self.adb.take_snapshot(slot); + handle.join().expect("Snapshot thread panicked"); + assert!(self.adb.snapshot_exists(slot), "Snapshot should exist"); } fn restore_to_slot(mut self, slot: u64) -> Self { diff --git a/magicblock-config/src/config/accounts.rs b/magicblock-config/src/config/accounts.rs index 25629566d..db2999d04 100644 --- a/magicblock-config/src/config/accounts.rs +++ b/magicblock-config/src/config/accounts.rs @@ -18,9 +18,6 @@ pub struct AccountsDbConfig { /// Maximum number of historical snapshots to retain. pub max_snapshots: u16, - /// Number of slots between generating a new snapshot. - pub snapshot_frequency: u64, - /// If true, wipes the accounts database on startup. pub reset: bool, } @@ -32,7 +29,6 @@ impl Default for AccountsDbConfig { database_size: consts::DEFAULT_ACCOUNTS_DB_SIZE, index_size: consts::DEFAULT_ACCOUNTS_INDEX_SIZE, max_snapshots: consts::DEFAULT_ACCOUNTS_MAX_SNAPSHOTS, - snapshot_frequency: consts::DEFAULT_ACCOUNTS_SNAPSHOT_FREQUENCY, reset: false, } } diff --git a/magicblock-config/src/consts.rs b/magicblock-config/src/consts.rs index e164d8485..ac8696719 100644 --- a/magicblock-config/src/consts.rs +++ b/magicblock-config/src/consts.rs @@ -47,9 +47,6 @@ pub const DEFAULT_ACCOUNTS_INDEX_SIZE: usize = 16 * 1024 * 1024; /// Maximum number of account snapshots to retain pub const DEFAULT_ACCOUNTS_MAX_SNAPSHOTS: u16 = 4; -/// Frequency of account snapshots (every N slots) -pub const DEFAULT_ACCOUNTS_SNAPSHOT_FREQUENCY: u64 = 1024; - /// Ledger Defaults /// Default block time in milliseconds pub const DEFAULT_LEDGER_BLOCK_TIME_MS: u64 = 50; diff --git a/magicblock-config/src/tests.rs b/magicblock-config/src/tests.rs index 12266ce9c..d8c4b4976 100644 --- a/magicblock-config/src/tests.rs +++ b/magicblock-config/src/tests.rs @@ -417,7 +417,6 @@ fn test_example_config_full_coverage() { assert!(matches!(config.accountsdb.block_size, BlockSize::Block256)); assert_eq!(config.accountsdb.index_size, 16_777_216); assert_eq!(config.accountsdb.max_snapshots, 4); - assert_eq!(config.accountsdb.snapshot_frequency, 1024); assert!(!config.accountsdb.reset); // ======================================================================== @@ -506,7 +505,6 @@ fn test_env_vars_full_coverage() { EnvVarGuard::new("MBV_ACCOUNTSDB__BLOCK_SIZE", "block512"), EnvVarGuard::new("MBV_ACCOUNTSDB__INDEX_SIZE", "2048"), EnvVarGuard::new("MBV_ACCOUNTSDB__MAX_SNAPSHOTS", "10"), - EnvVarGuard::new("MBV_ACCOUNTSDB__SNAPSHOT_FREQUENCY", "500"), EnvVarGuard::new("MBV_ACCOUNTSDB__RESET", "true"), // --- Ledger --- EnvVarGuard::new("MBV_LEDGER__BLOCK_TIME", "200ms"), @@ -568,7 +566,6 @@ fn test_env_vars_full_coverage() { assert!(matches!(config.accountsdb.block_size, BlockSize::Block512)); assert_eq!(config.accountsdb.index_size, 2048); assert_eq!(config.accountsdb.max_snapshots, 10); - assert_eq!(config.accountsdb.snapshot_frequency, 500); assert!(config.accountsdb.reset); // Ledger From 182b7307056e5c423a7e864ec2dc43326ef5a942 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Wed, 11 Mar 2026 13:36:38 +0400 Subject: [PATCH 3/5] fix: remove snapshot frequency from test-integration --- test-integration/Cargo.lock | 2 ++ test-integration/configs/api-conf.ephem.toml | 2 -- test-integration/configs/chainlink-conf.devnet.toml | 2 -- test-integration/configs/claim-fees-test.toml | 1 - test-integration/configs/cloning-conf.devnet.toml | 2 -- test-integration/configs/cloning-conf.ephem.toml | 2 -- test-integration/configs/committor-conf.devnet.toml | 2 -- test-integration/configs/config-conf.devnet.toml | 2 -- test-integration/configs/restore-ledger-conf.devnet.toml | 2 -- .../configs/schedulecommit-conf-fees.ephem.toml | 2 -- test-integration/configs/schedulecommit-conf.devnet.toml | 2 -- .../configs/schedulecommit-conf.ephem.frequent-commits.toml | 2 -- test-integration/configs/schedulecommit-conf.ephem.toml | 2 -- test-integration/configs/validator-offline.devnet.toml | 2 -- test-integration/test-ledger-restore/src/lib.rs | 6 +----- 15 files changed, 3 insertions(+), 30 deletions(-) diff --git a/test-integration/Cargo.lock b/test-integration/Cargo.lock index 5686dc552..bf7249bf4 100644 --- a/test-integration/Cargo.lock +++ b/test-integration/Cargo.lock @@ -3330,6 +3330,7 @@ dependencies = [ name = "magicblock-accounts-db" version = "0.8.2" dependencies = [ + "flate2", "lmdb-rkv", "magicblock-config", "memmap2 0.9.9", @@ -3337,6 +3338,7 @@ dependencies = [ "reflink-copy", "solana-account", "solana-pubkey", + "tar", "thiserror 1.0.69", "tracing", "twox-hash", diff --git a/test-integration/configs/api-conf.ephem.toml b/test-integration/configs/api-conf.ephem.toml index 77f344c7e..0fec3f375 100644 --- a/test-integration/configs/api-conf.ephem.toml +++ b/test-integration/configs/api-conf.ephem.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/chainlink-conf.devnet.toml b/test-integration/configs/chainlink-conf.devnet.toml index 16da71f89..171c4d3f2 100644 --- a/test-integration/configs/chainlink-conf.devnet.toml +++ b/test-integration/configs/chainlink-conf.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 20485760 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/claim-fees-test.toml b/test-integration/configs/claim-fees-test.toml index 65acb1b8e..cca8853c1 100644 --- a/test-integration/configs/claim-fees-test.toml +++ b/test-integration/configs/claim-fees-test.toml @@ -12,7 +12,6 @@ database-size = 1048576000 block-size = "block256" index-size = 2048576 max-snapshots = 7 -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/cloning-conf.devnet.toml b/test-integration/configs/cloning-conf.devnet.toml index c42d22316..763f4a579 100644 --- a/test-integration/configs/cloning-conf.devnet.toml +++ b/test-integration/configs/cloning-conf.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 20485760 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/cloning-conf.ephem.toml b/test-integration/configs/cloning-conf.ephem.toml index 5673ac1eb..beb1ec4ac 100644 --- a/test-integration/configs/cloning-conf.ephem.toml +++ b/test-integration/configs/cloning-conf.ephem.toml @@ -24,8 +24,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/committor-conf.devnet.toml b/test-integration/configs/committor-conf.devnet.toml index 62fc3ef10..97f871b0d 100644 --- a/test-integration/configs/committor-conf.devnet.toml +++ b/test-integration/configs/committor-conf.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/config-conf.devnet.toml b/test-integration/configs/config-conf.devnet.toml index 5b6bcdee7..40925ba5b 100644 --- a/test-integration/configs/config-conf.devnet.toml +++ b/test-integration/configs/config-conf.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/restore-ledger-conf.devnet.toml b/test-integration/configs/restore-ledger-conf.devnet.toml index d1007e412..79f936369 100644 --- a/test-integration/configs/restore-ledger-conf.devnet.toml +++ b/test-integration/configs/restore-ledger-conf.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/schedulecommit-conf-fees.ephem.toml b/test-integration/configs/schedulecommit-conf-fees.ephem.toml index daf1b20f0..04a62c568 100644 --- a/test-integration/configs/schedulecommit-conf-fees.ephem.toml +++ b/test-integration/configs/schedulecommit-conf-fees.ephem.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/schedulecommit-conf.devnet.toml b/test-integration/configs/schedulecommit-conf.devnet.toml index 2862ce1f4..9da9049ca 100644 --- a/test-integration/configs/schedulecommit-conf.devnet.toml +++ b/test-integration/configs/schedulecommit-conf.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 reset = true [ledger] diff --git a/test-integration/configs/schedulecommit-conf.ephem.frequent-commits.toml b/test-integration/configs/schedulecommit-conf.ephem.frequent-commits.toml index 5a2d72775..8796d82e4 100644 --- a/test-integration/configs/schedulecommit-conf.ephem.frequent-commits.toml +++ b/test-integration/configs/schedulecommit-conf.ephem.frequent-commits.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/schedulecommit-conf.ephem.toml b/test-integration/configs/schedulecommit-conf.ephem.toml index e10b6536a..8a87405d9 100644 --- a/test-integration/configs/schedulecommit-conf.ephem.toml +++ b/test-integration/configs/schedulecommit-conf.ephem.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/configs/validator-offline.devnet.toml b/test-integration/configs/validator-offline.devnet.toml index d4a931f81..0b9b76a3a 100644 --- a/test-integration/configs/validator-offline.devnet.toml +++ b/test-integration/configs/validator-offline.devnet.toml @@ -21,8 +21,6 @@ block-size = "block256" # possible values block128 | block256 | block512 index-size = 2048576 # max number of snapshots to keep around max-snapshots = 7 -# how frequently (slot-wise) we should take snapshots -snapshot-frequency = 1024 [ledger] reset = true diff --git a/test-integration/test-ledger-restore/src/lib.rs b/test-integration/test-ledger-restore/src/lib.rs index bff15fe01..6927037ca 100644 --- a/test-integration/test-ledger-restore/src/lib.rs +++ b/test-integration/test-ledger-restore/src/lib.rs @@ -58,10 +58,7 @@ pub fn setup_offline_validator( reset_ledger: bool, skip_keypair_match_check: bool, ) -> (TempDir, Child, IntegrationTestContext) { - let accountsdb_config = AccountsDbConfig { - snapshot_frequency: SNAPSHOT_FREQUENCY, - ..Default::default() - }; + let accountsdb_config = AccountsDbConfig::default(); let validator_config = ValidatorConfig::default(); @@ -134,7 +131,6 @@ pub fn setup_validator_with_local_remote_and_resume_strategy( loaded_accounts: &LoadedAccounts, ) -> (TempDir, Child, IntegrationTestContext) { let accountsdb_config = AccountsDbConfig { - snapshot_frequency: SNAPSHOT_FREQUENCY, reset: reset_ledger, ..Default::default() }; From 71a35670778905e48a890481fefc65890bbec771 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Wed, 11 Mar 2026 14:52:19 +0400 Subject: [PATCH 4/5] fix: corrent snapshot and archival logic --- magicblock-accounts-db/src/lib.rs | 77 ++++++++++++-------------- magicblock-accounts-db/src/snapshot.rs | 40 +++++++++---- magicblock-accounts-db/src/tests.rs | 14 +++-- 3 files changed, 76 insertions(+), 55 deletions(-) diff --git a/magicblock-accounts-db/src/lib.rs b/magicblock-accounts-db/src/lib.rs index d0ffc0a71..bbe10c482 100644 --- a/magicblock-accounts-db/src/lib.rs +++ b/magicblock-accounts-db/src/lib.rs @@ -1,10 +1,4 @@ -use std::{ - fs, - hash::Hasher, - path::{Path, PathBuf}, - sync::Arc, - thread::{self, JoinHandle}, -}; +use std::{fs, hash::Hasher, path::Path, sync::Arc, thread}; use error::{AccountsDbError, LogErr}; use index::{ @@ -12,7 +6,7 @@ use index::{ }; use lmdb::{RwTransaction, Transaction}; use magicblock_config::config::AccountsDbConfig; -use parking_lot::RwLock; +use parking_lot::{RwLock, RwLockWriteGuard}; use solana_account::{ cow::AccountBorrowed, AccountSharedData, ReadableAccount, }; @@ -283,45 +277,45 @@ impl AccountsDb { self.storage.slot() } - /// Updates the current slot + /// Updates the current slot. #[inline(always)] pub fn set_slot(self: &Arc, slot: u64) { self.storage.update_slot(slot); } - /// Spawns a background thread to take a snapshot. + /// Takes a database snapshot for the given slot. + /// /// The snapshot is created in two phases: - /// 1. Create snapshot directory (with write lock held) - /// 2. Archive directory to tar.gz and register (lock released) - pub fn take_snapshot( - self: &Arc, - slot: u64, - ) -> JoinHandle { + /// 1. **Synchronous**: Flush data, compute checksum, create snapshot directory + /// (with write lock held to ensure consistency) + /// 2. **Background**: Archive directory to tar.gz and register + /// + /// Returns the state checksum computed at snapshot time. + /// The checksum can be used to verify state consistency across nodes. + pub fn take_snapshot(self: &Arc, slot: u64) -> u64 { let this = self.clone(); - thread::spawn(move || { - // Phase 1: Create snapshot directory (with write lock) - let locked = this.write_lock.write(); - this.flush(); - // SAFETY: - // we have acquired the write lock above - let checksum = unsafe { this.checksum() }; - let used_storage = this.storage.active_segment(); - - let snapshot_dir = this.snapshot_manager.create_snapshot_dir( - slot, - used_storage, - locked, // Lock released when this returns - ); + // Phase 1: Create snapshot directory (with write lock) + let locked = this.write_lock.write(); + this.flush(); + // SAFETY: + // we have acquired the write lock above + let checksum = unsafe { this.checksum() }; + let used_storage = this.storage.active_segment(); + let snapshot_dir = this + .snapshot_manager + .create_snapshot_dir(slot, used_storage); + drop(locked); + thread::spawn(move || { // Phase 2: Archive directory (no lock needed) - let archive = snapshot_dir + let _ = snapshot_dir .and_then(|dir| { this.snapshot_manager.archive_and_register(&dir) }) .log_err(|| "failed to create accountsdb snapshot"); - AccountsDbSnapshotResult { checksum, archive } - }) + }); + checksum } /// Ensures the database state is at most `slot`. @@ -431,6 +425,15 @@ impl AccountsDb { } hasher.finish() } + + /// Acquires exclusive write access to the database. + /// + /// The returned guard blocks all other write operations while held. + /// Use this when you need to ensure the database state doesn't change + /// during operations like checksum computation. + pub fn lock_database(&self) -> RwLockWriteGuard<'_, ()> { + self.write_lock.write() + } } impl AccountsBank for AccountsDb { @@ -529,14 +532,6 @@ impl AccountsDb { } } -/// Result of a snapshot operation spawned via [`AccountsDb::take_snapshot`]. -pub struct AccountsDbSnapshotResult { - /// State checksum computed at snapshot time. - pub checksum: u64, - /// Path to the created archive, or error if archiving failed. - pub archive: AccountsDbResult, -} - pub mod error; mod index; mod snapshot; diff --git a/magicblock-accounts-db/src/snapshot.rs b/magicblock-accounts-db/src/snapshot.rs index 57d4fd1fc..585e3cf1f 100644 --- a/magicblock-accounts-db/src/snapshot.rs +++ b/magicblock-accounts-db/src/snapshot.rs @@ -8,7 +8,7 @@ use std::{ }; use flate2::{read::GzDecoder, write::GzEncoder, Compression}; -use parking_lot::{Mutex, RwLockWriteGuard}; +use parking_lot::Mutex; use tar::{Archive, Builder}; use tracing::{error, info, warn}; @@ -48,12 +48,10 @@ impl SnapshotStrategy { src: &Path, dst: &Path, memory_state: Vec, - lock: RwLockWriteGuard<()>, ) -> io::Result<()> { match self { Self::Reflink => fs_backend::reflink_dir(src, dst), Self::LegacyCopy => { - drop(lock); // Release lock before slow I/O fs_backend::deep_copy_dir(src, dst, &memory_state) } } @@ -106,7 +104,6 @@ impl SnapshotManager { &self, slot: u64, active_mem: &[u8], - lock: RwLockWriteGuard<()>, ) -> AccountsDbResult { self.prune_registry(); @@ -117,23 +114,27 @@ impl SnapshotManager { .unwrap_or_default(); self.strategy - .execute(&self.db_path, &snap_path, memory_capture, lock) + .execute(&self.db_path, &snap_path, memory_capture) .log_err(|| "Snapshot failed")?; Ok(snap_path) } /// Archives the snapshot directory to `.tar.gz` and removes the directory. + /// + /// Uses atomic write: writes to `.tmp` first, then renames to final path. pub fn archive_and_register( &self, snapshot_dir: &Path, ) -> AccountsDbResult { let archive_path = snapshot_dir.with_extension(ARCHIVE_EXT); + let tmp_path = archive_path.with_extension("tmp"); info!(archive_path = %archive_path.display(), "Archiving snapshot"); - let file = File::create(&archive_path).log_err(|| { - format!("Failed to create archive at {}", archive_path.display()) + // Write to temporary file first + let file = File::create(&tmp_path).log_err(|| { + format!("Failed to create temp archive at {}", tmp_path.display()) })?; let enc = GzEncoder::new(file, Compression::fast()); let mut tar = Builder::new(enc); @@ -141,6 +142,15 @@ impl SnapshotManager { .log_err(|| "Failed to append directory to tar")?; tar.finish().log_err(|| "Failed to finalize tar archive")?; + // Atomically rename to final path + fs::rename(&tmp_path, &archive_path).log_err(|| { + format!( + "Failed to rename {} to {}", + tmp_path.display(), + archive_path.display() + ) + })?; + fs::remove_dir_all(snapshot_dir).log_err(|| { format!( "Failed to remove snapshot directory {}", @@ -175,14 +185,24 @@ impl SnapshotManager { info!(slot, "Inserting external snapshot"); - // Write archive to disk - let mut file = File::create(&archive_path).log_err(|| { - format!("Failed to create archive at {}", archive_path.display()) + // Write to temporary file first, then atomically rename + let tmp_path = archive_path.with_extension("tmp"); + let mut file = File::create(&tmp_path).log_err(|| { + format!("Failed to create temp archive at {}", tmp_path.display()) })?; file.write_all(archive_bytes) .log_err(|| "Failed to write archive bytes")?; file.sync_all() .log_err(|| "Failed to sync archive to disk")?; + drop(file); + + fs::rename(&tmp_path, &archive_path).log_err(|| { + format!( + "Failed to rename {} to {}", + tmp_path.display(), + archive_path.display() + ) + })?; // Fast-forward if snapshot is newer than current state if slot > current_slot { diff --git a/magicblock-accounts-db/src/tests.rs b/magicblock-accounts-db/src/tests.rs index 7a572ea19..2d3c016a4 100644 --- a/magicblock-accounts-db/src/tests.rs +++ b/magicblock-accounts-db/src/tests.rs @@ -763,11 +763,17 @@ impl TestEnv { self.adb.set_slot(target_slot); } - /// Takes a snapshot and waits for it to complete. - fn take_snapshot_and_wait(&self, slot: u64) { - let handle = self.adb.take_snapshot(slot); - handle.join().expect("Snapshot thread panicked"); + /// Takes a snapshot and waits for archiving to complete. + fn take_snapshot_and_wait(&self, slot: u64) -> u64 { + let checksum = self.adb.take_snapshot(slot); + // Wait for background archiving to complete + let mut retries = 0; + while !self.adb.snapshot_exists(slot) && retries < 200 { + std::thread::sleep(std::time::Duration::from_millis(50)); + retries += 1; + } assert!(self.adb.snapshot_exists(slot), "Snapshot should exist"); + checksum } fn restore_to_slot(mut self, slot: u64) -> Self { From 2a3b1749c148d9348bb71c8b9e2c5c3798d19fd5 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Wed, 11 Mar 2026 15:03:56 +0400 Subject: [PATCH 5/5] fix: post review comments --- magicblock-accounts-db/src/lib.rs | 2 +- magicblock-accounts-db/src/snapshot.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/magicblock-accounts-db/src/lib.rs b/magicblock-accounts-db/src/lib.rs index bbe10c482..86c75d9cd 100644 --- a/magicblock-accounts-db/src/lib.rs +++ b/magicblock-accounts-db/src/lib.rs @@ -279,7 +279,7 @@ impl AccountsDb { /// Updates the current slot. #[inline(always)] - pub fn set_slot(self: &Arc, slot: u64) { + pub fn set_slot(&self, slot: u64) { self.storage.update_slot(slot); } diff --git a/magicblock-accounts-db/src/snapshot.rs b/magicblock-accounts-db/src/snapshot.rs index 585e3cf1f..8a4552ec2 100644 --- a/magicblock-accounts-db/src/snapshot.rs +++ b/magicblock-accounts-db/src/snapshot.rs @@ -105,8 +105,6 @@ impl SnapshotManager { slot: u64, active_mem: &[u8], ) -> AccountsDbResult { - self.prune_registry(); - let snap_path = self.slot_to_dir_path(slot); let memory_capture = matches!(self.strategy, SnapshotStrategy::LegacyCopy) @@ -116,6 +114,7 @@ impl SnapshotManager { self.strategy .execute(&self.db_path, &snap_path, memory_capture) .log_err(|| "Snapshot failed")?; + self.prune_registry(); Ok(snap_path) }