Skip to content
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

Reconsider Bootstrap interval #807

Closed
a1300 opened this issue Nov 24, 2020 · 7 comments
Closed

Reconsider Bootstrap interval #807

a1300 opened this issue Nov 24, 2020 · 7 comments
Labels
status/ready Ready to be worked

Comments

@a1300
Copy link
Contributor

a1300 commented Nov 24, 2020

  • Version: 0.29.3, even further on commit d0a9fad on the master branch
  • Platform: Linux
  • Subsystem:

Type:

Severity: Low

Description: The Bootstrap discovery mechanism only emits the event peer:discovery only once even when the interval property is specified

I would expect from the Bootstrap discovery mechanism to fire the peer:discovery event every x milliseconds

Steps to reproduce the error:

git clone https://github.com/libp2p/js-libp2p
cd js-libp2p
npm install
cd examples/discovery-mechanisms/
node 1.js
@vasco-santos
Copy link
Member

Hey @a1300
This is not an issue, but I understand the doubt here. In the discovery module level, libp2p-bootstrap, libp2p-mdns, ... the peer:discovery event is always triggered, like your expectation.
However, the same does not happen from the libp2p standpoint. What happens behind the scenes is that libp2p listens on peer:discovery events from the discovery modules and adds that information to the PeerStore. If it is a new entry in the PeerStore, it will emit the event from libp2p.
We should probably make it more clear. Is there any place you would see this information?

@a1300
Copy link
Contributor Author

a1300 commented Nov 24, 2020

Hi @vasco-santos
thanks for you fast response.
Does the libp2p-bootstrap still have its raison d'être? It only adds all Peers in the Bootstrap list to the PeerStore on startup and fires an event. Or maybe libp2p shouldn't allow the interval Bootstrap option. Otherwise people think that it will "discover" peers in a regular interval. The 1st example in example/discovery-mechanism also uses the interval property.

My problem is, that I have nodes that are not 100% of the time online. So in previous libp2p versions I dialed the bootstrap nodes in the peer.discovery event handler only if I am not connected. I will now use the following workaround. Is there a more elegant solution?

const myBootstrap = new Bootstrap({
  list: bootstrap,
  interval: 3000
})
myBootstrap.on('peer', (peer) => {
  console.log('this fires every x seconds')
})
options.modules.peerDiscovery = [myBootstrap];

How does libp2p-bootstrap work together with the autodial option? What happens if the target node is currently down, and starts in 5 minutes? Will there be a re-dial (even without a second peer:discovery event) ?

@vasco-santos
Copy link
Member

Does the libp2p-bootstrap still have its raison d'être?

Yes it has. It is basically a configurable way of attempting a dial to several peers on startup

Or maybe libp2p shouldn't allow the interval Bootstrap option.

That is probably true! I will need to think better about this for #744
So, for libp2p@0.30 we will be working on this connection manager overhaul. Some important things that should happen regarding this issue are:

  • we will probably remove the autoDial option
  • libp2p will become more auto sufficient by enabling users and libp2p subsystems to configure how many connections they will want and how their lifecycle should be
    • for example in bootstrap, the default behaviour would be connect to the provided peers and add a decaying tag to them. This way, once we leverage the bootstrap nodes to build our topology of important peers for the node, the bootstrap nodes should be disconnected as they typically have a large load of requests and their single purpose is bootstrapping a peer network
    • this above might not be the case for every single scenario. So, we will need to add features like protected peers (and other important tags), that make sure that connections are kept for them.

I will now use the following workaround. Is there a more elegant solution?

Not great, but a bit better would be to configure bootstrap as in the examples and then:

const Bootstrap = require('libp2p-bootstrap')

// ...
await libp2p.start()
// Listen on bootstrap events
libp2p._discovery.get(Bootstrap.tag).on('peer', (peer) => {})

How does libp2p-bootstrap work together with the autodial option? What happens if the target node is currently down, and starts in 5 minutes? Will there be a re-dial (even without a second peer:discovery event) ?

Per this it will not try to re dial the peer unfortunately. You can still manually try to dial it through your event handler. It will probably be the best option at the moment

@a1300
Copy link
Contributor Author

a1300 commented Nov 24, 2020

@vasco-santos thank you very much
Should this ticket be closed? Or should the documentation first be changed/enhanced?

@vasco-santos vasco-santos changed the title Bootstrap fires peer:discovery event only once Reconsider Bootstrap interval Nov 24, 2020
@vasco-santos
Copy link
Member

I renamed the issue to an action point on what we discussed with the bootstrap interval and I will keep it open for visibility while we do not get the connection management stuff in place

@vasco-santos
Copy link
Member

@jacobheun can you move this into the bootstrap repo?

@jacobheun
Copy link
Contributor

Issue moved to libp2p/js-libp2p-bootstrap #109 via ZenHub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

3 participants