-
Notifications
You must be signed in to change notification settings - Fork 493
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
Network: Class-based Peer Selector #5937
Conversation
…erance factor violations, priority and lastCheckedTime.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5937 +/- ##
=======================================
Coverage 55.71% 55.71%
=======================================
Files 487 488 +1
Lines 68048 68103 +55
=======================================
+ Hits 37911 37945 +34
- Misses 27572 27584 +12
- Partials 2565 2574 +9 ☔ View full report in Codecov by Sentry. |
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.
Added some comments/ideas
… check violations of tolerance factor during iteration of peer selectors in getNextPeer().
…ll of it's download functionality (implying that failures around catchpoint downloads for a peer from a class put that class at risk of being disabled temporarily).
…ises using actual peer selectors and expected ranking behavior.
… connected out, relays, archival nodes.
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.
I suggest to remove both priority
and lastCheckedTime
. It will make the initialization less verbose at least, and catchup itself faster (without re-checking disabled class every 10 minutes).
…d by order of wrappedPeerSelectors at construction.
…resets when all selectors have been disabled.
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.
Looks correct and I appreciate the simplifications
Summary
Introduce a class-based peer selector for explicitly ranking peer groups based on type and download performance. The intent is to be able to quickly move from one class of peers to the next in cases such as where a client needs older blocks that may only be present on archival nodes.
Test Plan
Introduce new unit tests for class-based peer selector, existing tests should be updated/pass.