Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

patches metrics for invalid cached vote/stake accounts #27266

Merged
merged 1 commit into from
Aug 20, 2022
Merged

patches metrics for invalid cached vote/stake accounts #27266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 31 additions & 39 deletions runtime/src/bank.rs
View file
Open in desktop
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
Open in desktop
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