Listener catch-up fix #450
Conversation
iansuvak
requested review from
feuGeneA,
geoff-vball,
minghinmatthewlam,
bernard-avalabs,
michaelkaplan13 and
cam-schultz
iansuvak
force-pushed
the
move-catchup-to-listener
branch
from
f84439a to
8a7c124
Compare
vms/evm/subscriber.go
Outdated
| ) | ||
| // Sleep if this wasn't the last retry | ||
| if i < rpcMaxRetries-1 { | ||
| time.Sleep(time.Duration(i+1) * time.Second) |
There was a problem hiding this comment.
nit: it might be more clear to specify the total timeout/sleep duration (10 seconds across 5 retries) as a constant. Any reason why the subscriber timeout is a constant on each retry, but the rpc timeout grows linearly with each retry?
There was a problem hiding this comment.
Agreed, we should be consistent here.
There was a problem hiding this comment.
I agree but in which direction? constant? Ideally with network resources I like exponential backoffs with jitter but seemed like an overkill here.
There was a problem hiding this comment.
Exponential backoff has come up in previous discussions, but we've come to similar conclusions that it's overkill. Since the overall number of RPC calls is low, optimizing how they're spaced hasn't been a top priority.
There was a problem hiding this comment.
I thought we were tracking that already, but apparently not. I created #453 to do so.
| var err error | ||
| var header *types.Header | ||
| attempt := 1 | ||
| for { |
There was a problem hiding this comment.
why don't we do a regular for attempt=1; attempt < retryAttempts; attempt ++ here
There was a problem hiding this comment.
matching the way that subscriber reattempts right below this
Co-authored-by: cam-schultz <78878559+cam-schultz@users.noreply.github.com> Signed-off-by: Ian Suvak <ian.suvak@avalabs.org>
Why this should be merged
Addresses #443
How this works
Moves catchup process from subscriber to listener along with the headers channelAfter doing this I realized that this change can be made lighter by passing in an RPC client to the Subscriber as well. That would mean that we can keep the catchup and the headers on the subscriber level.I pivoted to keeping things on the subscriber level and just changing the client and adding retries there
How this was tested
CI
How is this documented
N/A