Skip to content

Commit 39d519b

Browse files
craig[bot]stevendanna
andcommitted
Merge #148680
148680: kvserver,storage: allow sequence regression during lock flush r=arulajmani a=stevendanna Typically, when you acquire a replicated lock of the same or greater strength of an unreplicated lock, your unreplicated lock is freed from the lock table. However, this doesn't happen for all lock acquisition patterns. For instance, if the lock table is tracking a replicated lock for the key already, we do not free unreplicated locks for the new acquisition. However, even if we DID free such unreplicated locks, we may not want to in the future because we could be rolling back protection at a lower sequence number than the replicated lock. With our default settings, the above behaviour is fine. However, when we attempt to flush the unreplicated locks in the lock table to disk, we encounter an error: cannot acquire lock with strength Shared at seq number 0, already held at higher seq number 1 The comment around this error explains the two previous cases that justified this being an error. It assumed that any regression was either a replay or the result of a previous bug. Lock flushing produces a third case in which a regression of the sequence number is expected. Since sequence numbers are only used to track rolled back writes, it should be OK to allow it. But, since we only expect it in this single case, a new flag is added to MVCCAcquireLock for the caller to signal that they expect such a regression. Fixes #148674 Release note: None Co-authored-by: Steven Danna <[email protected]>
2 parents b0b5d89 + 2703877 commit 39d519b

17 files changed

+68
-44
lines changed

pkg/kv/kvserver/batcheval/cmd_lease.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func evalNewLease(
8484
} else {
8585
for _, acq := range acquisitions {
8686
if err := storage.MVCCAcquireLock(ctx, readWriter,
87-
&acq.Txn, acq.IgnoredSeqNums, acq.Strength, acq.Key, ms, 0, 0); err != nil {
87+
&acq.Txn, acq.IgnoredSeqNums, acq.Strength, acq.Key, ms, 0, 0, true /* allowSequenceNumberRegression */); err != nil {
8888
return newFailedLeaseTrigger(isTransfer), err
8989
}
9090
}

pkg/kv/kvserver/batcheval/cmd_query_resolved_timestamp_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestQueryResolvedTimestamp(t *testing.T) {
5353
}
5454
writeLock := func(k string, str lock.Strength) {
5555
txn := roachpb.MakeTransaction("test", roachpb.Key(k), 0, 0, makeTS(1), 0, 1, 0, false /* omitInRangefeeds */)
56-
err := storage.MVCCAcquireLock(ctx, db, &txn.TxnMeta, txn.IgnoredSeqNums, str, roachpb.Key(k), nil, 0, 0)
56+
err := storage.MVCCAcquireLock(ctx, db, &txn.TxnMeta, txn.IgnoredSeqNums, str, roachpb.Key(k), nil, 0, 0, false)
5757
require.NoError(t, err)
5858
}
5959

pkg/kv/kvserver/batcheval/cmd_subsume.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func Subsume(
199199
statsDelta := enginepb.MVCCStats{}
200200
for _, acq := range acquisitions {
201201
if err := storage.MVCCAcquireLock(ctx, readWriter,
202-
&acq.Txn, acq.IgnoredSeqNums, acq.Strength, acq.Key, &statsDelta, 0, 0); err != nil {
202+
&acq.Txn, acq.IgnoredSeqNums, acq.Strength, acq.Key, &statsDelta, 0, 0, true /* allowSequenceNumberRegression */); err != nil {
203203
return result.Result{}, err
204204
}
205205
}

pkg/kv/kvserver/batcheval/lock.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func acquireLockOnKey(
233233
// conflicts with un-contended replicated locks -- we need to do so before
234234
// we can acquire our own replicated lock; do that now, and also acquire
235235
// the replicated lock if no conflicts are found.
236-
if err := storage.MVCCAcquireLock(ctx, readWriter, &txn.TxnMeta, txn.IgnoredSeqNums, str, key, ms, maxLockConflicts, targetLockConflictBytes); err != nil {
236+
if err := storage.MVCCAcquireLock(ctx, readWriter, &txn.TxnMeta, txn.IgnoredSeqNums, str, key, ms, maxLockConflicts, targetLockConflictBytes, false); err != nil {
237237
return roachpb.LockAcquisition{}, err
238238
}
239239
default:

pkg/kv/kvserver/batcheval/lock_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,9 @@ func TestTxnBoundReplicatedLockTableView(t *testing.T) {
195195
txn2 := roachpb.MakeTransaction("txn2", keyA, isolation.Serializable, roachpb.NormalUserPriority, makeTS(100, 0), 0, 0, 0, false)
196196

197197
// Have txn1 acquire 2 locks with different strengths.
198-
err = storage.MVCCAcquireLock(ctx, engine, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Exclusive, keyA, nil, 0, 0)
198+
err = storage.MVCCAcquireLock(ctx, engine, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Exclusive, keyA, nil, 0, 0, false)
199199
require.NoError(t, err)
200-
err = storage.MVCCAcquireLock(ctx, engine, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Shared, keyB, nil, 0, 0)
200+
err = storage.MVCCAcquireLock(ctx, engine, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Shared, keyB, nil, 0, 0, false)
201201
require.NoError(t, err)
202202

203203
reader := engine.NewReader(storage.StandardDurability)

pkg/kv/kvserver/gc/gc_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,9 @@ func TestLockAgeThresholdSetting(t *testing.T) {
148148
require.NoError(t, err)
149149
// Acquire some shared and exclusive locks as well.
150150
for _, txn := range []*roachpb.Transaction{&txn1, &txn2} {
151-
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Shared, makeKey(local, lock.Shared), nil, 0, 0))
151+
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Shared, makeKey(local, lock.Shared), nil, 0, 0, false))
152152
}
153-
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Exclusive, makeKey(local, lock.Exclusive), nil, 0, 0))
153+
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Exclusive, makeKey(local, lock.Exclusive), nil, 0, 0, false))
154154
}
155155
require.NoError(t, eng.Flush())
156156

@@ -217,9 +217,9 @@ func TestIntentCleanupBatching(t *testing.T) {
217217
idx := i*len(objectKeys) + j
218218
switch idx % 3 {
219219
case 0:
220-
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Shared, key, nil, 0, 0))
220+
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Shared, key, nil, 0, 0, false))
221221
case 1:
222-
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Exclusive, key, nil, 0, 0))
222+
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Exclusive, key, nil, 0, 0, false))
223223
case 2:
224224
_, err := storage.MVCCPut(ctx, eng, key, intentHlc, value, storage.MVCCWriteOptions{Txn: &txn})
225225
require.NoError(t, err)

pkg/kv/kvserver/rangefeed/task_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ func TestInitResolvedTSScan(t *testing.T) {
311311
roachpb.MakeLock(&txn1.TxnMeta, roachpb.Key("p"), lock.Exclusive),
312312
}
313313
for _, l := range testLocks {
314-
err := storage.MVCCAcquireLock(ctx, engine, &txn1.TxnMeta, txn1.IgnoredSeqNums, l.Strength, l.Key, nil, 0, 0)
314+
err := storage.MVCCAcquireLock(ctx, engine, &txn1.TxnMeta, txn1.IgnoredSeqNums, l.Strength, l.Key, nil, 0, 0, false)
315315
require.NoError(t, err)
316316
}
317317
return engine

pkg/kv/kvserver/replica_consistency_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func TestReplicaChecksumSHA512(t *testing.T) {
328328
for i, l := range locks {
329329
txnID := uuid.FromUint128(uint128.FromInts(0, uint64(l.txnID)))
330330
txn := &roachpb.Transaction{TxnMeta: enginepb.TxnMeta{ID: txnID}}
331-
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, l.str, roachpb.Key(l.key), nil, 0, 0))
331+
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, l.str, roachpb.Key(l.key), nil, 0, 0, false))
332332

333333
rd, err = CalcReplicaDigest(ctx, desc, eng, kvpb.ChecksumMode_CHECK_FULL, unlim, nil /* settings */)
334334
require.NoError(t, err)

pkg/storage/bench_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,11 +1950,11 @@ func runMVCCAcquireLockCommon(
19501950
txn = &txn2
19511951
}
19521952
// Acquire a shared and an exclusive lock on the key.
1953-
err := MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Shared, key, nil, 0, 0)
1953+
err := MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Shared, key, nil, 0, 0, false)
19541954
if err != nil {
19551955
b.Fatal(err)
19561956
}
1957-
err = MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Exclusive, key, nil, 0, 0)
1957+
err = MVCCAcquireLock(ctx, eng, &txn.TxnMeta, txn.IgnoredSeqNums, lock.Exclusive, key, nil, 0, 0, false)
19581958
if err != nil {
19591959
b.Fatal(err)
19601960
}
@@ -1978,7 +1978,7 @@ func runMVCCAcquireLockCommon(
19781978
if checkFor {
19791979
err = MVCCCheckForAcquireLock(ctx, rw, txn, strength, key, 0, 0)
19801980
} else {
1981-
err = MVCCAcquireLock(ctx, rw, &txn.TxnMeta, txn.IgnoredSeqNums, strength, key, ms, 0, 0)
1981+
err = MVCCAcquireLock(ctx, rw, &txn.TxnMeta, txn.IgnoredSeqNums, strength, key, ms, 0, 0, false)
19821982
}
19831983
if heldOtherTxn {
19841984
if err == nil {

pkg/storage/engine_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,9 +1534,9 @@ func TestGetIntent(t *testing.T) {
15341534
// Key "b" has an intent, an exclusive lock, and a shared lock from txn1.
15351535
// NOTE: acquire in increasing strength order so that acquisition is never
15361536
// skipped.
1537-
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Shared, keyB, nil, 0, 0)
1537+
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Shared, keyB, nil, 0, 0, false)
15381538
require.NoError(t, err)
1539-
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Exclusive, keyB, nil, 0, 0)
1539+
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Exclusive, keyB, nil, 0, 0, false)
15401540
require.NoError(t, err)
15411541
_, err = MVCCPut(ctx, eng, keyB, txn1.ReadTimestamp, val, MVCCWriteOptions{Txn: txn1})
15421542
require.NoError(t, err)
@@ -1546,15 +1546,15 @@ func TestGetIntent(t *testing.T) {
15461546
require.NoError(t, err)
15471547

15481548
// Key "d" has an exclusive lock and a shared lock from txn2.
1549-
err = MVCCAcquireLock(ctx, eng, &txn2.TxnMeta, txn2.IgnoredSeqNums, lock.Shared, keyD, nil, 0, 0)
1549+
err = MVCCAcquireLock(ctx, eng, &txn2.TxnMeta, txn2.IgnoredSeqNums, lock.Shared, keyD, nil, 0, 0, false)
15501550
require.NoError(t, err)
1551-
err = MVCCAcquireLock(ctx, eng, &txn2.TxnMeta, txn2.IgnoredSeqNums, lock.Exclusive, keyD, nil, 0, 0)
1551+
err = MVCCAcquireLock(ctx, eng, &txn2.TxnMeta, txn2.IgnoredSeqNums, lock.Exclusive, keyD, nil, 0, 0, false)
15521552
require.NoError(t, err)
15531553

15541554
// Key "e" has a shared lock from each txn.
1555-
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Shared, keyE, nil, 0, 0)
1555+
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, lock.Shared, keyE, nil, 0, 0, false)
15561556
require.NoError(t, err)
1557-
err = MVCCAcquireLock(ctx, eng, &txn2.TxnMeta, txn2.IgnoredSeqNums, lock.Shared, keyE, nil, 0, 0)
1557+
err = MVCCAcquireLock(ctx, eng, &txn2.TxnMeta, txn2.IgnoredSeqNums, lock.Shared, keyE, nil, 0, 0, false)
15581558
require.NoError(t, err)
15591559

15601560
// Key "f" has no intent/locks.
@@ -1642,7 +1642,7 @@ func TestScanLocks(t *testing.T) {
16421642
if str == lock.Intent {
16431643
_, err = MVCCPut(ctx, eng, roachpb.Key(k), txn1.ReadTimestamp, roachpb.MakeValueFromBytes(roachpb.Key(k)), MVCCWriteOptions{Txn: txn1})
16441644
} else {
1645-
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, str, roachpb.Key(k), nil, 0, 0)
1645+
err = MVCCAcquireLock(ctx, eng, &txn1.TxnMeta, txn1.IgnoredSeqNums, str, roachpb.Key(k), nil, 0, 0, false)
16461646
}
16471647
require.NoError(t, err)
16481648
}
@@ -2211,11 +2211,11 @@ func TestScanConflictingIntentsForDroppingLatchesEarly(t *testing.T) {
22112211
setup: func(t *testing.T, rw ReadWriter, _ *roachpb.Transaction) {
22122212
txnA := newTxn(belowTxnTS)
22132213
txnB := newTxn(belowTxnTS)
2214-
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2214+
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22152215
require.NoError(t, err)
2216-
err = MVCCAcquireLock(ctx, rw, &txnB.TxnMeta, txnB.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2216+
err = MVCCAcquireLock(ctx, rw, &txnB.TxnMeta, txnB.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22172217
require.NoError(t, err)
2218-
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyB, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2218+
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyB, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22192219
require.NoError(t, err)
22202220
},
22212221
start: keyA,
@@ -2229,9 +2229,9 @@ func TestScanConflictingIntentsForDroppingLatchesEarly(t *testing.T) {
22292229
name: "shared and exclusive locks should be ignored no end key",
22302230
setup: func(t *testing.T, rw ReadWriter, _ *roachpb.Transaction) {
22312231
txnA := newTxn(belowTxnTS)
2232-
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2232+
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22332233
require.NoError(t, err)
2234-
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2234+
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22352235
require.NoError(t, err)
22362236
},
22372237
start: keyA,
@@ -2244,11 +2244,11 @@ func TestScanConflictingIntentsForDroppingLatchesEarly(t *testing.T) {
22442244
setup: func(t *testing.T, rw ReadWriter, _ *roachpb.Transaction) {
22452245
txnA := newTxn(belowTxnTS)
22462246
txnB := newTxn(belowTxnTS)
2247-
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2247+
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22482248
require.NoError(t, err)
2249-
err = MVCCAcquireLock(ctx, rw, &txnB.TxnMeta, txnB.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2249+
err = MVCCAcquireLock(ctx, rw, &txnB.TxnMeta, txnB.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22502250
require.NoError(t, err)
2251-
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyB, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2251+
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyB, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22522252
require.NoError(t, err)
22532253
require.NoError(t, err)
22542254
_, err = MVCCPut(ctx, rw, keyC, txnA.WriteTimestamp, val, MVCCWriteOptions{Txn: txnA})
@@ -2264,11 +2264,11 @@ func TestScanConflictingIntentsForDroppingLatchesEarly(t *testing.T) {
22642264
setup: func(t *testing.T, rw ReadWriter, txn *roachpb.Transaction) {
22652265
txnA := newTxn(belowTxnTS)
22662266
txnB := newTxn(belowTxnTS)
2267-
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2267+
err := MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22682268
require.NoError(t, err)
2269-
err = MVCCAcquireLock(ctx, rw, &txnB.TxnMeta, txnB.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2269+
err = MVCCAcquireLock(ctx, rw, &txnB.TxnMeta, txnB.IgnoredSeqNums, lock.Shared, keyA, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22702270
require.NoError(t, err)
2271-
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyB, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/)
2271+
err = MVCCAcquireLock(ctx, rw, &txnA.TxnMeta, txnA.IgnoredSeqNums, lock.Exclusive, keyB, nil /*ms*/, 0 /*maxConflicts*/, 0 /*targetLockConflictBytes*/, false)
22722272
require.NoError(t, err)
22732273
_, err = MVCCPut(ctx, rw, keyC, txn.WriteTimestamp, val, MVCCWriteOptions{Txn: txn})
22742274
require.NoError(t, err)

0 commit comments

Comments
 (0)