From 3edfdead0fb8388256d6620cbde41bfc27110e42 Mon Sep 17 00:00:00 2001
From: Pierre Krieger <pierre.krieger1708@gmail.com>
Date: Mon, 22 Mar 2021 17:51:57 +0100
Subject: [PATCH] Optimize the peerset a bit (#8416)
* Only allocate slots for the relevant peer set
* Do a pre-check before calling has_free_outgoing_slot
* Oops, fix infinite loop
---
substrate/client/peerset/src/lib.rs | 104 ++++++++++++---------
substrate/client/peerset/src/peersstate.rs | 9 +-
2 files changed, 64 insertions(+), 49 deletions(-)
diff --git a/substrate/client/peerset/src/lib.rs b/substrate/client/peerset/src/lib.rs
index 31162930efc..153e097dc8b 100644
--- a/substrate/client/peerset/src/lib.rs
+++ b/substrate/client/peerset/src/lib.rs
@@ -296,7 +296,10 @@ impl Peerset {
}
}
- peerset.alloc_slots();
+ for set_index in 0..peerset.data.num_sets() {
+ peerset.alloc_slots(SetId(set_index));
+ }
+
(peerset, handle)
}
@@ -307,7 +310,7 @@ impl Peerset {
}
self.data.add_no_slot_node(set_id.0, peer_id);
- self.alloc_slots();
+ self.alloc_slots(set_id);
}
fn on_remove_reserved_peer(&mut self, set_id: SetId, peer_id: PeerId) {
@@ -372,7 +375,7 @@ impl Peerset {
}
} else {
- self.alloc_slots();
+ self.alloc_slots(set_id);
}
}
@@ -383,7 +386,7 @@ impl Peerset {
pub fn add_to_peers_set(&mut self, set_id: SetId, peer_id: PeerId) {
if let peersstate::Peer::Unknown(entry) = self.data.peer(set_id.0, &peer_id) {
entry.discover();
- self.alloc_slots();
+ self.alloc_slots(set_id);
}
}
@@ -500,59 +503,68 @@ impl Peerset {
}
}
- /// Try to fill available out slots with nodes.
- fn alloc_slots(&mut self) {
+ /// Try to fill available out slots with nodes for the given set.
+ fn alloc_slots(&mut self, set_id: SetId) {
self.update_time();
// Try to connect to all the reserved nodes that we are not connected to.
- for set_index in 0..self.data.num_sets() {
- for reserved_node in &self.reserved_nodes[set_index].0 {
- let entry = match self.data.peer(set_index, reserved_node) {
- peersstate::Peer::Unknown(n) => n.discover(),
- peersstate::Peer::NotConnected(n) => n,
- peersstate::Peer::Connected(_) => continue,
- };
-
- match entry.try_outgoing() {
- Ok(conn) => self.message_queue.push_back(Message::Connect {
- set_id: SetId(set_index),
- peer_id: conn.into_peer_id()
- }),
- Err(_) => {
- // An error is returned only if no slot is available. Reserved nodes are
- // marked in the state machine with a flag saying "doesn't occupy a slot",
- // and as such this should never happen.
- debug_assert!(false);
- log::error!(
- target: "peerset",
- "Not enough slots to connect to reserved node"
- );
- }
+ for reserved_node in &self.reserved_nodes[set_id.0].0 {
+ let entry = match self.data.peer(set_id.0, reserved_node) {
+ peersstate::Peer::Unknown(n) => n.discover(),
+ peersstate::Peer::NotConnected(n) => n,
+ peersstate::Peer::Connected(_) => continue,
+ };
+
+ match entry.try_outgoing() {
+ Ok(conn) => self.message_queue.push_back(Message::Connect {
+ set_id,
+ peer_id: conn.into_peer_id()
+ }),
+ Err(_) => {
+ // An error is returned only if no slot is available. Reserved nodes are
+ // marked in the state machine with a flag saying "doesn't occupy a slot",
+ // and as such this should never happen.
+ debug_assert!(false);
+ log::error!(
+ target: "peerset",
+ "Not enough slots to connect to reserved node"
+ );
}
}
}
// Now, we try to connect to other nodes.
- for set_index in 0..self.data.num_sets() {
- // Nothing more to do if we're in reserved mode.
- if self.reserved_nodes[set_index].1 {
- continue;
+
+ // Nothing more to do if we're in reserved mode.
+ if self.reserved_nodes[set_id.0].1 {
+ return;
+ }
+
+ // Try to grab the next node to attempt to connect to.
+ // Since `highest_not_connected_peer` is rather expensive to call, check beforehand
+ // whether we have an available slot.
+ while self.data.has_free_outgoing_slot(set_id.0) {
+ let next = match self.data.highest_not_connected_peer(set_id.0) {
+ Some(n) => n,
+ None => break
+ };
+
+ // Don't connect to nodes with an abysmal reputation.
+ if next.reputation() < BANNED_THRESHOLD {
+ break;
}
- // Try to grab the next node to attempt to connect to.
- while let Some(next) = self.data.highest_not_connected_peer(set_index) {
- // Don't connect to nodes with an abysmal reputation.
- if next.reputation() < BANNED_THRESHOLD {
+ match next.try_outgoing() {
+ Ok(conn) => self.message_queue.push_back(Message::Connect {
+ set_id,
+ peer_id: conn.into_peer_id()
+ }),
+ Err(_) => {
+ // This branch can only be entered if there is no free slot, which is
+ // checked above.
+ debug_assert!(false);
break;
}
-
- match next.try_outgoing() {
- Ok(conn) => self.message_queue.push_back(Message::Connect {
- set_id: SetId(set_index),
- peer_id: conn.into_peer_id()
- }),
- Err(_) => break, // No more slots available.
- }
}
}
}
@@ -624,7 +636,7 @@ impl Peerset {
self.on_remove_from_peers_set(set_id, peer_id);
}
- self.alloc_slots();
+ self.alloc_slots(set_id);
}
/// Reports an adjustment to the reputation of the given peer.
diff --git a/substrate/client/peerset/src/peersstate.rs b/substrate/client/peerset/src/peersstate.rs
index c79dac5e10a..c200d2729e1 100644
--- a/substrate/client/peerset/src/peersstate.rs
+++ b/substrate/client/peerset/src/peersstate.rs
@@ -283,6 +283,11 @@ impl PeersState {
}
}
+ /// Returns `true` if there is a free outgoing slot available related to this set.
+ pub fn has_free_outgoing_slot(&self, set: usize) -> bool {
+ self.sets[set].num_out < self.sets[set].max_out
+ }
+
/// Add a node to the list of nodes that don't occupy slots.
///
/// Has no effect if the node was already in the group.
@@ -506,9 +511,7 @@ impl<'a> NotConnectedPeer<'a> {
// Note that it is possible for num_out to be strictly superior to the max, in case we were
// connected to reserved node then marked them as not reserved.
- if self.state.sets[self.set].num_out >= self.state.sets[self.set].max_out
- && !is_no_slot_occupy
- {
+ if !self.state.has_free_outgoing_slot(self.set) && !is_no_slot_occupy {
return Err(self);
}
--
GitLab