-
Notifications
You must be signed in to change notification settings - Fork 838
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
Backward sync: use retry switching peer when fetching data from peers #4656
Conversation
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
5ebd45f
to
e8190d1
Compare
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.
LGTM
peerResult -> { | ||
debugLambda( | ||
LOG, | ||
"Got {} blocks {} from peer {}, attempt {}", |
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.
don't need the {} after blocks
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.
good catch, fixed
this::getAssignedPeer, | ||
this::getRetryCount); | ||
} else { | ||
LOG.debug("Failed to get {} blocks after {} retries", headers.size(), getRetryCount()); |
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.
should the peer be logged here too
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 is the case when all peers have failed, for this the peer is not logged
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
…into bwsync-retry-switching-peer
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
# Conflicts: # CHANGELOG.md # ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java # ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncStep.java # ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncStep.java
…hyperledger#4656) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: wcgcyx <wcgcyx@gmail.com>
…hyperledger#4656) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
…hyperledger#4656) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
PR description
This PR is built on top of #4655, so please che it first.
Wait for at least one connected peer before starting the backwward sync, and use the retry mechanism that switches peer everytime it needs to retry to get data from peers, this helps reducing errors like the following ones, and makes the sync faster
Fixed Issue(s)
relates to #4446
fixes: #4698
Documentation
doc-change-required
label to this PR ifupdates are required.
Changelog