Skip to content

Commit

Permalink
pr feedback: gate metrics reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
AshwinSekar committed Jul 16, 2024
1 parent 509cdb9 commit 7daf78c
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 10 deletions.
1 change: 1 addition & 0 deletions core/src/replay_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3082,6 +3082,7 @@ impl ReplayStage {
match blockstore.check_last_fec_set_and_get_block_id(
bank.slot(),
bank.hash(),
bank.cluster_type(),
&bank.feature_set,
) {
Ok(block_id) => block_id,
Expand Down
13 changes: 9 additions & 4 deletions ledger/src/blockstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ use {
address_lookup_table::state::AddressLookupTable,
clock::{Slot, UnixTimestamp, DEFAULT_TICKS_PER_SECOND},
feature_set::FeatureSet,
genesis_config::{GenesisConfig, DEFAULT_GENESIS_ARCHIVE, DEFAULT_GENESIS_FILE},
genesis_config::{
ClusterType, GenesisConfig, DEFAULT_GENESIS_ARCHIVE, DEFAULT_GENESIS_FILE,
},
hash::Hash,
pubkey::Pubkey,
signature::{Keypair, Signature, Signer},
Expand Down Expand Up @@ -179,15 +181,17 @@ pub struct LastFECSetCheckResults {
}

impl LastFECSetCheckResults {
pub fn report_metrics(&self, slot: Slot, bank_hash: Hash) {
pub fn report_metrics(&self, slot: Slot, bank_hash: Hash, cluster_type: ClusterType) {
if self.block_id.is_none() {
datapoint_warn!(
"incomplete_final_fec_set",
("slot", slot, i64),
("bank_hash", bank_hash.to_string(), String)
);
}
if !self.is_retransmitter_signed {
// These metrics are expensive to send because hash does not compress well.
// Only send these metrics when we are sure the appropriate shred format is being sent
if !self.is_retransmitter_signed && shred::should_chain_merkle_shreds(slot, cluster_type) {
datapoint_warn!(
"invalid_retransmitter_signature_final_fec_set",
("slot", slot, i64),
Expand Down Expand Up @@ -3734,6 +3738,7 @@ impl Blockstore {
&self,
slot: Slot,
bank_hash: Hash,
cluster_type: ClusterType,
feature_set: &FeatureSet,
) -> std::result::Result<Option<Hash>, BlockstoreProcessorError> {
let results = self.check_last_fec_set(slot);
Expand All @@ -3749,7 +3754,7 @@ impl Blockstore {
return Ok(None);
};
// Update metrics
results.report_metrics(slot, bank_hash);
results.report_metrics(slot, bank_hash, cluster_type);
// Return block id / error based on feature flags
results.get_block_id(feature_set)
}
Expand Down
6 changes: 5 additions & 1 deletion ledger/src/shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ macro_rules! dispatch {
}
}

use dispatch;
use {dispatch, solana_sdk::genesis_config::ClusterType};

impl Shred {
dispatch!(fn common_header(&self) -> &ShredCommonHeader);
Expand Down Expand Up @@ -1283,6 +1283,10 @@ pub fn verify_test_data_shred(
}
}

pub fn should_chain_merkle_shreds(_slot: Slot, cluster_type: ClusterType) -> bool {
cluster_type == ClusterType::Development
}

#[cfg(test)]
mod tests {
use {
Expand Down
9 changes: 4 additions & 5 deletions turbine/src/broadcast_stage/standard_broadcast_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ use {
solana_entry::entry::Entry,
solana_ledger::{
blockstore,
shred::{shred_code, ProcessShredsStats, ReedSolomonCache, Shred, ShredFlags, Shredder},
shred::{
should_chain_merkle_shreds, shred_code, ProcessShredsStats, ReedSolomonCache, Shred,
ShredFlags, Shredder,
},
},
solana_sdk::{
genesis_config::ClusterType,
Expand Down Expand Up @@ -506,10 +509,6 @@ impl BroadcastRun for StandardBroadcastRun {
}
}

fn should_chain_merkle_shreds(_slot: Slot, cluster_type: ClusterType) -> bool {
cluster_type == ClusterType::Development
}

#[cfg(test)]
mod test {
use {
Expand Down

0 comments on commit 7daf78c

Please sign in to comment.