Skip to content

Commit

Permalink
[TieredStorage] Put commonly used test functions into test_utils.rs (s…
Browse files Browse the repository at this point in the history
…olana-labs#35065)

There're some test functions that have been used in different
mod in TieredStorage.  It's better to have one same place for
all tiere-storage related test functions.

Created test_utils.rs under /tiered_storage and move test-related
functions into it.

Existing tests.
  • Loading branch information
yhchiang-sol committed Feb 13, 2024
1 parent d71bf89 commit 909f73f
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 160 deletions.
76 changes: 12 additions & 64 deletions accounts-db/src/tiered_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod meta;
pub mod mmap_utils;
pub mod owners;
pub mod readable;
mod test_utils;
pub mod writer;

use {
Expand All @@ -27,7 +28,7 @@ use {
solana_sdk::account::ReadableAccount,
std::{
borrow::Borrow,
fs::OpenOptions,
fs::{self, OpenOptions},
path::{Path, PathBuf},
sync::OnceLock,
},
Expand All @@ -54,8 +55,11 @@ pub struct TieredStorage {

impl Drop for TieredStorage {
fn drop(&mut self) {
if let Err(err) = fs_err::remove_file(&self.path) {
panic!("TieredStorage failed to remove backing storage file: {err}");
if let Err(err) = fs::remove_file(&self.path) {
panic!(
"TieredStorage failed to remove backing storage file '{}': {err}",
self.path.display(),
);
}
}
}
Expand Down Expand Up @@ -157,24 +161,20 @@ impl TieredStorage {
mod tests {
use {
super::*,
crate::account_storage::meta::{StoredAccountMeta, StoredMeta, StoredMetaWriteVersion},
crate::account_storage::meta::StoredMetaWriteVersion,
footer::{TieredStorageFooter, TieredStorageMagicNumber},
hot::HOT_FORMAT,
index::IndexOffset,
owners::OWNER_NO_OWNER,
solana_accounts_db::rent_collector::RENT_EXEMPT_RENT_EPOCH,
solana_sdk::{
account::{Account, AccountSharedData},
clock::Slot,
hash::Hash,
pubkey::Pubkey,
account::AccountSharedData, clock::Slot, hash::Hash, pubkey::Pubkey,
system_instruction::MAX_PERMITTED_DATA_LENGTH,
},
std::{
collections::{HashMap, HashSet},
mem::ManuallyDrop,
},
tempfile::tempdir,
test_utils::{create_test_account, verify_test_account},
};

impl TieredStorage {
Expand Down Expand Up @@ -307,58 +307,6 @@ mod tests {
assert!(!tiered_storage_path.try_exists().unwrap());
}

/// Create a test account based on the specified seed.
fn create_account(seed: u64) -> (StoredMeta, AccountSharedData) {
let data_byte = seed as u8;
let account = Account {
lamports: seed,
data: std::iter::repeat(data_byte).take(seed as usize).collect(),
owner: Pubkey::new_unique(),
executable: seed % 2 > 0,
rent_epoch: if seed % 3 > 0 {
seed
} else {
RENT_EXEMPT_RENT_EPOCH
},
};

let stored_meta = StoredMeta {
write_version_obsolete: StoredMetaWriteVersion::default(),
pubkey: Pubkey::new_unique(),
data_len: seed,
};
(stored_meta, AccountSharedData::from(account))
}

fn verify_account(
stored_meta: &StoredAccountMeta<'_>,
account: Option<&impl ReadableAccount>,
account_hash: &AccountHash,
) {
let (lamports, owner, data, executable, account_hash) = account
.map(|acc| {
(
acc.lamports(),
acc.owner(),
acc.data(),
acc.executable(),
// only persist rent_epoch for those rent-paying accounts
Some(*account_hash),
)
})
.unwrap_or((0, &OWNER_NO_OWNER, &[], false, None));

assert_eq!(stored_meta.lamports(), lamports);
assert_eq!(stored_meta.data().len(), data.len());
assert_eq!(stored_meta.data(), data);
assert_eq!(stored_meta.executable(), executable);
assert_eq!(stored_meta.owner(), owner);
assert_eq!(
*stored_meta.hash(),
account_hash.unwrap_or(AccountHash(Hash::default()))
);
}

/// The helper function for all write_accounts tests.
/// Currently only supports hot accounts.
fn do_test_write_accounts(
Expand All @@ -368,7 +316,7 @@ mod tests {
) {
let accounts: Vec<_> = account_data_sizes
.iter()
.map(|size| create_account(*size))
.map(|size| create_test_account(*size))
.collect();

let account_refs: Vec<_> = accounts
Expand Down Expand Up @@ -412,7 +360,7 @@ mod tests {
let mut verified_accounts = HashSet::new();
while let Some((stored_meta, next)) = reader.get_account(index_offset).unwrap() {
if let Some((account, account_hash)) = expected_accounts_map.get(stored_meta.pubkey()) {
verify_account(&stored_meta, *account, account_hash);
verify_test_account(&stored_meta, *account, stored_meta.pubkey(), account_hash);
verified_accounts.insert(stored_meta.pubkey());
}
index_offset = next;
Expand Down
2 changes: 1 addition & 1 deletion accounts-db/src/tiered_storage/byte_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl ByteBlockWriter {

/// Write all the Some fields of the specified AccountMetaOptionalFields.
///
/// Note that the existance of each optional field is stored separately in
/// Note that the existence of each optional field is stored separately in
/// AccountMetaFlags.
pub fn write_optional_fields(
&mut self,
Expand Down
19 changes: 11 additions & 8 deletions accounts-db/src/tiered_storage/footer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ pub const FOOTER_MAGIC_NUMBER: u64 = 0x502A2AB5; // SOLALABS -> SOLANA LABS
#[repr(C)]
pub struct TieredStorageMagicNumber(pub u64);

// Ensure there are no implicit padding bytes
const _: () = assert!(std::mem::size_of::<TieredStorageMagicNumber>() == 8);

impl Default for TieredStorageMagicNumber {
fn default() -> Self {
Self(FOOTER_MAGIC_NUMBER)
Expand Down Expand Up @@ -123,12 +126,12 @@ pub struct TieredStorageFooter {
/// A hash that represents a tiered accounts file for consistency check.
pub hash: Hash,

/// The format version of the tiered accounts file.
pub format_version: u64,
// The below fields belong to footer tail.
// The sum of their sizes should match FOOTER_TAIL_SIZE.
/// The size of the footer including the magic number.
pub footer_size: u64,
/// The format version of the tiered accounts file.
pub format_version: u64,
// This field is persisted in the storage but not in this struct.
// The number should match FOOTER_MAGIC_NUMBER.
// pub magic_number: u64,
Expand All @@ -153,8 +156,8 @@ const _: () = assert!(
+ std::mem::size_of::<Pubkey>() // min_account_address
+ std::mem::size_of::<Pubkey>() // max_account_address
+ std::mem::size_of::<Hash>() // hash
+ std::mem::size_of::<u64>() // footer_size
+ std::mem::size_of::<u64>(), // format_version
+ std::mem::size_of::<u64>() // format_version
+ std::mem::size_of::<u64>(), // footer_size
"TieredStorageFooter cannot have any padding"
);

Expand All @@ -175,8 +178,8 @@ impl Default for TieredStorageFooter {
hash: Hash::new_unique(),
min_account_address: Pubkey::default(),
max_account_address: Pubkey::default(),
footer_size: FOOTER_SIZE as u64,
format_version: FOOTER_FORMAT_VERSION,
footer_size: FOOTER_SIZE as u64,
}
}
}
Expand Down Expand Up @@ -347,8 +350,8 @@ mod tests {
hash: Hash::new_unique(),
min_account_address: Pubkey::default(),
max_account_address: Pubkey::new_unique(),
footer_size: FOOTER_SIZE as u64,
format_version: FOOTER_FORMAT_VERSION,
footer_size: FOOTER_SIZE as u64,
};

// Persist the expected footer.
Expand Down Expand Up @@ -384,8 +387,8 @@ mod tests {
assert_eq!(offset_of!(TieredStorageFooter, min_account_address), 0x30);
assert_eq!(offset_of!(TieredStorageFooter, max_account_address), 0x50);
assert_eq!(offset_of!(TieredStorageFooter, hash), 0x70);
assert_eq!(offset_of!(TieredStorageFooter, footer_size), 0x90);
assert_eq!(offset_of!(TieredStorageFooter, format_version), 0x98);
assert_eq!(offset_of!(TieredStorageFooter, format_version), 0x90);
assert_eq!(offset_of!(TieredStorageFooter, footer_size), 0x98);
}

#[test]
Expand Down
106 changes: 25 additions & 81 deletions accounts-db/src/tiered_storage/hot.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![allow(dead_code)]
//! The account meta and related structs for hot accounts.
use {
Expand All @@ -23,7 +22,9 @@ use {
bytemuck::{Pod, Zeroable},
memmap2::{Mmap, MmapOptions},
modular_bitfield::prelude::*,
solana_sdk::{account::ReadableAccount, pubkey::Pubkey, stake_history::Epoch},
solana_sdk::{
account::ReadableAccount, pubkey::Pubkey, stake_history::Epoch,
},
std::{borrow::Borrow, fs::OpenOptions, option::Option, path::Path},
};

Expand Down Expand Up @@ -96,11 +97,17 @@ struct HotMetaPackedFields {
owner_offset: B29,
}

// Ensure there are no implicit padding bytes
const _: () = assert!(std::mem::size_of::<HotMetaPackedFields>() == 4);

/// The offset to access a hot account.
#[repr(C)]
#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Pod, Zeroable)]
pub struct HotAccountOffset(u32);

// Ensure there are no implicit padding bytes
const _: () = assert!(std::mem::size_of::<HotAccountOffset>() == 4);

impl AccountOffset for HotAccountOffset {}

impl HotAccountOffset {
Expand Down Expand Up @@ -143,6 +150,9 @@ pub struct HotAccountMeta {
flags: AccountMetaFlags,
}

// Ensure there are no implicit padding bytes
const _: () = assert!(std::mem::size_of::<HotAccountMeta>() == 8 + 4 + 4);

impl TieredAccountMeta for HotAccountMeta {
/// Construct a HotAccountMeta instance.
fn new() -> Self {
Expand Down Expand Up @@ -647,26 +657,21 @@ impl HotStorageWriter {
pub mod tests {
use {
super::*,
crate::{
account_storage::meta::StoredMeta,
tiered_storage::{
byte_block::ByteBlockWriter,
file::TieredStorageFile,
footer::{AccountBlockFormat, AccountMetaFormat, TieredStorageFooter, FOOTER_SIZE},
hot::{HotAccountMeta, HotStorageReader},
index::{AccountIndexWriterEntry, IndexBlockFormat, IndexOffset},
meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta},
owners::{OwnersBlockFormat, OwnersTable},
},
crate::tiered_storage::{
byte_block::ByteBlockWriter,
file::TieredStorageFile,
footer::{AccountBlockFormat, AccountMetaFormat, TieredStorageFooter, FOOTER_SIZE},
hot::{HotAccountMeta, HotStorageReader},
index::{AccountIndexWriterEntry, IndexBlockFormat, IndexOffset},
meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta},
owners::{OwnersBlockFormat, OwnersTable},
test_utils::{create_test_account, verify_test_account},
},
assert_matches::assert_matches,
memoffset::offset_of,
rand::{seq::SliceRandom, Rng},
solana_sdk::{
account::{Account, AccountSharedData, ReadableAccount},
hash::Hash,
pubkey::Pubkey,
slot_history::Slot,
account::ReadableAccount, hash::Hash, pubkey::Pubkey, slot_history::Slot,
stake_history::Epoch,
},
tempfile::TempDir,
Expand Down Expand Up @@ -1278,67 +1283,6 @@ pub mod tests {
assert_matches!(HotStorageWriter::new(&path), Err(_));
}

/// Create a test account based on the specified seed.
/// The created test account might have default rent_epoch
/// and write_version.
///
/// When the seed is zero, then a zero-lamport test account will be
/// created.
fn create_test_account(seed: u64) -> (StoredMeta, AccountSharedData) {
let data_byte = seed as u8;
let owner_byte = u8::MAX - data_byte;
let account = Account {
lamports: seed,
data: std::iter::repeat(data_byte).take(seed as usize).collect(),
// this will allow some test account sharing the same owner.
owner: [owner_byte; 32].into(),
executable: seed % 2 > 0,
rent_epoch: if seed % 3 > 0 {
seed
} else {
RENT_EXEMPT_RENT_EPOCH
},
};

let stored_meta = StoredMeta {
write_version_obsolete: u64::MAX,
pubkey: Pubkey::new_unique(),
data_len: seed,
};
(stored_meta, AccountSharedData::from(account))
}

fn verify_account(
stored_meta: &StoredAccountMeta<'_>,
account: Option<&impl ReadableAccount>,
address: &Pubkey,
account_hash: &AccountHash,
) {
let (lamports, owner, data, executable, account_hash) = account
.map(|acc| {
(
acc.lamports(),
acc.owner(),
acc.data(),
acc.executable(),
// only persist rent_epoch for those rent-paying accounts
Some(*account_hash),
)
})
.unwrap_or((0, &OWNER_NO_OWNER, &[], false, None));

assert_eq!(stored_meta.lamports(), lamports);
assert_eq!(stored_meta.data().len(), data.len());
assert_eq!(stored_meta.data(), data);
assert_eq!(stored_meta.executable(), executable);
assert_eq!(stored_meta.owner(), owner);
assert_eq!(stored_meta.pubkey(), address);
assert_eq!(
*stored_meta.hash(),
account_hash.unwrap_or(AccountHash(Hash::default()))
);
}

#[test]
fn test_write_account_and_index_blocks() {
let account_data_sizes = &[
Expand Down Expand Up @@ -1391,7 +1335,7 @@ pub mod tests {
.unwrap();

let (account, address, account_hash, _write_version) = storable_accounts.get(i);
verify_account(&stored_meta, account, address, account_hash);
verify_test_account(&stored_meta, account, address, account_hash);

assert_eq!(i + 1, next.0 as usize);
}
Expand All @@ -1410,7 +1354,7 @@ pub mod tests {

let (account, address, account_hash, _write_version) =
storable_accounts.get(stored_info.offset);
verify_account(&stored_meta, account, address, account_hash);
verify_test_account(&stored_meta, account, address, account_hash);
}

// verify get_accounts
Expand All @@ -1419,7 +1363,7 @@ pub mod tests {
// first, we verify everything
for (i, stored_meta) in accounts.iter().enumerate() {
let (account, address, account_hash, _write_version) = storable_accounts.get(i);
verify_account(stored_meta, account, address, account_hash);
verify_test_account(stored_meta, account, address, account_hash);
}

// second, we verify various initial position
Expand Down
Loading

0 comments on commit 909f73f

Please sign in to comment.