diff --git a/runtime/benches/accounts_index.rs b/runtime/benches/accounts_index.rs index bff139f857..5e53a62798 100644 --- a/runtime/benches/accounts_index.rs +++ b/runtime/benches/accounts_index.rs @@ -29,6 +29,7 @@ fn bench_accounts_index(bencher: &mut Bencher) { &AccountSecondaryIndexes::default(), AccountInfo::default(), &mut reclaims, + false, ); } } @@ -46,6 +47,7 @@ fn bench_accounts_index(bencher: &mut Bencher) { &AccountSecondaryIndexes::default(), AccountInfo::default(), &mut reclaims, + false, ); reclaims.clear(); } diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index d9f8142f5e..10af4144e2 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -5153,11 +5153,14 @@ impl AccountsDb { ret } + // previous_slot_entry_was_cached = true means we just need to assert that after this update is complete + // that there are no items we would have put in reclaims that are not cached fn update_index( &self, slot: Slot, infos: Vec, accounts: &[(&Pubkey, &impl ReadableAccount)], + previous_slot_entry_was_cached: bool, ) -> SlotList { let mut reclaims = SlotList::::with_capacity(infos.len() * 2); for (info, pubkey_account) in infos.into_iter().zip(accounts.iter()) { @@ -5170,6 +5173,7 @@ impl AccountsDb { &self.account_indexes, info, &mut reclaims, + previous_slot_entry_was_cached, ); } reclaims @@ -5711,11 +5715,13 @@ impl AccountsDb { .fetch_add(store_accounts_time.as_us(), Ordering::Relaxed); let mut update_index_time = Measure::start("update_index"); + let previous_slot_entry_was_cached = self.caching_enabled && is_cached_store; + // If the cache was flushed, then because `update_index` occurs // after the account are stored by the above `store_accounts_to` // call and all the accounts are stored, all reads after this point // will know to not check the cache anymore - let mut reclaims = self.update_index(slot, infos, accounts); + let mut reclaims = self.update_index(slot, infos, accounts, previous_slot_entry_was_cached); // For each updated account, `reclaims` should only have at most one // item (if the account was previously updated in this slot). @@ -5723,7 +5729,7 @@ impl AccountsDb { // to anything that needs to be cleaned in the backing storage // entries if self.caching_enabled { - reclaims.retain(|(_, r)| r.store_id != CACHE_VIRTUAL_STORAGE_ID); + reclaims.retain(|(_, r)| !r.is_cached()); if is_cached_store { assert!(reclaims.is_empty()); @@ -9848,6 +9854,8 @@ pub mod tests { ); } + const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: bool = false; + #[test] fn test_delete_dependencies() { solana_logger::setup(); @@ -9888,6 +9896,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), info0, &mut reclaims, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); accounts_index.upsert( 1, @@ -9897,6 +9906,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), info1.clone(), &mut reclaims, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); accounts_index.upsert( 1, @@ -9906,6 +9916,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), info1, &mut reclaims, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); accounts_index.upsert( 2, @@ -9915,6 +9926,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), info2.clone(), &mut reclaims, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); accounts_index.upsert( 2, @@ -9924,6 +9936,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), info2, &mut reclaims, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); accounts_index.upsert( 3, @@ -9933,6 +9946,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), info3, &mut reclaims, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); accounts_index.add_root(0, false); accounts_index.add_root(1, false); diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index 5f8579f9c4..2d8a367b6e 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -203,11 +203,17 @@ impl WriteAccountMapEntry { pubkey: &Pubkey, new_value: AccountMapEntry, reclaims: &mut SlotList, + previous_slot_entry_was_cached: bool, ) { match w_account_maps.entry(*pubkey) { Entry::Occupied(mut occupied) => { let current = occupied.get_mut(); - Self::lock_and_update_slot_list(current, &new_value, reclaims); + Self::lock_and_update_slot_list( + current, + &new_value, + reclaims, + previous_slot_entry_was_cached, + ); } Entry::Vacant(vacant) => { vacant.insert(new_value); @@ -222,9 +228,15 @@ impl WriteAccountMapEntry { pubkey: &Pubkey, new_value: &AccountMapEntry, reclaims: &mut SlotList, + previous_slot_entry_was_cached: bool, ) -> bool { if let Some(current) = r_account_maps.get(pubkey) { - Self::lock_and_update_slot_list(current, new_value, reclaims); + Self::lock_and_update_slot_list( + current, + new_value, + reclaims, + previous_slot_entry_was_cached, + ); true } else { false @@ -235,10 +247,17 @@ impl WriteAccountMapEntry { current: &Arc>, new_value: &AccountMapEntry, reclaims: &mut SlotList, + previous_slot_entry_was_cached: bool, ) { let mut slot_list = current.slot_list.write().unwrap(); let (slot, new_entry) = new_value.slot_list.write().unwrap().remove(0); - let addref = Self::update_slot_list(&mut slot_list, slot, new_entry, reclaims); + let addref = Self::update_slot_list( + &mut slot_list, + slot, + new_entry, + reclaims, + previous_slot_entry_was_cached, + ); if addref { Self::addref(¤t.ref_count); } @@ -251,6 +270,7 @@ impl WriteAccountMapEntry { slot: Slot, account_info: T, reclaims: &mut SlotList, + previous_slot_entry_was_cached: bool, ) -> bool { let mut addref = !account_info.is_cached(); @@ -258,11 +278,16 @@ impl WriteAccountMapEntry { for list_index in 0..list.len() { let (s, previous_update_value) = &list[list_index]; if *s == slot { - addref = addref && previous_update_value.is_cached(); + let previous_was_cached = previous_update_value.is_cached(); + addref = addref && previous_was_cached; let mut new_item = (slot, account_info); std::mem::swap(&mut new_item, &mut list[list_index]); - reclaims.push(new_item); + if previous_slot_entry_was_cached { + assert!(previous_was_cached); + } else { + reclaims.push(new_item); + } list[(list_index + 1)..] .iter() .for_each(|item| assert!(item.0 != slot)); @@ -281,7 +306,7 @@ impl WriteAccountMapEntry { pub fn update(&mut self, slot: Slot, account_info: T, reclaims: &mut SlotList) { let mut addref = !account_info.is_cached(); self.slot_list_mut(|list| { - addref = Self::update_slot_list(list, slot, account_info, reclaims); + addref = Self::update_slot_list(list, slot, account_info, reclaims, false); }); if addref { // If it's the first non-cache insert, also bump the stored ref count @@ -1520,6 +1545,7 @@ impl AccountsIndex { account_indexes: &AccountSecondaryIndexes, account_info: T, reclaims: &mut SlotList, + previous_slot_entry_was_cached: bool, ) { // We don't atomically update both primary index and secondary index together. // This certainly creates a small time window with inconsistent state across the two indexes. @@ -1536,10 +1562,21 @@ impl AccountsIndex { let map = &self.account_maps[self.bin_calculator.bin_from_pubkey(pubkey)]; let r_account_maps = map.read().unwrap(); - if !WriteAccountMapEntry::update_key_if_exists(r_account_maps, pubkey, &new_item, reclaims) - { + if !WriteAccountMapEntry::update_key_if_exists( + r_account_maps, + pubkey, + &new_item, + reclaims, + previous_slot_entry_was_cached, + ) { let w_account_maps = map.write().unwrap(); - WriteAccountMapEntry::upsert(w_account_maps, pubkey, new_item, reclaims); + WriteAccountMapEntry::upsert( + w_account_maps, + pubkey, + new_item, + reclaims, + previous_slot_entry_was_cached, + ); } self.update_secondary_indexes(pubkey, account_owner, account_data, account_indexes); } @@ -2586,6 +2623,8 @@ pub mod tests { assert!(index.include_key(&pk2)); } + const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: bool = false; + #[test] fn test_insert_no_ancestors() { let key = Keypair::new(); @@ -2599,6 +2638,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); @@ -2735,6 +2775,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), account_infos[0].clone(), &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); } else { let items = vec![(key, account_infos[0].clone())]; @@ -2769,6 +2810,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), account_infos[1].clone(), &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); } else { let items = vec![(key, account_infos[1].clone())]; @@ -2837,6 +2879,7 @@ pub mod tests { &key.pubkey(), &new_entry, &mut SlotList::default(), + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, )); assert_eq!( (slot, account_info), @@ -2850,6 +2893,7 @@ pub mod tests { &key.pubkey(), new_entry, &mut SlotList::default(), + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert_eq!(1, account_maps_len_expensive(&index)); @@ -2879,6 +2923,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); @@ -2903,6 +2948,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); @@ -2936,6 +2982,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); new_pubkey }) @@ -2952,6 +2999,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); } @@ -3083,6 +3131,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(iter.next().is_none()); } @@ -3108,6 +3157,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); @@ -3222,6 +3272,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); let (list, idx) = index.get(&key.pubkey(), Some(&ancestors), None).unwrap(); @@ -3237,6 +3288,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), false, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert_eq!(gc, vec![(0, true)]); let (list, idx) = index.get(&key.pubkey(), Some(&ancestors), None).unwrap(); @@ -3258,6 +3310,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); index.upsert( @@ -3268,6 +3321,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), false, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); let (list, idx) = index.get(&key.pubkey(), Some(&ancestors), None).unwrap(); @@ -3290,6 +3344,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(gc.is_empty()); index.upsert( @@ -3300,6 +3355,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), false, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); index.upsert( 2, @@ -3309,6 +3365,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); index.upsert( 3, @@ -3318,6 +3375,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); index.add_root(0, false); index.add_root(1, false); @@ -3330,6 +3388,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), true, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); // Updating index should not purge older roots, only purges @@ -3373,6 +3432,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), 12, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert_eq!(1, account_maps_len_expensive(&index)); @@ -3384,6 +3444,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), 10, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert_eq!(1, account_maps_len_expensive(&index)); @@ -3403,6 +3464,7 @@ pub mod tests { &AccountSecondaryIndexes::default(), 9, &mut gc, + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert_eq!(1, account_maps_len_expensive(&index)); } @@ -3478,6 +3540,7 @@ pub mod tests { secondary_indexes, true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); } @@ -3650,6 +3713,7 @@ pub mod tests { &secondary_indexes, true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(secondary_index.index.is_empty()); assert!(secondary_index.reverse_index.is_empty()); @@ -3663,6 +3727,7 @@ pub mod tests { &secondary_indexes, true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert!(secondary_index.index.is_empty()); assert!(secondary_index.reverse_index.is_empty()); @@ -3785,6 +3850,7 @@ pub mod tests { secondary_indexes, true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); // Now write a different mint index for the same account @@ -3796,6 +3862,7 @@ pub mod tests { secondary_indexes, true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); // Both pubkeys will now be present in the index @@ -3815,6 +3882,7 @@ pub mod tests { secondary_indexes, true, &mut vec![], + UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE, ); assert_eq!(secondary_index.get(&secondary_key1), vec![account_key]);