Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TieredStorage] Put commonly used test functions into test_utils.rs #35065

Merged
merged 3 commits into from
Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 6 additions & 61 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 Down Expand Up @@ -160,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_sdk::{
account::{Account, AccountSharedData},
clock::Slot,
hash::Hash,
pubkey::Pubkey,
rent_collector::RENT_EXEMPT_RENT_EPOCH,
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 @@ -310,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()))
);
}

Comment on lines -313 to -364
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two functions have been moved to test_utils.rs

/// The helper function for all write_accounts tests.
/// Currently only supports hot accounts.
fn do_test_write_accounts(
Expand All @@ -371,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 @@ -415,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
92 changes: 13 additions & 79 deletions accounts-db/src/tiered_storage/hot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,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 @@ -1288,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()))
);
}

Comment on lines -1291 to -1351
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two functions have been moved to test_utils.rs

#[test]
fn test_write_account_and_index_blocks() {
let account_data_sizes = &[
Expand Down Expand Up @@ -1401,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 @@ -1420,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 @@ -1429,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
76 changes: 76 additions & 0 deletions accounts-db/src/tiered_storage/test_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#![cfg(test)]
//! Helper functions for TieredStorage tests
brooksprumo marked this conversation as resolved.
Show resolved Hide resolved
use {
crate::{
account_storage::meta::{StoredAccountMeta, StoredMeta},
accounts_hash::AccountHash,
tiered_storage::owners::OWNER_NO_OWNER,
},
solana_sdk::{
account::{Account, AccountSharedData, ReadableAccount},
hash::Hash,
pubkey::Pubkey,
rent_collector::RENT_EXEMPT_RENT_EPOCH,
},
};

/// 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.
pub(super) 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))
}

pub(super) fn verify_test_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()))
);
}
Loading