-
Notifications
You must be signed in to change notification settings - Fork 111
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(sync): prevent synchronizer loop when very close to tip #3854
Conversation
Replace the use of loop labels and `continue` for control flow, and use early return from a separate method instead. This also allows removing the `started_once` flag.
Reduce duplicate code and make the main synchronization methods a little more concise to improve readability.
Leave active downloads running if the tips have been exhausted, because it could have reached the chain tip.
Codecov Report
@@ Coverage Diff @@
## main #3854 +/- ##
==========================================
- Coverage 78.90% 78.69% -0.21%
==========================================
Files 292 295 +3
Lines 33508 33769 +261
==========================================
+ Hits 26438 26576 +138
- Misses 7070 7193 +123 |
This fixes the issue for me 👍 |
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.
Nice catch!
Thanks for splitting this PR up into commits, it was very helpful during the review.
Is there something preventing this from moving out of draft? |
@jvff I've been running this since Friday and it's awesome, shall we promote this out of draft? |
Motivation
Zebra currently has some performance issues when block synchronization is close to the chain tip. During the investigation of the issue, I ran into two causes:
This PR fixes the first cause, and the second cause will very likely be fixed by the refactor to split the state service to increase throughput of read only requests (#3866).
Solution
Only cancel active block download tasks in case there's an error, preventing them from being cancelled when the prospective tips set is exhausted.
Closes #3375
Review
Reviewer Checklist
Follow Up Work
I'd like to have a regression test for this, but I'll try to open a separate optional PR for it.