client: support configuring backoff #8679
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
release-note-none
rleungx
force-pushed
the
introduce-backoffer
branch
from
17e8dd3
to
34b0620
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8679 +/- ##
==========================================
- Coverage 75.86% 75.81% -0.06%
==========================================
Files 460 460
Lines 70637 70678 +41
==========================================
- Hits 53591 53581 -10
- Misses 13673 13709 +36
- Partials 3373 3388 +15
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ti-chi-bot
bot
added
the
needs-rebase
client/retry/backoff.go
Outdated
| @@ -109,6 +109,57 @@ func (bo *Backoffer) Exec( | |||
| return err | |||
| } | |||
|
|
|||
| // ExecWithResult is a helper function to exec backoff with result. | |||
| func (bo *Backoffer) ExecWithResult( | |||
There was a problem hiding this comment.
we can wrap exec to this
And export Exec and ExecWithResult separately
client/client.go
Outdated
| } | ||
| return handleRegionResponse(resp), nil | ||
| } | ||
| if c.option.bo == nil { |
There was a problem hiding this comment.
How about using client interceptor to do backoff? We can delivery backoff option by context?
server/grpc_service.go
Outdated
| return &pdpb.GetMinTSResponse{ | ||
| Header: s.wrapErrorToHeader(pdpb.ErrorType_UNKNOWN, err.Error()), | ||
| }, nil | ||
| return nil, status.Error(codes.ResourceExhausted, err.Error()) |
There was a problem hiding this comment.
Maybe we can split these updates to another pr, and merge it firstly. It is simple
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
rleungx
force-pushed
the
introduce-backoffer
branch
from
41384b9
to
741f0e8
Compare
ti-chi-bot
bot
added
size/L
Signed-off-by: Ryan Leung <[email protected]>
rleungx
removed
the
needs-rebase
Signed-off-by: Ryan Leung <[email protected]>
|
/retest |
| @@ -41,6 +42,53 @@ const ( | |||
| FollowerHandleMetadataKey = "pd-allow-follower-handle" | |||
| ) | |||
|
|
|||
| // Add retry related CallOption | |||
| type retryCallOption struct { | |||
| grpc.EmptyCallOption | |||
There was a problem hiding this comment.
Otherwise, we need to implement before and after interfaces.
| // Record the start time | ||
| start := time.Now() | ||
|
|
||
| ctx = retry.WithBackoffer(ctx, bo) |
There was a problem hiding this comment.
Does the user need to specify it?
There was a problem hiding this comment.
Yes, but we also can provide a default one if needed.
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nolouch The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| type boCtxKey struct{} | ||
|
|
||
| // Key used to store backoffer | ||
| var backofferKey = boCtxKey{} |
There was a problem hiding this comment.
Why using struct{}, not string?
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
| // Calculate the elapsed time | ||
| elapsed := time.Since(start) | ||
| // Verify that some backoff occurred by checking if the elapsed time is greater than the base backoff | ||
| re.Greater(elapsed, base, "Expected some backoff to have occurred") |
What problem does this PR solve?
Issue Number: Close #8047
What is changed and how does it work?
Check List
Tests
Release note