accounts_db::load returns None for zero lamport accounts (#28311)

This commit is contained in:
Jeff Washington (jwash) 2022-10-11 07:43:03 -07:00 committed by GitHub
parent 391c15bb5b
commit 7a120b8b62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 78 additions and 24 deletions

View File

@ -635,26 +635,15 @@ impl Accounts {
}
}
fn filter_zero_lamport_account(
account: AccountSharedData,
slot: Slot,
) -> Option<(AccountSharedData, Slot)> {
if account.lamports() > 0 {
Some((account, slot))
} else {
None
}
}
/// Slow because lock is held for 1 operation instead of many
/// This always returns None for zero-lamport accounts.
fn load_slow(
&self,
ancestors: &Ancestors,
pubkey: &Pubkey,
load_hint: LoadHint,
) -> Option<(AccountSharedData, Slot)> {
let (account, slot) = self.accounts_db.load(ancestors, pubkey, load_hint)?;
Self::filter_zero_lamport_account(account, slot)
self.accounts_db.load(ancestors, pubkey, load_hint)
}
pub fn load_with_fixed_root(

View File

@ -145,6 +145,20 @@ pub enum StoreReclaims {
Ignore,
}
/// specifies how to return zero lamport accounts from a load
#[derive(Clone, Copy)]
enum LoadZeroLamports {
/// return None if loaded account has zero lamports
None,
/// return Some(account with zero lamports) if loaded account has zero lamports
/// This used to be the only behavior.
/// Note that this is non-deterministic if clean is running asynchronously.
/// If a zero lamport account exists in the index, then Some is returned.
/// Once it is cleaned from the index, None is returned.
#[cfg(test)]
SomeWithZeroLamportAccountForTests,
}
// the current best way to add filler accounts is gradually.
// In other scenarios, such as monitoring catchup with large # of accounts, it may be useful to be able to
// add filler accounts at the beginning, so that code path remains but won't execute at the moment.
@ -4829,11 +4843,19 @@ impl AccountsDb {
pubkey: &Pubkey,
load_hint: LoadHint,
) -> Option<(AccountSharedData, Slot)> {
self.do_load(ancestors, pubkey, None, load_hint)
self.do_load(ancestors, pubkey, None, load_hint, LoadZeroLamports::None)
}
pub fn load_account_into_read_cache(&self, ancestors: &Ancestors, pubkey: &Pubkey) {
self.do_load_with_populate_read_cache(ancestors, pubkey, None, LoadHint::Unspecified, true);
self.do_load_with_populate_read_cache(
ancestors,
pubkey,
None,
LoadHint::Unspecified,
true,
// no return from this function, so irrelevant
LoadZeroLamports::None,
);
}
/// note this returns None for accounts with zero lamports
@ -4843,7 +4865,6 @@ impl AccountsDb {
pubkey: &Pubkey,
) -> Option<(AccountSharedData, Slot)> {
self.load(ancestors, pubkey, LoadHint::FixedMaxRoot)
.filter(|(account, _)| !account.is_zero_lamport())
}
fn read_index_for_accessor_or_load_slow<'a>(
@ -5172,8 +5193,16 @@ impl AccountsDb {
pubkey: &Pubkey,
max_root: Option<Slot>,
load_hint: LoadHint,
load_zero_lamports: LoadZeroLamports,
) -> Option<(AccountSharedData, Slot)> {
self.do_load_with_populate_read_cache(ancestors, pubkey, max_root, load_hint, false)
self.do_load_with_populate_read_cache(
ancestors,
pubkey,
max_root,
load_hint,
false,
load_zero_lamports,
)
}
/// if 'load_into_read_cache_only', then return value is meaningless.
@ -5185,6 +5214,7 @@ impl AccountsDb {
max_root: Option<Slot>,
load_hint: LoadHint,
load_into_read_cache_only: bool,
load_zero_lamports: LoadZeroLamports,
) -> Option<(AccountSharedData, Slot)> {
#[cfg(not(test))]
assert!(max_root.is_none());
@ -5199,6 +5229,11 @@ impl AccountsDb {
if !in_write_cache {
let result = self.read_only_accounts_cache.load(*pubkey, slot);
if let Some(account) = result {
if matches!(load_zero_lamports, LoadZeroLamports::None)
&& account.is_zero_lamport()
{
return None;
}
return Some((account, slot));
}
}
@ -5226,6 +5261,9 @@ impl AccountsDb {
let loaded_account = account_accessor.check_and_get_loaded_account();
let is_cached = loaded_account.is_cached();
let account = loaded_account.take_account();
if matches!(load_zero_lamports, LoadZeroLamports::None) && account.is_zero_lamport() {
return None;
}
if self.caching_enabled && !is_cached {
/*
@ -9665,7 +9703,14 @@ pub mod tests {
ancestors: &Ancestors,
pubkey: &Pubkey,
) -> Option<(AccountSharedData, Slot)> {
self.load(ancestors, pubkey, LoadHint::Unspecified)
self.do_load(
ancestors,
pubkey,
None,
LoadHint::Unspecified,
// callers of this expect zero lamport accounts that exist in the index to be returned as Some(empty)
LoadZeroLamports::SomeWithZeroLamportAccountForTests,
)
}
}
@ -14080,6 +14125,9 @@ pub mod tests {
assert_eq!(db.read_only_accounts_cache.cache_len(), 1);
}
/// a test that will accept either answer
const LOAD_ZERO_LAMPORTS_ANY_TESTS: LoadZeroLamports = LoadZeroLamports::None;
#[test]
fn test_flush_cache_clean() {
let caching_enabled = true;
@ -14109,6 +14157,7 @@ pub mod tests {
&account_key,
Some(0),
LoadHint::Unspecified,
LoadZeroLamports::SomeWithZeroLamportAccountForTests,
)
.unwrap();
assert_eq!(account.0.lamports(), 0);
@ -14124,7 +14173,8 @@ pub mod tests {
&Ancestors::default(),
&account_key,
Some(0),
LoadHint::Unspecified
LoadHint::Unspecified,
LOAD_ZERO_LAMPORTS_ANY_TESTS
)
.is_none());
}
@ -14208,7 +14258,8 @@ pub mod tests {
&Ancestors::default(),
&zero_lamport_account_key,
max_root,
load_hint
load_hint,
LoadZeroLamports::SomeWithZeroLamportAccountForTests,
)
.unwrap()
.0
@ -14337,6 +14388,7 @@ pub mod tests {
&account_key,
Some(0),
LoadHint::Unspecified,
LoadZeroLamports::SomeWithZeroLamportAccountForTests,
)
.unwrap();
assert_eq!(account.0.lamports(), zero_lamport_account.lamports());
@ -14350,6 +14402,7 @@ pub mod tests {
&account_key,
Some(max_scan_root),
LoadHint::Unspecified,
LOAD_ZERO_LAMPORTS_ANY_TESTS,
)
.unwrap();
assert_eq!(account.0.lamports(), slot1_account.lamports());
@ -14364,6 +14417,7 @@ pub mod tests {
&account_key,
Some(max_scan_root),
LoadHint::Unspecified,
LOAD_ZERO_LAMPORTS_ANY_TESTS,
)
.unwrap();
assert_eq!(account.0.lamports(), slot1_account.lamports());
@ -14376,7 +14430,8 @@ pub mod tests {
&scan_ancestors,
&account_key,
Some(max_scan_root),
LoadHint::Unspecified
LoadHint::Unspecified,
LOAD_ZERO_LAMPORTS_ANY_TESTS
)
.is_none());
}
@ -14540,7 +14595,8 @@ pub mod tests {
&Ancestors::default(),
key,
Some(last_dead_slot),
LoadHint::Unspecified
LoadHint::Unspecified,
LOAD_ZERO_LAMPORTS_ANY_TESTS
)
.is_some());
}
@ -14568,7 +14624,8 @@ pub mod tests {
&Ancestors::default(),
key,
Some(last_dead_slot),
LoadHint::Unspecified
LoadHint::Unspecified,
LOAD_ZERO_LAMPORTS_ANY_TESTS
)
.is_none());
}
@ -15101,7 +15158,15 @@ pub mod tests {
.store(thread_rng().gen_range(0, 10) as u64, Ordering::Relaxed);
// Load should never be unable to find this key
let loaded_account = db.do_load(&ancestors, &pubkey, None, load_hint).unwrap();
let loaded_account = db
.do_load(
&ancestors,
&pubkey,
None,
load_hint,
LOAD_ZERO_LAMPORTS_ANY_TESTS,
)
.unwrap();
// slot + 1 == account.lamports because of the account-cache-flush thread
assert_eq!(
loaded_account.0.lamports(),