-
Notifications
You must be signed in to change notification settings - Fork 226
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
Double the usefulness interval for peers in the Routing Table #651
Conversation
The |
@Stebalien We now pass |
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 PR is fine and should only make things better/stay the same. However, I would've liked for us to run some tests to demonstrate that increasing the usefulness threshold actually helps us.
If we're going to be running testground tests on this in the near future to examine this parameter then we should probably make the usefulness threshold be configurable (see comment)
// use twice the theoritical usefulness threhold to keep older peers around longer | ||
rt, err := makeRoutingTable(dht, cfg, 2*maxLastSuccessfulOutboundThreshold) |
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.
@aarshkshah1992 what do you think about having the usefulness here be an option so that we can play around with it in testground?
Perhaps something like an option that takes func(refreshThreshold time.Duration) time.Duration
?
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.
@Stebalien @aschmahmann