From 6456b698ca32a88af9cc69a2757b8d2f21434f51 Mon Sep 17 00:00:00 2001 From: Wen <113942165+wen-coding@users.noreply.github.com> Date: Wed, 23 Jul 2025 15:10:09 -0700 Subject: [PATCH 1/4] Reject certs older than root and add CertificatePoolStats. --- programs/sbf/Cargo.lock | 1 + svm/examples/Cargo.lock | 1 + votor/src/certificate_pool.rs | 54 +++++- votor/src/certificate_pool/stats.rs | 249 ++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 2 deletions(-) create mode 100644 votor/src/certificate_pool/stats.rs diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 338324fb4e..20b604ae5d 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -7645,6 +7645,7 @@ dependencies = [ "num-traits", "num_cpus", "num_enum", + "parking_lot 0.12.2", "percentage", "qualifier_attr", "rand 0.8.5", diff --git a/svm/examples/Cargo.lock b/svm/examples/Cargo.lock index 77e1ebc6b4..c1daae01c3 100644 --- a/svm/examples/Cargo.lock +++ b/svm/examples/Cargo.lock @@ -7468,6 +7468,7 @@ dependencies = [ "num-traits", "num_cpus", "num_enum", + "parking_lot 0.12.3", "percentage", "qualifier_attr", "rand 0.8.5", diff --git a/votor/src/certificate_pool.rs b/votor/src/certificate_pool.rs index c6dbee47f8..4eb1470634 100644 --- a/votor/src/certificate_pool.rs +++ b/votor/src/certificate_pool.rs @@ -3,6 +3,7 @@ use { certificate_limits_and_vote_types, certificate_pool::{ parent_ready_tracker::ParentReadyTracker, + stats::CertificatePoolStats, vote_certificate::{CertificateError, VoteCertificate}, vote_pool::{ DuplicateBlockVotePool, SimpleVotePool, VotePool, VotePoolType, VotedBlockKey, @@ -37,6 +38,7 @@ use { }; pub mod parent_ready_tracker; +mod stats; mod vote_certificate; mod vote_pool; @@ -108,6 +110,8 @@ pub struct CertificatePool { root_epoch: Epoch, /// The certificate sender, if set, newly created certificates will be sent here certificate_sender: Option>, + /// Stats for the certificate pool + stats: CertificatePoolStats, } impl CertificatePool { @@ -136,6 +140,7 @@ impl CertificatePool { root_epoch: Epoch::default(), certificate_sender, parent_ready_tracker, + stats: CertificatePoolStats::new(), }; // Update the epoch_stakes_map and root @@ -245,6 +250,8 @@ impl CertificatePool { }); let new_cert = Arc::new(vote_certificate.certificate()); self.send_and_insert_certificate(cert_id, new_cert.clone(), events)?; + self.stats + .incr_cert_type(new_cert.certificate.certificate_type, true); new_certificates_to_send.push(new_cert); } Ok(new_certificates_to_send) @@ -387,6 +394,8 @@ impl CertificatePool { message: &BLSMessage, events: &mut Vec, ) -> Result<(Option, Vec>), AddVoteError> { + // We add stats reporting here because we should almost always have a message. + self.stats.maybe_report(); let current_highest_finalized_slot = self.highest_finalized_slot; let new_certficates_to_send = match message { BLSMessage::Vote(vote_message) => { @@ -408,7 +417,6 @@ impl CertificatePool { fn add_vote( &mut self, my_vote_pubkey: &Pubkey, - vote_message: &VoteMessage, events: &mut Vec, ) -> Result>, AddVoteError> { @@ -424,11 +432,14 @@ impl CertificatePool { "Validator stake is zero for pubkey: {validator_vote_key}" ); + CertificatePoolStats::incr_u32_array(&mut self.stats.incoming, true); if slot < self.root { + CertificatePoolStats::incr_u32_array(&mut self.stats.out_of_range, true); return Err(AddVoteError::UnrootedSlot); } // We only allow votes if slot > self.root.saturating_add(MAX_SLOT_AGE) { + CertificatePoolStats::incr_u32_array(&mut self.stats.out_of_range, true); return Err(AddVoteError::SlotInFuture); } @@ -447,6 +458,7 @@ impl CertificatePool { if let Some(conflicting_type) = self.has_conflicting_vote(slot, vote_type, &validator_vote_key, &voted_block_key) { + CertificatePoolStats::incr_u32(&mut self.stats.conflicting_votes); return Err(AddVoteError::ConflictingVoteType( vote_type, conflicting_type, @@ -462,6 +474,7 @@ impl CertificatePool { &validator_vote_key, validator_stake, ) { + CertificatePoolStats::incr_u32_array(&mut self.stats.exits, true); return Ok(vec![]); } // Check if this new vote generated a safe to notar or safe to skip @@ -470,11 +483,15 @@ impl CertificatePool { // everytime. if self.safe_to_skip(my_vote_pubkey, slot) { events.push(VotorEvent::SafeToSkip(slot)); + CertificatePoolStats::incr_u32(&mut self.stats.event_safe_to_skip); } for (block_id, bank_hash) in self.safe_to_notar(my_vote_pubkey, slot) { events.push(VotorEvent::SafeToNotar((slot, block_id, bank_hash))); + CertificatePoolStats::incr_u32(&mut self.stats.event_safe_to_notarize); } + self.stats.incr_ingested_vote_type(vote_type); + self.update_certificates(vote, voted_block_key, events, total_stake) } @@ -485,11 +502,21 @@ impl CertificatePool { ) -> Result>, AddVoteError> { let certificate = &certificate_message.certificate; let certificate_id = CertificateId::from(certificate); + CertificatePoolStats::incr_u32_array(&mut self.stats.incoming, false); + if certificate.slot < self.root { + CertificatePoolStats::incr_u32_array(&mut self.stats.out_of_range, false); + return Err(AddVoteError::UnrootedSlot); + } if self.completed_certificates.contains_key(&certificate_id) { + CertificatePoolStats::incr_u32_array(&mut self.stats.exits, false); return Ok(vec![]); } let new_certificate = Arc::new(certificate_message.clone()); self.send_and_insert_certificate(certificate_id, new_certificate.clone(), events)?; + + self.stats + .incr_cert_type(certificate.certificate_type, false); + Ok(vec![new_certificate]) } @@ -1724,7 +1751,7 @@ mod tests { } #[test] - fn test_reject_conflicting_votes() { + fn test_handle_new_root() { let validator_keypairs = (0..10) .map(|_| ValidatorVoteKeypairs::new_rand()) .collect::>(); @@ -1740,5 +1767,28 @@ mod tests { let new_bank = Arc::new(create_bank(3, new_bank, &Pubkey::new_unique())); pool.handle_new_root(new_bank); assert_eq!(pool.root(), 3); + // Send a vote on slot 1, it should be rejected + let vote = Vote::new_skip_vote(1); + assert!(pool + .add_message( + &Pubkey::new_unique(), + &dummy_transaction(&validator_keypairs, &vote, 0), + &mut vec![] + ) + .is_err()); + // Send a cert on slot 2, it should be rejected + let cert = BLSMessage::Certificate(CertificateMessage { + certificate: Certificate { + slot: 2, + certificate_type: CertificateType::Notarize, + block_id: Some(Hash::new_unique()), + replayed_bank_hash: Some(Hash::new_unique()), + }, + signature: BLSSignature::default(), + bitmap: BitVec::new(), + }); + assert!(pool + .add_message(&Pubkey::new_unique(), &cert, &mut vec![]) + .is_err()); } } diff --git a/votor/src/certificate_pool/stats.rs b/votor/src/certificate_pool/stats.rs new file mode 100644 index 0000000000..992a4dfe55 --- /dev/null +++ b/votor/src/certificate_pool/stats.rs @@ -0,0 +1,249 @@ +use { + crate::VoteType, + alpenglow_vote::certificate::CertificateType, + solana_metrics::datapoint_info, + std::time::{Duration, Instant}, +}; + +const STATS_REPORT_INTERVAL: Duration = Duration::from_secs(10); + +#[derive(Debug)] +pub(crate) struct CertificatePoolStats { + pub(crate) conflicting_votes: u32, + pub(crate) event_safe_to_skip: u32, + pub(crate) event_safe_to_notarize: u32, + + pub(crate) exits: [u32; 2], + pub(crate) incoming: [u32; 2], + pub(crate) out_of_range: [u32; 2], + + pub(crate) new_certs_by_type: [Vec; 2], // 0 for ingested, 1 for generated + pub(crate) ingested_votes_by_type: Vec, + last_reported: Instant, +} + +impl Default for CertificatePoolStats { + fn default() -> Self { + Self::new() + } +} + +impl CertificatePoolStats { + pub fn new() -> Self { + Self { + conflicting_votes: 0, + event_safe_to_skip: 0, + event_safe_to_notarize: 0, + exits: [0, 0], + incoming: [0, 0], + out_of_range: [0, 0], + new_certs_by_type: [Vec::new(), Vec::new()], + ingested_votes_by_type: Vec::new(), + last_reported: Instant::now(), + } + } + + pub fn incr_u32_array(value: &mut [u32; 2], is_vote: bool) { + if is_vote { + value[0] = value[0].saturating_add(1); + } else { + value[1] = value[1].saturating_add(1); + } + } + + pub fn incr_u32(value: &mut u32) { + *value = value.saturating_add(1); + } + + pub fn incr_ingested_vote_type(&mut self, vote_type: VoteType) { + let index = vote_type as usize; + + if self.ingested_votes_by_type.len() <= index { + self.ingested_votes_by_type + .resize(index.saturating_add(1), 0); + } + + self.ingested_votes_by_type[index] = self.ingested_votes_by_type[index].saturating_add(1); + } + + pub fn incr_cert_type(&mut self, cert_type: CertificateType, is_generated: bool) { + let index = cert_type as usize; + let array = if is_generated { + &mut self.new_certs_by_type[0] + } else { + &mut self.new_certs_by_type[1] + }; + + if array.len() <= index { + array.resize(index.saturating_add(1), 0); + } + + array[index] = array[index].saturating_add(1); + } + + fn report(&self) { + datapoint_info!( + "certificate_pool_stats", + ("conflicting_votes", self.conflicting_votes as i64, i64), + ("event_safe_to_skip", self.event_safe_to_skip as i64, i64), + ( + "event_safe_to_notarize", + self.event_safe_to_notarize as i64, + i64 + ), + ("exits_votes", self.exits[0] as i64, i64), + ("exits_certificates", self.exits[1] as i64, i64), + ("incoming_votes", self.incoming[0] as i64, i64), + ("incoming_certificates", self.incoming[1] as i64, i64), + ("out_of_range_votes", self.out_of_range[0] as i64, i64), + ( + "out_of_range_certificates", + self.out_of_range[1] as i64, + i64 + ), + ); + + datapoint_info!( + "certificate_pool_ingested_votes_by_type", + ( + "finalize", + *self + .ingested_votes_by_type + .get(VoteType::Finalize as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "notarize", + *self + .ingested_votes_by_type + .get(VoteType::Notarize as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "notarize_fallback", + *self + .ingested_votes_by_type + .get(VoteType::NotarizeFallback as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "skip", + *self + .ingested_votes_by_type + .get(VoteType::Skip as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "skip_fallback", + *self + .ingested_votes_by_type + .get(VoteType::SkipFallback as usize) + .unwrap_or(&0) as i64, + i64 + ), + ); + + datapoint_info!( + "certfificate_pool_ingested_certs_by_type", + ( + "finalize", + *self.new_certs_by_type[0] + .get(CertificateType::Finalize as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "finalize_fast", + *self.new_certs_by_type[0] + .get(CertificateType::FinalizeFast as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "notarize", + *self.new_certs_by_type[0] + .get(CertificateType::Notarize as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "notarize_fallback", + *self.new_certs_by_type[0] + .get(CertificateType::NotarizeFallback as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "skip", + *self.new_certs_by_type[0] + .get(CertificateType::Skip as usize) + .unwrap_or(&0) as i64, + i64 + ), + ); + + datapoint_info!( + "certificate_pool_generated_certs_by_type", + ( + "finalize", + *self.new_certs_by_type[1] + .get(CertificateType::Finalize as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "finalize_fast", + *self.new_certs_by_type[1] + .get(CertificateType::FinalizeFast as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "notarize", + *self.new_certs_by_type[1] + .get(CertificateType::Notarize as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "notarize_fallback", + *self.new_certs_by_type[1] + .get(CertificateType::NotarizeFallback as usize) + .unwrap_or(&0) as i64, + i64 + ), + ( + "skip", + *self.new_certs_by_type[1] + .get(CertificateType::Skip as usize) + .unwrap_or(&0) as i64, + i64 + ), + ); + } + + pub fn maybe_report(&mut self) { + if self.last_reported.elapsed() >= STATS_REPORT_INTERVAL { + self.report(); + self.reset(); + } + } + + fn reset(&mut self) { + self.conflicting_votes = 0; + self.event_safe_to_skip = 0; + self.event_safe_to_notarize = 0; + for i in 0..2 { + self.incoming[i] = 0; + self.out_of_range[i] = 0; + } + self.new_certs_by_type[0].clear(); + self.new_certs_by_type[1].clear(); + self.ingested_votes_by_type.clear(); + self.last_reported = Instant::now(); + } +} From 085a0cd738d09041f2de0baccda3dcd30ca2a347 Mon Sep 17 00:00:00 2001 From: Wen <113942165+wen-coding@users.noreply.github.com> Date: Wed, 23 Jul 2025 18:49:37 -0700 Subject: [PATCH 2/4] Change field types in stats. --- votor/src/certificate_pool.rs | 20 ++-- votor/src/certificate_pool/stats.rs | 172 ++++++++++++---------------- 2 files changed, 86 insertions(+), 106 deletions(-) diff --git a/votor/src/certificate_pool.rs b/votor/src/certificate_pool.rs index 4eb1470634..b9de2cf60b 100644 --- a/votor/src/certificate_pool.rs +++ b/votor/src/certificate_pool.rs @@ -432,14 +432,14 @@ impl CertificatePool { "Validator stake is zero for pubkey: {validator_vote_key}" ); - CertificatePoolStats::incr_u32_array(&mut self.stats.incoming, true); + self.stats.incoming_votes = self.stats.incoming_votes.saturating_add(1); if slot < self.root { - CertificatePoolStats::incr_u32_array(&mut self.stats.out_of_range, true); + self.stats.out_of_range_votes = self.stats.out_of_range_votes.saturating_add(1); return Err(AddVoteError::UnrootedSlot); } // We only allow votes if slot > self.root.saturating_add(MAX_SLOT_AGE) { - CertificatePoolStats::incr_u32_array(&mut self.stats.out_of_range, true); + self.stats.out_of_range_votes = self.stats.out_of_range_votes.saturating_add(1); return Err(AddVoteError::SlotInFuture); } @@ -458,7 +458,7 @@ impl CertificatePool { if let Some(conflicting_type) = self.has_conflicting_vote(slot, vote_type, &validator_vote_key, &voted_block_key) { - CertificatePoolStats::incr_u32(&mut self.stats.conflicting_votes); + self.stats.conflicting_votes = self.stats.conflicting_votes.saturating_add(1); return Err(AddVoteError::ConflictingVoteType( vote_type, conflicting_type, @@ -474,7 +474,7 @@ impl CertificatePool { &validator_vote_key, validator_stake, ) { - CertificatePoolStats::incr_u32_array(&mut self.stats.exits, true); + self.stats.exist_votes = self.stats.exist_votes.saturating_add(1); return Ok(vec![]); } // Check if this new vote generated a safe to notar or safe to skip @@ -483,11 +483,11 @@ impl CertificatePool { // everytime. if self.safe_to_skip(my_vote_pubkey, slot) { events.push(VotorEvent::SafeToSkip(slot)); - CertificatePoolStats::incr_u32(&mut self.stats.event_safe_to_skip); + self.stats.event_safe_to_skip = self.stats.event_safe_to_skip.saturating_add(1); } for (block_id, bank_hash) in self.safe_to_notar(my_vote_pubkey, slot) { events.push(VotorEvent::SafeToNotar((slot, block_id, bank_hash))); - CertificatePoolStats::incr_u32(&mut self.stats.event_safe_to_notarize); + self.stats.event_safe_to_notarize = self.stats.event_safe_to_notarize.saturating_add(1); } self.stats.incr_ingested_vote_type(vote_type); @@ -502,13 +502,13 @@ impl CertificatePool { ) -> Result>, AddVoteError> { let certificate = &certificate_message.certificate; let certificate_id = CertificateId::from(certificate); - CertificatePoolStats::incr_u32_array(&mut self.stats.incoming, false); + self.stats.incoming_certs = self.stats.incoming_certs.saturating_add(1); if certificate.slot < self.root { - CertificatePoolStats::incr_u32_array(&mut self.stats.out_of_range, false); + self.stats.out_of_range_certs = self.stats.out_of_range_certs.saturating_add(1); return Err(AddVoteError::UnrootedSlot); } if self.completed_certificates.contains_key(&certificate_id) { - CertificatePoolStats::incr_u32_array(&mut self.stats.exits, false); + self.stats.exist_certs = self.stats.exist_certs.saturating_add(1); return Ok(vec![]); } let new_certificate = Arc::new(certificate_message.clone()); diff --git a/votor/src/certificate_pool/stats.rs b/votor/src/certificate_pool/stats.rs index 992a4dfe55..a50a4a8180 100644 --- a/votor/src/certificate_pool/stats.rs +++ b/votor/src/certificate_pool/stats.rs @@ -10,15 +10,19 @@ const STATS_REPORT_INTERVAL: Duration = Duration::from_secs(10); #[derive(Debug)] pub(crate) struct CertificatePoolStats { pub(crate) conflicting_votes: u32, - pub(crate) event_safe_to_skip: u32, pub(crate) event_safe_to_notarize: u32, + pub(crate) event_safe_to_skip: u32, + pub(crate) exist_certs: u32, + pub(crate) exist_votes: u32, + pub(crate) incoming_certs: u32, + pub(crate) incoming_votes: u32, + pub(crate) out_of_range_certs: u32, + pub(crate) out_of_range_votes: u32, - pub(crate) exits: [u32; 2], - pub(crate) incoming: [u32; 2], - pub(crate) out_of_range: [u32; 2], + pub(crate) new_certs_generated: Vec, + pub(crate) new_certs_ingested: Vec, + pub(crate) ingested_votes: Vec, - pub(crate) new_certs_by_type: [Vec; 2], // 0 for ingested, 1 for generated - pub(crate) ingested_votes_by_type: Vec, last_reported: Instant, } @@ -30,54 +34,41 @@ impl Default for CertificatePoolStats { impl CertificatePoolStats { pub fn new() -> Self { + let num_vote_types = (VoteType::SkipFallback as usize).saturating_add(1); + let num_cert_types = (CertificateType::Skip as usize).saturating_add(1); Self { conflicting_votes: 0, - event_safe_to_skip: 0, event_safe_to_notarize: 0, - exits: [0, 0], - incoming: [0, 0], - out_of_range: [0, 0], - new_certs_by_type: [Vec::new(), Vec::new()], - ingested_votes_by_type: Vec::new(), - last_reported: Instant::now(), - } - } + event_safe_to_skip: 0, + exist_certs: 0, + exist_votes: 0, + incoming_certs: 0, + incoming_votes: 0, + out_of_range_certs: 0, + out_of_range_votes: 0, - pub fn incr_u32_array(value: &mut [u32; 2], is_vote: bool) { - if is_vote { - value[0] = value[0].saturating_add(1); - } else { - value[1] = value[1].saturating_add(1); - } - } + new_certs_ingested: vec![0; num_cert_types], + new_certs_generated: vec![0; num_cert_types], + ingested_votes: vec![0; num_vote_types], - pub fn incr_u32(value: &mut u32) { - *value = value.saturating_add(1); + last_reported: Instant::now(), + } } pub fn incr_ingested_vote_type(&mut self, vote_type: VoteType) { let index = vote_type as usize; - if self.ingested_votes_by_type.len() <= index { - self.ingested_votes_by_type - .resize(index.saturating_add(1), 0); - } - - self.ingested_votes_by_type[index] = self.ingested_votes_by_type[index].saturating_add(1); + self.ingested_votes[index] = self.ingested_votes[index].saturating_add(1); } pub fn incr_cert_type(&mut self, cert_type: CertificateType, is_generated: bool) { let index = cert_type as usize; let array = if is_generated { - &mut self.new_certs_by_type[0] + &mut self.new_certs_generated } else { - &mut self.new_certs_by_type[1] + &mut self.new_certs_ingested }; - if array.len() <= index { - array.resize(index.saturating_add(1), 0); - } - array[index] = array[index].saturating_add(1); } @@ -91,136 +82,139 @@ impl CertificatePoolStats { self.event_safe_to_notarize as i64, i64 ), - ("exits_votes", self.exits[0] as i64, i64), - ("exits_certificates", self.exits[1] as i64, i64), - ("incoming_votes", self.incoming[0] as i64, i64), - ("incoming_certificates", self.incoming[1] as i64, i64), - ("out_of_range_votes", self.out_of_range[0] as i64, i64), - ( - "out_of_range_certificates", - self.out_of_range[1] as i64, - i64 - ), + ("exist_votes", self.exist_votes as i64, i64), + ("exist_certs", self.exist_certs as i64, i64), + ("incoming_votes", self.incoming_votes as i64, i64), + ("incoming_certs", self.incoming_certs as i64, i64), + ("out_of_range_votes", self.out_of_range_votes as i64, i64), + ("out_of_range_certs", self.out_of_range_certs as i64, i64), ); datapoint_info!( - "certificate_pool_ingested_votes_by_type", + "certificate_pool_ingested_votes", ( "finalize", *self - .ingested_votes_by_type + .ingested_votes .get(VoteType::Finalize as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "notarize", *self - .ingested_votes_by_type + .ingested_votes .get(VoteType::Notarize as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "notarize_fallback", *self - .ingested_votes_by_type + .ingested_votes .get(VoteType::NotarizeFallback as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "skip", - *self - .ingested_votes_by_type - .get(VoteType::Skip as usize) - .unwrap_or(&0) as i64, + *self.ingested_votes.get(VoteType::Skip as usize).unwrap() as i64, i64 ), ( "skip_fallback", *self - .ingested_votes_by_type + .ingested_votes .get(VoteType::SkipFallback as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ); datapoint_info!( - "certfificate_pool_ingested_certs_by_type", + "certfificate_pool_ingested_certs", ( "finalize", - *self.new_certs_by_type[0] + *self + .new_certs_ingested .get(CertificateType::Finalize as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "finalize_fast", - *self.new_certs_by_type[0] + *self + .new_certs_ingested .get(CertificateType::FinalizeFast as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "notarize", - *self.new_certs_by_type[0] + *self + .new_certs_ingested .get(CertificateType::Notarize as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "notarize_fallback", - *self.new_certs_by_type[0] + *self + .new_certs_ingested .get(CertificateType::NotarizeFallback as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "skip", - *self.new_certs_by_type[0] + *self + .new_certs_ingested .get(CertificateType::Skip as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ); datapoint_info!( - "certificate_pool_generated_certs_by_type", + "certificate_pool_generated_certs", ( "finalize", - *self.new_certs_by_type[1] + *self + .new_certs_generated .get(CertificateType::Finalize as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "finalize_fast", - *self.new_certs_by_type[1] + *self + .new_certs_generated .get(CertificateType::FinalizeFast as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "notarize", - *self.new_certs_by_type[1] + *self + .new_certs_generated .get(CertificateType::Notarize as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "notarize_fallback", - *self.new_certs_by_type[1] + *self + .new_certs_generated .get(CertificateType::NotarizeFallback as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ( "skip", - *self.new_certs_by_type[1] + *self + .new_certs_generated .get(CertificateType::Skip as usize) - .unwrap_or(&0) as i64, + .unwrap() as i64, i64 ), ); @@ -229,21 +223,7 @@ impl CertificatePoolStats { pub fn maybe_report(&mut self) { if self.last_reported.elapsed() >= STATS_REPORT_INTERVAL { self.report(); - self.reset(); - } - } - - fn reset(&mut self) { - self.conflicting_votes = 0; - self.event_safe_to_skip = 0; - self.event_safe_to_notarize = 0; - for i in 0..2 { - self.incoming[i] = 0; - self.out_of_range[i] = 0; + *self = Self::new(); } - self.new_certs_by_type[0].clear(); - self.new_certs_by_type[1].clear(); - self.ingested_votes_by_type.clear(); - self.last_reported = Instant::now(); } } From 6fd26b96939c0e3d30e3ba143095077ccad9735d Mon Sep 17 00:00:00 2001 From: Wen <113942165+wen-coding@users.noreply.github.com> Date: Thu, 24 Jul 2025 12:31:13 -0700 Subject: [PATCH 3/4] Trigger CertificatePoolStats reporting from CertificatePoolService. --- votor/src/certificate_pool.rs | 6 ++++-- votor/src/certificate_pool/stats.rs | 21 +++------------------ votor/src/certificate_pool_service.rs | 5 ++++- votor/src/certificate_pool_service/stats.rs | 5 ++++- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/votor/src/certificate_pool.rs b/votor/src/certificate_pool.rs index b9de2cf60b..f96521d67b 100644 --- a/votor/src/certificate_pool.rs +++ b/votor/src/certificate_pool.rs @@ -394,8 +394,6 @@ impl CertificatePool { message: &BLSMessage, events: &mut Vec, ) -> Result<(Option, Vec>), AddVoteError> { - // We add stats reporting here because we should almost always have a message. - self.stats.maybe_report(); let current_highest_finalized_slot = self.highest_finalized_slot; let new_certficates_to_send = match message { BLSMessage::Vote(vote_message) => { @@ -778,6 +776,10 @@ impl CertificatePool { pub fn update_pubkey(&mut self, new_pubkey: Pubkey) { self.parent_ready_tracker.update_pubkey(new_pubkey); } + + pub fn report_stats(&mut self) { + self.stats.report(); + } } pub fn load_from_blockstore( diff --git a/votor/src/certificate_pool/stats.rs b/votor/src/certificate_pool/stats.rs index a50a4a8180..1cbd89b2aa 100644 --- a/votor/src/certificate_pool/stats.rs +++ b/votor/src/certificate_pool/stats.rs @@ -1,12 +1,7 @@ use { - crate::VoteType, - alpenglow_vote::certificate::CertificateType, - solana_metrics::datapoint_info, - std::time::{Duration, Instant}, + crate::VoteType, alpenglow_vote::certificate::CertificateType, solana_metrics::datapoint_info, }; -const STATS_REPORT_INTERVAL: Duration = Duration::from_secs(10); - #[derive(Debug)] pub(crate) struct CertificatePoolStats { pub(crate) conflicting_votes: u32, @@ -22,8 +17,6 @@ pub(crate) struct CertificatePoolStats { pub(crate) new_certs_generated: Vec, pub(crate) new_certs_ingested: Vec, pub(crate) ingested_votes: Vec, - - last_reported: Instant, } impl Default for CertificatePoolStats { @@ -50,8 +43,6 @@ impl CertificatePoolStats { new_certs_ingested: vec![0; num_cert_types], new_certs_generated: vec![0; num_cert_types], ingested_votes: vec![0; num_vote_types], - - last_reported: Instant::now(), } } @@ -72,7 +63,7 @@ impl CertificatePoolStats { array[index] = array[index].saturating_add(1); } - fn report(&self) { + pub fn report(&mut self) { datapoint_info!( "certificate_pool_stats", ("conflicting_votes", self.conflicting_votes as i64, i64), @@ -218,12 +209,6 @@ impl CertificatePoolStats { i64 ), ); - } - - pub fn maybe_report(&mut self) { - if self.last_reported.elapsed() >= STATS_REPORT_INTERVAL { - self.report(); - *self = Self::new(); - } + *self = Self::new(); } } diff --git a/votor/src/certificate_pool_service.rs b/votor/src/certificate_pool_service.rs index 860a0107f9..68f091de99 100644 --- a/votor/src/certificate_pool_service.rs +++ b/votor/src/certificate_pool_service.rs @@ -275,7 +275,10 @@ impl CertificatePoolService { return Ok(()); } } - stats.maybe_report(); + if stats.maybe_report() { + // Stats were reported, report the certificate pool stats + cert_pool.report_stats(); + } } Ok(()) } diff --git a/votor/src/certificate_pool_service/stats.rs b/votor/src/certificate_pool_service/stats.rs index d6b8e55278..1bb2d978f3 100644 --- a/votor/src/certificate_pool_service/stats.rs +++ b/votor/src/certificate_pool_service/stats.rs @@ -83,10 +83,13 @@ impl CertificatePoolServiceStats { ); } - pub fn maybe_report(&mut self) { + pub fn maybe_report(&mut self) -> bool { if self.last_request_time.elapsed() >= STATS_REPORT_INTERVAL { self.report(); self.reset(); + true + } else { + false } } } From 77fa024a3a9ffd8635ff0edea3806c80dd57bd80 Mon Sep 17 00:00:00 2001 From: Wen <113942165+wen-coding@users.noreply.github.com> Date: Thu, 24 Jul 2025 12:40:42 -0700 Subject: [PATCH 4/4] Decouple CertificatePool and CertificatePoolService stats. --- votor/src/certificate_pool.rs | 4 ++-- votor/src/certificate_pool/stats.rs | 21 ++++++++++++++++++--- votor/src/certificate_pool_service.rs | 6 ++---- votor/src/certificate_pool_service/stats.rs | 5 +---- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/votor/src/certificate_pool.rs b/votor/src/certificate_pool.rs index f96521d67b..9d2cbfc8f7 100644 --- a/votor/src/certificate_pool.rs +++ b/votor/src/certificate_pool.rs @@ -777,8 +777,8 @@ impl CertificatePool { self.parent_ready_tracker.update_pubkey(new_pubkey); } - pub fn report_stats(&mut self) { - self.stats.report(); + pub fn maybe_report(&mut self) { + self.stats.maybe_report(); } } diff --git a/votor/src/certificate_pool/stats.rs b/votor/src/certificate_pool/stats.rs index 1cbd89b2aa..b7a57a3bad 100644 --- a/votor/src/certificate_pool/stats.rs +++ b/votor/src/certificate_pool/stats.rs @@ -1,7 +1,12 @@ use { - crate::VoteType, alpenglow_vote::certificate::CertificateType, solana_metrics::datapoint_info, + crate::VoteType, + alpenglow_vote::certificate::CertificateType, + solana_metrics::datapoint_info, + std::time::{Duration, Instant}, }; +const STATS_REPORT_INTERVAL: Duration = Duration::from_secs(10); + #[derive(Debug)] pub(crate) struct CertificatePoolStats { pub(crate) conflicting_votes: u32, @@ -17,6 +22,8 @@ pub(crate) struct CertificatePoolStats { pub(crate) new_certs_generated: Vec, pub(crate) new_certs_ingested: Vec, pub(crate) ingested_votes: Vec, + + pub(crate) last_request_time: Instant, } impl Default for CertificatePoolStats { @@ -43,6 +50,8 @@ impl CertificatePoolStats { new_certs_ingested: vec![0; num_cert_types], new_certs_generated: vec![0; num_cert_types], ingested_votes: vec![0; num_vote_types], + + last_request_time: Instant::now(), } } @@ -63,7 +72,7 @@ impl CertificatePoolStats { array[index] = array[index].saturating_add(1); } - pub fn report(&mut self) { + fn report(&self) { datapoint_info!( "certificate_pool_stats", ("conflicting_votes", self.conflicting_votes as i64, i64), @@ -209,6 +218,12 @@ impl CertificatePoolStats { i64 ), ); - *self = Self::new(); + } + + pub fn maybe_report(&mut self) { + if self.last_request_time.elapsed() >= STATS_REPORT_INTERVAL { + self.report(); + *self = Self::new(); + } } } diff --git a/votor/src/certificate_pool_service.rs b/votor/src/certificate_pool_service.rs index 68f091de99..d8ce249f37 100644 --- a/votor/src/certificate_pool_service.rs +++ b/votor/src/certificate_pool_service.rs @@ -275,10 +275,8 @@ impl CertificatePoolService { return Ok(()); } } - if stats.maybe_report() { - // Stats were reported, report the certificate pool stats - cert_pool.report_stats(); - } + stats.maybe_report(); + cert_pool.maybe_report(); } Ok(()) } diff --git a/votor/src/certificate_pool_service/stats.rs b/votor/src/certificate_pool_service/stats.rs index 1bb2d978f3..d6b8e55278 100644 --- a/votor/src/certificate_pool_service/stats.rs +++ b/votor/src/certificate_pool_service/stats.rs @@ -83,13 +83,10 @@ impl CertificatePoolServiceStats { ); } - pub fn maybe_report(&mut self) -> bool { + pub fn maybe_report(&mut self) { if self.last_request_time.elapsed() >= STATS_REPORT_INTERVAL { self.report(); self.reset(); - true - } else { - false } } }