Add aggressive-locking mechanism and support locking with conflict #528
+966
−190
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
MyonKeminta
force-pushed
the
m/pessimistic-lock-optimization
branch
from
f11b130 to
552885c
Compare
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
12 tasks
Signed-off-by: MyonKeminta <[email protected]>
MyonKeminta
force-pushed
the
m/pessimistic-lock-optimization
branch
9 times, most recently
from
7241c1c to
7736f37
Compare
Signed-off-by: MyonKeminta <[email protected]>
MyonKeminta
force-pushed
the
m/pessimistic-lock-optimization
branch
from
7736f37 to
1078efa
Compare
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
MyonKeminta
changed the title
|
/race-test |
|
/run-race-test |
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
|
The tests look not sufficient to me now, but this PR is large enough. Considering that this PR is not expected to affect the original logic, I think I can add more tests in further PRs. |
30 tasks
cfzjywxk
reviewed
Nov 28, 2022
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
…simistic-lock-optimization
cfzjywxk
reviewed
Nov 30, 2022
Signed-off-by: MyonKeminta <[email protected]>
cfzjywxk
reviewed
Nov 30, 2022
txnkv/transaction/pessimistic.go
Outdated
Comment on lines
228
to
253
| if err != nil { | ||
| return true, err | ||
| } | ||
| if regionErr != nil { | ||
| // For other region error and the fake region error, backoff because | ||
| // there's something wrong. | ||
| // For the real EpochNotMatch error, don't backoff. | ||
| if regionErr.GetEpochNotMatch() == nil || locate.IsFakeRegionError(regionErr) { | ||
| err = bo.Backoff(retry.BoRegionMiss, errors.New(regionErr.String())) | ||
| if err != nil { | ||
| return true, err | ||
| } | ||
| } | ||
| same, err := batch.relocate(bo, c.store.GetRegionCache()) | ||
| if err != nil { | ||
| return true, err | ||
| } | ||
| if same { | ||
| return false, nil | ||
| } | ||
| err = c.pessimisticLockMutations(bo, action.LockCtx, action.wakeUpMode, batch.mutations) | ||
| return true, err | ||
| } | ||
| if resp.Resp == nil { | ||
| return true, errors.WithStack(tikverr.ErrBodyMissing) | ||
| } |
There was a problem hiding this comment.
There seem to be some duplicates with the implementation in handlePessimisticLockResponseForceMode, could it be abstracted to a single function?
There was a problem hiding this comment.
Ok. Btw I just realize that some of the code to handle the partially-successful cases is not removed. I'll remove them.
txnkv/transaction/pessimistic.go
Outdated
Comment on lines
294
to
311
| var locks []*txnlock.Lock | ||
| for _, keyErr := range keyErrs { | ||
| // Check already exists error | ||
| if alreadyExist := keyErr.GetAlreadyExist(); alreadyExist != nil { | ||
| e := &tikverr.ErrKeyExist{AlreadyExist: alreadyExist} | ||
| return true, c.extractKeyExistsErr(e) | ||
| } | ||
| if deadlock := keyErr.Deadlock; deadlock != nil { | ||
| return true, errors.WithStack(&tikverr.ErrDeadlock{Deadlock: deadlock}) | ||
| } | ||
|
|
||
| // Extract lock from key error | ||
| lock, err1 := txnlock.ExtractLockFromKeyErr(keyErr) | ||
| if err1 != nil { | ||
| return true, err1 | ||
| } | ||
| locks = append(locks, lock) | ||
| } |
There was a problem hiding this comment.
There seem to be some duplicates with the implementation in handlePessimisticLockResponseForceMode, could it be abstracted to a single function?
cfzjywxk
reviewed
Nov 30, 2022
| @@ -594,6 +650,185 @@ func (txn *KVTxn) LockKeysWithWaitTime(ctx context.Context, lockWaitTime int64, | |||
| return txn.LockKeys(ctx, lockCtx, keysInput...) | |||
| } | |||
|
|
|||
| // StartAggressiveLocking makes the transaction enters aggressive locking state. | |||
There was a problem hiding this comment.
The aggressive locked keys processing functions are complex, we may need more SQL tests to verify it's working as expected.
sticnarf
reviewed
Nov 30, 2022
txnkv/transaction/pessimistic.go
Outdated
|
|
||
| if retryMutations != nil && retryMutations.Len() < batch.mutations.Len() { | ||
| // The request may be partially succeeded. Replace the request with remaining mutations. | ||
| // The primary lock (if it's in this request) must have been succeeded. |
There was a problem hiding this comment.
Where is it ensured? (Though it's not important when we only enable this mode for single key)
There was a problem hiding this comment.
TiKV should ensure it. But since this kind of code is useless in current version, I'm now removing them.
… happen currently Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
cfzjywxk
approved these changes
Dec 1, 2022
|
@sticnarf PTAL |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires: tikv/tikv#12749
This PR adapts to the new behavior of acquire_pessimistic_lock requests in the PR above.
This PR is experimental and still under development.