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: print error logs only if necessary #10349

Merged
merged 4 commits into from
May 6, 2019

Conversation

hicqu
Copy link
Contributor

@hicqu hicqu commented May 5, 2019

What problem does this PR solve?

When TiDB loses a connection to TiKV, it will try to re-establish it if necessary. Only in this case it needs to print error logs. If the connection is closed by TiDB itself, error logs is not necessary.

What is changed and how it works?

Move error log print code to better place.

Check List

Tests
No code.
We are working on adding a test framework for tikvclient. We will add a new check rule for that: if the connArray is closed, all goroutines created by it shoud exit gracefully.

Code changes
No visible changes out of tikvclient.

Close tikv/client-go#19 and pingcap/go-ycsb#97.

@codecov
Copy link

codecov bot commented May 5, 2019

Codecov Report

Merging #10349 into master will decrease coverage by 0.0219%.
The diff coverage is 4.1666%.

@@               Coverage Diff               @@
##             master     #10349       +/-   ##
===============================================
- Coverage   77.6897%   77.6677%   -0.022%     
===============================================
  Files           411        411               
  Lines         85436      85455       +19     
===============================================
- Hits          66375      66371        -4     
- Misses        14101      14124       +23     
  Partials       4960       4960

@hicqu hicqu requested review from lysu and tiancaiamao May 5, 2019 08:51
@lysu
Copy link
Contributor

lysu commented May 5, 2019

@hicqu plz help add target host info into batchRecvLoop re-create streaming fail and batchRecvLoop error when receive thx :D

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.

LGTM

@lysu lysu added component/tikv status/LGT1 Indicates that a PR has LGTM 1. labels May 5, 2019
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 6, 2019
@tiancaiamao
Copy link
Contributor

/run-all-tests

@tiancaiamao tiancaiamao merged commit 461512b into pingcap:master May 6, 2019
@hicqu hicqu deleted the tikv-client-improvement branch May 6, 2019 05:54
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve the error log "batch commands send error: EOF"
3 participants