-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
tikv: forbid to try to get a connection forever #11391
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
store/tikv/client_test.go
Outdated
@@ -22,6 +22,9 @@ import ( | |||
"github.com/pingcap/errors" | |||
"github.com/pingcap/kvproto/pkg/tikvpb" | |||
"github.com/pingcap/tidb/config" | |||
"github.com/pingcap/tidb/store/tikv/tikvrpc" | |||
|
|||
"fmt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need move import to first group
@hicqu maybe better edit the description to follow TIDB PR template, our QA's tools will rely on PR template |
Codecov Report
@@ Coverage Diff @@
## master #11391 +/- ##
================================================
- Coverage 81.3685% 81.3024% -0.0662%
================================================
Files 423 423
Lines 90648 90616 -32
================================================
- Hits 73759 73673 -86
- Misses 11577 11619 +42
- Partials 5312 5324 +12 |
/run-all-tests |
LGTM |
Please fix CI. |
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cherry pick to release-3.0 in PR #11404 |
What problem does this PR solve?
SendRequest
could be block forever if all connectons are broken.What is changed and how it works?
Forbid to try to get a connection forever so that
SendRequest
can return immediately and try an another target peer.Check List
Tests
Related changes