diff --git a/accounts-db/src/tiered_storage/file.rs b/accounts-db/src/tiered_storage/file.rs index fae9a6a4d1513b..7f234527b154b0 100644 --- a/accounts-db/src/tiered_storage/file.rs +++ b/accounts-db/src/tiered_storage/file.rs @@ -10,7 +10,7 @@ use { }; /// The ending 8 bytes of a valid tiered account storage file. -pub const FOOTER_MAGIC_NUMBER: u64 = 0x502A2AB5; // SOLALABS -> SOLANA LABS +pub const FILE_MAGIC_NUMBER: u64 = 0xA72A2AB5; // ANZALABS #[derive(Debug, PartialEq, Eq, Clone, Copy, Pod, Zeroable)] #[repr(C)] @@ -21,7 +21,7 @@ const _: () = assert!(std::mem::size_of::() == 8); impl Default for TieredStorageMagicNumber { fn default() -> Self { - Self(FOOTER_MAGIC_NUMBER) + Self(FILE_MAGIC_NUMBER) } } @@ -136,19 +136,70 @@ impl TieredStorageFile { #[cfg(test)] mod tests { - use {super::TieredStorageFile, tempfile::TempDir}; + use { + crate::tiered_storage::{ + error::TieredStorageError, + file::{TieredStorageFile, FILE_MAGIC_NUMBER}, + }, + std::{fs::OpenOptions, path::Path}, + tempfile::TempDir, + }; + + fn generate_test_file_with_number(path: impl AsRef, number: u64) { + // Generate a new temp path that is guaranteed to NOT already have a file. + let file = TieredStorageFile::new_writable(&path).unwrap(); + file.write_pod(&number).unwrap(); + } #[test] - #[should_panic(expected = "MagicNumberMismatch")] fn test_magic_number() { - // Generate a new temp path that is guaranteed to NOT already have a file. let temp_dir = TempDir::new().unwrap(); - let path = temp_dir.path().join("test_magic_number"); - { - let file = TieredStorageFile::new_writable(&path).unwrap(); - let unmagic_number: u64 = 0x12345678; - file.write_pod(&unmagic_number).unwrap(); - } + let path = temp_dir.path().join("test_check_magic_number"); + generate_test_file_with_number(&path, FILE_MAGIC_NUMBER); + let file = TieredStorageFile( + OpenOptions::new() + .read(true) + .create(false) + .open(&path) + .unwrap(), + ); + assert!(matches!(file.check_magic_number(), Ok(()))); + } + + #[test] + fn test_check_magic_number_mismatch() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("test_check_magic_number_mismatch"); + generate_test_file_with_number(&path, !FILE_MAGIC_NUMBER); + let file = TieredStorageFile( + OpenOptions::new() + .read(true) + .create(false) + .open(&path) + .unwrap(), + ); + assert!(matches!( + file.check_magic_number(), + Err(TieredStorageError::MagicNumberMismatch(_, _)) + )); + } + + #[test] + fn test_new_readonly() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("test_new_readonly"); + generate_test_file_with_number(&path, FILE_MAGIC_NUMBER); TieredStorageFile::new_readonly(&path).unwrap(); } + + #[test] + fn test_new_readonly_mismatch() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("test_new_readonly_mismatch"); + generate_test_file_with_number(&path, !FILE_MAGIC_NUMBER); + assert!(matches!( + TieredStorageFile::new_readonly(&path), + Err(TieredStorageError::MagicNumberMismatch(_, _)) + )); + } } diff --git a/accounts-db/src/tiered_storage/footer.rs b/accounts-db/src/tiered_storage/footer.rs index d10ebee8c09714..58b42a387f78b2 100644 --- a/accounts-db/src/tiered_storage/footer.rs +++ b/accounts-db/src/tiered_storage/footer.rs @@ -117,7 +117,7 @@ pub struct TieredStorageFooter { /// The size of the footer including the magic number. pub footer_size: u64, // This field is persisted in the storage but not in this struct. - // The number should match FOOTER_MAGIC_NUMBER. + // The number should match FILE_MAGIC_NUMBER. // pub magic_number: u64, } diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index d18bdf483434f2..43bd4f2a7d67a7 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -346,13 +346,7 @@ pub struct HotStorageReader { } impl HotStorageReader { - /// Constructs a HotStorageReader from the specified path. - pub fn new_from_path(path: impl AsRef) -> TieredStorageResult { - let file = TieredStorageFile::new_readonly(&path)?; - Self::new_from_file(file) - } - - pub fn new_from_file(file: TieredStorageFile) -> TieredStorageResult { + pub fn new(file: TieredStorageFile) -> TieredStorageResult { let mmap = unsafe { MmapOptions::new().map(&file.0)? }; // Here we are copying the footer, as accessing any data in a // TieredStorage instance requires accessing its Footer. @@ -903,7 +897,8 @@ pub mod tests { // Reopen the same storage, and expect the persisted footer is // the same as what we have written. { - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); assert_eq!(expected_footer, *hot_storage.footer()); } } @@ -949,7 +944,8 @@ pub mod tests { footer.write_footer_block(&file).unwrap(); } - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); for (offset, expected_meta) in account_offsets.iter().zip(hot_account_metas.iter()) { let meta = hot_storage.get_account_meta_from_offset(*offset).unwrap(); @@ -979,7 +975,8 @@ pub mod tests { footer.write_footer_block(&file).unwrap(); } - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); let offset = HotAccountOffset::new(footer.index_block_offset as usize).unwrap(); // Read from index_block_offset, which offset doesn't belong to // account blocks. Expect assert failure here @@ -1030,7 +1027,8 @@ pub mod tests { footer.write_footer_block(&file).unwrap(); } - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); for (i, index_writer_entry) in index_writer_entries.iter().enumerate() { let account_offset = hot_storage .get_account_offset(IndexOffset(i as u32)) @@ -1079,7 +1077,8 @@ pub mod tests { footer.write_footer_block(&file).unwrap(); } - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); for (i, address) in addresses.iter().enumerate() { assert_eq!( hot_storage @@ -1153,7 +1152,8 @@ pub mod tests { footer.write_footer_block(&file).unwrap(); } - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); // First, verify whether we can find the expected owners. let mut owner_candidates = owner_addresses.clone(); @@ -1285,7 +1285,8 @@ pub mod tests { footer.write_footer_block(&file).unwrap(); } - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); for i in 0..NUM_ACCOUNTS { let (stored_meta, next) = hot_storage @@ -1366,10 +1367,10 @@ pub mod tests { writer.write_accounts(&storable_accounts, 0).unwrap() }; - let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); + let file = TieredStorageFile::new_readonly(&path).unwrap(); + let hot_storage = HotStorageReader::new(file).unwrap(); let num_accounts = account_data_sizes.len(); - for i in 0..num_accounts { let (stored_meta, next) = hot_storage .get_account(IndexOffset(i as u32)) diff --git a/accounts-db/src/tiered_storage/readable.rs b/accounts-db/src/tiered_storage/readable.rs index 0a4b5ea16d1e23..a121feba3b3de1 100644 --- a/accounts-db/src/tiered_storage/readable.rs +++ b/accounts-db/src/tiered_storage/readable.rs @@ -26,7 +26,7 @@ impl TieredStorageReader { let file = TieredStorageFile::new_readonly(&path)?; let footer = TieredStorageFooter::new_from_footer_block(&file)?; match footer.account_meta_format { - AccountMetaFormat::Hot => Ok(Self::Hot(HotStorageReader::new_from_file(file)?)), + AccountMetaFormat::Hot => Ok(Self::Hot(HotStorageReader::new(file)?)), } }