From ea1a6be9baf3737350b9c895d22634ec3b5f9691 Mon Sep 17 00:00:00 2001 From: Lldenaurois Date: Sat, 20 Nov 2021 04:25:32 +0100 Subject: [PATCH] Replicate Rob's PR --- node/core/backing/src/lib.rs | 7 ++++ .../src/real/initialized.rs | 8 +++- .../src/sender/send_task.rs | 42 ++----------------- 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 81c778f85dd2..7cb37bcd399f 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -932,6 +932,13 @@ impl CandidateBackingJob { // Sanity check that candidate is from our assignment. if Some(candidate.descriptor().para_id) != self.assignment { + tracing::debug!( + target: LOG_TARGET, + our_assignment = ?self.assignment, + collation = ?candidate.descriptor().para_id, + "Subsystem asked to second for para outside of our assignment", + ); + return Ok(()) } diff --git a/node/core/dispute-coordinator/src/real/initialized.rs b/node/core/dispute-coordinator/src/real/initialized.rs index 59d9d4f87bd0..f3c99a86d071 100644 --- a/node/core/dispute-coordinator/src/real/initialized.rs +++ b/node/core/dispute-coordinator/src/real/initialized.rs @@ -649,6 +649,9 @@ impl Initialized { }, }; let candidate_receipt = votes.candidate_receipt.clone(); + let was_concluded_valid = votes.valid.len() >= supermajority_threshold; + let was_concluded_invalid = votes.invalid.len() >= supermajority_threshold; + let mut recent_disputes = overlay_db.load_recent_disputes()?.unwrap_or_default(); let controlled_indices = find_controlled_validator_indices(&self.keystore, &validators); @@ -813,10 +816,11 @@ impl Initialized { self.metrics.on_open(); } - if concluded_valid { + if !was_concluded_valid && concluded_valid { self.metrics.on_concluded_valid(); } - if concluded_invalid { + + if !was_concluded_invalid && concluded_invalid { self.metrics.on_concluded_invalid(); } diff --git a/node/network/dispute-distribution/src/sender/send_task.rs b/node/network/dispute-distribution/src/sender/send_task.rs index b75ea8d6a1c3..8a71a0cacbfe 100644 --- a/node/network/dispute-distribution/src/sender/send_task.rs +++ b/node/network/dispute-distribution/src/sender/send_task.rs @@ -58,16 +58,6 @@ pub struct SendTask { /// Whether we have any tasks failed since the last refresh. has_failed_sends: bool, - /// Total count of failed transmissions. - /// - /// Used for issuing a warning, if that number gets above a certain threshold. - failed_count: usize, - - /// Total number of initiated requests. - /// - /// Used together with `failed_count` for issuing a warning on too many failed attempts. - send_count: usize, - /// Sender to be cloned for tasks. tx: mpsc::Sender, } @@ -120,14 +110,8 @@ impl SendTask { request: DisputeRequest, metrics: &Metrics, ) -> Result { - let mut send_task = Self { - request, - deliveries: HashMap::new(), - has_failed_sends: false, - tx, - failed_count: 0, - send_count: 0, - }; + let mut send_task = + Self { request, deliveries: HashMap::new(), has_failed_sends: false, tx }; send_task.refresh_sends(ctx, runtime, active_sessions, metrics).await?; Ok(send_task) } @@ -160,7 +144,6 @@ impl SendTask { .await?; self.has_failed_sends = false; - self.send_count += new_statuses.len(); self.deliveries.extend(new_statuses.into_iter()); Ok(()) } @@ -176,7 +159,7 @@ impl SendTask { pub fn on_finished_send(&mut self, authority: &AuthorityDiscoveryId, result: TaskResult) { match result { TaskResult::Failed(err) => { - tracing::debug!( + tracing::trace!( target: LOG_TARGET, ?authority, candidate_hash = %self.request.0.candidate_receipt.hash(), @@ -184,25 +167,6 @@ impl SendTask { "Error sending dispute statements to node." ); - self.failed_count += 1; - let error_rate = (100 * self.failed_count).checked_div(self.send_count).expect( - "We cannot receive a failed request, without having sent one first. qed.", - ); - // 10% seems to be a sensible threshold to become alert - note that - // self.send_count gets increased in batches of the full validator set, so we don't - // need to account for a low send_count. - if error_rate > 10 { - tracing::warn!( - target: LOG_TARGET, - candidate_hash = %self.request.0.candidate_receipt.hash(), - last_authority = ?authority, - last_error = %err, - failed_count = ?self.failed_count, - total_attempts = ?self.send_count, - "Sending our dispute vote failed for more than 10% of total attempts!" - ); - } - self.has_failed_sends = true; // Remove state, so we know what to try again: self.deliveries.remove(authority);