-
Notifications
You must be signed in to change notification settings - Fork 106
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
fix(net): Fix potential network hangs, and reduce code complexity #7859
Conversation
327e4a7
to
aa05c71
Compare
aa05c71
to
2a92792
Compare
Co-authored-by: Arya <aryasolhi@gmail.com>
2a92792
to
21b5c39
Compare
This should be ready to go now, there was a minor issue with the |
Does this need a full sync before it merges, or can we wait until Friday's scheduled full sync? If we do one now, the previous time was 2d 3h 15m: |
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.
Does this need a full sync before it merges, or can we wait until Friday's scheduled full sync?
I think we can wait until the next scheduled full sync.
Co-authored-by: Arya <aryasolhi@gmail.com>
Motivation
Zebra's peer handling code is complex, and it contains some potential hangs. This PR fixes some hangs, and simplifies the code.
Follow up to #7772
Close #7858
Specifications
Rust futures wake a task when any waker cloned from the most recently passed Context is woken:
https://doc.rust-lang.org/std/future/trait.Future.html#tymethod.poll
Complex Code or Requirements
This PR contains a lot of refactors to make future changes easier.
It also eliminates some wakers entirely to simplify the code and waking logic.
Solution
Bug fixes:
Code simplification:
futures::ready!
(the macro) and?
to simplify pollingRefactors:
Poll
, andOk
if they do somethingTesting
The existing tests cover this behaviour fairly well. It is hard to check for hangs in tests.
Some tests have been updated for the new polling behaviour.
Review
This might still need some test fixes.
Reviewer Checklist
Follow Up Work