Create backward sync retries on demand #4095
Conversation
fab-10
force-pushed
the
backwardsync-ondemand-retries
branch
2 times, most recently
from
3114b6b
to
9442dde
Compare
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
fab-10
force-pushed
the
backwardsync-ondemand-retries
branch
from
9442dde
to
b54ddfb
Compare
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
| return ethContext | ||
| .getScheduler() | ||
| .scheduleFutureTask( | ||
| () -> prepareBackwardSyncFutureWithRetry(retries - 1), Duration.ofSeconds(5)); |
There was a problem hiding this comment.
Are you convinced this will not stackoverflow?
There was a problem hiding this comment.
the method is called with an always decreasing number of remaining retries, and when that number reaches 0 the recursion is stopped, so it should not do an infinite recursion, and MAX_RETRIES is a small number that should not produce a stack overflow, am I missing some specific cases?
I am adding unit test to check that this not goes on stackoverflow.
I suggest also to reduce the MAX_RETRIES to something like 20, so it retries for 20*5sec = 2minutes, what do you think?
There was a problem hiding this comment.
I just know that retries using recursion and futures are tricky because of stack overflow. I am fine with reducing the retries amount.
|
converting to draft until unit tests are ready |
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
fab-10
force-pushed
the
backwardsync-ondemand-retries
branch
from
b0a6a57
to
1a18de7
Compare
# Conflicts: # CHANGELOG.md # ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java
* Create backward sync retries on demand Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
* Create backward sync retries on demand Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: Cody Born <codyborn@outlook.com>
* Create backward sync retries on demand Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio fabio.difabio@consensys.net
PR description
Currently backward sync create the max number of retries ahead of time, and this could cause unneeded retries in case of non recoverable error like in this log
backward-sync-retries.txt
This PR rework the retry strategy to create retries on demand and fast fail in case of a non recoverable exception.
Fixed Issue(s)
Documentation
doc-change-requiredlabel to this PR ifupdates are required.
Changelog