-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Make z2 work again - and incidentally, make networks more robust to stalls. #1947
base: main
Are you sure you want to change the base?
Conversation
dc85271
to
95c2b72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking ages to get to this :(
self.swarm | ||
.behaviour_mut() | ||
.kademlia | ||
.add_address(peer, address.clone()); | ||
self.swarm.behaviour_mut().kademlia.bootstrap()?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kademlia already does this periodically by itself:
https://github.com/libp2p/rust-libp2p/blob/master/protocols/kad/src/behaviour.rs#L990-L996
It also has logic to do this more sanely by triggering a bootstrap when new peers are added and avoid making more than one bootstrap at once.
Does z2
work without this change? Can you see if the automatic bootstrap is working? We can also adjust the automatic_bootstrap_throttle
configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
z2 doesn't work without this change, sadly...
The logic here is that, at least at startup, a node starts before the bootstrap; it fails to add the peer (because it's not up yet), and drops the bootstrap. Now it won't add the peer again, even if it comes up, and z2 then deadlocks essentially forever.
I'll see if automatic_bootstrap_throttle
will work, but if it's currently set to less than a few hours, I suspect it is already triggering. We could perhaps omit the .kademlia_bootstrap()
, but the .add_address()
definitely isn't optional.
This also relates to one of the crashing bugs which I have utterly failed (sorry!) to tag as such and now can't find, where all peers get dropped and the network then dissociates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JamesHinshelwood - OK. I've checked. You can't get away without re-adding the address, but (as expected) kademlia will eventually bootstrap itself automatically.
…ons, unles you are the bootstrap, in which case wait for someone to contact you.
95c2b72
to
63b3e71
Compare
Bencher Report
Click to view all benchmark results
|
No description provided.