From 7adc1da789f2738f9b3d264209c5a38a933cfed4 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 11 Aug 2023 17:05:54 +0700 Subject: [PATCH 1/3] networking: Reduce loggin noise for connected-peers protocol. --- .../src/connected_peers.rs | 73 +++++++++++++++++-- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/crates/subspace-networking/src/connected_peers.rs b/crates/subspace-networking/src/connected_peers.rs index 8f98c8e8ed..f4b2f9b2bd 100644 --- a/crates/subspace-networking/src/connected_peers.rs +++ b/crates/subspace-networking/src/connected_peers.rs @@ -79,6 +79,17 @@ enum ConnectionState { NotInterested, } +impl ToString for ConnectionState { + fn to_string(&self) -> String { + match self { + ConnectionState::Connecting { .. } => "Connecting".to_string(), + ConnectionState::Deciding => "Deciding".to_string(), + ConnectionState::Permanent => "Permanent".to_string(), + ConnectionState::NotInterested => "NotInterested".to_string(), + } + } +} + /// Connected peers protocol configuration. #[derive(Debug, Clone)] pub struct Config { @@ -86,6 +97,8 @@ pub struct Config { pub log_target: &'static str, /// Interval between new dialing attempts. pub dialing_interval: Duration, + /// Interval between logging of the internal state. + pub logging_interval: Duration, /// Number of connected peers that protocol will maintain. pub target_connected_peers: u32, /// We dial peers using this batch size. @@ -101,6 +114,7 @@ impl Default for Config { Self { log_target: DEFAULT_CONNECTED_PEERS_LOG_TARGET, dialing_interval: Duration::from_secs(3), + logging_interval: Duration::from_secs(5), target_connected_peers: 30, dialing_peer_batch_size: 5, decision_timeout: Duration::from_secs(10), @@ -122,6 +136,13 @@ struct PeerConnectionDecisionUpdate { keep_alive: KeepAlive, } +#[derive(Debug, Default)] +#[allow(dead_code)] // actually, we use the fields implicitly using the `Debug` on logging. +struct ConnectedPeerStats { + status_count: HashMap, + peer_status: HashMap>, +} + /// `Behaviour` for `connected peers` protocol. #[derive(Debug)] pub struct Behaviour { @@ -145,12 +166,16 @@ pub struct Behaviour { /// Instance type marker. phantom_data: PhantomData, + + /// Delay between logging of the internal state. + logging_delay: Delay, } impl Behaviour { /// Creates a new `Behaviour`. pub fn new(config: Config) -> Self { let dialing_delay = Delay::new(config.dialing_interval); + let logging_delay = Delay::new(config.logging_interval); Self { config, known_peers: HashMap::new(), @@ -159,6 +184,7 @@ impl Behaviour { peer_cache: Vec::new(), waker: None, phantom_data: PhantomData, + logging_delay, } } @@ -251,6 +277,35 @@ impl Behaviour { waker.wake_by_ref() } } + + fn gather_stats(&self) -> ConnectedPeerStats { + let status_count = + self.known_peers + .iter() + .fold(HashMap::new(), |mut result, (_, state)| { + result + .entry(state.connection_state.to_string()) + .and_modify(|count| *count += 1) + .or_insert(1); + result + }); + + let peer_status = self.known_peers.iter().fold( + HashMap::>::new(), + |mut result, (peer_id, state)| { + result + .entry(state.connection_state.to_string()) + .and_modify(|peers| peers.push(*peer_id)) + .or_insert(vec![*peer_id]); + result + }, + ); + + ConnectedPeerStats { + status_count, + peer_status, + } + } } impl NetworkBehaviour for Behaviour { @@ -372,12 +427,6 @@ impl NetworkBehaviour for Behaviour { // Check decision statuses. for (peer_id, state) in self.known_peers.iter_mut() { - trace!( - %peer_id, - ?state, - target=%self.config.log_target, - "Peer decisions for connected peers protocol." - ); match state.connection_state.clone() { ConnectionState::Connecting { peer_address: address, @@ -435,6 +484,18 @@ impl NetworkBehaviour for Behaviour { } } + // Log the internal state. + match self.logging_delay.poll_unpin(cx) { + Poll::Pending => {} + Poll::Ready(()) => { + self.logging_delay.reset(self.config.logging_interval); + + let stats = self.gather_stats(); + + debug!(instance=%self.config.log_target, ?stats, "Connected peers protocol statistics."); + } + } + self.waker.replace(cx.waker().clone()); Poll::Pending } From 0bfc35e04e824e0db729b3a95f9775b67738c682 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 11 Aug 2023 18:37:05 +0700 Subject: [PATCH 2/3] networking: Add logging on temporary banned peers. --- crates/subspace-networking/src/node_runner.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/subspace-networking/src/node_runner.rs b/crates/subspace-networking/src/node_runner.rs index 437aa04eaa..1aed47b13c 100644 --- a/crates/subspace-networking/src/node_runner.rs +++ b/crates/subspace-networking/src/node_runner.rs @@ -591,8 +591,10 @@ where } _ => true, }; + if should_temporary_ban { self.temporary_bans.lock().create_or_extend(peer_id); + debug!(%peer_id, ?error, "Peer was temporarily banned."); } } }; From ded9d07b1994a07cdf29e20b288b2dd208299252 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 11 Aug 2023 20:38:09 +0700 Subject: [PATCH 3/3] networking: Refactor connected-peers protocol. --- crates/subspace-networking/src/connected_peers.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/subspace-networking/src/connected_peers.rs b/crates/subspace-networking/src/connected_peers.rs index f4b2f9b2bd..c0a3be85b5 100644 --- a/crates/subspace-networking/src/connected_peers.rs +++ b/crates/subspace-networking/src/connected_peers.rs @@ -79,8 +79,9 @@ enum ConnectionState { NotInterested, } -impl ToString for ConnectionState { - fn to_string(&self) -> String { +impl ConnectionState { + /// Converts [`ConnectionState`] to a string with information loss. + fn stringify(&self) -> String { match self { ConnectionState::Connecting { .. } => "Connecting".to_string(), ConnectionState::Deciding => "Deciding".to_string(), @@ -284,7 +285,7 @@ impl Behaviour { .iter() .fold(HashMap::new(), |mut result, (_, state)| { result - .entry(state.connection_state.to_string()) + .entry(state.connection_state.stringify()) .and_modify(|count| *count += 1) .or_insert(1); result @@ -294,7 +295,7 @@ impl Behaviour { HashMap::>::new(), |mut result, (peer_id, state)| { result - .entry(state.connection_state.to_string()) + .entry(state.connection_state.stringify()) .and_modify(|peers| peers.push(*peer_id)) .or_insert(vec![*peer_id]); result