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
Asynchronous lookups #498
Asynchronous lookups #498
Changes from all commits
2dfbbc4
49b07fa
516cd0b
f65f562
1c33021
7cf6b8f
f9f08b5
86fd84a
a4fcc84
9476ad8
b04ec8a
c9d93bd
6a56205
11d4d73
15e343b
8656be1
de9d5ae
a21162d
f9a79c5
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.
This test is pretty strange but indicative of the changes we've been making. The test seems to setup a ring of peers and then calls
GetClosestPeers
. In the past we were much more thorough in our queries and effectively ignored most of the benefits that can be assumed in a Kademlia network with fairly accurate routing tables. This meant we could have a ring of 30 peers and get 20 of them no problem.Now, we more strongly rely on the routing tables being good and so in this ring setup it seems like we're only guaranteed to return > beta peers instead of k.
Thoughts?
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've basically broken the contract of
GetClosestPeers
as I've commented elsewhere. There are a couple of things we can/should do to address these which have been mentioned in the comment.This file was deleted.
This file was deleted.