Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Discovery #184
Add Discovery #184
Changes from all commits
c837436
5ef7439
f9c26c2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're going to spawn a bunch of goroutines. We really do need to listen on peer join/leave events to determine when we're done bootstrapping. At the moment:
The right way to do this is:
Note: We can probably also find a way to hack in a fix but, at the very least, we can't spawn a bunch of goroutines connecting to the same peers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emitting an "I don't need more peers" signal is likely much more invasive then just checking from the outside. However, if we're certain that the "how many peers" functions that we care about is static per PubSub instance then we can use the PeerJoin events instead of
time.Sleep(100 ms)
(although if the number of peers we wait on can change over time this won't work).This won't be very expensive since it should be backed by a backoff cache of some sort anyhow.
I'm fine replacing this with storing a local cache of queried peers. I only left it this way bc I though when we talked about this previously that you didn't want to store that state locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this on a call. It's going to be fine as-is as long as we remember which peers we're currently dialing and/or have recently dialed.