Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Pause Kademlia if too many connections #4828

Merged
merged 4 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion client/network/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Behaviour<B, S, H> {
known_addresses: Vec<(PeerId, Multiaddr)>,
enable_mdns: bool,
allow_private_ipv4: bool,
discovery_only_if_under_num: u64,
) -> Self {
Behaviour {
substrate,
Expand All @@ -73,7 +74,8 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Behaviour<B, S, H> {
local_public_key,
known_addresses,
enable_mdns,
allow_private_ipv4
allow_private_ipv4,
discovery_only_if_under_num,
).await,
events: Vec::new(),
}
Expand Down
26 changes: 21 additions & 5 deletions client/network/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ pub struct DiscoveryBehaviour<TSubstream> {
/// If false, `addresses_of_peer` won't return any private IPv4 address, except for the ones
/// stored in `user_defined`.
allow_private_ipv4: bool,
/// Number of active connections over which we interrupt the discovery process.
discovery_only_if_under_num: u64,
}

impl<TSubstream> DiscoveryBehaviour<TSubstream> {
Expand All @@ -98,6 +100,7 @@ impl<TSubstream> DiscoveryBehaviour<TSubstream> {
user_defined: Vec<(PeerId, Multiaddr)>,
enable_mdns: bool,
allow_private_ipv4: bool,
discovery_only_if_under_num: u64,
) -> Self {
if enable_mdns {
#[cfg(target_os = "unknown")]
Expand All @@ -120,6 +123,7 @@ impl<TSubstream> DiscoveryBehaviour<TSubstream> {
local_peer_id: local_public_key.into_peer_id(),
num_connections: 0,
allow_private_ipv4,
discovery_only_if_under_num,
#[cfg(not(target_os = "unknown"))]
mdns: if enable_mdns {
match Mdns::new() {
Expand Down Expand Up @@ -302,11 +306,19 @@ where

// Poll the stream that fires when we need to start a random Kademlia query.
while let Poll::Ready(_) = self.next_kad_random_query.poll_unpin(cx) {
let random_peer_id = PeerId::random();
debug!(target: "sub-libp2p", "Libp2p <= Starting random Kademlia request for \
{:?}", random_peer_id);
if self.num_connections < self.discovery_only_if_under_num {
let random_peer_id = PeerId::random();
debug!(target: "sub-libp2p", "Libp2p <= Starting random Kademlia request for \
{:?}", random_peer_id);

self.kademlia.get_closest_peers(random_peer_id);
self.kademlia.get_closest_peers(random_peer_id);
} else {
debug!(
target: "sub-libp2p",
"Kademlia paused due to high number of connections ({})",
self.num_connections
);
}

// Schedule the next random query with exponentially increasing delay,
// capped at 60 seconds.
Expand Down Expand Up @@ -406,6 +418,10 @@ where
NetworkBehaviourAction::GenerateEvent(event) => {
match event {
MdnsEvent::Discovered(list) => {
if self.num_connections >= self.discovery_only_if_under_num {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not familiar with this code so might be a stupid question. If we already discovered the node why not keep it? Kademlia and sub protocol account for the peer limits separately right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it doesn't really matter for mDNS.
I applied the change to mDNS as well because the variable name is about stopping discovery altogether, and not just Kademlia.

continue;
}

self.discoveries.extend(list.into_iter().map(|(peer_id, _)| peer_id));
if let Some(peer_id) = self.discoveries.pop_front() {
let ev = DiscoveryOut::Discovered(peer_id);
Expand Down Expand Up @@ -473,7 +489,7 @@ mod tests {
let user_defined = user_defined.clone();
let keypair_public = keypair.public();
async move {
DiscoveryBehaviour::new(keypair_public, user_defined, false, true).await
DiscoveryBehaviour::new(keypair_public, user_defined, false, true, 50).await
}
});
let mut swarm = Swarm::new(transport, behaviour, keypair.public().into_peer_id());
Expand Down
1 change: 1 addition & 0 deletions client/network/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ impl<B: BlockT + 'static, S: NetworkSpecialization<B>, H: ExHashT> NetworkWorker
TransportConfig::MemoryOnly => false,
TransportConfig::Normal { allow_private_ipv4, .. } => allow_private_ipv4,
},
u64::from(params.network_config.out_peers) + 15,
));
let (transport, bandwidth) = {
let (config_mem, config_wasm) = match params.network_config.transport {
Expand Down