Implement Exponential Backoff for Transient Sync Errors #588
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks!
CI is unhappy though, please check that the code compiles before pushing.
src/chain/mod.rs
Outdated
| log_error!(logger, "Failed to synchronize chain listeners: {:?}", e); | ||
| tokio::time::sleep(Duration::from_secs(CHAIN_POLLING_INTERVAL_SECS)) | ||
| .await; | ||
| backoff_delay_multiplier += 1; |
There was a problem hiding this comment.
I don't think we should let the backoff delay grow infinitely. Please include a sane upper bound.
There was a problem hiding this comment.
Got it, I am adding a max backoff delay of 300 secs for this
src/chain/mod.rs
Outdated
| } else { | ||
| log_error!( | ||
| logger, | ||
| "Failed to synchronize chain listeners: {:?}, e" |
There was a problem hiding this comment.
This doesn't compile. Also, as discussed elsewhere, we at least need to error out. But technically, we should keep retrying currently as otherwise we'd need to add panic here as permanently being unable to connect to the chain source is an unrecoverable error.
There was a problem hiding this comment.
This doesn't compile. Also, as discussed elsewhere, we at least need to error out. But technically, we should keep retrying currently as otherwise we'd need to add panic here as permanently being unable to connect to the chain source is an unrecoverable error.
@tnull As you suggested that panicking isnt best and not being able to error out I stuck with retrying but with the delay of 300 secs. Exponential backoff from CHAIN_POLLING_INTERVAL_SECS was only introduced for transient errors
Yeah, just saw that, would check it out now.. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Please also make sure to always provide rationale for the change in the commit message, and format it properly (heading shouldn't be larger than one line). Feel free to refer to https://cbea.ms/git-commit/ on how to write good commit messages.
Previously, chain synchronization failures would retry immediately without any delay, which could lead to tight retry loops and high CPU usage during failures. This change introduces exponential backoff for transient errors, starting at 2 seconds and doubling each time up to a maximum of 300 seconds. Persistent errors also now delay retries by the maximum backoff duration to prevent rapid loops while maintaining eventual recovery. Fixes lightningdevkit#587
3 participants
This PR attempts to improve retry in the bitcoind RPC synchronization loop from #587 by replacing the existing linear backoff with a proper exponential backoff strategy. We could also add a maximum delay to prevent excessively long waits, but I am not sure if it would really help.