From 67837c6233b7908fddff8d3f56b7f83854fa1a90 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Wed, 4 Mar 2020 20:24:17 +0100
Subject: [PATCH] Forget peerset manager nodes after 5mn (#5108)
* Forget peerstore nodes after 5mn
* Bump to one hour
* Also bump on rediscover
---
substrate/client/peerset/src/lib.rs | 27 +++++--
substrate/client/peerset/src/peersstate.rs | 84 +++++++++++++++++++---
2 files changed, 95 insertions(+), 16 deletions(-)
diff --git a/substrate/client/peerset/src/lib.rs b/substrate/client/peerset/src/lib.rs
index 53e3b35297b..bd6c19a62d1 100644
--- a/substrate/client/peerset/src/lib.rs
+++ b/substrate/client/peerset/src/lib.rs
@@ -23,7 +23,7 @@ use std::{collections::{HashSet, HashMap}, collections::VecDeque};
use futures::{prelude::*, channel::mpsc};
use log::{debug, error, trace};
use serde_json::json;
-use std::{pin::Pin, task::Context, task::Poll};
+use std::{pin::Pin, task::{Context, Poll}, time::Duration};
use wasm_timer::Instant;
pub use libp2p::PeerId;
@@ -34,6 +34,9 @@ const BANNED_THRESHOLD: i32 = 82 * (i32::min_value() / 100);
const DISCONNECT_REPUTATION_CHANGE: i32 = -256;
/// Reserved peers group ID
const RESERVED_NODES: &'static str = "reserved";
+/// Amount of time between the moment we disconnect from a node and the moment we remove it from
+/// the list.
+const FORGET_AFTER: Duration = Duration::from_secs(3600);
#[derive(Debug)]
enum Action {
@@ -310,10 +313,11 @@ impl Peerset {
/// Updates the value of `self.latest_time_update` and performs all the updates that happen
/// over time, such as reputation increases for staying connected.
fn update_time(&mut self) {
+ let now = Instant::now();
+
// We basically do `(now - self.latest_update).as_secs()`, except that by the way we do it
// we know that we're not going to miss seconds because of rounding to integers.
let secs_diff = {
- let now = Instant::now();
let elapsed_latest = self.latest_time_update - self.created;
let elapsed_now = now - self.created;
self.latest_time_update = now;
@@ -345,10 +349,16 @@ impl Peerset {
peer.set_reputation(after)
}
peersstate::Peer::NotConnected(mut peer) => {
- let before = peer.reputation();
- let after = reput_tick(before);
- trace!(target: "peerset", "Fleeting {}: {} -> {}", peer_id, before, after);
- peer.set_reputation(after)
+ if peer.reputation() == 0 &&
+ peer.last_connected_or_discovered() + FORGET_AFTER < now
+ {
+ peer.forget_peer();
+ } else {
+ let before = peer.reputation();
+ let after = reput_tick(before);
+ trace!(target: "peerset", "Fleeting {}: {} -> {}", peer_id, before, after);
+ peer.set_reputation(after)
+ }
}
peersstate::Peer::Unknown(_) => unreachable!("We iterate over known peers; qed")
};
@@ -414,7 +424,10 @@ impl Peerset {
let not_connected = match self.data.peer(&peer_id) {
// If we're already connected, don't answer, as the docs mention.
peersstate::Peer::Connected(_) => return,
- peersstate::Peer::NotConnected(entry) => entry,
+ peersstate::Peer::NotConnected(mut entry) => {
+ entry.bump_last_connected_or_discovered();
+ entry
+ },
peersstate::Peer::Unknown(entry) => entry.discover(),
};
diff --git a/substrate/client/peerset/src/peersstate.rs b/substrate/client/peerset/src/peersstate.rs
index 96a6698734b..7abf17a5f83 100644
--- a/substrate/client/peerset/src/peersstate.rs
+++ b/substrate/client/peerset/src/peersstate.rs
@@ -17,8 +17,9 @@
//! Contains the state storage behind the peerset.
use libp2p::PeerId;
+use log::{error, warn};
use std::{borrow::Cow, collections::{HashSet, HashMap}};
-use log::warn;
+use wasm_timer::Instant;
/// State storage behind the peerset.
///
@@ -69,7 +70,9 @@ struct Node {
impl Default for Node {
fn default() -> Node {
Node {
- connection_state: ConnectionState::NotConnected,
+ connection_state: ConnectionState::NotConnected {
+ last_connected: Instant::now(),
+ },
reputation: 0,
}
}
@@ -83,7 +86,11 @@ enum ConnectionState {
/// We are connected through an outgoing connection.
Out,
/// We are not connected to this node.
- NotConnected,
+ NotConnected {
+ /// When we were last connected to the node, or if we were never connected when we
+ /// discovered it.
+ last_connected: Instant,
+ },
}
impl ConnectionState {
@@ -92,7 +99,7 @@ impl ConnectionState {
match self {
ConnectionState::In => true,
ConnectionState::Out => true,
- ConnectionState::NotConnected => false,
+ ConnectionState::NotConnected { .. } => false,
}
}
}
@@ -212,11 +219,13 @@ impl PeersState {
match node.connection_state {
ConnectionState::In => self.num_in -= 1,
ConnectionState::Out => self.num_out -= 1,
- ConnectionState::NotConnected =>
+ ConnectionState::NotConnected { .. } =>
debug_assert!(false, "State inconsistency: disconnecting a disconnected node")
}
}
- node.connection_state = ConnectionState::NotConnected;
+ node.connection_state = ConnectionState::NotConnected {
+ last_connected: Instant::now(),
+ };
} else {
warn!(target: "peerset", "Attempting to disconnect unknown peer {}", peer_id);
}
@@ -292,7 +301,7 @@ impl PeersState {
match peer.connection_state {
ConnectionState::In => self.num_in += 1,
ConnectionState::Out => self.num_out += 1,
- ConnectionState::NotConnected => {},
+ ConnectionState::NotConnected { .. } => {},
}
}
}
@@ -305,7 +314,7 @@ impl PeersState {
match peer.connection_state {
ConnectionState::In => self.num_in -= 1,
ConnectionState::Out => self.num_out -= 1,
- ConnectionState::NotConnected => {},
+ ConnectionState::NotConnected { .. } => {},
}
}
}
@@ -467,6 +476,45 @@ impl<'a> NotConnectedPeer<'a> {
self.peer_id.into_owned()
}
+ /// Bumps the value that `last_connected_or_discovered` would return to now, even if we
+ /// didn't connect or disconnect.
+ pub fn bump_last_connected_or_discovered(&mut self) {
+ let state = match self.state.nodes.get_mut(&*self.peer_id) {
+ Some(s) => s,
+ None => return,
+ };
+
+ if let ConnectionState::NotConnected { last_connected } = &mut state.connection_state {
+ *last_connected = Instant::now();
+ }
+ }
+
+ /// Returns when we were last connected to this peer, or when we discovered it if we were
+ /// never connected.
+ ///
+ /// Guaranteed to be earlier than calling `Instant::now()` after the function returns.
+ pub fn last_connected_or_discovered(&self) -> Instant {
+ let state = match self.state.nodes.get(&*self.peer_id) {
+ Some(s) => s,
+ None => {
+ error!(
+ target: "peerset",
+ "State inconsistency with {}; not connected after borrow",
+ self.peer_id
+ );
+ return Instant::now();
+ }
+ };
+
+ match state.connection_state {
+ ConnectionState::NotConnected { last_connected } => last_connected,
+ _ => {
+ error!(target: "peerset", "State inconsistency with {}", self.peer_id);
+ Instant::now()
+ }
+ }
+ }
+
/// Tries to set the peer as connected as an outgoing connection.
///
/// If there are enough slots available, switches the node to "connected" and returns `Ok`. If
@@ -518,6 +566,22 @@ impl<'a> NotConnectedPeer<'a> {
pub fn add_reputation(&mut self, modifier: i32) {
self.state.add_reputation(&self.peer_id, modifier)
}
+
+ /// Un-discovers the peer. Removes it from the list.
+ pub fn forget_peer(self) -> UnknownPeer<'a> {
+ if self.state.nodes.remove(&*self.peer_id).is_none() {
+ error!(
+ target: "peerset",
+ "State inconsistency with {} when forgetting peer",
+ self.peer_id
+ );
+ }
+
+ UnknownPeer {
+ parent: self.state,
+ peer_id: self.peer_id,
+ }
+ }
}
/// A peer that we have never heard of.
@@ -533,7 +597,9 @@ impl<'a> UnknownPeer<'a> {
/// values using the `NotConnectedPeer` that this method returns.
pub fn discover(self) -> NotConnectedPeer<'a> {
self.parent.nodes.insert(self.peer_id.clone().into_owned(), Node {
- connection_state: ConnectionState::NotConnected,
+ connection_state: ConnectionState::NotConnected {
+ last_connected: Instant::now(),
+ },
reputation: 0,
});
--
GitLab