diff --git a/polkadot/node/core/approval-voting/src/approval_checking.rs b/polkadot/node/core/approval-voting/src/approval_checking.rs
index ec623a25bf996b7ca612a08b093d7064023fca2f..d8384f88d4c9598c528ee097ec148d887e26d29e 100644
--- a/polkadot/node/core/approval-voting/src/approval_checking.rs
+++ b/polkadot/node/core/approval-voting/src/approval_checking.rs
@@ -59,17 +59,40 @@ pub enum RequiredTranches {
}
}
+/// The result of a check.
+#[derive(Debug, Clone, Copy)]
+pub enum Check {
+ /// The candidate is unapproved.
+ Unapproved,
+ /// The candidate is approved, with the given amount of no-shows.
+ Approved(usize),
+}
+
+impl Check {
+ /// Whether the candidate is approved.
+ pub fn is_approved(&self) -> bool {
+ match *self {
+ Check::Unapproved => false,
+ Check::Approved(_) => true,
+ }
+ }
+}
+
/// Check the approval of a candidate.
pub fn check_approval(
candidate: &CandidateEntry,
approval: &ApprovalEntry,
required: RequiredTranches,
-) -> bool {
+) -> Check {
match required {
- RequiredTranches::Pending { .. } => false,
+ RequiredTranches::Pending { .. } => Check::Unapproved,
RequiredTranches::All => {
let approvals = candidate.approvals();
- 3 * approvals.count_ones() > 2 * approvals.len()
+ if 3 * approvals.count_ones() > 2 * approvals.len() {
+ Check::Approved(0)
+ } else {
+ Check::Unapproved
+ }
}
RequiredTranches::Exact { needed, tolerated_missing, .. } => {
// whether all assigned validators up to `needed` less no_shows have approved.
@@ -93,7 +116,11 @@ pub fn check_approval(
// note: the process of computing `required` only chooses `exact` if
// that will surpass a minimum amount of checks.
// shouldn't typically go above, since all no-shows are supposed to be covered.
- n_approved + tolerated_missing >= n_assigned
+ if n_approved + tolerated_missing >= n_assigned {
+ Check::Approved(tolerated_missing)
+ } else {
+ Check::Unapproved
+ }
}
}
}
@@ -380,7 +407,7 @@ mod tests {
maximum_broadcast: 0,
clock_drift: 0,
},
- ));
+ ).is_approved());
}
#[test]
@@ -404,10 +431,10 @@ mod tests {
approved: false,
}.into();
- assert!(!check_approval(&candidate, &approval_entry, RequiredTranches::All));
+ assert!(!check_approval(&candidate, &approval_entry, RequiredTranches::All).is_approved());
candidate.mark_approval(ValidatorIndex(6));
- assert!(check_approval(&candidate, &approval_entry, RequiredTranches::All));
+ assert!(check_approval(&candidate, &approval_entry, RequiredTranches::All).is_approved());
}
#[test]
@@ -452,7 +479,7 @@ mod tests {
tolerated_missing: 0,
next_no_show: None,
},
- ));
+ ).is_approved());
assert!(!check_approval(
&candidate,
&approval_entry,
@@ -461,7 +488,7 @@ mod tests {
tolerated_missing: 0,
next_no_show: None,
},
- ));
+ ).is_approved());
assert!(check_approval(
&candidate,
&approval_entry,
@@ -470,7 +497,7 @@ mod tests {
tolerated_missing: 4,
next_no_show: None,
},
- ));
+ ).is_approved());
}
#[test]
diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs
index 1183a141b87cc9d229a3fd0091a0ed2a2540fb4d..82424586ebff6c01ceb3be5646a916627f82b875 100644
--- a/polkadot/node/core/approval-voting/src/lib.rs
+++ b/polkadot/node/core/approval-voting/src/lib.rs
@@ -105,6 +105,10 @@ struct MetricsInner {
imported_candidates_total: prometheus::Counter<prometheus::U64>,
assignments_produced_total: prometheus::Counter<prometheus::U64>,
approvals_produced_total: prometheus::Counter<prometheus::U64>,
+ no_shows_total: prometheus::Counter<prometheus::U64>,
+ wakeups_triggered_total: prometheus::Counter<prometheus::U64>,
+ candidate_approval_time_ticks: prometheus::Histogram,
+ block_approval_time_ticks: prometheus::Histogram,
}
/// Aproval Voting metrics.
@@ -129,6 +133,30 @@ impl Metrics {
metrics.approvals_produced_total.inc();
}
}
+
+ fn on_no_shows(&self, n: usize) {
+ if let Some(metrics) = &self.0 {
+ metrics.no_shows_total.inc_by(n as u64);
+ }
+ }
+
+ fn on_wakeup(&self) {
+ if let Some(metrics) = &self.0 {
+ metrics.wakeups_triggered_total.inc();
+ }
+ }
+
+ fn on_candidate_approved(&self, ticks: Tick) {
+ if let Some(metrics) = &self.0 {
+ metrics.candidate_approval_time_ticks.observe(ticks as f64);
+ }
+ }
+
+ fn on_block_approved(&self, ticks: Tick) {
+ if let Some(metrics) = &self.0 {
+ metrics.block_approval_time_ticks.observe(ticks as f64);
+ }
+ }
}
impl metrics::Metrics for Metrics {
@@ -157,7 +185,40 @@ impl metrics::Metrics for Metrics {
)?,
registry,
)?,
+ no_shows_total: prometheus::register(
+ prometheus::Counter::new(
+ "parachain_approvals_no_shows_total",
+ "Number of assignments which became no-shows in the approval voting subsystem",
+ )?,
+ registry,
+ )?,
+ wakeups_triggered_total: prometheus::register(
+ prometheus::Counter::new(
+ "parachain_approvals_wakeups_total",
+ "Number of times we woke up to process a candidate in the approval voting subsystem",
+ )?,
+ registry,
+ )?,
+ candidate_approval_time_ticks: prometheus::register(
+ prometheus::Histogram::with_opts(
+ prometheus::HistogramOpts::new(
+ "parachain_approvals_candidate_approval_time_ticks",
+ "Number of ticks (500ms) to approve candidates.",
+ ).buckets(vec![6.0, 12.0, 18.0, 24.0, 30.0, 36.0, 72.0, 100.0, 144.0]),
+ )?,
+ registry,
+ )?,
+ block_approval_time_ticks: prometheus::register(
+ prometheus::Histogram::with_opts(
+ prometheus::HistogramOpts::new(
+ "parachain_approvals_blockapproval_time_ticks",
+ "Number of ticks (500ms) to approve blocks.",
+ ).buckets(vec![6.0, 12.0, 18.0, 24.0, 30.0, 36.0, 72.0, 100.0, 144.0]),
+ )?,
+ registry,
+ )?,
};
+
Ok(Metrics(Some(metrics)))
}
}
@@ -400,6 +461,7 @@ async fn run<C>(
loop {
let actions = futures::select! {
(tick, woken_block, woken_candidate) = wakeups.next(&*state.clock).fuse() => {
+ subsystem.metrics.on_wakeup();
process_wakeup(
&mut state,
woken_block,
@@ -589,12 +651,13 @@ async fn handle_from_overseer(
}
FromOverseer::Communication { msg } => match msg {
ApprovalVotingMessage::CheckAndImportAssignment(a, claimed_core, res) => {
- let (check_outcome, actions) = check_and_import_assignment(state, a, claimed_core)?;
+ let (check_outcome, actions)
+ = check_and_import_assignment(state, metrics, a, claimed_core)?;
let _ = res.send(check_outcome);
actions
}
ApprovalVotingMessage::CheckAndImportApproval(a, res) => {
- check_and_import_approval(state, a, |r| { let _ = res.send(r); })?.0
+ check_and_import_approval(state, metrics, a, |r| { let _ = res.send(r); })?.0
}
ApprovalVotingMessage::ApprovedAncestor(target, lower_bound, res ) => {
match handle_approved_ancestor(ctx, &state.db, target, lower_bound).await {
@@ -858,6 +921,7 @@ fn schedule_wakeup_action(
fn check_and_import_assignment(
state: &State<impl DBReader>,
+ metrics: &Metrics,
assignment: IndirectAssignmentCert,
candidate_index: CandidateIndex,
) -> SubsystemResult<(AssignmentCheckResult, Vec<Action>)> {
@@ -963,6 +1027,7 @@ fn check_and_import_assignment(
// It also produces actions to schedule wakeups for the candidate.
let actions = check_and_apply_full_approval(
state,
+ &metrics,
Some((assignment.block_hash, block_entry)),
assigned_candidate_hash,
candidate_entry,
@@ -974,6 +1039,7 @@ fn check_and_import_assignment(
fn check_and_import_approval<T>(
state: &State<impl DBReader>,
+ metrics: &Metrics,
approval: IndirectSignedApprovalVote,
with_response: impl FnOnce(ApprovalCheckResult) -> T,
) -> SubsystemResult<(Vec<Action>, T)> {
@@ -1050,6 +1116,7 @@ fn check_and_import_approval<T>(
let actions = import_checked_approval(
state,
+ &metrics,
Some((approval.block_hash, block_entry)),
approved_candidate_hash,
candidate_entry,
@@ -1061,6 +1128,7 @@ fn check_and_import_approval<T>(
fn import_checked_approval(
state: &State<impl DBReader>,
+ metrics: &Metrics,
already_loaded: Option<(Hash, BlockEntry)>,
candidate_hash: CandidateHash,
mut candidate_entry: CandidateEntry,
@@ -1076,6 +1144,7 @@ fn import_checked_approval(
// This may include blocks beyond the already loaded block.
let actions = check_and_apply_full_approval(
state,
+ metrics,
already_loaded,
candidate_hash,
candidate_entry,
@@ -1092,6 +1161,7 @@ fn import_checked_approval(
// the candidate under any blocks filtered.
fn check_and_apply_full_approval(
state: &State<impl DBReader>,
+ metrics: &Metrics,
mut already_loaded: Option<(Hash, BlockEntry)>,
candidate_hash: CandidateHash,
mut candidate_entry: CandidateEntry,
@@ -1150,13 +1220,13 @@ fn check_and_apply_full_approval(
session_info.needed_approvals as _
);
- let now_approved = approval_checking::check_approval(
+ let check = approval_checking::check_approval(
&candidate_entry,
approval_entry,
required_tranches.clone(),
);
- if now_approved {
+ if let approval_checking::Check::Approved(no_shows) = check {
tracing::trace!(
target: LOG_TARGET,
"Candidate approved {} under block {}",
@@ -1164,8 +1234,21 @@ fn check_and_apply_full_approval(
block_hash,
);
+ let was_approved = block_entry.is_fully_approved();
+
newly_approved.push(*block_hash);
block_entry.mark_approved_by_hash(&candidate_hash);
+ let is_approved = block_entry.is_fully_approved();
+
+ if no_shows != 0 {
+ metrics.on_no_shows(no_shows);
+ }
+
+ metrics.on_candidate_approved(tranche_now as _);
+
+ if is_approved && !was_approved {
+ metrics.on_block_approved(tranche_now as _)
+ }
actions.push(Action::WriteBlockEntry(block_entry));
}
@@ -1204,7 +1287,7 @@ fn should_trigger_assignment(
&candidate_entry,
&approval_entry,
RequiredTranches::All,
- ),
+ ).is_approved(),
RequiredTranches::Pending {
maximum_broadcast,
clock_drift,
@@ -1620,6 +1703,7 @@ async fn issue_approval(
let actions = import_checked_approval(
state,
+ metrics,
Some((block_hash, block_entry)),
candidate_hash,
candidate_entry,
diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs
index 3959a1468be67e4a526c593849044ac0777fdb17..3c94c7c0918e782be4eefbb2ea107c7f0b4b2ddb 100644
--- a/polkadot/node/core/approval-voting/src/tests.rs
+++ b/polkadot/node/core/approval-voting/src/tests.rs
@@ -376,6 +376,7 @@ fn rejects_bad_assignment() {
let res = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment_good.clone(),
candidate_index,
).unwrap();
@@ -396,6 +397,7 @@ fn rejects_bad_assignment() {
let res = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment,
candidate_index,
).unwrap();
@@ -411,6 +413,7 @@ fn rejects_bad_assignment() {
// same assignment, but this time rejected
let res = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment_good,
candidate_index,
).unwrap();
@@ -441,6 +444,7 @@ fn rejects_assignment_in_future() {
let res = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment.clone(),
candidate_index,
).unwrap();
@@ -455,6 +459,7 @@ fn rejects_assignment_in_future() {
let res = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment.clone(),
candidate_index,
).unwrap();
@@ -479,6 +484,7 @@ fn rejects_assignment_with_unknown_candidate() {
let res = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment.clone(),
candidate_index,
).unwrap();
@@ -510,6 +516,7 @@ fn assignment_import_updates_candidate_entry_and_schedules_wakeup() {
let (res, actions) = check_and_import_assignment(
&mut state,
+ &Metrics(None),
assignment.clone(),
candidate_index,
).unwrap();
@@ -560,6 +567,7 @@ fn rejects_approval_before_assignment() {
let (actions, res) = check_and_import_approval(
&state,
+ &Metrics(None),
vote,
|r| r
).unwrap();
@@ -591,6 +599,7 @@ fn rejects_approval_if_no_candidate_entry() {
let (actions, res) = check_and_import_approval(
&state,
+ &Metrics(None),
vote,
|r| r
).unwrap();
@@ -628,6 +637,7 @@ fn rejects_approval_if_no_block_entry() {
let (actions, res) = check_and_import_approval(
&state,
+ &Metrics(None),
vote,
|r| r
).unwrap();
@@ -669,6 +679,7 @@ fn accepts_and_imports_approval_after_assignment() {
let (actions, res) = check_and_import_approval(
&state,
+ &Metrics(None),
vote,
|r| r
).unwrap();
@@ -722,6 +733,7 @@ fn second_approval_import_is_no_op() {
let (actions, res) = check_and_import_approval(
&state,
+ &Metrics(None),
vote,
|r| r
).unwrap();
@@ -767,6 +779,7 @@ fn check_and_apply_full_approval_sets_flag_and_bit() {
let actions = check_and_apply_full_approval(
&state,
+ &Metrics(None),
None,
candidate_hash,
state.db.candidate_entries.get(&candidate_hash).unwrap().clone(),
@@ -830,6 +843,7 @@ fn check_and_apply_full_approval_does_not_load_cached_block_from_db() {
let actions = check_and_apply_full_approval(
&state,
+ &Metrics(None),
Some((block_hash, block_entry)),
candidate_hash,
state.db.candidate_entries.get(&candidate_hash).unwrap().clone(),
@@ -1329,6 +1343,7 @@ fn block_not_approved_until_all_candidates_approved() {
let actions = check_and_apply_full_approval(
&state,
+ &Metrics(None),
None,
candidate_hash_2,
state.db.candidate_entries.get(&candidate_hash_2).unwrap().clone(),
@@ -1423,6 +1438,7 @@ fn candidate_approval_applied_to_all_blocks() {
let actions = check_and_apply_full_approval(
&state,
+ &Metrics(None),
None,
candidate_hash,
state.db.candidate_entries.get(&candidate_hash).unwrap().clone(),