consul
consul copied to clipboard
feat: update rpc retry criteria
Description
This change makes the following change to the rate-limiting retry logic:
- Client RPC to Server -> Retry should be in the client, only the "ErrRetryElsewhere" error
- Client RPC to Follower Forwarded to Leader -> Retry should be implemented in the client, only the "ErrRetryElsewhere" error
- Server RPC executed locally -> Should retry, only the "ErrRetryElsewhere" error
- Server RPC forwarded to Leader -> Should retry, only "ErrRetryLater" error
PR Checklist
- [ ] ~updated test coverage~
- [ ] ~external facing docs updated~
- [ ] ~not a security concern~
Thank you @JadhavPoonam for working on this.
Looking to the change in here, I think it make the canRetry logic a bit confusing.
Another approach that I think would be better, is to extract retriableErrors as a func parameter and pass it in both cases. What you think about that?
I have passed the retryable messages separately now and is based on the following criteria:
- On the client side we want to retry for
ErrChunkingResubmit
and both rate limiting errorsErrRetryElsewhere
- In the
forwardRequestToLeader
method we retryErrRetryLater
error
@JadhavPoonam we need to add retries in the server RPC for the rate limiter errors, this is to cover the case of direct HTTP to a server.