From 41b1b3c769b4172f850fa8f3cfeee7f67db5e43a Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 30 Mar 2023 00:14:43 +0200 Subject: [PATCH] perf(net): cap backoff duration and only apply to severe backoff kinds (#2022) --- crates/net/network/src/error.rs | 9 +++++ crates/net/network/src/peers/manager.rs | 54 +++++++++++++++++++------ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/crates/net/network/src/error.rs b/crates/net/network/src/error.rs index 887dff0a49..2d4d4ad8ac 100644 --- a/crates/net/network/src/error.rs +++ b/crates/net/network/src/error.rs @@ -69,6 +69,15 @@ pub enum BackoffKind { High, } +// === impl BackoffKind === + +impl BackoffKind { + /// Returns true if the backoff is considered severe. + pub(crate) fn is_severe(&self) -> bool { + matches!(self, BackoffKind::Medium | BackoffKind::High) + } +} + impl SessionError for EthStreamError { fn merits_discovery_ban(&self) -> bool { match self { diff --git a/crates/net/network/src/peers/manager.rs b/crates/net/network/src/peers/manager.rs index c75551e081..2c7184b3f2 100644 --- a/crates/net/network/src/peers/manager.rs +++ b/crates/net/network/src/peers/manager.rs @@ -347,7 +347,7 @@ impl PeersManager { // reset the peer's state // we reset the backoff counter since we're able to establish a successful // session to that peer - entry.get_mut().backoff_counter = 0; + entry.get_mut().severe_backoff_counter = 0; entry.get_mut().state = PeerConnectionState::Idle; return } @@ -415,10 +415,12 @@ impl PeersManager { if let Some(mut peer) = self.peers.get_mut(peer_id) { let reputation_change = if let Some(kind) = err.should_backoff() { // Increment peer.backoff_counter - peer.backoff_counter += 1; + if kind.is_severe() { + peer.severe_backoff_counter += 1; + } let backoff_time = - self.backoff_durations.backoff_until(kind, peer.backoff_counter); + self.backoff_durations.backoff_until(kind, peer.severe_backoff_counter); backoff_until = Some(backoff_time); @@ -766,8 +768,8 @@ pub struct Peer { remove_after_disconnect: bool, /// The kind of peer kind: PeerKind, - /// Counts number of times the peer was backed off - backoff_counter: u32, + /// Counts number of times the peer was backed off due to a severe [BackoffKind]. + severe_backoff_counter: u32, } // === impl Peer === @@ -789,7 +791,7 @@ impl Peer { fork_id: None, remove_after_disconnect: false, kind: Default::default(), - backoff_counter: 0, + severe_backoff_counter: 0, } } @@ -1071,6 +1073,9 @@ pub struct PeerBackoffDurations { /// Intended for spammers, or bad peers in general. #[cfg_attr(feature = "serde", serde(with = "humantime_serde"))] pub high: Duration, + /// Maximum total backoff duration. + #[cfg_attr(feature = "serde", serde(with = "humantime_serde"))] + pub max: Duration, } impl PeerBackoffDurations { @@ -1083,11 +1088,14 @@ impl PeerBackoffDurations { } } - /// Returns the timestamp until which we should backoff + /// Returns the timestamp until which we should backoff. + /// + /// The Backoff duration is capped by the configured maximum backoff duration. pub fn backoff_until(&self, kind: BackoffKind, backoff_counter: u32) -> std::time::Instant { - let backoff_time = self.backoff(kind) * backoff_counter; + let backoff_time = self.backoff(kind); + let backoff_time = backoff_time + backoff_time * backoff_counter; let now = std::time::Instant::now(); - now + backoff_time + now + backoff_time.min(self.max) } } @@ -1099,6 +1107,8 @@ impl Default for PeerBackoffDurations { medium: Duration::from_secs(60 * 3), // 15min high: Duration::from_secs(60 * 15), + // 1h + max: Duration::from_secs(60 * 60), } } } @@ -1375,6 +1385,23 @@ mod test { } } + #[tokio::test] + async fn test_low_backoff() { + let peer = PeerId::random(); + let socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 1, 2)), 8008); + let config = PeersConfig::default(); + let mut peers = PeersManager::new(config); + peers.add_peer(peer, socket_addr, None); + let peer_struct = peers.peers.get_mut(&peer).unwrap(); + + let backoff_timestamp = peers + .backoff_durations + .backoff_until(BackoffKind::Low, peer_struct.severe_backoff_counter); + + let expected = std::time::Instant::now() + peers.backoff_durations.low; + assert!(backoff_timestamp <= expected); + } + #[tokio::test] async fn test_multiple_backoff_calculations() { let peer = PeerId::random(); @@ -1385,15 +1412,16 @@ mod test { let peer_struct = peers.peers.get_mut(&peer).unwrap(); // Simulate a peer that was already backed off once - peer_struct.backoff_counter = 1; + peer_struct.severe_backoff_counter = 1; let now = std::time::Instant::now(); // Simulate the increment that happens in on_connection_failure - peer_struct.backoff_counter += 1; + peer_struct.severe_backoff_counter += 1; // Get official backoff time - let backoff_time = - peers.backoff_durations.backoff_until(BackoffKind::High, peer_struct.backoff_counter); + let backoff_time = peers + .backoff_durations + .backoff_until(BackoffKind::High, peer_struct.severe_backoff_counter); // Duration of the backoff should be 2 * 15 minutes = 30 minutes let backoff_duration = std::time::Duration::new(30 * 60, 0);