From 7daf78ccf4cff7231ed8db0236720cffb9a00320 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Tue, 16 Jul 2024 19:26:45 +0000 Subject: [PATCH] pr feedback: gate metrics reporting --- core/src/replay_stage.rs | 1 + ledger/src/blockstore.rs | 13 +++++++++---- ledger/src/shred.rs | 6 +++++- .../src/broadcast_stage/standard_broadcast_run.rs | 9 ++++----- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 4a6fb1962cdb67..6131c493798dda 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -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, diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 13340fee9bda31..588f9991f04b2c 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -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}, @@ -179,7 +181,7 @@ 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", @@ -187,7 +189,9 @@ impl LastFECSetCheckResults { ("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), @@ -3734,6 +3738,7 @@ impl Blockstore { &self, slot: Slot, bank_hash: Hash, + cluster_type: ClusterType, feature_set: &FeatureSet, ) -> std::result::Result, BlockstoreProcessorError> { let results = self.check_last_fec_set(slot); @@ -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) } diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index 814ec2b5bf303a..3086cd7894eae7 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -341,7 +341,7 @@ macro_rules! dispatch { } } -use dispatch; +use {dispatch, solana_sdk::genesis_config::ClusterType}; impl Shred { dispatch!(fn common_header(&self) -> &ShredCommonHeader); @@ -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 { diff --git a/turbine/src/broadcast_stage/standard_broadcast_run.rs b/turbine/src/broadcast_stage/standard_broadcast_run.rs index f108fc08226a6b..e4ec736f184da0 100644 --- a/turbine/src/broadcast_stage/standard_broadcast_run.rs +++ b/turbine/src/broadcast_stage/standard_broadcast_run.rs @@ -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, @@ -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 {