This repository has been archived by the owner on Aug 23, 2020. It is now read-only.
Feature: Sync check added to broadcast stage #1856
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kwek20
force-pushed
the
sync-check-broadcast
branch
3 times, most recently
from
05fd88c to
65873be
Compare
GalRogozinski
suggested changes
May 10, 2020
| /** | ||
| * If this node is currently seen as in sync | ||
| */ | ||
| private boolean isInSync; |
There was a problem hiding this comment.
Even though currently we have one thread per stage, let's do this correctly and make this variable volatile/atomic.. since supposedly with a small change we can have more than one broadcast thread
kwek20
force-pushed
the
sync-check-broadcast
branch
from
2780beb to
ae372b3
Compare
|
@GalRogozinski can we merge this, so i can easily merge 1.8.6 onto the progressbar PR? |
GalRogozinski
approved these changes
May 12, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description of change
Extracts our sync checker to a service, and uses that service in the BroadcastStage of the Network pipeline.
This is required because if you get all your new transactions from 1 neighbour, and have 1 other neighbour which doesnt have the transaction you need, there is no chance of syncing.
If that second neighbour sends you a very low amount of new transactions(compared to the other neighbour) you will still sync, but very slow.
This has been introduced due to our removal of "ping pong" in #1745
Type of change
How the change has been tested
The stuck node got unstuck in the scenario described above