Skip to content

Commit

Permalink
feat(kad): improve automatic bootstrap
Browse files Browse the repository at this point in the history
As discussed in the last maintainer call, some improvements are probably necessary for the automatic bootstrap feature (introduced by #4838). Indeed, like @drHuangMHT mentioned in #5341 and like @guillaumemichel has agreed, triggering a bootstrap every time an update happens inside the routing table consumes a lot more resources.

The idea behind the automatic bootstrap feature it that, when a peer is starting, if a routing table update happens we probably don't want to wait for the periodic bootstrap to trigger and we want to trigger it right now. However, like @guillaumemichel said, this is something we want to do at startup or when a network connectivity problem happens, we don't want to do that all the time.

This PR is a proposal to trigger automatically a bootstrap on routing table update but only when we have less that `K_VALUE` peers in it (meaning that we are starting up or something went wrong and the fact that a new peer is inserted is probably a sign that the network connectivity issue is resolved).

I have also added a new triggering condition like mentioned in the maintainer call. When discovering a new listen address and if we have no connected peers, we trigger a bootstrap. This condition is based on our own experience at Stormshield : some peers were starting before the network interfaces were up, doing so, the automatic and periodic bootstrap failed, but when the network interfaces were finally up, we were waiting X minutes for the periodic bootstrap to actually trigger a bootstrap and join the p2p network.

Pull-Request: #5474.
  • Loading branch information
stormshield-frb authored Jul 4, 2024
1 parent 23eb4f8 commit 36c2f94
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 11 deletions.
4 changes: 4 additions & 0 deletions protocols/kad/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
See [PR 5347](https://github.com/libp2p/rust-libp2p/pull/5347).
- Correctly handle the `NoKnownPeers` error on automatic bootstrap.
See [PR 5349](https://github.com/libp2p/rust-libp2p/pull/5349).
- Improve automatic bootstrap triggering conditions:
trigger when the routing table is updated and we have less that `K_VALUE` peers in it,
trigger when a new listen address is discovered and we have no connected peers.
See [PR 5474](https://github.com/libp2p/rust-libp2p/pull/5474).

## 0.45.3

Expand Down
28 changes: 25 additions & 3 deletions protocols/kad/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
mod test;

use crate::addresses::Addresses;
use crate::bootstrap;
use crate::handler::{Handler, HandlerEvent, HandlerIn, RequestId};
use crate::kbucket::{self, Distance, KBucketsTable, NodeStatus};
use crate::protocol::{ConnectionType, KadPeer, ProtocolConfig};
Expand All @@ -33,6 +32,7 @@ use crate::record::{
store::{self, RecordStore},
ProviderRecord, Record,
};
use crate::{bootstrap, K_VALUE};
use crate::{jobs::*, protocol};
use fnv::FnvHashSet;
use libp2p_core::{ConnectedPoint, Endpoint, Multiaddr};
Expand Down Expand Up @@ -604,7 +604,8 @@ where
};
match entry.insert(addresses.clone(), status) {
kbucket::InsertResult::Inserted => {
self.bootstrap_status.on_new_peer_in_routing_table();
self.bootstrap_on_low_peers();

self.queued_events.push_back(ToSwarm::GenerateEvent(
Event::RoutingUpdated {
peer: *peer,
Expand Down Expand Up @@ -1324,7 +1325,8 @@ where
let addresses = Addresses::new(a);
match entry.insert(addresses.clone(), new_status) {
kbucket::InsertResult::Inserted => {
self.bootstrap_status.on_new_peer_in_routing_table();
self.bootstrap_on_low_peers();

let event = Event::RoutingUpdated {
peer,
is_new_peer: true,
Expand Down Expand Up @@ -1375,6 +1377,20 @@ where
}
}

/// A new peer has been inserted in the routing table but we check if the routing
/// table is currently small (less that `K_VALUE` peers are present) and only
/// trigger a bootstrap in that case
fn bootstrap_on_low_peers(&mut self) {
if self
.kbuckets()
.map(|kbucket| kbucket.num_entries())
.sum::<usize>()
< K_VALUE.get()
{
self.bootstrap_status.trigger();
}
}

/// Handles a finished (i.e. successful) query.
fn query_finished(&mut self, q: Query) -> Option<Event> {
let query_id = q.id();
Expand Down Expand Up @@ -2613,6 +2629,12 @@ where
}
FromSwarm::DialFailure(dial_failure) => self.on_dial_failure(dial_failure),
FromSwarm::AddressChange(address_change) => self.on_address_change(address_change),
FromSwarm::NewListenAddr(_) if self.connected_peers.is_empty() => {
// A new listen addr was just discovered and we have no connected peers,
// it can mean that our network interfaces were not up but they are now
// so it might be a good idea to trigger a bootstrap.
self.bootstrap_status.trigger();
}
_ => {}
}
}
Expand Down
17 changes: 9 additions & 8 deletions protocols/kad/src/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ impl Status {
}
}

pub(crate) fn on_new_peer_in_routing_table(&mut self) {
/// Trigger a bootstrap now or after the configured `automatic_throttle` if configured.
pub(crate) fn trigger(&mut self) {
// Registering `self.throttle_timer` means scheduling a bootstrap.
// A bootstrap will be triggered when `self.throttle_timer` finishes.
// A `throttle_timer` is useful to not trigger a batch of bootstraps when a
Expand Down Expand Up @@ -201,7 +202,7 @@ mod tests {
"bootstrap to not be triggered immediately because periodic bootstrap is in ~1s"
);

status.on_new_peer_in_routing_table(); // Connected to a new peer though!
status.trigger(); // Connected to a new peer though!
assert!(
status.next().now_or_never().is_some(),
"bootstrap to be triggered immediately because we connected to a new peer"
Expand All @@ -226,7 +227,7 @@ mod tests {
"bootstrap to not be triggered immediately because periodic bootstrap is in ~1s"
);

status.on_new_peer_in_routing_table(); // Connected to a new peer though!
status.trigger(); // Connected to a new peer though!
assert!(
status.next().now_or_never().is_none(),
"bootstrap to not be triggered immediately because throttle is 5ms"
Expand All @@ -247,7 +248,7 @@ mod tests {
// User manually triggered a bootstrap
do_bootstrap(&mut status);

status.on_new_peer_in_routing_table(); // Connected to a new peer though!
status.trigger(); // Connected to a new peer though!

assert!(
status.next().now_or_never().is_some(),
Expand All @@ -260,7 +261,7 @@ mod tests {
) {
let mut status = Status::new(Some(MS_100), Some(MS_5));

status.on_new_peer_in_routing_table();
status.trigger();

let start = Instant::now();
await_and_do_bootstrap(&mut status).await;
Expand All @@ -280,7 +281,7 @@ mod tests {
) {
let mut status = Status::new(None, Some(Duration::ZERO));

status.on_new_peer_in_routing_table();
status.trigger();

status.next().await;
}
Expand All @@ -304,10 +305,10 @@ mod tests {
) {
let mut status = Status::new(None, Some(MS_100));

status.on_new_peer_in_routing_table();
status.trigger();
for _ in 0..10 {
Delay::new(MS_100 / 2).await;
status.on_new_peer_in_routing_table(); // should reset throttle_timer
status.trigger(); // should reset throttle_timer
}
assert!(
status.next().now_or_never().is_none(),
Expand Down

0 comments on commit 36c2f94

Please sign in to comment.