From ee113de4ce7243628e8ecfb1dfceba7999a72f66 Mon Sep 17 00:00:00 2001 From: Brooks Date: Thu, 18 Apr 2024 15:45:44 -0400 Subject: [PATCH] Refactors stored accounts info (#887) --- accounts-db/benches/append_vec.rs | 14 +++--- accounts-db/benches/bench_accounts_file.rs | 4 +- accounts-db/src/account_storage/meta.rs | 6 --- accounts-db/src/accounts_db.rs | 37 +++++++++----- accounts-db/src/accounts_file.rs | 21 +++++--- accounts-db/src/append_vec.rs | 41 ++++++++-------- accounts-db/src/tiered_storage.rs | 38 ++++++++++++--- accounts-db/src/tiered_storage/footer.rs | 10 ++-- accounts-db/src/tiered_storage/hot.rs | 57 ++++++++++------------ 9 files changed, 132 insertions(+), 96 deletions(-) diff --git a/accounts-db/benches/append_vec.rs b/accounts-db/benches/append_vec.rs index fd8e267c7cf704..0116cfb13dc5df 100644 --- a/accounts-db/benches/append_vec.rs +++ b/accounts-db/benches/append_vec.rs @@ -4,7 +4,8 @@ extern crate test; use { rand::{thread_rng, Rng}, solana_accounts_db::{ - account_storage::meta::{StoredAccountInfo, StoredMeta}, + account_storage::meta::StoredMeta, + accounts_file::StoredAccountsInfo, append_vec::{ test_utils::{create_test_account, get_append_vec_path}, AppendVec, @@ -29,13 +30,12 @@ fn append_account( vec: &AppendVec, storage_meta: StoredMeta, account: &AccountSharedData, -) -> Option { +) -> Option { let slot_ignored = Slot::MAX; let accounts = [(&storage_meta.pubkey, account)]; let slice = &accounts[..]; let storable_accounts = (slot_ignored, slice); - let res = vec.append_accounts(&storable_accounts, 0); - res.and_then(|res| res.first().cloned()) + vec.append_accounts(&storable_accounts, 0) } #[bench] @@ -54,7 +54,7 @@ fn add_test_accounts(vec: &AppendVec, size: usize) -> Vec<(usize, usize)> { (0..size) .filter_map(|sample| { let (meta, account) = create_test_account(sample); - append_account(vec, meta, &account).map(|info| (sample, info.offset)) + append_account(vec, meta, &account).map(|info| (sample, info.offsets[0])) }) .collect() } @@ -100,7 +100,7 @@ fn append_vec_concurrent_append_read(bencher: &mut Bencher) { let sample = indexes1.lock().unwrap().len(); let (meta, account) = create_test_account(sample); if let Some(info) = append_account(&vec1, meta, &account) { - indexes1.lock().unwrap().push((sample, info.offset)) + indexes1.lock().unwrap().push((sample, info.offsets[0])) } else { break; } @@ -140,7 +140,7 @@ fn append_vec_concurrent_read_append(bencher: &mut Bencher) { let sample: usize = thread_rng().gen_range(0..256); let (meta, account) = create_test_account(sample); if let Some(info) = append_account(&vec, meta, &account) { - indexes.lock().unwrap().push((sample, info.offset)) + indexes.lock().unwrap().push((sample, info.offsets[0])) } }); } diff --git a/accounts-db/benches/bench_accounts_file.rs b/accounts-db/benches/bench_accounts_file.rs index fc450439f38a14..4efdd5550552a7 100644 --- a/accounts-db/benches/bench_accounts_file.rs +++ b/accounts-db/benches/bench_accounts_file.rs @@ -54,7 +54,7 @@ fn bench_write_accounts_file(c: &mut Criterion) { }, |append_vec| { let res = append_vec.append_accounts(&storable_accounts, 0).unwrap(); - let accounts_written_count = res.len(); + let accounts_written_count = res.offsets.len(); assert_eq!(accounts_written_count, accounts_count); }, BatchSize::SmallInput, @@ -72,7 +72,7 @@ fn bench_write_accounts_file(c: &mut Criterion) { }, |hot_storage| { let res = hot_storage.write_accounts(&storable_accounts, 0).unwrap(); - let accounts_written_count = res.len(); + let accounts_written_count = res.offsets.len(); assert_eq!(accounts_written_count, accounts_count); }, BatchSize::SmallInput, diff --git a/accounts-db/src/account_storage/meta.rs b/accounts-db/src/account_storage/meta.rs index 445000a329e411..d2624d75df3ae9 100644 --- a/accounts-db/src/account_storage/meta.rs +++ b/accounts-db/src/account_storage/meta.rs @@ -9,12 +9,6 @@ use { }; pub type StoredMetaWriteVersion = u64; -// A tuple that stores offset and size respectively -#[derive(Debug, Clone)] -pub struct StoredAccountInfo { - pub offset: usize, - pub size: usize, -} lazy_static! { static ref DEFAULT_ACCOUNT_HASH: AccountHash = AccountHash(Hash::default()); diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 30d3b9b396bbbd..392b46421c02d6 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -1138,10 +1138,11 @@ impl AccountStorageEntry { self.accounts.get_account_shared_data(offset) } - fn add_account(&self, num_bytes: usize) { + fn add_accounts(&self, num_accounts: usize, num_bytes: usize) { let mut count_and_status = self.count_and_status.lock_write(); - *count_and_status = (count_and_status.0 + 1, count_and_status.1); - self.approx_store_count.fetch_add(1, Ordering::Relaxed); + *count_and_status = (count_and_status.0 + num_accounts, count_and_status.1); + self.approx_store_count + .fetch_add(num_accounts, Ordering::Relaxed); self.alive_bytes.fetch_add(num_bytes, Ordering::SeqCst); } @@ -5976,12 +5977,12 @@ impl AccountsDb { let mut total_append_accounts_us = 0; while infos.len() < accounts_and_meta_to_store.len() { let mut append_accounts = Measure::start("append_accounts"); - let rvs = storage + let stored_accounts_info = storage .accounts .append_accounts(accounts_and_meta_to_store, infos.len()); append_accounts.stop(); total_append_accounts_us += append_accounts.as_us(); - if rvs.is_none() { + let Some(stored_accounts_info) = stored_accounts_info else { storage.set_status(AccountStorageStatus::Full); // See if an account overflows the append vecs in the slot. @@ -6005,18 +6006,21 @@ impl AccountsDb { }, ); continue; - } + }; let store_id = storage.append_vec_id(); - for (i, stored_account_info) in rvs.unwrap().into_iter().enumerate() { - storage.add_account(stored_account_info.size); - + for (i, offset) in stored_accounts_info.offsets.iter().enumerate() { infos.push(AccountInfo::new( - StorageLocation::AppendVec(store_id, stored_account_info.offset), + StorageLocation::AppendVec(store_id, *offset), accounts_and_meta_to_store .account_default_if_zero_lamport(i, |account| account.lamports()), )); } + storage.add_accounts( + stored_accounts_info.offsets.len(), + stored_accounts_info.size, + ); + // restore the state to available storage.set_status(AccountStorageStatus::Available); } @@ -9614,6 +9618,12 @@ pub mod tests { } } + impl AccountStorageEntry { + fn add_account(&self, num_bytes: usize) { + self.add_accounts(1, num_bytes) + } + } + impl CurrentAncientAccountsFile { /// note this requires that 'slot_and_accounts_file' is Some fn append_vec_id(&self) -> AccountsFileId { @@ -10673,12 +10683,15 @@ pub mod tests { .unwrap(); if mark_alive { // updates 'alive_bytes' on the storage - storage.add_account(stored_accounts_info[0].size); + storage.add_account(stored_accounts_info.size); } if let Some(index) = add_to_index { let account_info = AccountInfo::new( - StorageLocation::AppendVec(storage.append_vec_id(), stored_accounts_info[0].offset), + StorageLocation::AppendVec( + storage.append_vec_id(), + stored_accounts_info.offsets[0], + ), account.lamports(), ); index.upsert( diff --git a/accounts-db/src/accounts_file.rs b/accounts-db/src/accounts_file.rs index b0671f7f20ed67..2fd5be2f1418a7 100644 --- a/accounts-db/src/accounts_file.rs +++ b/accounts-db/src/accounts_file.rs @@ -1,7 +1,7 @@ use { crate::{ account_info::AccountInfo, - account_storage::meta::{StoredAccountInfo, StoredAccountMeta}, + account_storage::meta::StoredAccountMeta, accounts_db::AccountsFileId, append_vec::{AppendVec, AppendVecError, IndexInfo}, storable_accounts::StorableAccounts, @@ -268,7 +268,7 @@ impl AccountsFile { &self, accounts: &impl StorableAccounts<'a>, skip: usize, - ) -> Option> { + ) -> Option { match self { Self::AppendVec(av) => av.append_accounts(accounts, skip), // Note: The conversion here is needed as the AccountsDB currently @@ -276,11 +276,11 @@ impl AccountsFile { // IndexOffset that is equivalent to AccountInfo::reduced_offset. Self::TieredStorage(ts) => ts .write_accounts(accounts, skip, &HOT_FORMAT) - .map(|mut infos| { - infos.iter_mut().for_each(|info| { - info.offset = AccountInfo::reduced_offset_to_offset(info.offset as u32); + .map(|mut stored_accounts_info| { + stored_accounts_info.offsets.iter_mut().for_each(|offset| { + *offset = AccountInfo::reduced_offset_to_offset(*offset as u32); }); - infos + stored_accounts_info }) .ok(), } @@ -344,6 +344,15 @@ impl AccountsFileProvider { } } +/// Information after storing accounts +#[derive(Debug)] +pub struct StoredAccountsInfo { + /// offset in the storage where each account was stored + pub offsets: Vec, + /// total size of all the stored accounts + pub size: usize, +} + #[cfg(test)] pub mod tests { use crate::accounts_file::AccountsFile; diff --git a/accounts-db/src/append_vec.rs b/accounts-db/src/append_vec.rs index 1fdb99f60d1015..a460e9445caa61 100644 --- a/accounts-db/src/append_vec.rs +++ b/accounts-db/src/append_vec.rs @@ -7,9 +7,12 @@ use { crate::{ account_storage::meta::{ - AccountMeta, StoredAccountInfo, StoredAccountMeta, StoredMeta, StoredMetaWriteVersion, + AccountMeta, StoredAccountMeta, StoredMeta, StoredMetaWriteVersion, + }, + accounts_file::{ + AccountsFileError, MatchAccountOwnerError, Result, StoredAccountsInfo, + ALIGN_BOUNDARY_OFFSET, }, - accounts_file::{AccountsFileError, MatchAccountOwnerError, Result, ALIGN_BOUNDARY_OFFSET}, accounts_hash::AccountHash, storable_accounts::StorableAccounts, u64_align, @@ -748,14 +751,14 @@ impl AppendVec { &self, accounts: &impl StorableAccounts<'a>, skip: usize, - ) -> Option> { + ) -> Option { let _lock = self.append_lock.lock().unwrap(); let default_hash: Hash = Hash::default(); // [0_u8; 32]; let mut offset = self.len(); let len = accounts.len(); // Here we have `len - skip` number of accounts. The +1 extra capacity - // is for storing the aligned offset of the last entry to that is used - // to compute the StoredAccountInfo of the last entry. + // is for storing the aligned offset of the last-plus-one entry, + // which is used to compute the size of the last stored account. let offsets_len = len - skip + 1; let mut offsets = Vec::with_capacity(offsets_len); let mut stop = false; @@ -787,31 +790,25 @@ impl AppendVec { (hash_ptr, mem::size_of::()), (data_ptr, data_len), ]; - if let Some(res) = self.append_ptrs_locked(&mut offset, &ptrs) { - offsets.push(res) + if let Some(start_offset) = self.append_ptrs_locked(&mut offset, &ptrs) { + offsets.push(start_offset) } else { stop = true; } }); } - if offsets.is_empty() { - None - } else { - let mut rv = Vec::with_capacity(offsets.len()); - - // The last entry in this offset needs to be the u64 aligned offset, because that's + (!offsets.is_empty()).then(|| { + // The last entry in the offsets needs to be the u64 aligned `offset`, because that's // where the *next* entry will begin to be stored. + // This is used to compute the size of the last stored account; make sure to remove + // it afterwards! offsets.push(u64_align!(offset)); - for offsets in offsets.windows(2) { - rv.push(StoredAccountInfo { - offset: offsets[0], - size: offsets[1] - offsets[0], - }); - } + let size = offsets.windows(2).map(|offset| offset[1] - offset[0]).sum(); + offsets.pop(); - Some(rv) - } + StoredAccountsInfo { offsets, size } + }) } /// Returns a slice suitable for use when archiving append vecs @@ -846,7 +843,7 @@ pub mod tests { let storable_accounts = (slot_ignored, slice); self.append_accounts(&storable_accounts, 0) - .map(|res| res[0].offset) + .map(|res| res.offsets[0]) } } diff --git a/accounts-db/src/tiered_storage.rs b/accounts-db/src/tiered_storage.rs index a958c2acc5180a..0fee6594c26d65 100644 --- a/accounts-db/src/tiered_storage.rs +++ b/accounts-db/src/tiered_storage.rs @@ -13,7 +13,7 @@ pub mod readable; mod test_utils; use { - crate::{account_storage::meta::StoredAccountInfo, storable_accounts::StorableAccounts}, + crate::{accounts_file::StoredAccountsInfo, storable_accounts::StorableAccounts}, error::TieredStorageError, footer::{AccountBlockFormat, AccountMetaFormat}, hot::{HotStorageWriter, HOT_FORMAT}, @@ -110,7 +110,7 @@ impl TieredStorage { accounts: &impl StorableAccounts<'a>, skip: usize, format: &TieredStorageFormat, - ) -> TieredStorageResult> { + ) -> TieredStorageResult { let was_written = self.already_written.swap(true, Ordering::AcqRel); if was_written { @@ -196,7 +196,7 @@ mod tests { /// to persist non-account blocks such as footer, index block, etc. fn write_zero_accounts( tiered_storage: &TieredStorage, - expected_result: TieredStorageResult>, + expected_result: TieredStorageResult, ) { let slot_ignored = Slot::MAX; let account_refs = Vec::<(&Pubkey, &AccountSharedData)>::new(); @@ -239,7 +239,13 @@ mod tests { assert_eq!(tiered_storage.path(), tiered_storage_path); assert_eq!(tiered_storage.len(), 0); - write_zero_accounts(&tiered_storage, Ok(vec![])); + write_zero_accounts( + &tiered_storage, + Ok(StoredAccountsInfo { + offsets: vec![], + size: 0, + }), + ); } let tiered_storage_readonly = TieredStorage::new_readonly(&tiered_storage_path).unwrap(); @@ -265,7 +271,13 @@ mod tests { let tiered_storage_path = temp_dir.path().join("test_write_accounts_twice"); let tiered_storage = TieredStorage::new_writable(&tiered_storage_path); - write_zero_accounts(&tiered_storage, Ok(vec![])); + write_zero_accounts( + &tiered_storage, + Ok(StoredAccountsInfo { + offsets: vec![], + size: 0, + }), + ); // Expect AttemptToUpdateReadOnly error as write_accounts can only // be invoked once. write_zero_accounts( @@ -283,7 +295,13 @@ mod tests { let tiered_storage_path = temp_dir.path().join("test_remove_on_drop"); { let tiered_storage = TieredStorage::new_writable(&tiered_storage_path); - write_zero_accounts(&tiered_storage, Ok(vec![])); + write_zero_accounts( + &tiered_storage, + Ok(StoredAccountsInfo { + offsets: vec![], + size: 0, + }), + ); } // expect the file does not exists as it has been removed on drop assert!(!tiered_storage_path.try_exists().unwrap()); @@ -291,7 +309,13 @@ mod tests { { let tiered_storage = ManuallyDrop::new(TieredStorage::new_writable(&tiered_storage_path)); - write_zero_accounts(&tiered_storage, Ok(vec![])); + write_zero_accounts( + &tiered_storage, + Ok(StoredAccountsInfo { + offsets: vec![], + size: 0, + }), + ); } // expect the file exists as we have ManuallyDrop this time. assert!(tiered_storage_path.try_exists().unwrap()); diff --git a/accounts-db/src/tiered_storage/footer.rs b/accounts-db/src/tiered_storage/footer.rs index 652b160cb67998..f93f7f88195e67 100644 --- a/accounts-db/src/tiered_storage/footer.rs +++ b/accounts-db/src/tiered_storage/footer.rs @@ -174,12 +174,14 @@ impl TieredStorageFooter { Self::new_from_footer_block(&file) } - pub fn write_footer_block(&self, file: &mut TieredWritableFile) -> TieredStorageResult<()> { + pub fn write_footer_block(&self, file: &mut TieredWritableFile) -> TieredStorageResult { + let mut bytes_written = 0; + // SAFETY: The footer does not contain any uninitialized bytes. - unsafe { file.write_type(self)? }; - file.write_pod(&TieredStorageMagicNumber::default())?; + bytes_written += unsafe { file.write_type(self)? }; + bytes_written += file.write_pod(&TieredStorageMagicNumber::default())?; - Ok(()) + Ok(bytes_written) } pub fn new_from_footer_block(file: &TieredReadableFile) -> TieredStorageResult { diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index a0406b75cd5ec8..97c8783fea6bdc 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -3,8 +3,8 @@ use { crate::{ account_info::AccountInfo, - account_storage::meta::{StoredAccountInfo, StoredAccountMeta}, - accounts_file::MatchAccountOwnerError, + account_storage::meta::StoredAccountMeta, + accounts_file::{MatchAccountOwnerError, StoredAccountsInfo}, append_vec::{IndexInfo, IndexInfoInner}, tiered_storage::{ byte_block, @@ -743,17 +743,18 @@ impl HotStorageWriter { &mut self, accounts: &impl StorableAccounts<'a>, skip: usize, - ) -> TieredStorageResult> { + ) -> TieredStorageResult { let mut footer = new_hot_footer(); let mut index = vec![]; let mut owners_table = OwnersTable::default(); let mut cursor = 0; let mut address_range = AccountAddressRange::default(); - // writing accounts blocks let len = accounts.len(); - let total_input_accounts = len - skip; - let mut stored_infos = Vec::with_capacity(total_input_accounts); + let total_input_accounts = len.saturating_sub(skip); + let mut offsets = Vec::with_capacity(total_input_accounts); + + // writing accounts blocks for i in skip..len { accounts.account_default_if_zero_lamport::>(i, |account| { let index_entry = AccountIndexWriterEntry { @@ -776,24 +777,15 @@ impl HotStorageWriter { ) }; let owner_offset = owners_table.insert(owner); - let stored_size = + cursor += self.write_account(lamports, owner_offset, data, executable, rent_epoch)?; - cursor += stored_size; - - stored_infos.push(StoredAccountInfo { - // Here we pass the IndexOffset as the get_account() API - // takes IndexOffset. Given the account address is also - // maintained outside the TieredStorage, a potential optimization - // is to store AccountOffset instead, which can further save - // one jump from the index block to the accounts block. - offset: index.len(), - // Here we only include the stored size that the account directly - // contribute (i.e., account entry + index entry that include the - // account meta, data, optional fields, its address, and AccountOffset). - // Storage size from those shared blocks like footer and owners block - // is not included. - size: stored_size + footer.index_block_format.entry_size::(), - }); + + // Here we pass the IndexOffset as the get_account() API + // takes IndexOffset. Given the account address is also + // maintained outside the TieredStorage, a potential optimization + // is to store AccountOffset instead, which can further save + // one jump from the index block to the accounts block. + offsets.push(index.len()); index.push(index_entry); Ok(()) })?; @@ -819,14 +811,19 @@ impl HotStorageWriter { assert!(cursor % HOT_BLOCK_ALIGNMENT == 0); footer.owners_block_offset = cursor as u64; footer.owner_count = owners_table.len() as u32; - footer + cursor += footer .owners_block_format .write_owners_block(&mut self.storage, &owners_table)?; + + // writing footer footer.min_account_address = address_range.min; footer.max_account_address = address_range.max; - footer.write_footer_block(&mut self.storage)?; + cursor += footer.write_footer_block(&mut self.storage)?; - Ok(stored_infos) + Ok(StoredAccountsInfo { + offsets, + size: cursor, + }) } } @@ -1546,7 +1543,7 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let path = temp_dir.path().join("test_write_account_and_index_blocks"); - let stored_infos = { + let stored_accounts_info = { let mut writer = HotStorageWriter::new(&path).unwrap(); writer.write_accounts(&storable_accounts, 0).unwrap() }; @@ -1578,13 +1575,13 @@ mod tests { Ok(None) ); - for stored_info in stored_infos { + for offset in stored_accounts_info.offsets { let (stored_account_meta, _) = hot_storage - .get_stored_account_meta(IndexOffset(stored_info.offset as u32)) + .get_stored_account_meta(IndexOffset(offset as u32)) .unwrap() .unwrap(); - storable_accounts.account_default_if_zero_lamport(stored_info.offset, |account| { + storable_accounts.account_default_if_zero_lamport(offset, |account| { verify_test_account( &stored_account_meta, &account.to_account_shared_data(),