-
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
executor: support innodb_lock_wait_timeout for pessimistic transaction #13103
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -447,6 +447,10 @@ type SessionVars struct { | |
isolationReadEngines map[kv.StoreType]struct{} | ||
|
||
PlannerSelectBlockAsName []ast.HintTable | ||
|
||
// LockWaitTimeout is the duration waiting for pessimistic lock in milliseconds | ||
// negative value means nowait, 0 means default behavior, others means actual wait time | ||
LockWaitTimeout int64 | ||
} | ||
|
||
// PreparedParams contains the parameters of the current prepared statement when executing it. | ||
|
@@ -524,6 +528,7 @@ func NewSessionVars() *SessionVars { | |
AllowRemoveAutoInc: DefTiDBAllowRemoveAutoInc, | ||
UsePlanBaselines: DefTiDBUsePlanBaselines, | ||
isolationReadEngines: map[kv.StoreType]struct{}{kv.TiKV: {}, kv.TiFlash: {}}, | ||
LockWaitTimeout: DefInnodbLockWaitTimeout * 1000, | ||
} | ||
vars.Concurrency = Concurrency{ | ||
IndexLookupConcurrency: DefIndexLookupConcurrency, | ||
|
@@ -812,6 +817,9 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { | |
case MaxExecutionTime: | ||
timeoutMS := tidbOptPositiveInt32(val, 0) | ||
s.MaxExecutionTime = uint64(timeoutMS) | ||
case InnodbLockWaitTimeout: | ||
lockWaitSec := tidbOptInt64(val, DefInnodbLockWaitTimeout) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to validate the range. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is done in function
|
||
s.LockWaitTimeout = int64(lockWaitSec * 1000) | ||
case TiDBSkipUTF8Check: | ||
s.SkipUTF8Check = TiDBOptOn(val) | ||
case TiDBOptAggPushDown: | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -676,7 +676,17 @@ func (action actionPessimisticLock) handleSingleBatch(c *twoPhaseCommitter, bo * | |||||
IsFirstLock: c.isFirstLock, | ||||||
WaitTimeout: action.lockWaitTime, | ||||||
}, pb.Context{Priority: c.priority, SyncLog: c.syncLog}) | ||||||
lockWaitStartTime := time.Now() | ||||||
for { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to update the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. roger |
||||||
// if lockWaitTime set, refine the request `WaitTimeout` field based on timeout limit | ||||||
if action.lockWaitTime > 0 { | ||||||
timeLeft := action.lockWaitTime - (time.Since(lockWaitStartTime)).Milliseconds() | ||||||
if timeLeft <= 0 { | ||||||
req.PessimisticLock().WaitTimeout = kv.LockNoWait | ||||||
} else { | ||||||
req.PessimisticLock().WaitTimeout = timeLeft | ||||||
} | ||||||
} | ||||||
resp, err := c.store.SendReq(bo, req, batch.region, readTimeoutShort) | ||||||
if err != nil { | ||||||
return errors.Trace(err) | ||||||
|
@@ -726,15 +736,28 @@ func (action actionPessimisticLock) handleSingleBatch(c *twoPhaseCommitter, bo * | |||||
// if the lock left behind whose related txn is already committed or rollbacked, | ||||||
// (eg secondary locks not committed or rollbacked yet) | ||||||
// we cant return "nowait conflict" directly | ||||||
if action.lockWaitTime == kv.LockNoWait && lock.LockType == pb.Op_PessimisticLock { | ||||||
// the pessimistic lock found could be lock left behind(timeout but not recycled yet) | ||||||
if !c.store.oracle.IsExpired(lock.TxnID, lock.TTL) { | ||||||
return ErrLockAcquireFailAndNoWaitSet | ||||||
if lock.LockType == pb.Op_PessimisticLock { | ||||||
if action.lockWaitTime == kv.LockNoWait { | ||||||
// the pessimistic lock found could be invalid locks which is timeout but not recycled yet | ||||||
if !c.store.oracle.IsExpired(lock.TxnID, lock.TTL) { | ||||||
return ErrLockAcquireFailAndNoWaitSet | ||||||
} | ||||||
} else if action.lockWaitTime == kv.LockAlwaysWait { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to handle There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coocood some inner usages which calls There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about
Suggested change
then do something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. current we have 3 paths(negative, 0, positive value), I think it's more clear to make them like noodle style, and we just need to add/remove one more |
||||||
// do nothing but keep wait | ||||||
} else { | ||||||
// the lockWaitTime is set, check the lock wait timeout or not | ||||||
// the pessimistic lock found could be invalid locks which is timeout but not recycled yet | ||||||
if !c.store.oracle.IsExpired(lock.TxnID, lock.TTL) { | ||||||
if time.Since(lockWaitStartTime).Milliseconds() >= action.lockWaitTime { | ||||||
return ErrLockWaitTimeout | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
locks = append(locks, lock) | ||||||
} | ||||||
// Because we already waited on tikv, no need to Backoff here. | ||||||
// tikv default will wait 3s(also the maximum wait value) when lock error occurs | ||||||
_, err = c.store.lockResolver.ResolveLocks(bo, c.startTS, locks) | ||||||
if err != nil { | ||||||
return errors.Trace(err) | ||||||
|
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.
why set autocommit here?
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.
set this before next
begin
statement below