From ffb04e1a9e2b55f66145daa1d4266dac4cc7c627 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 1 Nov 2021 05:52:31 +0000 Subject: [PATCH] Add op pool metrics for attestations (#2758) ## Proposed Changes Add several metrics for the number of attestations in the op pool. These give us a way to observe the number of valid, non-trivial attestations during block packing rather than just the size of the entire op pool. --- beacon_node/beacon_chain/src/metrics.rs | 16 ++++- beacon_node/operation_pool/src/lib.rs | 77 ++++++++++++++++----- beacon_node/operation_pool/src/max_cover.rs | 19 +++-- beacon_node/operation_pool/src/metrics.rs | 13 ++++ 4 files changed, 100 insertions(+), 25 deletions(-) diff --git a/beacon_node/beacon_chain/src/metrics.rs b/beacon_node/beacon_chain/src/metrics.rs index 2128eace4..2967d40a1 100644 --- a/beacon_node/beacon_chain/src/metrics.rs +++ b/beacon_node/beacon_chain/src/metrics.rs @@ -365,6 +365,10 @@ lazy_static! { */ pub static ref OP_POOL_NUM_ATTESTATIONS: Result = try_create_int_gauge("beacon_op_pool_attestations_total", "Count of attestations in the op pool"); + pub static ref OP_POOL_NUM_ATTESTATION_DATA: Result = + try_create_int_gauge("beacon_op_pool_attestation_data_total", "Count of attestation data in the op pool"); + pub static ref OP_POOL_MAX_AGGREGATES_PER_DATA: Result = + try_create_int_gauge("beacon_op_pool_max_aggregates_per_data", "Max aggregates per AttestationData"); pub static ref OP_POOL_NUM_ATTESTER_SLASHINGS: Result = try_create_int_gauge("beacon_op_pool_attester_slashings_total", "Count of attester slashings in the op pool"); pub static ref OP_POOL_NUM_PROPOSER_SLASHINGS: Result = @@ -886,9 +890,19 @@ pub fn scrape_for_metrics(beacon_chain: &BeaconChain) { scrape_sync_committee_observation(slot, beacon_chain); } + let attestation_stats = beacon_chain.op_pool.attestation_stats(); + set_gauge_by_usize( &OP_POOL_NUM_ATTESTATIONS, - beacon_chain.op_pool.num_attestations(), + attestation_stats.num_attestations, + ); + set_gauge_by_usize( + &OP_POOL_NUM_ATTESTATION_DATA, + attestation_stats.num_attestation_data, + ); + set_gauge_by_usize( + &OP_POOL_MAX_AGGREGATES_PER_DATA, + attestation_stats.max_aggregates_per_data, ); set_gauge_by_usize( &OP_POOL_NUM_ATTESTER_SLASHINGS, diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 0b979c9f4..2cc3ffaf6 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -57,6 +57,15 @@ pub enum OpPoolError { IncorrectOpPoolVariant, } +pub struct AttestationStats { + /// Total number of attestations for all committeees/indices/votes. + pub num_attestations: usize, + /// Number of unique `AttestationData` attested to. + pub num_attestation_data: usize, + /// Maximum number of aggregates for a single `AttestationData`. + pub max_aggregates_per_data: usize, +} + impl From for OpPoolError { fn from(e: SyncAggregateError) -> Self { OpPoolError::SyncAggregateError(e) @@ -207,6 +216,23 @@ impl OperationPool { self.attestations.read().values().map(Vec::len).sum() } + pub fn attestation_stats(&self) -> AttestationStats { + let mut num_attestations = 0; + let mut num_attestation_data = 0; + let mut max_aggregates_per_data = 0; + + for aggregates in self.attestations.read().values() { + num_attestations += aggregates.len(); + num_attestation_data += 1; + max_aggregates_per_data = std::cmp::max(max_aggregates_per_data, aggregates.len()); + } + AttestationStats { + num_attestations, + num_attestation_data, + max_aggregates_per_data, + } + } + /// Return all valid attestations for the given epoch, for use in max cover. fn get_valid_attestations_for_epoch<'a>( &'a self, @@ -265,22 +291,29 @@ impl OperationPool { // Split attestations for the previous & current epochs, so that we // can optimise them individually in parallel. - let prev_epoch_att = self.get_valid_attestations_for_epoch( - prev_epoch, - &*all_attestations, - state, - total_active_balance, - prev_epoch_validity_filter, - spec, - ); - let curr_epoch_att = self.get_valid_attestations_for_epoch( - current_epoch, - &*all_attestations, - state, - total_active_balance, - curr_epoch_validity_filter, - spec, - ); + let mut num_prev_valid = 0_i64; + let mut num_curr_valid = 0_i64; + + let prev_epoch_att = self + .get_valid_attestations_for_epoch( + prev_epoch, + &*all_attestations, + state, + total_active_balance, + prev_epoch_validity_filter, + spec, + ) + .inspect(|_| num_prev_valid += 1); + let curr_epoch_att = self + .get_valid_attestations_for_epoch( + current_epoch, + &*all_attestations, + state, + total_active_balance, + curr_epoch_validity_filter, + spec, + ) + .inspect(|_| num_curr_valid += 1); let prev_epoch_limit = if let BeaconState::Base(base_state) = state { std::cmp::min( @@ -299,15 +332,22 @@ impl OperationPool { if prev_epoch == current_epoch { vec![] } else { - maximum_cover(prev_epoch_att, prev_epoch_limit) + maximum_cover(prev_epoch_att, prev_epoch_limit, "prev_epoch_attestations") } }, move || { let _timer = metrics::start_timer(&metrics::ATTESTATION_CURR_EPOCH_PACKING_TIME); - maximum_cover(curr_epoch_att, T::MaxAttestations::to_usize()) + maximum_cover( + curr_epoch_att, + T::MaxAttestations::to_usize(), + "curr_epoch_attestations", + ) }, ); + metrics::set_gauge(&metrics::NUM_PREV_EPOCH_ATTESTATIONS, num_prev_valid); + metrics::set_gauge(&metrics::NUM_CURR_EPOCH_ATTESTATIONS, num_curr_valid); + Ok(max_cover::merge_solutions( curr_cover, prev_cover, @@ -394,6 +434,7 @@ impl OperationPool { let attester_slashings = maximum_cover( relevant_attester_slashings, T::MaxAttesterSlashings::to_usize(), + "attester_slashings", ) .into_iter() .map(|cover| { diff --git a/beacon_node/operation_pool/src/max_cover.rs b/beacon_node/operation_pool/src/max_cover.rs index 9f9adbb82..8e50b8152 100644 --- a/beacon_node/operation_pool/src/max_cover.rs +++ b/beacon_node/operation_pool/src/max_cover.rs @@ -1,3 +1,4 @@ +use crate::metrics; use itertools::Itertools; /// Trait for types that we can compute a maximum cover for. @@ -44,7 +45,7 @@ impl MaxCoverItem { /// /// * Time complexity: `O(limit * items_iter.len())` /// * Space complexity: `O(item_iter.len())` -pub fn maximum_cover(items_iter: I, limit: usize) -> Vec +pub fn maximum_cover(items_iter: I, limit: usize, label: &str) -> Vec where I: IntoIterator, T: MaxCover, @@ -56,6 +57,12 @@ where .filter(|x| x.item.score() != 0) .collect(); + metrics::set_int_gauge( + &metrics::MAX_COVER_NON_ZERO_ITEMS, + &[label], + all_items.len() as i64, + ); + let mut result = vec![]; for _ in 0..limit { @@ -146,14 +153,14 @@ mod test { #[test] fn zero_limit() { - let cover = maximum_cover(example_system(), 0); + let cover = maximum_cover(example_system(), 0, "test"); assert_eq!(cover.len(), 0); } #[test] fn one_limit() { let sets = example_system(); - let cover = maximum_cover(sets.clone(), 1); + let cover = maximum_cover(sets.clone(), 1, "test"); assert_eq!(cover.len(), 1); assert_eq!(cover[0], sets[1]); } @@ -163,7 +170,7 @@ mod test { fn exclude_zero_score() { let sets = example_system(); for k in 2..10 { - let cover = maximum_cover(sets.clone(), k); + let cover = maximum_cover(sets.clone(), k, "test"); assert_eq!(cover.len(), 2); assert_eq!(cover[0], sets[1]); assert_eq!(cover[1], sets[0]); @@ -187,7 +194,7 @@ mod test { HashSet::from_iter(vec![5, 6, 7, 8]), // 4, 4* HashSet::from_iter(vec![0, 1, 2, 3, 4]), // 5* ]; - let cover = maximum_cover(sets, 3); + let cover = maximum_cover(sets, 3, "test"); assert_eq!(quality(&cover), 11); } @@ -202,7 +209,7 @@ mod test { HashSet::from_iter(vec![1, 5, 6, 8]), HashSet::from_iter(vec![1, 7, 11, 19]), ]; - let cover = maximum_cover(sets, 5); + let cover = maximum_cover(sets, 5, "test"); assert_eq!(quality(&cover), 19); assert_eq!(cover.len(), 5); } diff --git a/beacon_node/operation_pool/src/metrics.rs b/beacon_node/operation_pool/src/metrics.rs index e69dfaa16..3fa5208a3 100644 --- a/beacon_node/operation_pool/src/metrics.rs +++ b/beacon_node/operation_pool/src/metrics.rs @@ -11,4 +11,17 @@ lazy_static! { "op_pool_attestation_curr_epoch_packing_time", "Time to pack current epoch attestations" ); + pub static ref NUM_PREV_EPOCH_ATTESTATIONS: Result = try_create_int_gauge( + "op_pool_prev_epoch_attestations", + "Number of valid attestations considered for packing from the previous epoch" + ); + pub static ref NUM_CURR_EPOCH_ATTESTATIONS: Result = try_create_int_gauge( + "op_pool_curr_epoch_attestations", + "Number of valid attestations considered for packing from the current epoch" + ); + pub static ref MAX_COVER_NON_ZERO_ITEMS: Result = try_create_int_gauge_vec( + "op_pool_max_cover_non_zero_items", + "Number of non-trivial items considered in a max coverage optimisation", + &["label"] + ); }