txnkv: add new API for lock->put optimization #748
Conversation
Signed-off-by: zyguan <[email protected]>
| @@ -581,8 +606,8 @@ func (c *twoPhaseCommitter) initKeysAndMutations(ctx context.Context) error { | |||
| if c.txn.schemaAmender != nil || c.txn.assertionLevel == kvrpcpb.AssertionLevel_Off { | |||
| mustExist, mustNotExist, hasAssertUnknown = false, false, false | |||
| } | |||
| c.mutations.Push(op, isPessimistic, mustExist, mustNotExist, it.Handle()) | |||
| size += len(key) + len(value) | |||
| c.mutations.Push(op, isPessimistic, mustExist, mustNotExist, it.Handle(), cachedValue) | |||
There was a problem hiding this comment.
Some comments are left here, maybe avoiding using nil checking in the push function is safer.
There was a problem hiding this comment.
Push needs to take a parameter of type []byte as the lock->put value, since we do not overwrite membuf anymore (even on initKeysAndMutations).
| if m.overlay == nil { | ||
| m.overlay = make(map[unionstore.MemKeyHandle][]byte) | ||
| } | ||
| m.overlay[handle] = value |
There was a problem hiding this comment.
Maybe we could assert isPessimisticLock == true here?
There was a problem hiding this comment.
Do you mean panic on assertion failed?
There was a problem hiding this comment.
Yes, as it may prevent some data inconsistency risks.
integration_tests/2pc_test.go
Outdated
| mustLockKey := func(txn transaction.TxnProbe, key []byte) { | ||
| s.Require().NoError(txn.LockKeys(ctx, &kv.LockCtx{ForUpdateTS: txn.StartTS(), WaitStartTime: time.Now()}, key)) | ||
| } | ||
| mutOpVals := func(opVals ...interface{}) func(m transaction.CommitterMutations) { |
There was a problem hiding this comment.
I just have a question: what does mut mean here?
There was a problem hiding this comment.
It's the abbr of 'mutation'. The function means to create a check function from mutation op-val pairs. I just want to make the name shorter, but yes, it's little hard to understand.
integration_tests/2pc_test.go
Outdated
| func(txn transaction.TxnProbe) { mustLockKey(txn, k1) }, | ||
| func(txn transaction.TxnProbe) { s.Require().NoError(txn.Delete(k1)) }, | ||
| }, | ||
| mutOpVals(kvrpcpb.Op_Del, []byte{}), |
There was a problem hiding this comment.
Is it necessary to test in different order, like calling txn.SetLockedKeyValue after calling LockKeys or any mutations?
There was a problem hiding this comment.
In tidb, SetLockedKeyValue is always called after LockKeys in point-get/batch-point-get (like the following). But I think it's ok to test it in arbitrary orders.
LockKeys(...)
for each unique keys {
if uk not in membuf {
SetLockedKeyValue(uk, val)
}
}
Signed-off-by: zyguan <[email protected]>
Signed-off-by: zyguan <[email protected]>
This is a part of hotfix for pingcap/tidb#28011 . It allow us do lock->put optimization without touching membuf.