From be9f17f053a99afbbab491b42e37150c85843430 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:33:42 -0800 Subject: [PATCH] [TieredStorage] Have HotStorageWriter::write_account() return Vec (#34929) #### Problem To allow hot-storage to use HotStorageWriter::write_account() to implement AccountsFile::append_accounts(), it is required to provide a Vector of StoredAccountInfo to allow AccountsDB to properly prepare the entry for each account. #### Summary of Changes This PR enables HotStorageWriter::write_account() to return Vec. #### Test Plan Extend existing tests for HotStorageWriter to verify the correctness of the returned Vec. --- accounts-db/src/tiered_storage/hot.rs | 101 ++++++++++++++++++-------- 1 file changed, 69 insertions(+), 32 deletions(-) diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 311da9916..c6e3efdbb 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -2,7 +2,7 @@ use { crate::{ - account_storage::meta::StoredAccountMeta, + account_storage::meta::{StoredAccountInfo, StoredAccountMeta}, accounts_file::MatchAccountOwnerError, accounts_hash::AccountHash, rent_collector::RENT_EXEMPT_RENT_EPOCH, @@ -543,7 +543,7 @@ impl HotStorageWriter { &self, accounts: &StorableAccountsWithHashesAndWriteVersions<'a, 'b, T, U, V>, skip: usize, - ) -> TieredStorageResult<()> { + ) -> TieredStorageResult> { let mut footer = new_hot_footer(); let mut index = vec![]; let mut owners_table = OwnersTable::default(); @@ -551,6 +551,8 @@ impl HotStorageWriter { // writing accounts blocks let len = accounts.accounts.len(); + let total_input_accounts = len - skip; + let mut stored_infos = Vec::with_capacity(total_input_accounts); for i in skip..len { let (account, address, account_hash, _write_version) = accounts.get(i); let index_entry = AccountIndexWriterEntry { @@ -574,7 +576,7 @@ impl HotStorageWriter { }) .unwrap_or((0, &OWNER_NO_OWNER, &[], false, None, None)); let owner_offset = owners_table.insert(owner); - cursor += self.write_account( + let stored_size = self.write_account( lamports, owner_offset, data, @@ -582,9 +584,25 @@ impl HotStorageWriter { rent_epoch, account_hash, )?; + 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::(), + }); index.push(index_entry); } - footer.account_entry_count = (len - skip) as u32; + footer.account_entry_count = total_input_accounts as u32; // writing index block // expect the offset of each block aligned. @@ -611,7 +629,7 @@ impl HotStorageWriter { footer.write_footer_block(&self.storage)?; - Ok(()) + Ok(stored_infos) } } @@ -1280,6 +1298,37 @@ pub mod tests { (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 = &[ @@ -1316,11 +1365,10 @@ pub mod tests { let temp_dir = TempDir::new().unwrap(); let path = temp_dir.path().join("test_write_account_and_index_blocks"); - - { + let stored_infos = { let writer = HotStorageWriter::new(&path).unwrap(); - writer.write_accounts(&storable_accounts, 0).unwrap(); - } + writer.write_accounts(&storable_accounts, 0).unwrap() + }; let hot_storage = HotStorageReader::new_from_path(&path).unwrap(); @@ -1333,29 +1381,7 @@ pub mod tests { .unwrap(); let (account, address, account_hash, _write_version) = storable_accounts.get(i); - 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())) - ); + verify_account(&stored_meta, account, address, account_hash); assert_eq!(i + 1, next); } @@ -1365,5 +1391,16 @@ pub mod tests { hot_storage.get_account(IndexOffset(num_accounts as u32)), Ok(None) ); + + for stored_info in stored_infos { + let (stored_meta, _) = hot_storage + .get_account(IndexOffset(stored_info.offset as u32)) + .unwrap() + .unwrap(); + + let (account, address, account_hash, _write_version) = + storable_accounts.get(stored_info.offset); + verify_account(&stored_meta, account, address, account_hash); + } } }