Skip to content

Improve peer performance for NAT'd nodes #5345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 7, 2024
Merged

Improve peer performance for NAT'd nodes #5345

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af66843
Peer discovery for Natd peers
AgeManning Mar 4, 2024
4e3d14c
Reduce logic, discover up to max peers
AgeManning Mar 6, 2024
1f8d215
Reduce diff
AgeManning Mar 6, 2024
d5dee22
Merge latest unstable
AgeManning Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions beacon_node/lighthouse_network/src/peer_manager/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pub const MIN_OUTBOUND_ONLY_FACTOR: f32 = 0.2;
/// limit is 55, and we are at 55 peers, the following parameter provisions a few more slots of
/// dialing priority peers we need for validator duties.
pub const PRIORITY_PEER_EXCESS: f32 = 0.2;
/// The number of inbound peers that are connected that indicate this node is not behind a NAT.
pub const INBOUND_PEERS_NAT: usize = 5;
Copy link
Member

@pawanjay176 pawanjay176 Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 5? How's the node getting any inbound peers at all if it does not have a reachable address?!

Copy link
Member

@jxs jxs Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the node may still be reached by peers that are public, i.e. the node dials a public peer so the router adds that public address to it's table and therefore it may be dialed from them in the future right?


/// The main struct that handles peer's reputation and connection status.
pub struct PeerManager<TSpec: EthSpec> {
Expand Down Expand Up @@ -878,10 +880,25 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
if self.discovery_enabled {
let peer_count = self.network_globals.connected_or_dialing_peers();
let outbound_only_peer_count = self.network_globals.connected_outbound_only_peers();
let inbound_peer_count = self
.network_globals
.connected_peers()
.saturating_sub(outbound_only_peer_count);
let wanted_peers = if peer_count < self.target_peers.saturating_sub(dialing_peers) {
// We need more peers in general.
// Note: The maximum discovery query is bounded by `Discovery`.
self.target_peers.saturating_sub(dialing_peers) - peer_count

// If we are behind a NAT, we are likely to have little to no inbound peers. Having
// an excess amount of peers helps with peer management via pruning. We search for
// these extra peers in this case.
if inbound_peer_count < INBOUND_PEERS_NAT {
self.max_peers()
.saturating_sub(dialing_peers)
.saturating_sub(peer_count)
} else {
// Normal operation, search up to the target
self.target_peers.saturating_sub(dialing_peers) - peer_count
}
} else if outbound_only_peer_count < self.min_outbound_only_peers()
&& peer_count < self.max_outbound_dialing_peers()
{
Expand All @@ -894,7 +911,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {

if wanted_peers != 0 {
// We need more peers, re-queue a discovery lookup.
debug!(self.log, "Starting a new peer discovery query"; "connected" => peer_count, "target" => self.target_peers, "outbound" => outbound_only_peer_count, "wanted" => wanted_peers);
debug!(self.log, "Starting a new peer discovery query"; "connected" => peer_count, "target" => self.target_peers, "outbound" => outbound_only_peer_count, "wanted" => wanted_peers, "inbound" => inbound_peer_count);
self.events
.push(PeerManagerEvent::DiscoverPeers(wanted_peers));
}
Expand Down