Skip to content

Commit

Permalink
Consistent tracking of disconnected peers (#2650)
Browse files Browse the repository at this point in the history
## Issue Addressed

N/A

## Proposed Changes

When peers switching to a disconnecting state, decrement the disconnected peers counter. This also downgrades some crit logs to errors. 

I've also added a re-sync point when peers get unbanned the disconnected peer count will match back to the number of disconnected peers if it has gone out of sync previously.
  • Loading branch information
AgeManning committed Sep 30, 2021
1 parent db4d72c commit 29a8865
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 15 deletions.
12 changes: 6 additions & 6 deletions beacon_node/eth2_libp2p/src/peer_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use futures::Stream;
use hashset_delay::HashSetDelay;
use libp2p::core::ConnectedPoint;
use libp2p::identify::IdentifyInfo;
use slog::{crit, debug, error, warn};
use slog::{debug, error, warn};
use smallvec::SmallVec;
use std::{
pin::Pin,
Expand Down Expand Up @@ -529,7 +529,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
};
}
} else {
crit!(self.log, "Received an Identify response from an unknown peer"; "peer_id" => peer_id.to_string());
error!(self.log, "Received an Identify response from an unknown peer"; "peer_id" => peer_id.to_string());
}
}

Expand Down Expand Up @@ -672,7 +672,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
self.events.push(PeerManagerEvent::MetaData(*peer_id));
}
} else {
crit!(self.log, "Received a PING from an unknown peer";
error!(self.log, "Received a PING from an unknown peer";
"peer_id" => %peer_id);
}
}
Expand All @@ -696,7 +696,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
self.events.push(PeerManagerEvent::MetaData(*peer_id));
}
} else {
crit!(self.log, "Received a PONG from an unknown peer"; "peer_id" => %peer_id);
error!(self.log, "Received a PONG from an unknown peer"; "peer_id" => %peer_id);
}
}

Expand All @@ -720,7 +720,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
}
peer_info.meta_data = Some(meta_data);
} else {
crit!(self.log, "Received METADATA from an unknown peer";
error!(self.log, "Received METADATA from an unknown peer";
"peer_id" => %peer_id);
}
}
Expand Down Expand Up @@ -842,7 +842,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
let mut peerdb = self.network_globals.peers.write();
if !matches!(peerdb.ban_status(peer_id), BanResult::NotBanned) {
// don't connect if the peer is banned
slog::crit!(self.log, "Connection has been allowed to a banned peer"; "peer_id" => %peer_id);
error!(self.log, "Connection has been allowed to a banned peer"; "peer_id" => %peer_id);
}

match connection {
Expand Down
15 changes: 10 additions & 5 deletions beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,15 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
/// Notifies the peer manager that the peer is undergoing a normal disconnect. Optionally tag
/// the peer to be banned after the disconnect.
pub fn notify_disconnecting(&mut self, peer_id: PeerId, to_ban_afterwards: bool) {
self.peers
.entry(peer_id)
.or_default()
.disconnecting(to_ban_afterwards);
let peer_info = self.peers.entry(peer_id).or_default();

if matches!(
peer_info.connection_status(),
PeerConnectionStatus::Disconnected { .. }
) {
self.disconnected_peers = self.disconnected_peers.saturating_sub(1);
}
peer_info.disconnecting(to_ban_afterwards);
}

/// Marks a peer to be disconnected and then banned.
Expand Down Expand Up @@ -609,7 +614,7 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
info.update_state();

// This transitions a banned peer to a disconnected peer
self.disconnected_peers = self.disconnected_peers.saturating_add(1);
self.disconnected_peers = self.disconnected_peers().count().saturating_add(1);
self.shrink_to_fit();
Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions beacon_node/eth2_libp2p/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,14 @@ impl<TSpec: EthSpec> Service<TSpec> {
.with_max_established_incoming(Some(
(config.target_peers as f32
* (1.0 + PEER_EXCESS_FACTOR - MIN_OUTBOUND_ONLY_FACTOR))
as u32,
.ceil() as u32,
))
.with_max_established_outgoing(Some(
(config.target_peers as f32 * (1.0 + PEER_EXCESS_FACTOR)) as u32,
(config.target_peers as f32 * (1.0 + PEER_EXCESS_FACTOR)).ceil() as u32,
))
.with_max_established(Some(
(config.target_peers as f32 * (1.0 + PEER_EXCESS_FACTOR + PRIORITY_PEER_EXCESS))
as u32,
.ceil() as u32,
))
.with_max_established_per_peer(Some(MAX_CONNECTIONS_PER_PEER));

Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/sync/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
}
peer_info.is_connected()
} else {
crit!(self.log, "Status'd peer is unknown"; "peer_id" => %peer_id);
error!(self.log, "Status'd peer is unknown"; "peer_id" => %peer_id);
false
}
}
Expand Down

0 comments on commit 29a8865

Please sign in to comment.