From 57a99c960fd50e3a65c2192403c2b724cac03830 Mon Sep 17 00:00:00 2001 From: Robert Klotzner <eskimor@users.noreply.github.com> Date: Wed, 6 Oct 2021 19:00:51 +0200 Subject: [PATCH] Silence some alerts due to overly verbose warnings. (#3946) * statement-distribution: Only warn on relevant stuff. * Silence warnings in availability-distribution. * Demote more warnings. * More consistency. * info -> debug --- .../availability-distribution/src/error.rs | 13 +++++++- .../src/requester/fetch_task/mod.rs | 4 +-- .../src/validator_side/mod.rs | 4 +-- .../statement-distribution/src/error.rs | 12 +++++-- .../network/statement-distribution/src/lib.rs | 32 ++++++------------- 5 files changed, 36 insertions(+), 29 deletions(-) diff --git a/polkadot/node/network/availability-distribution/src/error.rs b/polkadot/node/network/availability-distribution/src/error.rs index 881e00ac28a..d9db0ec42fa 100644 --- a/polkadot/node/network/availability-distribution/src/error.rs +++ b/polkadot/node/network/availability-distribution/src/error.rs @@ -117,7 +117,18 @@ pub fn log_error(result: Result<()>, ctx: &'static str) -> std::result::Result<( match result { Err(Error::Fatal(f)) => Err(f), Err(Error::NonFatal(error)) => { - tracing::warn!(target: LOG_TARGET, error = ?error, ctx); + match error { + NonFatal::UnexpectedPoV | + NonFatal::InvalidValidatorIndex | + NonFatal::NoSuchCachedSession | + NonFatal::QueryAvailableDataResponseChannel(_) | + NonFatal::QueryChunkResponseChannel(_) => + tracing::warn!(target: LOG_TARGET, error = %error, ctx), + NonFatal::FetchPoV(_) | + NonFatal::SendResponse | + NonFatal::NoSuchPoV | + NonFatal::Runtime(_) => tracing::debug!(target: LOG_TARGET, error = ?error, ctx), + } Ok(()) }, Ok(()) => Ok(()), diff --git a/polkadot/node/network/availability-distribution/src/requester/fetch_task/mod.rs b/polkadot/node/network/availability-distribution/src/requester/fetch_task/mod.rs index f1615d1f33a..0c5dd6e684c 100644 --- a/polkadot/node/network/availability-distribution/src/requester/fetch_task/mod.rs +++ b/polkadot/node/network/availability-distribution/src/requester/fetch_task/mod.rs @@ -344,7 +344,7 @@ impl RunningTask { Err(TaskError::PeerError) }, Err(RequestError::NetworkError(err)) => { - tracing::warn!( + tracing::debug!( target: LOG_TARGET, origin= ?validator, err= ?err, @@ -353,7 +353,7 @@ impl RunningTask { Err(TaskError::PeerError) }, Err(RequestError::Canceled(oneshot::Canceled)) => { - tracing::warn!(target: LOG_TARGET, + tracing::debug!(target: LOG_TARGET, origin= ?validator, "Erasure chunk request got canceled"); Err(TaskError::PeerError) diff --git a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs index 3f77e499de0..d9e1de06255 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -1362,7 +1362,7 @@ where .await; }, Err(RequestError::NetworkError(err)) => { - tracing::warn!( + tracing::debug!( target: LOG_TARGET, hash = ?pending_collation.relay_parent, para_id = ?pending_collation.para_id, @@ -1377,7 +1377,7 @@ where modify_reputation(ctx, pending_collation.peer_id.clone(), COST_NETWORK_ERROR).await; }, Err(RequestError::Canceled(_)) => { - tracing::warn!( + tracing::debug!( target: LOG_TARGET, hash = ?pending_collation.relay_parent, para_id = ?pending_collation.para_id, diff --git a/polkadot/node/network/statement-distribution/src/error.rs b/polkadot/node/network/statement-distribution/src/error.rs index 819440e6f29..ac997544b5c 100644 --- a/polkadot/node/network/statement-distribution/src/error.rs +++ b/polkadot/node/network/statement-distribution/src/error.rs @@ -87,8 +87,12 @@ pub enum NonFatal { #[error("Relay parent could not be found in active heads")] NoSuchHead(Hash), + /// Received message from actually disconnected peer. + #[error("Message from not connected peer")] + NoSuchPeer(PeerId), + /// Peer requested statement data for candidate that was never announced to it. - #[error("Peer requested data for candidate it never received a notification for")] + #[error("Peer requested data for candidate it never received a notification for (malicious?)")] RequestedUnannouncedCandidate(PeerId, CandidateHash), /// A large statement status was requested, which could not be found. @@ -112,7 +116,11 @@ pub fn log_error(result: Result<()>, ctx: &'static str) -> std::result::Result<( match result { Err(Error::Fatal(f)) => Err(f), Err(Error::NonFatal(error)) => { - tracing::warn!(target: LOG_TARGET, error = ?error, ctx); + match error { + NonFatal::RequestedUnannouncedCandidate(_, _) => + tracing::warn!(target: LOG_TARGET, error = %error, ctx), + _ => tracing::debug!(target: LOG_TARGET, error = %error, ctx), + } Ok(()) }, Ok(()) => Ok(()), diff --git a/polkadot/node/network/statement-distribution/src/lib.rs b/polkadot/node/network/statement-distribution/src/lib.rs index e2c08c8216b..a810a663a87 100644 --- a/polkadot/node/network/statement-distribution/src/lib.rs +++ b/polkadot/node/network/statement-distribution/src/lib.rs @@ -1625,7 +1625,7 @@ impl StatementDistribution { &requesting_peer, &relay_parent, &candidate_hash, - ) { + )? { return Err(NonFatal::RequestedUnannouncedCandidate( requesting_peer, candidate_hash, @@ -1896,27 +1896,15 @@ fn requesting_peer_knows_about_candidate( requesting_peer: &PeerId, relay_parent: &Hash, candidate_hash: &CandidateHash, -) -> bool { - requesting_peer_knows_about_candidate_inner( - peers, - requesting_peer, - relay_parent, - candidate_hash, - ) - .is_some() -} - -/// Helper function for `requesting_peer_knows_about_statement`. -fn requesting_peer_knows_about_candidate_inner( - peers: &HashMap<PeerId, PeerData>, - requesting_peer: &PeerId, - relay_parent: &Hash, - candidate_hash: &CandidateHash, -) -> Option<()> { - let peer_data = peers.get(requesting_peer)?; - let knowledge = peer_data.view_knowledge.get(relay_parent)?; - knowledge.sent_candidates.get(&candidate_hash)?; - Some(()) +) -> NonFatalResult<bool> { + let peer_data = peers + .get(requesting_peer) + .ok_or_else(|| NonFatal::NoSuchPeer(*requesting_peer))?; + let knowledge = peer_data + .view_knowledge + .get(relay_parent) + .ok_or_else(|| NonFatal::NoSuchHead(*relay_parent))?; + Ok(knowledge.sent_candidates.get(&candidate_hash).is_some()) } #[derive(Clone)] -- GitLab