feat(kad): improve automatic bootstrap#5474
Merged
mergify[bot] merged 4 commits intolibp2p:masterfrom Jul 4, 2024
Merged
Conversation
stormshield-frb
changed the title
stormshield-frb
changed the title
stormshield-frb
force-pushed
the
feat/improve-automatic-bootstrap
branch
from
3229558 to
ee83002
Compare
stormshield-frb
changed the title
Contributor
|
This pull request has merge conflicts. Could you please resolve them @stormshield-frb? 🙏 |
dariusc93
reviewed
Jun 19, 2024
stormshield-frb
force-pushed
the
feat/improve-automatic-bootstrap
branch
from
ee83002 to
7268d7e
Compare
guillaumemichel
approved these changes
Jun 26, 2024
jxs
approved these changes
Jul 3, 2024
Member
jxs
left a comment
jxs
left a comment
There was a problem hiding this comment.
LGTM!
Thanks François, and Darius and Gui for the review
Member
|
@stormshield-frb can you check what's happening with https://github.com/libp2p/rust-libp2p/actions/runs/9777683040/job/26994305156?pr=5474#step:6:46 ? |
stormshield-frb
force-pushed
the
feat/improve-automatic-bootstrap
branch
from
1982c05 to
0ae16d1
Compare
Contributor
Author
Should be corrected @jxs. The |
Contributor
Author
|
It seems there is a problem with the runner no ? Almost every job fails, complaining about a unknown command |
Member
yeah submitted #5481 to address it, thanks François |
TimTinkers
pushed a commit
to unattended-backpack/rust-libp2p
that referenced
this pull request
Sep 14, 2024
As discussed in the last maintainer call, some improvements are probably necessary for the automatic bootstrap feature (introduced by libp2p#4838). Indeed, like @drHuangMHT mentioned in libp2p#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: libp2p#5474.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
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_VALUEpeers 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.
Notes & open questions
Change checklist