-
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 select for update no wait #12775
Changes from 23 commits
f242af2
9e8d6cf
70dc1eb
d2bc394
c103a77
b69efa6
3f4205b
d5ef7bf
38c5caf
16616e0
99935c3
34dccb9
a70b11f
5c52dbd
1a4d0b3
4afc621
de469ca
31f2c67
fddc2e8
119a307
09bdcb2
4565b05
7f73591
99bada6
afa7bea
4b3dfce
edcaf66
16b26c4
cd7151a
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 |
---|---|---|
|
@@ -169,7 +169,7 @@ type Transaction interface { | |
// String implements fmt.Stringer interface. | ||
String() string | ||
// LockKeys tries to lock the entries with the keys in KV store. | ||
LockKeys(ctx context.Context, killed *uint32, forUpdateTS uint64, keys ...Key) error | ||
LockKeys(ctx context.Context, killed *uint32, forUpdateTS uint64, lockWaitTime uint64, keys ...Key) error | ||
// SetOption sets an option with a value, when val is nil, uses the default | ||
// value of this option. | ||
SetOption(opt Option, val interface{}) | ||
|
@@ -366,3 +366,11 @@ type SplitableStore interface { | |
WaitScatterRegionFinish(regionID uint64, backOff int) error | ||
CheckRegionInScattering(regionID uint64) (bool, error) | ||
} | ||
|
||
// Used for pessimistic lock wait time | ||
// these two constants are special for lock protocol with tikv | ||
// 0 means always wait, 1 means nowait, others meaning lock wait in milliseconds | ||
var ( | ||
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. A gentle remind, better code style here is defining a type like this:
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, the |
||
LockAlwaysWait = uint64(0) | ||
LockNoWait = uint64(1) | ||
) |
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.
if fail happened here,
UpdateTS
will not be updated, and the previous async rollback is possiblely ongoing,still risk that the later
select for update
rollbacked ? @coocoodThere 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.
This error rarely happens, so even if this happened, the async rollback causes the transaction to fail, it is acceptable.
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.
@tiancaiamao @coocood @jackysp
I'll add and use
getWithRetry
, but I think here still risk withnowait
. User get this statement result error, next statement pessimistic lockgetForUpdateTs
not updated(select for update, update, delete operations), ths "async rollback" risk still exists, how should we process the error here?Or maybe we make rollback sync for "nowait" locked 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.
sync rollback may still send duplicated requests later.