-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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: reduce wait backoff time when lock has be expired #10006
Conversation
/run-all-tests |
@tiancaiamao PTAL can we do this? thx |
I like this idea @lysu |
You may remove the WIP label if it's ready for review |
Codecov Report
@@ Coverage Diff @@
## master #10006 +/- ##
================================================
+ Coverage 77.2589% 77.2756% +0.0166%
================================================
Files 413 413
Lines 86931 86964 +33
================================================
+ Hits 67162 67202 +40
+ Misses 14602 14599 -3
+ Partials 5167 5163 -4 |
/run-all-tests |
/rebuild |
/run-all-tests |
@@ -265,47 +266,59 @@ func (lr *LockResolver) BatchResolveLocks(bo *Backoffer, locks []*Lock, loc Regi | |||
// commit status. | |||
// 3) Send `ResolveLock` cmd to the lock's region to resolve all locks belong to | |||
// the same transaction. | |||
func (lr *LockResolver) ResolveLocks(bo *Backoffer, locks []*Lock) (ok bool, err error) { | |||
func (lr *LockResolver) ResolveLocks(bo *Backoffer, locks []*Lock) (msBeforeTxnExpired int64, err error) { |
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.
msBeforeTxnExpired
stands for "milliseconds before transaction expired"?
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.
Seems yes.
LGTM |
@@ -102,8 +102,13 @@ func NewBackoffFn(base, cap, jitter int) func(ctx context.Context) int { | |||
logutil.Logger(context.Background()).Debug("backoff", | |||
zap.Int("base", base), | |||
zap.Int("sleep", sleep)) | |||
|
|||
realSleep := sleep | |||
if maxSleepMs >= 0 && realSleep > maxSleepMs { |
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.
When the maxSleepMs will be 0?
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.
maxSleepMs never should be 0 in current usage, but backoff is a common function and we add new maxSleepMs Param, so we should take care about it and in logic "force sleep zero" is useful.
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
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
/run-all-tests |
/run-unit-test |
/run-all-tests |
What problem does this PR solve?
we can precal lock expired time from kv response, then:
this PR maybe work better if tikv's tikv/tikv#4589 be merged(that decrease TTL)
What is changed and how it works?
calculate wait time by
UntilExpired
and force sleep not more than that.Check List
Tests
Code changes
Side effects
Related changes
This change is