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.

Sign up for GitHub

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

make store_id an Option in mem acct idx #30512

Closed
wants to merge 4 commits into from
Closed

make store_id an Option in mem acct idx #30512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ff3025b
add index_entries_being_shrunk to ShrinkCollect
jeffwashington Feb 24, 2023
0b6670f
make store_id an Option in mem acct idx
jeffwashington Feb 24, 2023
bb17a70
make store_id an Option in mem acct idx
jeffwashington Feb 25, 2023
96224a3
after shrinking, wait until fg processing is complete before swapping…
jeffwashington Feb 27, 2023
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
37 changes: 26 additions & 11 deletions runtime/src/account_info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub type StoredSize = u32;
/// specify where account data is located
#[derive(Debug, PartialEq, Eq)]
pub enum StorageLocation {
AppendVec(AppendVecId, Offset),
AppendVec(Option<AppendVecId>, Offset),
Cached,
}

Expand Down Expand Up @@ -84,8 +84,9 @@ pub struct PackedOffsetAndFlags {

#[derive(Default, Debug, PartialEq, Eq, Clone, Copy)]
pub struct AccountInfo {
/// index identifying the append storage
store_id: AppendVecId,
/// index identifying the append vec storage
/// None means the default one associated with the slot
store_id: Option<AppendVecId>,

account_offset_and_flags: AccountOffsetAndFlags,
}
Expand All @@ -97,6 +98,23 @@ pub struct AccountOffsetAndFlags {
packed_offset_and_flags: PackedOffsetAndFlags,
}

impl From<AccountOffsetAndFlags> for AccountInfo {
fn from(account_offset_and_flags: AccountOffsetAndFlags) -> Self {
Self {
store_id: None,
account_offset_and_flags,
}
}
}

impl From<AccountInfo> for AccountOffsetAndFlags {
fn from(info: AccountInfo) -> Self {
Self {
packed_offset_and_flags: info.account_offset_and_flags.packed_offset_and_flags,
}
}
}

impl ZeroLamport for AccountInfo {
fn is_zero_lamport(&self) -> bool {
self.account_offset_and_flags
Expand All @@ -120,9 +138,6 @@ impl IsCached for StorageLocation {
}
}

/// We have to have SOME value for store_id when we are cached
const CACHE_VIRTUAL_STORAGE_ID: AppendVecId = AppendVecId::MAX;

impl AccountInfo {
pub fn new(storage_location: StorageLocation, lamports: u64) -> Self {
let mut packed_offset_and_flags = PackedOffsetAndFlags::default();
Expand All @@ -143,7 +158,7 @@ impl AccountInfo {
}
StorageLocation::Cached => {
packed_offset_and_flags.set_offset_reduced(CACHED_OFFSET);
CACHE_VIRTUAL_STORAGE_ID
None
}
};
packed_offset_and_flags.set_is_zero_lamport(lamports == 0);
Expand All @@ -160,7 +175,7 @@ impl AccountInfo {
(offset / ALIGN_BOUNDARY_OFFSET) as OffsetReduced
}

pub fn store_id(&self) -> AppendVecId {
pub fn store_id(&self) -> Option<AppendVecId> {
// if the account is in a cached store, the store_id is meaningless
assert!(!self.is_cached());
self.store_id
Expand Down Expand Up @@ -202,7 +217,7 @@ mod test {
ALIGN_BOUNDARY_OFFSET,
4 * ALIGN_BOUNDARY_OFFSET,
] {
let info = AccountInfo::new(StorageLocation::AppendVec(0, offset), 0);
let info = AccountInfo::new(StorageLocation::AppendVec(None, offset), 0);
assert!(info.offset() == offset);
}
}
Expand All @@ -211,13 +226,13 @@ mod test {
#[should_panic(expected = "illegal offset")]
fn test_illegal_offset() {
let offset = (MAXIMUM_APPEND_VEC_FILE_SIZE - (ALIGN_BOUNDARY_OFFSET as u64)) as Offset;
AccountInfo::new(StorageLocation::AppendVec(0, offset), 0);
AccountInfo::new(StorageLocation::AppendVec(None, offset), 0);
}

#[test]
#[should_panic(expected = "illegal offset")]
fn test_alignment() {
let offset = 1; // not aligned
AccountInfo::new(StorageLocation::AppendVec(0, offset), 0);
AccountInfo::new(StorageLocation::AppendVec(None, offset), 0);
}
}
42 changes: 25 additions & 17 deletions runtime/src/account_storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,30 @@ impl AccountStorage {
pub(crate) fn get_account_storage_entry(
&self,
slot: Slot,
store_id: AppendVecId,
store_id: Option<AppendVecId>,
) -> Option<Arc<AccountStorageEntry>> {
let lookup_in_map = || {
self.map
.get(&slot)
.and_then(|r| (r.id == store_id).then_some(Arc::clone(&r.storage)))
self.map.get(&slot).and_then(|r| {
Self::match_store_id(store_id, r.id).then_some(Arc::clone(&r.storage))
})
};

lookup_in_map()
.or_else(|| {
self.shrink_in_progress_map.get(&slot).and_then(|entry| {
(entry.value().append_vec_id() == store_id).then(|| Arc::clone(entry.value()))
Self::match_store_id(store_id, entry.value().append_vec_id())
.then(|| Arc::clone(entry.value()))
})
})
.or_else(lookup_in_map)
}

fn match_store_id(expected: Option<AppendVecId>, store_id: AppendVecId) -> bool {
expected
.map(|expected| expected == store_id)
.unwrap_or(true)
}

/// assert if shrink in progress is active
pub(crate) fn assert_no_shrink_in_progress(&self) {
assert!(self.shrink_in_progress_map.is_empty());
Expand Down Expand Up @@ -276,7 +283,7 @@ pub(crate) mod tests {
let slot = 0;
let id = 0;
// empty everything
assert!(storage.get_account_storage_entry(slot, id).is_none());
assert!(storage.get_account_storage_entry(slot, None).is_none());

// add a map store
let common_store_path = Path::new("");
Expand All @@ -299,11 +306,12 @@ pub(crate) mod tests {
.map
.insert(slot, AccountStorageReference { id, storage: entry });

//todo not sure about these append vec id values
// look in map
assert_eq!(
store_file_size,
storage
.get_account_storage_entry(slot, id)
.get_account_storage_entry(slot, None)
.map(|entry| entry.accounts.capacity())
.unwrap_or_default()
);
Expand All @@ -315,7 +323,7 @@ pub(crate) mod tests {
assert_eq!(
store_file_size,
storage
.get_account_storage_entry(slot, id)
.get_account_storage_entry(slot, None)
.map(|entry| entry.accounts.capacity())
.unwrap_or_default()
);
Expand All @@ -327,7 +335,7 @@ pub(crate) mod tests {
assert_eq!(
store_file_size2,
storage
.get_account_storage_entry(slot, id)
.get_account_storage_entry(slot, None)
.map(|entry| entry.accounts.capacity())
.unwrap_or_default()
);
Expand Down Expand Up @@ -515,10 +523,10 @@ pub(crate) mod tests {
let missing_id = 9999;
let slot = sample.slot();
// id is missing since not in maps at all
assert!(storage.get_account_storage_entry(slot, id).is_none());
assert!(storage.get_account_storage_entry(slot, None).is_none());
// missing should always be missing
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
storage.map.insert(
slot,
Expand All @@ -528,23 +536,23 @@ pub(crate) mod tests {
},
);
// id is found in map
assert!(storage.get_account_storage_entry(slot, id).is_some());
assert!(storage.get_account_storage_entry(slot, None).is_some());
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
storage
.shrink_in_progress_map
.insert(slot, Arc::clone(&sample));
// id is found in map
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
assert!(storage.get_account_storage_entry(slot, id).is_some());
assert!(storage.get_account_storage_entry(slot, None).is_some());
storage.map.remove(&slot);
// id is found in shrink_in_progress_map
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
assert!(storage.get_account_storage_entry(slot, id).is_some());
assert!(storage.get_account_storage_entry(slot, None).is_some());
}
}
Loading