From 3ea1ad0d0d7164da361d5c6fe7b8752dae33751b Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Tue, 5 Dec 2023 17:53:28 -0800 Subject: [PATCH 1/7] [TieredStorage] Make AccountOffset a trait and introduce HotAccountOffset --- accounts-db/src/tiered_storage/error.rs | 9 +++ accounts-db/src/tiered_storage/hot.rs | 90 ++++++++++++++++++++----- accounts-db/src/tiered_storage/index.rs | 64 ++++++++++++------ 3 files changed, 124 insertions(+), 39 deletions(-) diff --git a/accounts-db/src/tiered_storage/error.rs b/accounts-db/src/tiered_storage/error.rs index 9b9d07d977e804..47b5a49413a420 100644 --- a/accounts-db/src/tiered_storage/error.rs +++ b/accounts-db/src/tiered_storage/error.rs @@ -25,4 +25,13 @@ pub enum TieredStorageError { #[error("footer is unsanitary: {0}")] SanitizeFooter(#[from] SanitizeFooterError), + + #[error("OffsetOutOfBounds: offset {0} is larger than the supported size {1}")] + OffsetOutOfBounds(usize, usize), + + #[error("OffsetAlignmentError: offset {0} must be multiple of {1}")] + OffsetAlignmentError(usize, usize), + + #[error("IntraBlockOffsetOutOfBounds: offset {0} is larger than the supported size {1}")] + IntraBlockOffsetOutOfBounds(usize, usize), } diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 28f09a9e664358..561058ad352c23 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -12,7 +12,7 @@ use { meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta}, mmap_utils::get_type, owners::{OwnerOffset, OwnersBlock}, - TieredStorageFormat, TieredStorageResult, + TieredStorageError, TieredStorageFormat, TieredStorageResult, }, }, memmap2::{Mmap, MmapOptions}, @@ -35,9 +35,12 @@ const MAX_HOT_PADDING: u8 = 7; /// The maximum allowed value for the owner index of a hot account. const MAX_HOT_OWNER_OFFSET: OwnerOffset = OwnerOffset((1 << 29) - 1); -/// The multiplier for converting AccountOffset to the internal hot account +/// The multiplier for converting HotAccountOffset to the internal hot account /// offset. This increases the maximum size of a hot accounts file. -const HOT_ACCOUNT_OFFSET_MULTIPLIER: usize = 8; +pub(crate) const HOT_ACCOUNT_OFFSET_MULTIPLIER: usize = 8; + +/// The maximum supported offset for hot accounts storage. +const MAX_HOT_ACCOUNT_OFFSET: usize = u32::MAX as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER; #[bitfield(bits = 32)] #[repr(C)] @@ -57,6 +60,56 @@ struct HotMetaPackedFields { owner_offset: B29, } +/// The offset to access a hot account. +#[repr(C)] +#[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] +pub struct HotAccountOffset(u32); + +impl AccountOffset for HotAccountOffset { + /// Creates a new AccountOffset instance given the block offset and the + /// intra-block offset. + fn new(block_offset: usize, intra_block_offset: usize) -> TieredStorageResult { + if block_offset >= MAX_HOT_ACCOUNT_OFFSET { + return Err(TieredStorageError::OffsetOutOfBounds( + block_offset, + MAX_HOT_ACCOUNT_OFFSET, + )); + } + + // Hot accounts are aligned based on HOT_ACCOUNT_OFFSET_MULTIPLIER. + if block_offset % HOT_ACCOUNT_OFFSET_MULTIPLIER != 0 { + return Err(TieredStorageError::OffsetAlignmentError( + block_offset, + HOT_ACCOUNT_OFFSET_MULTIPLIER, + )); + } + + // Each hot account has its own account block. As a result, its + // intra-block offset is always 0. Any intra-block offset that is + // greater than 0 is invalid. + if intra_block_offset > 0 { + return Err(TieredStorageError::IntraBlockOffsetOutOfBounds( + block_offset, + 0, + )); + } + + Ok(HotAccountOffset( + (block_offset / HOT_ACCOUNT_OFFSET_MULTIPLIER) as u32, + )) + } + + /// The offset to the block that contains the account. + fn block_offset(&self) -> usize { + self.0 as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER + } + + /// The offset to the account inside the account block. + fn intra_block_offset(&self) -> usize { + 0 + } +} + /// The storage and in-memory representation of the metadata entry for a /// hot account. #[derive(Debug, PartialEq, Eq)] @@ -229,19 +282,22 @@ impl HotStorageReader { /// Returns the account meta located at the specified offset. fn get_account_meta_from_offset( &self, - account_offset: AccountOffset, + account_offset: HotAccountOffset, ) -> TieredStorageResult<&HotAccountMeta> { - let internal_account_offset = account_offset.block as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER; + let internal_account_offset = account_offset.block_offset(); let (meta, _) = get_type::(&self.mmap, internal_account_offset)?; Ok(meta) } /// Returns the offset to the account given the specified index. - fn get_account_offset(&self, index_offset: IndexOffset) -> TieredStorageResult { + fn get_account_offset( + &self, + index_offset: IndexOffset, + ) -> TieredStorageResult<&HotAccountOffset> { self.footer .index_block_format - .get_account_offset(&self.mmap, &self.footer, index_offset) + .get_account_offset::(&self.mmap, &self.footer, index_offset) } /// Returns the address of the account associated with the specified index. @@ -270,7 +326,7 @@ pub mod tests { FOOTER_SIZE, }, hot::{HotAccountMeta, HotStorageReader}, - index::{AccountIndexWriterEntry, AccountOffset, IndexBlockFormat, IndexOffset}, + index::{AccountIndexWriterEntry, IndexBlockFormat, IndexOffset}, meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta}, }, memoffset::offset_of, @@ -472,11 +528,8 @@ pub mod tests { .iter() .map(|meta| { let prev_offset = current_offset; - current_offset += file.write_type(meta).unwrap() as u32; - assert_eq!(prev_offset % HOT_ACCOUNT_OFFSET_MULTIPLIER as u32, 0); - AccountOffset { - block: prev_offset / HOT_ACCOUNT_OFFSET_MULTIPLIER as u32, - } + current_offset += file.write_type(meta).unwrap(); + HotAccountOffset::new(prev_offset, 0).unwrap() }) .collect(); // while the test only focuses on account metas, writing a footer @@ -511,8 +564,8 @@ pub mod tests { .iter() .map(|address| AccountIndexWriterEntry { address, - block_offset: rng.gen_range(0..u32::MAX), - intra_block_offset: rng.gen_range(0..4096), + block_offset: rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, + intra_block_offset: 0, }) .collect(); @@ -529,7 +582,7 @@ pub mod tests { footer .index_block_format - .write_index_block(&file, &index_writer_entries) + .write_index_block::(&file, &index_writer_entries) .unwrap(); // while the test only focuses on account metas, writing a footer @@ -542,7 +595,10 @@ pub mod tests { let account_offset = hot_storage .get_account_offset(IndexOffset(i as u32)) .unwrap(); - assert_eq!(account_offset.block, index_writer_entry.block_offset); + assert_eq!( + account_offset.block_offset(), + index_writer_entry.block_offset + ); let account_address = hot_storage .get_account_address(IndexOffset(i as u32)) diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index 778eb3237e304e..5e9da26281b6da 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -13,17 +13,27 @@ use { #[derive(Debug)] pub struct AccountIndexWriterEntry<'a> { pub address: &'a Pubkey, - pub block_offset: u32, - pub intra_block_offset: u32, + /// The offset to the block that contains the account. + pub block_offset: usize, + /// The offset to the account inside the account block. + pub intra_block_offset: usize, } /// The offset to an account stored inside its accounts block. /// This struct is used to access the meta and data of an account by looking through /// its accounts block. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct AccountOffset { - /// The offset to the accounts block that contains the account meta/data. - pub block: u32, +pub trait AccountOffset { + /// Creates a new AccountOffset instance given the block offset and the + /// intra-block offset. + fn new(block_offset: usize, intra_block_offset: usize) -> TieredStorageResult + where + Self: Sized; + + /// The offset to the block that contains the account. + fn block_offset(&self) -> usize; + + /// The offset to the account inside the account block. + fn intra_block_offset(&self) -> usize; } /// The offset to an account/address entry in the accounts index block. @@ -56,7 +66,7 @@ pub enum IndexBlockFormat { impl IndexBlockFormat { /// Persists the specified index_entries to the specified file and returns /// the total number of bytes written. - pub fn write_index_block( + pub fn write_index_block( &self, file: &TieredStorageFile, index_entries: &[AccountIndexWriterEntry], @@ -68,7 +78,10 @@ impl IndexBlockFormat { bytes_written += file.write_type(index_entry.address)?; } for index_entry in index_entries { - bytes_written += file.write_type(&index_entry.block_offset)?; + bytes_written += file.write_type(&T::new( + index_entry.block_offset, + index_entry.intra_block_offset, + )?)?; } Ok(bytes_written) } @@ -93,22 +106,20 @@ impl IndexBlockFormat { } /// Returns the offset to the account given the specified index. - pub fn get_account_offset( + pub fn get_account_offset<'a, T: AccountOffset>( &self, - mmap: &Mmap, + mmap: &'a Mmap, footer: &TieredStorageFooter, index_offset: IndexOffset, - ) -> TieredStorageResult { + ) -> TieredStorageResult<&'a T> { match self { Self::AddressAndBlockOffsetOnly => { - let account_offset = footer.index_block_offset as usize + let offset = footer.index_block_offset as usize + std::mem::size_of::() * footer.account_entry_count as usize + std::mem::size_of::() * index_offset.0 as usize; - let (block_offset, _) = get_type(mmap, account_offset)?; + let (account_offset, _) = get_type::(mmap, offset)?; - Ok(AccountOffset { - block: *block_offset, - }) + Ok(account_offset) } } } @@ -126,8 +137,15 @@ impl IndexBlockFormat { #[cfg(test)] mod tests { use { - super::*, crate::tiered_storage::file::TieredStorageFile, memmap2::MmapOptions, rand::Rng, - std::fs::OpenOptions, tempfile::TempDir, + super::*, + crate::tiered_storage::{ + file::TieredStorageFile, + hot::{HotAccountOffset, HOT_ACCOUNT_OFFSET_MULTIPLIER}, + }, + memmap2::MmapOptions, + rand::Rng, + std::fs::OpenOptions, + tempfile::TempDir, }; #[test] @@ -147,7 +165,7 @@ mod tests { .iter() .map(|address| AccountIndexWriterEntry { address, - block_offset: rng.gen_range(128..2048), + block_offset: rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, intra_block_offset: 0, }) .collect(); @@ -155,7 +173,9 @@ mod tests { { let file = TieredStorageFile::new_writable(&path).unwrap(); let indexer = IndexBlockFormat::AddressAndBlockOffsetOnly; - indexer.write_index_block(&file, &index_entries).unwrap(); + indexer + .write_index_block::(&file, &index_entries) + .unwrap(); } let indexer = IndexBlockFormat::AddressAndBlockOffsetOnly; @@ -167,9 +187,9 @@ mod tests { let mmap = unsafe { MmapOptions::new().map(&file).unwrap() }; for (i, index_entry) in index_entries.iter().enumerate() { let account_offset = indexer - .get_account_offset(&mmap, &footer, IndexOffset(i as u32)) + .get_account_offset::(&mmap, &footer, IndexOffset(i as u32)) .unwrap(); - assert_eq!(index_entry.block_offset, account_offset.block); + assert_eq!(index_entry.block_offset, account_offset.block_offset()); let address = indexer .get_account_address(&mmap, &footer, IndexOffset(i as u32)) .unwrap(); From a42a85cdfd45d4fec5b7f9e781d06e212620ab0f Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Thu, 7 Dec 2023 13:35:37 -0800 Subject: [PATCH 2/7] * Remove currently unused intra_block_offset() API. Correct a boundary check. --- accounts-db/src/tiered_storage/hot.rs | 7 +------ accounts-db/src/tiered_storage/index.rs | 3 --- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 561058ad352c23..08735afc22ce32 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -69,7 +69,7 @@ impl AccountOffset for HotAccountOffset { /// Creates a new AccountOffset instance given the block offset and the /// intra-block offset. fn new(block_offset: usize, intra_block_offset: usize) -> TieredStorageResult { - if block_offset >= MAX_HOT_ACCOUNT_OFFSET { + if block_offset > MAX_HOT_ACCOUNT_OFFSET { return Err(TieredStorageError::OffsetOutOfBounds( block_offset, MAX_HOT_ACCOUNT_OFFSET, @@ -103,11 +103,6 @@ impl AccountOffset for HotAccountOffset { fn block_offset(&self) -> usize { self.0 as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER } - - /// The offset to the account inside the account block. - fn intra_block_offset(&self) -> usize { - 0 - } } /// The storage and in-memory representation of the metadata entry for a diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index 5e9da26281b6da..c0a93980a85a5f 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -31,9 +31,6 @@ pub trait AccountOffset { /// The offset to the block that contains the account. fn block_offset(&self) -> usize; - - /// The offset to the account inside the account block. - fn intra_block_offset(&self) -> usize; } /// The offset to an account/address entry in the accounts index block. From 55e740ea78e162bf24a4b5d749cf9ed433ef4a4e Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Fri, 8 Dec 2023 09:59:32 -0800 Subject: [PATCH 3/7] Remove public APIs from AccountOffset trait. Make AccountIndexWriterEntry hold AccountOffset directly. --- accounts-db/src/tiered_storage/error.rs | 3 -- accounts-db/src/tiered_storage/hot.rs | 50 ++++++++++--------------- accounts-db/src/tiered_storage/index.rs | 47 ++++++++--------------- 3 files changed, 35 insertions(+), 65 deletions(-) diff --git a/accounts-db/src/tiered_storage/error.rs b/accounts-db/src/tiered_storage/error.rs index 47b5a49413a420..e0c8ffa5ca482d 100644 --- a/accounts-db/src/tiered_storage/error.rs +++ b/accounts-db/src/tiered_storage/error.rs @@ -31,7 +31,4 @@ pub enum TieredStorageError { #[error("OffsetAlignmentError: offset {0} must be multiple of {1}")] OffsetAlignmentError(usize, usize), - - #[error("IntraBlockOffsetOutOfBounds: offset {0} is larger than the supported size {1}")] - IntraBlockOffsetOutOfBounds(usize, usize), } diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 08735afc22ce32..3a704e97e971ca 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -65,42 +65,33 @@ struct HotMetaPackedFields { #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] pub struct HotAccountOffset(u32); -impl AccountOffset for HotAccountOffset { - /// Creates a new AccountOffset instance given the block offset and the - /// intra-block offset. - fn new(block_offset: usize, intra_block_offset: usize) -> TieredStorageResult { - if block_offset > MAX_HOT_ACCOUNT_OFFSET { +impl AccountOffset for HotAccountOffset {} + +impl HotAccountOffset { + /// Creates a new AccountOffset instance + pub fn new(offset: usize) -> TieredStorageResult { + if offset > MAX_HOT_ACCOUNT_OFFSET { return Err(TieredStorageError::OffsetOutOfBounds( - block_offset, + offset, MAX_HOT_ACCOUNT_OFFSET, )); } // Hot accounts are aligned based on HOT_ACCOUNT_OFFSET_MULTIPLIER. - if block_offset % HOT_ACCOUNT_OFFSET_MULTIPLIER != 0 { + if offset % HOT_ACCOUNT_OFFSET_MULTIPLIER != 0 { return Err(TieredStorageError::OffsetAlignmentError( - block_offset, + offset, HOT_ACCOUNT_OFFSET_MULTIPLIER, )); } - // Each hot account has its own account block. As a result, its - // intra-block offset is always 0. Any intra-block offset that is - // greater than 0 is invalid. - if intra_block_offset > 0 { - return Err(TieredStorageError::IntraBlockOffsetOutOfBounds( - block_offset, - 0, - )); - } - Ok(HotAccountOffset( - (block_offset / HOT_ACCOUNT_OFFSET_MULTIPLIER) as u32, + (offset / HOT_ACCOUNT_OFFSET_MULTIPLIER) as u32, )) } - /// The offset to the block that contains the account. - fn block_offset(&self) -> usize { + /// Returns the offset to the account. + fn offset(&self) -> usize { self.0 as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER } } @@ -279,7 +270,7 @@ impl HotStorageReader { &self, account_offset: HotAccountOffset, ) -> TieredStorageResult<&HotAccountMeta> { - let internal_account_offset = account_offset.block_offset(); + let internal_account_offset = account_offset.offset(); let (meta, _) = get_type::(&self.mmap, internal_account_offset)?; Ok(meta) @@ -524,7 +515,7 @@ pub mod tests { .map(|meta| { let prev_offset = current_offset; current_offset += file.write_type(meta).unwrap(); - HotAccountOffset::new(prev_offset, 0).unwrap() + HotAccountOffset::new(prev_offset).unwrap() }) .collect(); // while the test only focuses on account metas, writing a footer @@ -559,8 +550,10 @@ pub mod tests { .iter() .map(|address| AccountIndexWriterEntry { address, - block_offset: rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, - intra_block_offset: 0, + offset: HotAccountOffset::new( + rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, + ) + .unwrap(), }) .collect(); @@ -577,7 +570,7 @@ pub mod tests { footer .index_block_format - .write_index_block::(&file, &index_writer_entries) + .write_index_block(&file, &index_writer_entries) .unwrap(); // while the test only focuses on account metas, writing a footer @@ -590,10 +583,7 @@ pub mod tests { let account_offset = hot_storage .get_account_offset(IndexOffset(i as u32)) .unwrap(); - assert_eq!( - account_offset.block_offset(), - index_writer_entry.block_offset - ); + assert_eq!(*account_offset, index_writer_entry.offset); let account_address = hot_storage .get_account_address(IndexOffset(i as u32)) diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index c0a93980a85a5f..71411acf8bde5f 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -8,30 +8,16 @@ use { }; /// The in-memory struct for the writing index block. -/// The actual storage format of a tiered account index entry might be different -/// from this. #[derive(Debug)] -pub struct AccountIndexWriterEntry<'a> { +pub struct AccountIndexWriterEntry<'a, T: AccountOffset> { + /// The account address. pub address: &'a Pubkey, - /// The offset to the block that contains the account. - pub block_offset: usize, - /// The offset to the account inside the account block. - pub intra_block_offset: usize, + /// The offset to the account. + pub offset: T, } -/// The offset to an account stored inside its accounts block. -/// This struct is used to access the meta and data of an account by looking through -/// its accounts block. -pub trait AccountOffset { - /// Creates a new AccountOffset instance given the block offset and the - /// intra-block offset. - fn new(block_offset: usize, intra_block_offset: usize) -> TieredStorageResult - where - Self: Sized; - - /// The offset to the block that contains the account. - fn block_offset(&self) -> usize; -} +/// The offset to an account. +pub trait AccountOffset {} /// The offset to an account/address entry in the accounts index block. /// This can be used to obtain the AccountOffset and address by looking through @@ -63,10 +49,10 @@ pub enum IndexBlockFormat { impl IndexBlockFormat { /// Persists the specified index_entries to the specified file and returns /// the total number of bytes written. - pub fn write_index_block( + pub fn write_index_block( &self, file: &TieredStorageFile, - index_entries: &[AccountIndexWriterEntry], + index_entries: &[AccountIndexWriterEntry], ) -> TieredStorageResult { match self { Self::AddressAndBlockOffsetOnly => { @@ -75,10 +61,7 @@ impl IndexBlockFormat { bytes_written += file.write_type(index_entry.address)?; } for index_entry in index_entries { - bytes_written += file.write_type(&T::new( - index_entry.block_offset, - index_entry.intra_block_offset, - )?)?; + bytes_written += file.write_type(&index_entry.offset)?; } Ok(bytes_written) } @@ -162,17 +145,17 @@ mod tests { .iter() .map(|address| AccountIndexWriterEntry { address, - block_offset: rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, - intra_block_offset: 0, + offset: HotAccountOffset::new( + rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, + ) + .unwrap(), }) .collect(); { let file = TieredStorageFile::new_writable(&path).unwrap(); let indexer = IndexBlockFormat::AddressAndBlockOffsetOnly; - indexer - .write_index_block::(&file, &index_entries) - .unwrap(); + indexer.write_index_block(&file, &index_entries).unwrap(); } let indexer = IndexBlockFormat::AddressAndBlockOffsetOnly; @@ -186,7 +169,7 @@ mod tests { let account_offset = indexer .get_account_offset::(&mmap, &footer, IndexOffset(i as u32)) .unwrap(); - assert_eq!(index_entry.block_offset, account_offset.block_offset()); + assert_eq!(index_entry.offset, *account_offset); let address = indexer .get_account_address(&mmap, &footer, IndexOffset(i as u32)) .unwrap(); From 314bc561cce4774069126a60960a49e4c7238f1a Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Fri, 8 Dec 2023 10:12:53 -0800 Subject: [PATCH 4/7] Have get_account_offset() returns an instance instead of a reference. --- accounts-db/src/tiered_storage/hot.rs | 4 ++-- accounts-db/src/tiered_storage/index.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 3a704e97e971ca..21183ca4b71ee0 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -280,7 +280,7 @@ impl HotStorageReader { fn get_account_offset( &self, index_offset: IndexOffset, - ) -> TieredStorageResult<&HotAccountOffset> { + ) -> TieredStorageResult { self.footer .index_block_format .get_account_offset::(&self.mmap, &self.footer, index_offset) @@ -583,7 +583,7 @@ pub mod tests { let account_offset = hot_storage .get_account_offset(IndexOffset(i as u32)) .unwrap(); - assert_eq!(*account_offset, index_writer_entry.offset); + assert_eq!(account_offset, index_writer_entry.offset); let account_address = hot_storage .get_account_address(IndexOffset(i as u32)) diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index 71411acf8bde5f..c3b1f802f855fd 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -86,12 +86,12 @@ impl IndexBlockFormat { } /// Returns the offset to the account given the specified index. - pub fn get_account_offset<'a, T: AccountOffset>( + pub fn get_account_offset<'a, T: AccountOffset + Copy>( &self, - mmap: &'a Mmap, + mmap: &Mmap, footer: &TieredStorageFooter, index_offset: IndexOffset, - ) -> TieredStorageResult<&'a T> { + ) -> TieredStorageResult { match self { Self::AddressAndBlockOffsetOnly => { let offset = footer.index_block_offset as usize @@ -99,7 +99,7 @@ impl IndexBlockFormat { + std::mem::size_of::() * index_offset.0 as usize; let (account_offset, _) = get_type::(mmap, offset)?; - Ok(account_offset) + Ok(*account_offset) } } } @@ -169,7 +169,7 @@ mod tests { let account_offset = indexer .get_account_offset::(&mmap, &footer, IndexOffset(i as u32)) .unwrap(); - assert_eq!(index_entry.offset, *account_offset); + assert_eq!(index_entry.offset, account_offset); let address = indexer .get_account_address(&mmap, &footer, IndexOffset(i as u32)) .unwrap(); From 2d354ec68b0640bcc573798982dab465418f3f5f Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Fri, 8 Dec 2023 10:17:45 -0800 Subject: [PATCH 5/7] * Replaced u32 to T: AccountOffset in other index.rs functions. --- accounts-db/src/tiered_storage/index.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index c3b1f802f855fd..af699ed143bbfb 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -96,7 +96,7 @@ impl IndexBlockFormat { Self::AddressAndBlockOffsetOnly => { let offset = footer.index_block_offset as usize + std::mem::size_of::() * footer.account_entry_count as usize - + std::mem::size_of::() * index_offset.0 as usize; + + std::mem::size_of::() * index_offset.0 as usize; let (account_offset, _) = get_type::(mmap, offset)?; Ok(*account_offset) @@ -105,10 +105,10 @@ impl IndexBlockFormat { } /// Returns the size of one index entry. - pub fn entry_size(&self) -> usize { + pub fn entry_size(&self) -> usize { match self { Self::AddressAndBlockOffsetOnly => { - std::mem::size_of::() + std::mem::size_of::() + std::mem::size_of::() + std::mem::size_of::() } } } From e9ead661e0ea83e1620d14a48a43fbf24b50c786 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Fri, 8 Dec 2023 10:50:14 -0800 Subject: [PATCH 6/7] Removed one unused 'a --- accounts-db/src/tiered_storage/index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index af699ed143bbfb..62825961e28d27 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -86,7 +86,7 @@ impl IndexBlockFormat { } /// Returns the offset to the account given the specified index. - pub fn get_account_offset<'a, T: AccountOffset + Copy>( + pub fn get_account_offset( &self, mmap: &Mmap, footer: &TieredStorageFooter, From e3869c366115d0ebdfd087b3058408ebc36ac645 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Fri, 8 Dec 2023 14:45:59 -0800 Subject: [PATCH 7/7] * T -> Offset. HOT_ACCOUNT_OFFSET_MULTIPLIER -> HOT_ACCOUNT_OFFSET_MULTIPLIER. --- accounts-db/src/tiered_storage/hot.rs | 21 +++++++++++---------- accounts-db/src/tiered_storage/index.rs | 20 ++++++++++---------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 21183ca4b71ee0..e365b638dff3f3 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -35,12 +35,13 @@ const MAX_HOT_PADDING: u8 = 7; /// The maximum allowed value for the owner index of a hot account. const MAX_HOT_OWNER_OFFSET: OwnerOffset = OwnerOffset((1 << 29) - 1); -/// The multiplier for converting HotAccountOffset to the internal hot account -/// offset. This increases the maximum size of a hot accounts file. -pub(crate) const HOT_ACCOUNT_OFFSET_MULTIPLIER: usize = 8; +/// The alignment for HotAccountOffset. It is also a multiplier for converting +/// HotAccountOffset to the internal hot account offset that increases the maximum +/// size of a hot accounts file. +pub(crate) const HOT_ACCOUNT_OFFSET_ALIGNMENT: usize = 8; /// The maximum supported offset for hot accounts storage. -const MAX_HOT_ACCOUNT_OFFSET: usize = u32::MAX as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER; +const MAX_HOT_ACCOUNT_OFFSET: usize = u32::MAX as usize * HOT_ACCOUNT_OFFSET_ALIGNMENT; #[bitfield(bits = 32)] #[repr(C)] @@ -77,22 +78,22 @@ impl HotAccountOffset { )); } - // Hot accounts are aligned based on HOT_ACCOUNT_OFFSET_MULTIPLIER. - if offset % HOT_ACCOUNT_OFFSET_MULTIPLIER != 0 { + // Hot accounts are aligned based on HOT_ACCOUNT_OFFSET_ALIGNMENT. + if offset % HOT_ACCOUNT_OFFSET_ALIGNMENT != 0 { return Err(TieredStorageError::OffsetAlignmentError( offset, - HOT_ACCOUNT_OFFSET_MULTIPLIER, + HOT_ACCOUNT_OFFSET_ALIGNMENT, )); } Ok(HotAccountOffset( - (offset / HOT_ACCOUNT_OFFSET_MULTIPLIER) as u32, + (offset / HOT_ACCOUNT_OFFSET_ALIGNMENT) as u32, )) } /// Returns the offset to the account. fn offset(&self) -> usize { - self.0 as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER + self.0 as usize * HOT_ACCOUNT_OFFSET_ALIGNMENT } } @@ -551,7 +552,7 @@ pub mod tests { .map(|address| AccountIndexWriterEntry { address, offset: HotAccountOffset::new( - rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, + rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_ALIGNMENT, ) .unwrap(), }) diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index 62825961e28d27..0921bf6259cead 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -9,11 +9,11 @@ use { /// The in-memory struct for the writing index block. #[derive(Debug)] -pub struct AccountIndexWriterEntry<'a, T: AccountOffset> { +pub struct AccountIndexWriterEntry<'a, Offset: AccountOffset> { /// The account address. pub address: &'a Pubkey, /// The offset to the account. - pub offset: T, + pub offset: Offset, } /// The offset to an account. @@ -86,18 +86,18 @@ impl IndexBlockFormat { } /// Returns the offset to the account given the specified index. - pub fn get_account_offset( + pub fn get_account_offset( &self, mmap: &Mmap, footer: &TieredStorageFooter, index_offset: IndexOffset, - ) -> TieredStorageResult { + ) -> TieredStorageResult { match self { Self::AddressAndBlockOffsetOnly => { let offset = footer.index_block_offset as usize + std::mem::size_of::() * footer.account_entry_count as usize - + std::mem::size_of::() * index_offset.0 as usize; - let (account_offset, _) = get_type::(mmap, offset)?; + + std::mem::size_of::() * index_offset.0 as usize; + let (account_offset, _) = get_type::(mmap, offset)?; Ok(*account_offset) } @@ -105,10 +105,10 @@ impl IndexBlockFormat { } /// Returns the size of one index entry. - pub fn entry_size(&self) -> usize { + pub fn entry_size(&self) -> usize { match self { Self::AddressAndBlockOffsetOnly => { - std::mem::size_of::() + std::mem::size_of::() + std::mem::size_of::() + std::mem::size_of::() } } } @@ -120,7 +120,7 @@ mod tests { super::*, crate::tiered_storage::{ file::TieredStorageFile, - hot::{HotAccountOffset, HOT_ACCOUNT_OFFSET_MULTIPLIER}, + hot::{HotAccountOffset, HOT_ACCOUNT_OFFSET_ALIGNMENT}, }, memmap2::MmapOptions, rand::Rng, @@ -146,7 +146,7 @@ mod tests { .map(|address| AccountIndexWriterEntry { address, offset: HotAccountOffset::new( - rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_MULTIPLIER, + rng.gen_range(0..u32::MAX) as usize * HOT_ACCOUNT_OFFSET_ALIGNMENT, ) .unwrap(), })