Skip to content

Commit

Permalink
patches metrics for invalid cached vote/stake accounts (#27266)
Browse files Browse the repository at this point in the history 
patches invalid cached vote/stake accounts metrics

Invalid cached vote accounts is overcounting actual mismatches, and
invalid cached stake accounts is undercounting.
  • Loading branch information
@behzadnouri
behzadnouri authored and haoran committed Aug 21, 2022
1 parent 3a80118 commit bdf3735
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 46 deletions.
70 changes: 31 additions & 39 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2736,13 +2736,11 @@ impl Bank {
"distributed inflation: {} (rounded from: {})",
validator_rewards_paid, validator_rewards
);
// TODO: staked_nodes forces an eager stakes calculation. remove it!
let (num_stake_accounts, num_vote_accounts, num_staked_nodes) = {
let (num_stake_accounts, num_vote_accounts) = {
let stakes = self.stakes_cache.stakes();
(
stakes.stake_delegations().len(),
stakes.vote_accounts().len(),
stakes.staked_nodes().len(),
)
};
self.capitalization
Expand All @@ -2769,7 +2767,6 @@ impl Bank {
("post_capitalization", self.capitalization(), i64),
("num_stake_accounts", num_stake_accounts as i64, i64),
("num_vote_accounts", num_vote_accounts as i64, i64),
("num_staked_nodes", num_staked_nodes as i64, i64)
);
}

Expand Down Expand Up @@ -2807,9 +2804,26 @@ impl Bank {
None => {
invalid_stake_keys
.insert(*stake_pubkey, InvalidCacheEntryReason::Missing);
invalid_cached_stake_accounts.fetch_add(1, Relaxed);
return;
}
};
if cached_stake_account.account() != &stake_account {
invalid_cached_stake_accounts.fetch_add(1, Relaxed);
let cached_stake_account = cached_stake_account.account();
if cached_stake_account.lamports() == stake_account.lamports()
&& cached_stake_account.data() == stake_account.data()
&& cached_stake_account.owner() == stake_account.owner()
&& cached_stake_account.executable() == stake_account.executable()
{
invalid_cached_stake_accounts_rent_epoch.fetch_add(1, Relaxed);
} else {
debug!(
"cached stake account mismatch: {}: {:?}, {:?}",
stake_pubkey, stake_account, cached_stake_account
);
}
}
let stake_account = match StakeAccount::<()>::try_from(stake_account) {
Ok(stake_account) => stake_account,
Err(stake_account::Error::InvalidOwner { .. }) => {
Expand All @@ -2832,33 +2846,6 @@ impl Bank {
return;
}
};
if cached_stake_account != &stake_account {
invalid_cached_stake_accounts.fetch_add(1, Relaxed);
let mut cached_account = cached_stake_account.account().clone();
// We could have collected rent on the loaded account already in this new epoch (we could be at partition_index 12, for example).
// So, we may need to adjust the rent_epoch of the cached account. So, update rent_epoch and compare just the accounts.
ExpectedRentCollection::maybe_update_rent_epoch_on_load(
&mut cached_account,
&SlotInfoInEpoch::new_small(self.slot()),
&SlotInfoInEpoch::new_small(self.slot()),
self.epoch_schedule(),
self.rent_collector(),
stake_pubkey,
&self.rewrites_skipped_this_slot,
);
if &cached_account != stake_account.account() {
info!(
"cached stake account mismatch: {}: {:?}, {:?}",
stake_pubkey,
cached_account,
stake_account.account()
);
} else {
// track how many of 'invalid_cached_stake_accounts' were due to rent_epoch changes
// subtract these to find real invalid cached accounts
invalid_cached_stake_accounts_rent_epoch.fetch_add(1, Relaxed);
}
}
let stake_delegation = (*stake_pubkey, stake_account);
let mut vote_delegations = if let Some(vote_delegations) =
vote_with_stake_delegations_map.get_mut(vote_pubkey)
Expand All @@ -2868,16 +2855,12 @@ impl Bank {
let cached_vote_account = cached_vote_accounts.get(vote_pubkey);
let vote_account = match self.get_account_with_fixed_root(vote_pubkey) {
Some(vote_account) => {
match cached_vote_account {
Some(cached_vote_account)
if cached_vote_account == &vote_account => {}
_ => {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
};
if vote_account.owner() != &solana_vote_program::id() {
invalid_vote_keys
.insert(*vote_pubkey, InvalidCacheEntryReason::WrongOwner);
if cached_vote_account.is_some() {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
return;
}
vote_account
Expand All @@ -2899,9 +2882,18 @@ impl Bank {
} else {
invalid_vote_keys
.insert(*vote_pubkey, InvalidCacheEntryReason::BadState);
if cached_vote_account.is_some() {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
return;
};

match cached_vote_account {
Some(cached_vote_account)
if cached_vote_account.account() == &vote_account => {}
_ => {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
};
vote_with_stake_delegations_map
.entry(*vote_pubkey)
.or_insert_with(|| VoteWithStakeDelegations {
Expand Down
12 changes: 5 additions & 7 deletions runtime/src/vote_account.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use {
once_cell::sync::OnceCell,
serde::ser::{Serialize, Serializer},
solana_sdk::{
account::{accounts_equal, AccountSharedData, ReadableAccount},
account::{AccountSharedData, ReadableAccount},
instruction::InstructionError,
pubkey::Pubkey,
},
Expand Down Expand Up @@ -53,6 +53,10 @@ pub struct VoteAccounts {
}

impl VoteAccount {
pub(crate) fn account(&self) -> &AccountSharedData {
&self.0.account
}

pub(crate) fn lamports(&self) -> u64 {
self.0.account.lamports()
}
Expand Down Expand Up @@ -255,12 +259,6 @@ impl PartialEq<VoteAccountInner> for VoteAccountInner {
}
}

impl PartialEq<AccountSharedData> for VoteAccount {
fn eq(&self, other: &AccountSharedData) -> bool {
accounts_equal(&self.0.account, other)
}
}

impl Default for VoteAccounts {
fn default() -> Self {
Self {
Expand Down

0 comments on commit bdf3735

Please sign in to comment.