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

tikvclient: fix a bug that recreate connection forever #10301

Merged
merged 4 commits into from
Apr 30, 2019

Conversation

hicqu
Copy link
Contributor

@hicqu hicqu commented Apr 29, 2019

What problem does this PR solve?

When TiDB trys to re-create the connection to one TiKV, it just checks itself is shutdown or not for the first time. After that TiDB will keep retry if the re-create always fail.

What is changed and how it works?

With this PR TiDB will check it's shutdown or not for every retry, so it can exit successfully.

@hicqu hicqu requested a review from tiancaiamao April 29, 2019 03:33
@codecov
Copy link

codecov bot commented Apr 29, 2019

Codecov Report

Merging #10301 into master will decrease coverage by 0.0235%.
The diff coverage is 0%.

@@               Coverage Diff                @@
##             master     #10301        +/-   ##
================================================
- Coverage   77.8087%   77.7852%   -0.0236%     
================================================
  Files           410        410                
  Lines         84966      84930        -36     
================================================
- Hits          66111      66063        -48     
- Misses        13909      13917         +8     
- Partials       4946       4950         +4

@codecov
Copy link

codecov bot commented Apr 29, 2019

Codecov Report

Merging #10301 into master will decrease coverage by 0.0106%.
The diff coverage is 0%.

@@               Coverage Diff                @@
##             master     #10301        +/-   ##
================================================
- Coverage   77.6681%   77.6575%   -0.0107%     
================================================
  Files           411        411                
  Lines         85452      85447         -5     
================================================
- Hits          66369      66356        -13     
- Misses        14121      14127         +6     
- Partials       4962       4964         +2

@tiancaiamao
Copy link
Contributor

LGTM
PTAL @lysu

@tiancaiamao tiancaiamao added type/bugfix This PR fixes a bug. status/LGT1 Indicates that a PR has LGTM 1. component/tikv labels Apr 30, 2019
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGMT

@lysu
Copy link
Contributor

lysu commented Apr 30, 2019

@hicqu maybe we can add target addr in this log to help know fail target address

logutil.Logger(context.Background()).Error("batchRecvLoop re-create streaming fail", zap.Error(err))

@lysu lysu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 30, 2019
@lysu
Copy link
Contributor

lysu commented Apr 30, 2019

/run-all-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants