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

client: check the timeout for creating stream (#2616) #2648

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented Jul 15, 2020

What problem does this PR solve?

cherry-pick #2616.

What is changed and how it works?

This PR uses an extra goroutine to check if creating stream causes too much time and will call cancel once it reaches the timeout setting of the client.

Release note

  • Fix the issue that creating TSO stream may block for a while if the server crashes

@rleungx rleungx added this to the v4.0.3 milestone Jul 15, 2020
@rleungx rleungx requested review from nolouch, disksing and lhy1024 July 15, 2020 03:07
@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 15, 2020
@disksing disksing added the require-LGT1 Indicates that the PR requires an LGTM. label Jul 15, 2020
@disksing
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 15, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@codecov-commenter
Copy link

Codecov Report

Merging #2648 into release-4.0 will decrease coverage by 0.08%.
The diff coverage is 83.33%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-4.0    #2648      +/-   ##
===============================================
- Coverage        77.14%   77.06%   -0.09%     
===============================================
  Files              205      205              
  Lines            22173    22185      +12     
===============================================
- Hits             17105    17096       -9     
- Misses            3765     3782      +17     
- Partials          1303     1307       +4     
Impacted Files Coverage Δ
client/client.go 71.95% <83.33%> (+0.34%) ⬆️
pkg/etcdutil/etcdutil.go 82.60% <0.00%> (-5.80%) ⬇️
server/kv/etcd_kv.go 84.41% <0.00%> (-3.90%) ⬇️
server/member/leader.go 71.98% <0.00%> (-1.56%) ⬇️
server/cluster/coordinator.go 74.54% <0.00%> (-1.30%) ⬇️
pkg/btree/btree.go 86.84% <0.00%> (-0.81%) ⬇️
client/base_client.go 90.60% <0.00%> (-0.68%) ⬇️
server/handler.go 50.32% <0.00%> (-0.44%) ⬇️
server/schedule/operator_controller.go 81.25% <0.00%> (-0.17%) ⬇️
server/server.go 76.62% <0.00%> (+0.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f68d227...b284107. Read the comment docs.

@disksing disksing merged commit ae0b004 into tikv:release-4.0 Jul 15, 2020
@lhy1024 lhy1024 added the type/bugfix This PR fixes a bug. label Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
require-LGT1 Indicates that the PR requires an LGTM. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants