peerdiscovery icon indicating copy to clipboard operation
peerdiscovery copied to clipboard

Question regarding PeerDiscovery api

Open bearsh opened this issue 1 year ago • 5 comments

Hi, I'm about to use this library in a client-server project where the client should be able to discover servers in the lan.

I looked at the PeerDiscovery object and its attached methods. For me, it's strange how it is implemented. NewPeerDiscovery() creates this object and does a peer discover in a blocking way. Still PeerDiscovery has a Shutdown() method to stop discovery. But how should this work, if the object creation is blocking until discovering has been finished?

Wouldn't it be better to create the option, have a Serve() and Shutdown() method and get the notification about new peer via callback (or channel)?

bearsh avatar Sep 19 '24 06:09 bearsh

You can use Notify to get a callback and you can run this in a Go routine for async. Check out croc for a battle-tested example. Croc needs blocking because it decides to switch to LAN only if peers are connected.

Can you describe your use case some more for peer discovery?

schollz avatar Sep 19 '24 09:09 schollz

I didn't notice, notify also gets called by the listen() method. But why does it then get called again after discovery in the NewPeerDiscovery()?

Yes, I can run NewPeerDiscovery() in a go routine (with TimeLimit = -1), but then the PeerDiscovery does not get returned and there's AFAIK no clean way to stop it again.

Can you describe your use case some more for peer discovery?

I have a cli application which I can use for local operation. But it is also possible to start it as a server which accepts rpc calls from a client. In this way, the client is able to perform operations on a remote server. To ease the server discovery in a LAN, I want to start peerdiscovery on the server with TimeLimit=-1. Theoretically, on the server, notification of available clients is not necessary. Everything is already possible with the current API of peerdiscovery but I see no way to stop the discovery service in a clean way.

bearsh avatar Sep 19 '24 09:09 bearsh

You can use StopChan or Shutdown to stop peer discovery.

Can you link to your code? It will be easier to see your code to get an idea of what might work best.

schollz avatar Sep 19 '24 09:09 schollz

I missed the StopChan. That works indeed.

Shutdown() will not work:

ps, err := peerdiscovery.NewPeerDiscovery(ps)
ps.Shutdown() // either TimeLimit = -1 and this point will not be reached or there's a limit but than discovery has already been stopped

or do I miss something regarding the Shutdown() method?

bearsh avatar Sep 19 '24 11:09 bearsh

@kirillDanshin

schollz avatar Sep 19 '24 11:09 schollz