remove per-account slot in ancient append vecs (#28851)

* remove per-account slot in ancient append vecs

* review fixes
This commit is contained in:
Jeff Washington (jwash) 2022-11-18 10:15:41 -08:00 committed by GitHub
parent 4c85850a73
commit 9acfa08838
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 40 deletions

View File

@ -226,16 +226,17 @@ impl CurrentAncientAppendVec {
fn store_ancient_accounts(
&self,
db: &AccountsDb,
accounts: &AccountsToStore,
accounts_to_store: &AccountsToStore,
storage_selector: StorageSelector,
) -> StoreAccountsTiming {
let (accounts, hashes) = accounts.get(storage_selector);
let (accounts, hashes) = accounts_to_store.get(storage_selector);
db.store_accounts_frozen(
(
self.slot(),
accounts,
INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION,
accounts_to_store.slot,
),
Some(hashes),
Some(self.append_vec()),
@ -4360,14 +4361,14 @@ impl AccountsDb {
/// returns the pubkeys that are in 'accounts' that are already in 'existing_ancient_pubkeys'
/// Also updated 'existing_ancient_pubkeys' to include all pubkeys in 'accounts' since they will soon be written into the ancient slot.
fn get_keys_to_unref_ancient<'a>(
accounts: &'a [(&StoredAccountMeta<'_>, Slot)],
accounts: &'a [&StoredAccountMeta<'_>],
existing_ancient_pubkeys: &mut HashSet<Pubkey>,
) -> HashSet<&'a Pubkey> {
let mut unref = HashSet::<&Pubkey>::default();
// for each key that we're about to add that already exists in this storage, we need to unref. The account was in a different storage.
// Now it is being put into an ancient storage again, but it is already there, so maintain max of 1 ref per storage in the accounts index.
// The slot that currently references the account is going away, so unref to maintain # slots that reference the pubkey = refcount.
accounts.iter().for_each(|(account, _)| {
accounts.iter().for_each(|account| {
let key = account.pubkey();
if !existing_ancient_pubkeys.insert(*key) {
// this key exists BOTH in 'accounts' and already in the ancient append vec, so we need to unref it
@ -4382,7 +4383,7 @@ impl AccountsDb {
/// As a side effect, on exit, 'existing_ancient_pubkeys' will now contain all pubkeys in 'accounts'.
fn unref_accounts_already_in_storage(
&self,
accounts: &[(&StoredAccountMeta<'_>, Slot)],
accounts: &[&StoredAccountMeta<'_>],
existing_ancient_pubkeys: &mut HashSet<Pubkey>,
) {
let unref = Self::get_keys_to_unref_ancient(accounts, existing_ancient_pubkeys);
@ -10106,9 +10107,8 @@ pub mod tests {
stored_size,
hash: &hash,
};
let slot0 = 0;
let mut existing_ancient_pubkeys = HashSet::default();
let accounts = [(&stored_account, slot0)];
let accounts = [&stored_account];
// pubkey NOT in existing_ancient_pubkeys, so do NOT unref, but add to existing_ancient_pubkeys
let unrefs =
AccountsDb::get_keys_to_unref_ancient(&accounts, &mut existing_ancient_pubkeys);
@ -10126,7 +10126,7 @@ pub mod tests {
);
assert_eq!(unrefs.iter().cloned().collect::<Vec<_>>(), vec![&pubkey]);
// pubkey2 NOT in existing_ancient_pubkeys, so do NOT unref, but add to existing_ancient_pubkeys
let accounts = [(&stored_account2, slot0)];
let accounts = [&stored_account2];
let unrefs =
AccountsDb::get_keys_to_unref_ancient(&accounts, &mut existing_ancient_pubkeys);
assert!(unrefs.is_empty());
@ -10149,7 +10149,7 @@ pub mod tests {
);
assert_eq!(unrefs.iter().cloned().collect::<Vec<_>>(), vec![&pubkey2]);
// pubkey3/4 NOT in existing_ancient_pubkeys, so do NOT unref, but add to existing_ancient_pubkeys
let accounts = [(&stored_account3, slot0), (&stored_account4, slot0)];
let accounts = [&stored_account3, &stored_account4];
let unrefs =
AccountsDb::get_keys_to_unref_ancient(&accounts, &mut existing_ancient_pubkeys);
assert!(unrefs.is_empty());

View File

@ -26,10 +26,11 @@ pub enum StorageSelector {
/// The slice arithmetic accross both hashes and account data gets messy. So, this struct abstracts that.
pub struct AccountsToStore<'a> {
hashes: Vec<&'a Hash>,
accounts: Vec<(&'a StoredAccountMeta<'a>, Slot)>,
accounts: Vec<&'a StoredAccountMeta<'a>>,
/// if 'accounts' contains more items than can be contained in the primary storage, then we have to split these accounts.
/// 'index_first_item_overflow' specifies the index of the first item in 'accounts' that will go into the overflow storage
index_first_item_overflow: usize,
pub slot: Slot,
}
impl<'a> AccountsToStore<'a> {
@ -57,12 +58,13 @@ impl<'a> AccountsToStore<'a> {
hashes.push(account.account.hash);
// we have to specify 'slot' here because we are writing to an ancient append vec and squashing slots,
// so we need to update the previous accounts index entry for this account from 'slot' to 'ancient_slot'
accounts.push((&account.account, slot));
accounts.push(&account.account);
});
Self {
hashes,
accounts,
index_first_item_overflow,
slot,
}
}
@ -72,10 +74,7 @@ impl<'a> AccountsToStore<'a> {
}
/// get the accounts and hashes to store in the given 'storage'
pub fn get(
&self,
storage: StorageSelector,
) -> (&[(&'a StoredAccountMeta<'a>, Slot)], &[&'a Hash]) {
pub fn get(&self, storage: StorageSelector) -> (&[&'a StoredAccountMeta<'a>], &[&'a Hash]) {
let range = match storage {
StorageSelector::Primary => 0..self.index_first_item_overflow,
StorageSelector::Overflow => self.index_first_item_overflow..self.accounts.len(),
@ -168,7 +167,7 @@ pub mod tests {
let (accounts, hashes) = accounts_to_store.get(selector);
assert_eq!(
accounts,
map.iter().map(|b| (&b.account, slot)).collect::<Vec<_>>(),
map.iter().map(|b| &b.account).collect::<Vec<_>>(),
"mismatch"
);
assert_eq!(hashes, vec![&hash]);

View File

@ -128,23 +128,24 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>>
}
}
/// this tuple contains slot info PER account
/// this tuple contains a single different source slot that applies to all accounts
impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>>
for (
Slot,
&'a [(&'a StoredAccountMeta<'a>, Slot)],
&'a [&'a StoredAccountMeta<'a>],
IncludeSlotInHash,
Slot,
)
{
fn pubkey(&self, index: usize) -> &Pubkey {
self.1[index].0.pubkey()
self.1[index].pubkey()
}
fn account(&self, index: usize) -> &StoredAccountMeta<'a> {
self.1[index].0
self.1[index]
}
fn slot(&self, index: usize) -> Slot {
// note that this could be different than 'target_slot()' PER account
self.1[index].1
fn slot(&self, _index: usize) -> Slot {
// same other slot for all accounts
self.3
}
fn target_slot(&self) -> Slot {
self.0
@ -153,14 +154,7 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>>
self.1.len()
}
fn contains_multiple_slots(&self) -> bool {
let len = self.len();
if len > 0 {
let slot = self.slot(0);
// true if any item has a different slot than the first item
(1..len).any(|i| slot != self.slot(i))
} else {
false
}
false
}
fn include_slot_in_hash(&self) -> IncludeSlotInHash {
self.2
@ -231,16 +225,11 @@ pub mod tests {
let test3 = (
slot,
&vec![(&stored_account, slot), (&stored_account, slot)][..],
&vec![&stored_account, &stored_account][..],
INCLUDE_SLOT_IN_HASH_TESTS,
slot,
);
assert!(!test3.contains_multiple_slots());
let test3 = (
slot,
&vec![(&stored_account, slot), (&stored_account, slot + 1)][..],
INCLUDE_SLOT_IN_HASH_TESTS,
);
assert!(test3.contains_multiple_slots());
}
#[test]
@ -297,11 +286,17 @@ pub mod tests {
let mut three = Vec::new();
raw.iter().zip(raw2.iter()).for_each(|(raw, raw2)| {
two.push((&raw.0, &raw.1)); // 2 item tuple
three.push((raw2, raw.2)); // 2 item tuple, including slot
three.push(raw2);
});
let test2 = (target_slot, &two[..], INCLUDE_SLOT_IN_HASH_TESTS);
let test3 = (target_slot, &three[..], INCLUDE_SLOT_IN_HASH_TESTS);
let source_slot = starting_slot % max_slots;
let test3 = (
target_slot,
&three[..],
INCLUDE_SLOT_IN_HASH_TESTS,
source_slot,
);
let old_slot = starting_slot;
let test_moving_slots = StorableAccountsMovingSlots {
accounts: &two[..],