From e2cfb7946c865461c041288a403fd0eb9b4e446a Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 23 Oct 2023 14:37:00 -0400 Subject: [PATCH] kv: tolerate lock acquisition replay with writes in same batch Informs #112221. Informs #112174. Informs #112173. Informs #111984. Informs #111893. Informs #111564. Informs #111530. This commit fixes the handling of replayed batches that contain a replicated lock acquisition and a later write to the same key. In such cases, the write at the higher sequence number should not be detected as an error during the replay. Instead, it should simply be ignored. Release note: None --- pkg/storage/mvcc.go | 20 ++++++++++++++++++- .../testdata/mvcc_histories/replicated_locks | 14 +++++++------ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go index 38d8567a0e17..c86bad0f3e81 100644 --- a/pkg/storage/mvcc.go +++ b/pkg/storage/mvcc.go @@ -5645,7 +5645,25 @@ func MVCCAcquireLock( // Acquiring at new epoch. rolledBack = true } else if foundLock.Txn.Sequence > txn.Sequence { - // Acquiring at same epoch and old sequence number. + // Acquiring at same epoch and an old sequence number. + // + // If the found lock has a different strength than the acquisition then we + // ignore it and continue. We are likely part of a replayed batch where a + // later request in the batch acquired a lock with a higher strength (or + // performed an intent write) on the same key. + if iterStr != str { + continue + } + // If the found lock has the same strength as the acquisition then this is + // an unexpected case. We are likely part of a replayed batch and either: + // 1. the lock was reacquired at a later sequence number and the minimum + // acquisition sequence number was not properly retained (bug!). See + // below about why we preserve the earliest non-rolled back sequence + // number for each lock strength. + // 2. this acquisition's sequence number was rolled back and the lock was + // subsequently acquired again at a higher sequence number. In such + // cases, we can return an error as the client is no longer waiting for + // a response. return errors.Errorf( "cannot acquire lock with strength %s at seq number %d, "+ "already held at higher seq number %d", diff --git a/pkg/storage/testdata/mvcc_histories/replicated_locks b/pkg/storage/testdata/mvcc_histories/replicated_locks index 660427221a78..74870779febf 100644 --- a/pkg/storage/testdata/mvcc_histories/replicated_locks +++ b/pkg/storage/testdata/mvcc_histories/replicated_locks @@ -920,7 +920,7 @@ stats: key_count=3 key_bytes=45 val_count=3 val_bytes=21 live_count=3 live_bytes # Replay lock acquisition with an acquisition with a stronger strength in the # same batch. -run stats error +run stats ok with t=C txn_step seq=1 acquire_lock k=k1 str=shared @@ -938,15 +938,16 @@ stats: no change stats: lock_count=+1 lock_bytes=+69 lock_age=+88 >> acquire_lock k=k1 str=shared t=C stats: no change +>> acquire_lock k=k1 str=exclusive t=C +stats: no change >> at end: -txn: "C" meta={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=1} lock=true stat=PENDING rts=12.000000000,0 wto=false gul=0,0 +txn: "C" meta={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=2} lock=true stat=PENDING rts=12.000000000,0 wto=false gul=0,0 lock (Replicated): "k1"/Exclusive -> txn={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=2} ts=12.000000000,0 del=false klen=0 vlen=0 mergeTs= txnDidNotUpdateMeta=true lock (Replicated): "k1"/Shared -> txn={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=1} ts=12.000000000,0 del=false klen=0 vlen=0 mergeTs= txnDidNotUpdateMeta=true stats: key_count=3 key_bytes=45 val_count=3 val_bytes=21 live_count=3 live_bytes=66 lock_count=2 lock_bytes=138 lock_age=176 -error: (*withstack.withStack:) cannot acquire lock with strength Shared at seq number 1, already held at higher seq number 2 # Replay lock acquisition with a write to the same key in the same batch. -run stats error +run stats ok with t=C txn_step seq=1 acquire_lock k=k2 str=shared @@ -964,8 +965,10 @@ stats: lock_count=+1 lock_bytes=+69 lock_age=+88 stats: key_bytes=+12 val_count=+1 val_bytes=+59 live_bytes=+52 gc_bytes_age=+1672 intent_count=+1 intent_bytes=+21 lock_count=+1 lock_age=+88 >> acquire_lock k=k2 str=shared t=C stats: no change +>> put k=k2 v=v2_2 t=C +stats: no change >> at end: -txn: "C" meta={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=1} lock=true stat=PENDING rts=12.000000000,0 wto=false gul=0,0 +txn: "C" meta={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=2} lock=true stat=PENDING rts=12.000000000,0 wto=false gul=0,0 data: "k1"/5.000000000,0 -> /BYTES/v1 meta: "k2"/0,0 -> txn={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=2} ts=12.000000000,0 del=false klen=12 vlen=9 mergeTs= txnDidNotUpdateMeta=true data: "k2"/12.000000000,0 -> /BYTES/v2_2 @@ -975,4 +978,3 @@ lock (Replicated): "k1"/Exclusive -> txn={id=00000003 key=/Min iso=Serializable lock (Replicated): "k1"/Shared -> txn={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=1} ts=12.000000000,0 del=false klen=0 vlen=0 mergeTs= txnDidNotUpdateMeta=true lock (Replicated): "k2"/Shared -> txn={id=00000003 key=/Min iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=1} ts=12.000000000,0 del=false klen=0 vlen=0 mergeTs= txnDidNotUpdateMeta=true stats: key_count=3 key_bytes=57 val_count=4 val_bytes=80 live_count=3 live_bytes=118 gc_bytes_age=1672 intent_count=1 intent_bytes=21 lock_count=4 lock_bytes=207 lock_age=352 -error: (*withstack.withStack:) cannot acquire lock with strength Shared at seq number 1, already held at higher seq number 2