polygon/p2p: retry fetch headers on response timeout #9537
Conversation
…c-header-downloader-interface
…ledgerwatch/erigon into astrid-sync-header-downloader-interface
…-fetch-headers-validations
…gerwatch/erigon into astrid-p2p-chunk-fetch-headers
…ch/erigon into astrid-p2p-retry-fetch-headers-timeouts
…-retry-fetch-headers-timeouts
…-retry-fetch-headers-timeouts
…-retry-fetch-headers-timeouts
| @@ -26,6 +26,7 @@ require ( | |||
| github.com/benesch/cgosymbolizer v0.0.0-20190515212042-bec6fe6e597b | |||
| github.com/btcsuite/btcd/btcec/v2 v2.1.3 | |||
| github.com/c2h5oh/datasize v0.0.0-20220606134207-859f65c6625b | |||
| github.com/cenkalti/backoff/v4 v4.2.1 | |||
There was a problem hiding this comment.
I've also implemented a simple generic retry in devnet request_generator.go. Maybe we can migrate it to this lib.
| type RequestIdGenerator func() uint64 | ||
|
|
||
| type FetcherConfig struct { | ||
| responseTimeout time.Duration | ||
| retryBackOff time.Duration |
There was a problem hiding this comment.
what does it mean? will it add retryBackOff for each subsequent request? e.g. retryBackOff=1 results in delays 1, 2, 3 (retries at t=1, t=3, t=6)?
There was a problem hiding this comment.
we're using constant backoff retry mechanism which keeps the backoff the same, so request->fail->wait for 1 sec->request->fail->wait for 1 sec->request->fail->wait for 1 sec->request... and so on until max retries
the library has also the exponential backoff mechanism which is a bit more advanced (cam read about it in the docs)
There was a problem hiding this comment.
👌 please check out my retry function in devnet request_generator.go, and we can discuss later which one we replace going forward
There was a problem hiding this comment.
personally I'd use this open source library since it is a popular choice & less code for us to maintain if we were to have our own retry implementations
There was a problem hiding this comment.
but no strong preference really - happy with either
| func NewChanMessageObserver[M any](ctx context.Context) ChanMessageObserver[M] { | ||
| return ChanMessageObserver[M]{ | ||
| ctx: ctx, | ||
| channel: make(chan M), | ||
| } | ||
| } | ||
|
|
||
| type ChanMessageObserver[M any] struct { | ||
| ctx context.Context | ||
| channel chan M | ||
| } | ||
|
|
||
| func (cmo ChanMessageObserver[M]) Notify(msg M) { |
There was a problem hiding this comment.
I'd rather pass ctx to Notify as parameter, because at the point of observer creation it might not be available yet.
There was a problem hiding this comment.
will tidy this up as part of the refactoring we discussed just now
No description provided.