introduce UpsertReclaim (#26462)

This commit is contained in:
Jeff Washington (jwash) 2022-07-07 15:40:17 -05:00 committed by GitHub
parent 5bffee248c
commit b582e4ce0f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 56 additions and 47 deletions

View File

@ -7,7 +7,8 @@ use {
solana_runtime::{ solana_runtime::{
account_info::AccountInfo, account_info::AccountInfo,
accounts_index::{ accounts_index::{
AccountSecondaryIndexes, AccountsIndex, ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS, AccountSecondaryIndexes, AccountsIndex, UpsertReclaim,
ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS,
}, },
}, },
solana_sdk::{account::AccountSharedData, pubkey}, solana_sdk::{account::AccountSharedData, pubkey},
@ -33,7 +34,7 @@ fn bench_accounts_index(bencher: &mut Bencher) {
&AccountSecondaryIndexes::default(), &AccountSecondaryIndexes::default(),
AccountInfo::default(), AccountInfo::default(),
&mut reclaims, &mut reclaims,
false, UpsertReclaim::PopulateReclaims,
); );
} }
} }
@ -51,7 +52,7 @@ fn bench_accounts_index(bencher: &mut Bencher) {
&AccountSecondaryIndexes::default(), &AccountSecondaryIndexes::default(),
AccountInfo::default(), AccountInfo::default(),
&mut reclaims, &mut reclaims,
false, UpsertReclaim::PopulateReclaims,
); );
reclaims.clear(); reclaims.clear();
} }

View File

@ -30,7 +30,7 @@ use {
accounts_index::{ accounts_index::{
AccountIndexGetResult, AccountSecondaryIndexes, AccountsIndex, AccountsIndexConfig, AccountIndexGetResult, AccountSecondaryIndexes, AccountsIndex, AccountsIndexConfig,
AccountsIndexRootsStats, AccountsIndexScanResult, IndexKey, IndexValue, IsCached, AccountsIndexRootsStats, AccountsIndexScanResult, IndexKey, IndexValue, IsCached,
RefCount, ScanConfig, ScanResult, SlotList, SlotSlice, ZeroLamport, RefCount, ScanConfig, ScanResult, SlotList, SlotSlice, UpsertReclaim, ZeroLamport,
ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS, ACCOUNTS_INDEX_CONFIG_FOR_TESTING, ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS, ACCOUNTS_INDEX_CONFIG_FOR_TESTING,
}, },
accounts_index_storage::Startup, accounts_index_storage::Startup,
@ -7107,7 +7107,7 @@ impl AccountsDb {
&self, &self,
infos: Vec<AccountInfo>, infos: Vec<AccountInfo>,
accounts: impl StorableAccounts<'a, T>, accounts: impl StorableAccounts<'a, T>,
previous_slot_entry_was_cached: bool, reclaim: UpsertReclaim,
) -> SlotList<AccountInfo> { ) -> SlotList<AccountInfo> {
let target_slot = accounts.target_slot(); let target_slot = accounts.target_slot();
// using a thread pool here results in deadlock panics from bank_hashes.write() // using a thread pool here results in deadlock panics from bank_hashes.write()
@ -7130,7 +7130,7 @@ impl AccountsDb {
&self.account_indexes, &self.account_indexes,
info, info,
&mut reclaims, &mut reclaims,
previous_slot_entry_was_cached, reclaim,
); );
}); });
reclaims reclaims
@ -7716,7 +7716,11 @@ impl AccountsDb {
.fetch_add(store_accounts_time.as_us(), Ordering::Relaxed); .fetch_add(store_accounts_time.as_us(), Ordering::Relaxed);
let mut update_index_time = Measure::start("update_index"); let mut update_index_time = Measure::start("update_index");
let previous_slot_entry_was_cached = self.caching_enabled && is_cached_store; let reclaim = if self.caching_enabled && is_cached_store {
UpsertReclaim::PreviousSlotEntryWasCached
} else {
UpsertReclaim::PopulateReclaims
};
// if we are squashing a single slot, then we can expect a single dead slot // if we are squashing a single slot, then we can expect a single dead slot
let expected_single_dead_slot = let expected_single_dead_slot =
@ -7726,7 +7730,7 @@ impl AccountsDb {
// after the account are stored by the above `store_accounts_to` // after the account are stored by the above `store_accounts_to`
// call and all the accounts are stored, all reads after this point // call and all the accounts are stored, all reads after this point
// will know to not check the cache anymore // will know to not check the cache anymore
let mut reclaims = self.update_index(infos, accounts, previous_slot_entry_was_cached); let mut reclaims = self.update_index(infos, accounts, reclaim);
// For each updated account, `reclaims` should only have at most one // For each updated account, `reclaims` should only have at most one
// item (if the account was previously updated in this slot). // item (if the account was previously updated in this slot).
@ -12690,7 +12694,8 @@ pub mod tests {
); );
} }
const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: bool = false; const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: UpsertReclaim =
UpsertReclaim::PopulateReclaims;
#[test] #[test]
fn test_delete_dependencies() { fn test_delete_dependencies() {

View File

@ -70,6 +70,16 @@ pub type SlotSlice<'s, T> = &'s [(Slot, T)];
pub type RefCount = u64; pub type RefCount = u64;
pub type AccountMap<V> = Arc<InMemAccountsIndex<V>>; pub type AccountMap<V> = Arc<InMemAccountsIndex<V>>;
#[derive(Debug, Clone, Copy)]
/// how accounts index 'upsert' should handle reclaims
pub enum UpsertReclaim {
/// previous entry for this slot in the index is expected to be cached, so irrelevant to reclaims
PreviousSlotEntryWasCached,
/// previous entry for this slot in the index may need to be reclaimed, so return it.
/// reclaims is the only output of upsert, requiring a synchronous execution
PopulateReclaims,
}
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct ScanConfig { pub struct ScanConfig {
/// checked by the scan. When true, abort scan. /// checked by the scan. When true, abort scan.
@ -1605,7 +1615,7 @@ impl<T: IndexValue> AccountsIndex<T> {
account_indexes: &AccountSecondaryIndexes, account_indexes: &AccountSecondaryIndexes,
account_info: T, account_info: T,
reclaims: &mut SlotList<T>, reclaims: &mut SlotList<T>,
previous_slot_entry_was_cached: bool, reclaim: UpsertReclaim,
) { ) {
// vast majority of updates are to item already in accounts index, so store as raw to avoid unnecessary allocations // vast majority of updates are to item already in accounts index, so store as raw to avoid unnecessary allocations
let store_raw = true; let store_raw = true;
@ -1631,13 +1641,7 @@ impl<T: IndexValue> AccountsIndex<T> {
{ {
let r_account_maps = map.read().unwrap(); let r_account_maps = map.read().unwrap();
r_account_maps.upsert( r_account_maps.upsert(pubkey, new_item, Some(old_slot), reclaims, reclaim);
pubkey,
new_item,
Some(old_slot),
reclaims,
previous_slot_entry_was_cached,
);
} }
self.update_secondary_indexes(pubkey, account, account_indexes); self.update_secondary_indexes(pubkey, account, account_indexes);
} }
@ -2301,7 +2305,8 @@ pub mod tests {
assert!(index.include_key(&pk2)); assert!(index.include_key(&pk2));
} }
const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: bool = false; const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: UpsertReclaim =
UpsertReclaim::PopulateReclaims;
#[test] #[test]
fn test_insert_no_ancestors() { fn test_insert_no_ancestors() {

View File

@ -2,7 +2,7 @@ use {
crate::{ crate::{
accounts_index::{ accounts_index::{
AccountMapEntry, AccountMapEntryInner, AccountMapEntryMeta, IndexValue, AccountMapEntry, AccountMapEntryInner, AccountMapEntryMeta, IndexValue,
PreAllocatedAccountMapEntry, RefCount, SlotList, SlotSlice, ZeroLamport, PreAllocatedAccountMapEntry, RefCount, SlotList, SlotSlice, UpsertReclaim, ZeroLamport,
}, },
bucket_map_holder::{Age, BucketMapHolder}, bucket_map_holder::{Age, BucketMapHolder},
bucket_map_holder_stats::BucketMapHolderStats, bucket_map_holder_stats::BucketMapHolderStats,
@ -364,7 +364,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_value: PreAllocatedAccountMapEntry<T>, new_value: PreAllocatedAccountMapEntry<T>,
other_slot: Option<Slot>, other_slot: Option<Slot>,
reclaims: &mut SlotList<T>, reclaims: &mut SlotList<T>,
previous_slot_entry_was_cached: bool, reclaim: UpsertReclaim,
) { ) {
let mut updated_in_mem = true; let mut updated_in_mem = true;
// try to get it just from memory first using only a read lock // try to get it just from memory first using only a read lock
@ -375,7 +375,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_value.into(), new_value.into(),
other_slot, other_slot,
reclaims, reclaims,
previous_slot_entry_was_cached, reclaim,
); );
// age is incremented by caller // age is incremented by caller
} else { } else {
@ -392,7 +392,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_value.into(), new_value.into(),
other_slot, other_slot,
reclaims, reclaims,
previous_slot_entry_was_cached, reclaim,
); );
current.set_age(self.storage.future_age_to_flush()); current.set_age(self.storage.future_age_to_flush());
} }
@ -407,13 +407,8 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
// We may like this to always run, but it is unclear. // We may like this to always run, but it is unclear.
// If disk bucket needs to resize, then this call can stall for a long time. // If disk bucket needs to resize, then this call can stall for a long time.
// Right now, we know it is safe during startup. // Right now, we know it is safe during startup.
let already_existed = self.upsert_on_disk( let already_existed = self
vacant, .upsert_on_disk(vacant, new_value, other_slot, reclaims, reclaim);
new_value,
other_slot,
reclaims,
previous_slot_entry_was_cached,
);
if !already_existed { if !already_existed {
self.stats().inc_insert(); self.stats().inc_insert();
} }
@ -427,7 +422,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_value.into(), new_value.into(),
other_slot, other_slot,
reclaims, reclaims,
previous_slot_entry_was_cached, reclaim,
); );
disk_entry disk_entry
} else { } else {
@ -471,7 +466,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_value: (Slot, T), new_value: (Slot, T),
other_slot: Option<Slot>, other_slot: Option<Slot>,
reclaims: &mut SlotList<T>, reclaims: &mut SlotList<T>,
previous_slot_entry_was_cached: bool, reclaim: UpsertReclaim,
) { ) {
let mut slot_list = current.slot_list.write().unwrap(); let mut slot_list = current.slot_list.write().unwrap();
let (slot, new_entry) = new_value; let (slot, new_entry) = new_value;
@ -481,7 +476,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_entry, new_entry,
other_slot, other_slot,
reclaims, reclaims,
previous_slot_entry_was_cached, reclaim,
); );
if addref { if addref {
current.add_un_ref(true); current.add_un_ref(true);
@ -504,7 +499,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
account_info: T, account_info: T,
mut other_slot: Option<Slot>, mut other_slot: Option<Slot>,
reclaims: &mut SlotList<T>, reclaims: &mut SlotList<T>,
previous_slot_entry_was_cached: bool, reclaim: UpsertReclaim,
) -> bool { ) -> bool {
let mut addref = !account_info.is_cached(); let mut addref = !account_info.is_cached();
@ -546,11 +541,14 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
// already replaced one entry, so this one has to be removed // already replaced one entry, so this one has to be removed
slot_list.remove(slot_list_index) slot_list.remove(slot_list_index)
}; };
if previous_slot_entry_was_cached { match reclaim {
assert!(is_cur_account_cached); UpsertReclaim::PopulateReclaims => {
} else {
reclaims.push(reclaim_item); reclaims.push(reclaim_item);
} }
UpsertReclaim::PreviousSlotEntryWasCached => {
assert!(is_cur_account_cached);
}
}
if matched_slot { if matched_slot {
found_slot = true; found_slot = true;
@ -618,7 +616,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
(slot, account_info), (slot, account_info),
None, // should be None because we don't expect a different slot # during index generation None, // should be None because we don't expect a different slot # during index generation
&mut Vec::default(), &mut Vec::default(),
false, UpsertReclaim::PopulateReclaims, // this should be ignore?
); );
( (
true, /* found in mem */ true, /* found in mem */
@ -637,7 +635,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_entry, new_entry,
None, // not changing slots here since it doesn't exist in the index at all None, // not changing slots here since it doesn't exist in the index at all
&mut Vec::default(), &mut Vec::default(),
false, UpsertReclaim::PopulateReclaims,
); );
(false, already_existed) (false, already_existed)
} else { } else {
@ -652,7 +650,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
// There can be no 'other' slot in the list. // There can be no 'other' slot in the list.
None, None,
&mut Vec::default(), &mut Vec::default(),
false, UpsertReclaim::PopulateReclaims,
); );
vacant.insert(disk_entry); vacant.insert(disk_entry);
( (
@ -694,7 +692,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
new_entry: PreAllocatedAccountMapEntry<T>, new_entry: PreAllocatedAccountMapEntry<T>,
other_slot: Option<Slot>, other_slot: Option<Slot>,
reclaims: &mut SlotList<T>, reclaims: &mut SlotList<T>,
previous_slot_entry_was_cached: bool, reclaim: UpsertReclaim,
) -> bool { ) -> bool {
if let Some(disk) = self.bucket.as_ref() { if let Some(disk) = self.bucket.as_ref() {
let mut existed = false; let mut existed = false;
@ -709,7 +707,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
account_info, account_info,
other_slot, other_slot,
reclaims, reclaims,
previous_slot_entry_was_cached, reclaim,
); );
if addref { if addref {
ref_count += 1 ref_count += 1
@ -1672,7 +1670,7 @@ mod tests {
#[test] #[test]
fn test_update_slot_list_other() { fn test_update_slot_list_other() {
solana_logger::setup(); solana_logger::setup();
let previous_slot_entry_was_cached = false; let reclaim = UpsertReclaim::PopulateReclaims;
let new_slot = 0; let new_slot = 0;
let info = 1; let info = 1;
let other_value = info + 1; let other_value = info + 1;
@ -1689,7 +1687,7 @@ mod tests {
info, info,
other_slot, other_slot,
&mut reclaims, &mut reclaims,
previous_slot_entry_was_cached reclaim
), ),
"other_slot: {:?}", "other_slot: {:?}",
other_slot other_slot
@ -1711,7 +1709,7 @@ mod tests {
info, info,
other_slot, other_slot,
&mut reclaims, &mut reclaims,
previous_slot_entry_was_cached reclaim
), ),
"other_slot: {:?}", "other_slot: {:?}",
other_slot other_slot
@ -1732,7 +1730,7 @@ mod tests {
info, info,
other_slot, other_slot,
&mut reclaims, &mut reclaims,
previous_slot_entry_was_cached reclaim
), ),
"other_slot: {:?}", "other_slot: {:?}",
other_slot other_slot
@ -1805,7 +1803,7 @@ mod tests {
info, info,
other_slot, other_slot,
&mut reclaims, &mut reclaims,
previous_slot_entry_was_cached, reclaim,
); );
// calculate expected results // calculate expected results