chore(sequencer): implement retry logic for connecting to the oracle sidecar #1788
Conversation
…er of injected txs
…GRPC_ADDR, increment next_currency_pair_id in put_price_for_currency_pair
…/ENG-996/retry-connect
| let _ = oracle_client | ||
| .prices(QueryPricesRequest::default()) | ||
| .await | ||
| .wrap_err("oracle sidecar responded with error to query for prices"); |
There was a problem hiding this comment.
i think we should move this out of the retried function and call it after we successfully connect to the client, as if the grpc call for prices fails, it indicates a more serious error that probably isn't fixable upon retry. thoughts?
There was a problem hiding this comment.
I think this is maybe a general problem with most or even all of our retry loops like this. We probably have some error cases which are permanent/unrecoverable, and others which are transient. However we always treat them as transient and keep retrying.
In this specific case, I guess we could also get a transient failure of the gRPC (e.g. timeout or dropped connection)? We should probably look to abandon the retry loop if the error indicates a permanent failure, but keep trying if not. However, that's another can of worms, since matching on error variants isn't a lot of fun when using eyre/anyhow.
There was a problem hiding this comment.
yeah, maybe we can do a larger revisit of the retry functions. this is fine for now, but something to keep in mind
Summary
This changes sequencer initialization to be blocked while attempting to connect to the oracle sidecar (if configured to use the sidecar), and to fail if repeated connection attempts fail.
Background
This was a todo in the code.
Changes
Testing
Added a unit test to confirm the process waits for the expected duration. We could probably do with adding gRPC mock tests for the sidecar, but I think that's outside the scope of this PR.
Changelogs
No updates required as this is (effectively) a feature branch which will have all relevant changelogs updated as one of the final steps of completing the feature.
Related Issues
Closes #1787