libp2p / js-libp2p-bootstrap

Rail a libp2p node through a bootstrap peer list
https://libp2p.io
Other
30 stars 21 forks source link

Reconsider Bootstrap interval #109

Closed jacobheun closed 2 years ago

jacobheun commented 3 years ago

@a1300 commented on Tue Nov 24 2020

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 commented on Tue Nov 24 2020

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 commented on Tue 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 commented on Tue Nov 24 2020

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 https://github.com/libp2p/js-libp2p/issues/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:

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 commented on Tue Nov 24 2020

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


@vasco-santos commented on Tue Nov 24 2020

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 commented on Fri Nov 27 2020

@jacobheun can you move this into the bootstrap repo?

mpetrunic commented 2 years ago

I guess closed by https://github.com/libp2p/js-libp2p-bootstrap/pull/142