Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check time spent on attempting RPC to avoid spending too much time on retrying #1117

Merged

Conversation

MyonKeminta
Copy link
Contributor

@MyonKeminta
Copy link
Contributor Author

I'm still considering whether the values passed to these isExhausted function calls are proper enough. Comments are welcome.

@MyonKeminta
Copy link
Contributor Author

PTAL while I'm testing how it works in a real cluster with io injection

@@ -2396,3 +2419,10 @@ func (s *replicaSelector) patchRequestSource(req *tikvrpc.Request, rpcCtx *RPCCo
}
sb.WriteString(req.ReadType)
}

func (s *replicaSelector) recordAttemptedTime(duration time.Duration) {
s.targetReplica().attemptedTime += duration
Copy link
Contributor

@zyguan zyguan Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check s.targetReplica() != nil, otherwise the method is required to be called at right place.

@MyonKeminta
Copy link
Contributor Author

I didn't finish testing it due to not yet successfully allocate resources for testing 😥 shall we consider merging it first?

Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta MyonKeminta marked this pull request as ready for review January 18, 2024 08:29
@cfzjywxk cfzjywxk merged commit b8a6587 into tikv:master Jan 18, 2024
10 checks passed
@MyonKeminta MyonKeminta deleted the m/avoid-too-much-rpc-retry-on-timeout branch January 18, 2024 15:38
lance6716 pushed a commit to lance6716/client-go that referenced this pull request Jan 22, 2024
… retrying (tikv#1117)

* Check time spent on attempting RPC to avoid spending too much time on retrying

Signed-off-by: MyonKeminta <[email protected]>

* Handle refreshRegionStore

Signed-off-by: MyonKeminta <[email protected]>

* Add test

Signed-off-by: MyonKeminta <[email protected]>

* Address comments

Signed-off-by: MyonKeminta <[email protected]>

---------

Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
Signed-off-by: lance6716 <[email protected]>
MyonKeminta added a commit to MyonKeminta/client-go that referenced this pull request Jan 24, 2024
… retrying (tikv#1117)

* Check time spent on attempting RPC to avoid spending too much time on retrying

Signed-off-by: MyonKeminta <[email protected]>

* Handle refreshRegionStore

Signed-off-by: MyonKeminta <[email protected]>

* Add test

Signed-off-by: MyonKeminta <[email protected]>

* Address comments

Signed-off-by: MyonKeminta <[email protected]>

---------

Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
disksing pushed a commit that referenced this pull request Jan 24, 2024
… retrying (#1117) (#1131)

Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
MyonKeminta added a commit to MyonKeminta/client-go that referenced this pull request Jan 24, 2024
MyonKeminta added a commit to MyonKeminta/client-go that referenced this pull request Jan 24, 2024
… time on retrying (tikv#1117) (tikv#1131)"

This reverts commit 062f4c9.

Signed-off-by: MyonKeminta <[email protected]>
cfzjywxk pushed a commit that referenced this pull request Jan 24, 2024
… time on retrying (#1117) (#1131)" (#1133)

This reverts commit 062f4c9.

Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
@ti-chi-bot ti-chi-bot added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Feb 5, 2024
MyonKeminta added a commit to MyonKeminta/client-go that referenced this pull request Feb 19, 2024
… retrying (tikv#1117)

* Check time spent on attempting RPC to avoid spending too much time on retrying

Signed-off-by: MyonKeminta <[email protected]>

* Handle refreshRegionStore

Signed-off-by: MyonKeminta <[email protected]>

* Add test

Signed-off-by: MyonKeminta <[email protected]>

* Address comments

Signed-off-by: MyonKeminta <[email protected]>

---------

Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
@ti-chi-bot ti-chi-bot removed the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Feb 19, 2024
@ti-chi-bot ti-chi-bot added needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. and removed needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. labels Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants