-
Notifications
You must be signed in to change notification settings - Fork 22
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
Better discovery and dialing heuristic #153
Comments
@pgte one thing I'm not sure about, what is the purpose of this block? |
Been deployed now |
@victorb wow you guys don't mess around :) Thanks! |
@dirkmc this is great, thank you! (Thinking about this again, I'm not sure of why the peer would be in the inbound list if it wasn't in the peer ring, but I may be missing something...) |
@pgte Yes, it was fixed - closing |
There are some issues with discovery:
peer:connect
event and make a connection. Currently we wait a random amount of time (up to 5s) before dialing a peer so as not to overload new peers.In practice the peer may already have been dialed by the websocket star transport.
1. I propose a dialing heuristic that takes into account the number of peers known to the app.
2. We should listen for this event instead of polling
This is because rendezvous repeatedly emits
peer:connect
(every 10s by default)3. I propose adding a wrapper around discovery that remembers which peers have been dialed
I suggest the wrapper's memory should have a time limit, and a limit on the number of peers
peer
events, I think because of a bug in the dialing mechanism4. Implementing the wrapper should take care of this
I'm happy to work on a PR to fix these issues
The text was updated successfully, but these errors were encountered: