From a72cbf14e318aad970ee2bb33ba6db7648c3c727 Mon Sep 17 00:00:00 2001 From: MyonKeminta Date: Tue, 28 Mar 2023 13:33:09 +0800 Subject: [PATCH 1/3] Add exitAggressiveLockingIfInapplicable function and call within mutex of lockKeys Signed-off-by: MyonKeminta --- txnkv/transaction/txn.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/txnkv/transaction/txn.go b/txnkv/transaction/txn.go index 7074b967bc..698a2e84c0 100644 --- a/txnkv/transaction/txn.go +++ b/txnkv/transaction/txn.go @@ -861,6 +861,19 @@ func (txn *KVTxn) collectAggressiveLockingStats(lockCtx *tikv.LockCtx, keys int, lockCtx.Stats.AggressiveLockDerivedCount += filteredAggressiveLockedKeysCount } +func (txn *KVTxn) exitAggressiveLockingIfInapplicable(ctx context.Context, keys [][]byte) error { + if len(keys) > 1 && txn.IsInAggressiveLockingMode() { + // Only allow fair locking if it only needs to lock one key. Considering that it's possible that a + // statement causes multiple calls to `LockKeys` (which means some keys may have been locked in fair + // locking mode), here we exit fair locking mode by calling DoneFairLocking instead of cancelling. + // Then the previously-locked keys during execution in this statement (if any) will be turned into the state + // as if they were locked in normal way. + // Note that the issue https://github.com/pingcap/tidb/issues/35682 also exists here. + txn.DoneAggressiveLocking(ctx) + } + return nil +} + // LockKeys tries to lock the entries with the keys in KV store. // lockCtx is the context for lock, lockCtx.lockWaitTime in ms func (txn *KVTxn) LockKeys(ctx context.Context, lockCtx *tikv.LockCtx, keysInput ...[]byte) error { @@ -889,6 +902,12 @@ func (txn *KVTxn) lockKeys(ctx context.Context, lockCtx *tikv.LockCtx, fn func() startTime := time.Now() txn.mu.Lock() defer txn.mu.Unlock() + + err = txn.exitAggressiveLockingIfInapplicable(ctx, keysInput) + if err != nil { + return err + } + defer func() { if txn.isInternal() { metrics.TxnCmdHistogramWithLockKeysInternal.Observe(time.Since(startTime).Seconds()) From 7bb959d257194f9434df9b4e865a28d06232e85f Mon Sep 17 00:00:00 2001 From: MyonKeminta Date: Tue, 28 Mar 2023 14:23:30 +0800 Subject: [PATCH 2/3] fix fmt Signed-off-by: MyonKeminta --- txnkv/transaction/txn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txnkv/transaction/txn.go b/txnkv/transaction/txn.go index 698a2e84c0..92e38e74eb 100644 --- a/txnkv/transaction/txn.go +++ b/txnkv/transaction/txn.go @@ -902,7 +902,7 @@ func (txn *KVTxn) lockKeys(ctx context.Context, lockCtx *tikv.LockCtx, fn func() startTime := time.Now() txn.mu.Lock() defer txn.mu.Unlock() - + err = txn.exitAggressiveLockingIfInapplicable(ctx, keysInput) if err != nil { return err From 26e426c613f6bf1bc28f146ef929cf5686865152 Mon Sep 17 00:00:00 2001 From: MyonKeminta Date: Tue, 28 Mar 2023 15:26:36 +0800 Subject: [PATCH 3/3] Add test Signed-off-by: MyonKeminta --- integration_tests/2pc_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/integration_tests/2pc_test.go b/integration_tests/2pc_test.go index 28f0eeb6c2..cdce59af0d 100644 --- a/integration_tests/2pc_test.go +++ b/integration_tests/2pc_test.go @@ -1496,6 +1496,30 @@ func (s *testCommitterSuite) TestAggressiveLockingLoadValueOptionChanges() { } } +func (s *testCommitterSuite) TestAggressiveLockingExitIfInapplicable() { + txn := s.begin() + txn.SetPessimistic(true) + txn.StartAggressiveLocking() + s.True(txn.IsInAggressiveLockingMode()) + lockCtx := &kv.LockCtx{ForUpdateTS: txn.StartTS(), WaitStartTime: time.Now()} + s.NoError(txn.LockKeys(context.Background(), lockCtx, []byte("k1"))) + + txn.RetryAggressiveLocking(context.Background()) + lockCtx = &kv.LockCtx{ForUpdateTS: txn.StartTS(), WaitStartTime: time.Now()} + s.NoError(txn.LockKeys(context.Background(), lockCtx, []byte("k2"))) + s.True(txn.IsInAggressiveLockingMode()) + + lockCtx = &kv.LockCtx{ForUpdateTS: txn.StartTS(), WaitStartTime: time.Now()} + s.NoError(txn.LockKeys(context.Background(), lockCtx, []byte("k3"), []byte("k4"))) + s.False(txn.IsInAggressiveLockingMode()) + s.checkIsKeyLocked([]byte("k1"), false) + s.checkIsKeyLocked([]byte("k2"), true) + s.checkIsKeyLocked([]byte("k3"), true) + s.checkIsKeyLocked([]byte("k4"), true) + + s.NoError(txn.Rollback()) +} + // TestElapsedTTL tests that elapsed time is correct even if ts physical time is greater than local time. func (s *testCommitterSuite) TestElapsedTTL() { key := []byte("key")