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

tikv: non-blocking establish superbatch connection with timeout (#12733) #12814

Merged
merged 2 commits into from Oct 18, 2019
Merged

tikv: non-blocking establish superbatch connection with timeout (#12733) #12814

merged 2 commits into from Oct 18, 2019

Conversation

lysu
Copy link
Collaborator

@lysu lysu commented Oct 18, 2019

cherry-pick #12733 to 3.0

without any conflict just fix logger compile error
9f30108

What problem does this PR solve?
fix the potential problem that slow establishes the connection make other kv requests be slow

5 min dial timeout doesn't work in superbatch
createConnArray will block wait conn establish on create tikvClient.BatchCommands
long time hold lock in createConnArray will block getConnArray
What is changed and how it works?
make establish grpc client in other goroutine which is blocking process
check estDone before send request, send directly if done
wait estReady channel if it's not done(only need select channel for first time)
set right dialTimeout for batch client creation
more words:

current impl is using connectivity state API, to wait non-blocking dial result with context timeout.
new but not released grpc, we maybe can use grpc/grpc-go#2960 to simplify impl
also take care grpc/grpc-go#1786 if upgrade to new grpc in future
Check List
Tests

Unit test
Integration test
Code changes

impl
Side effects

n/a
Related changes

Need to cherry-pick to the release branch
Release note

fix the potential problem that slow establishes the connection make other kv requests be slow 

This change is Reviewable

@lysu lysu added component/tikv priority/release-blocker This PR blocks a release. Please review it ASAP. type/3.0 cherry-pick type/bug-fix This PR fixes a bug. labels Oct 18, 2019
@lysu
Copy link
Collaborator Author

lysu commented Oct 18, 2019

/run-all-tests

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 18, 2019
Copy link
Member

@coocood coocood left a comment

Choose a reason for hiding this comment

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

LGTM

@coocood coocood merged commit 7749430 into pingcap:release-3.0 Oct 18, 2019
@lysu
Copy link
Collaborator Author

lysu commented Nov 8, 2019

/run-cherry-picker

@sre-bot
Copy link
Contributor

sre-bot commented Nov 8, 2019

cherry pick to release-3.1 in PR #13277

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv priority/release-blocker This PR blocks a release. Please review it ASAP. status/LGT1 Indicates that a PR has LGTM 1. type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants