schollz / peerdiscovery

Pure-Go library for cross-platform local peer discovery using UDP multicast :woman: :repeat: :woman:
MIT License
638 stars 55 forks source link

Question regarding PeerDiscovery api #38

Open bearsh opened 1 week ago

bearsh commented 1 week ago

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)?

schollz commented 1 week ago

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?

bearsh commented 1 week ago

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.

schollz commented 1 week ago

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.

bearsh commented 1 week ago

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?

schollz commented 1 week ago

@kirillDanshin