Backward sync exception improvements #4092
Merged
+87
−50
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
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
fab-10
force-pushed
the
backwardsync-exceptions
branch
from
5cf3f34
to
caf08a1
Compare
gezero
approved these changes
Jul 13, 2022
| @@ -165,7 +165,13 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) | |||
| new Block(newBlockHeader, new BlockBody(transactions, Collections.emptyList())); | |||
|
|
|||
| if (mergeContext.isSyncing() || parentHeader.isEmpty()) { | |||
| mergeCoordinator.appendNewPayloadToSync(block); | |||
| mergeCoordinator | |||
There was a problem hiding this comment.
Should this be part of this PR? Is this related to BWSync?
There was a problem hiding this comment.
The idea is to log also the not recoverable backward sync exceptions, and to do that closest possible to where the sync was invoked, and possibly do other actions in case of these exceptions.
So it related to backward sync since the exception is triggered there, but no problem for me to move it to a separate PR if you think it does not belong here.
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
fab-10
force-pushed
the
backwardsync-exceptions
branch
from
6f90f53
to
cda207c
Compare
codyborn
pushed a commit
to codyborn/besu
that referenced
this pull request
Jul 31, 2022
* Remove backward sync exception recursive nesting Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
codyborn
pushed a commit
to codyborn/besu
that referenced
this pull request
Jul 31, 2022
* Remove backward sync exception recursive nesting Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: Cody Born <codyborn@outlook.com>
eum602
pushed a commit
to lacchain/besu
that referenced
this pull request
Nov 3, 2023
* Remove backward sync exception recursive nesting Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: Fabio Di Fabio fabio.difabio@consensys.net
PR description
During the investigation of a Hive test that triggers backward sync, found some improvements that could be applied to backward sync exceptions.
Remove of recoursive exceptions nesting, that produces monster exception like this one
deeply-nested-exception.txt
Avoid to return
nullfromsyncBackwardsUntilmethods in case the block is already trustedLog a warning for backward sync exception that are not recoverable, likewise it is already done for recoverable exceptions
Rephrase the error messages when there is a validator error found during the backward sync, with the assumption that the child block is the invalid one, and the parent is correct, this makes clearer the error message for the Hive test
Fixed Issue(s)
Documentation
doc-change-requiredlabel to this PR ifupdates are required.
Changelog