diff --git a/pkg/kv/kvserver/batch_spanset_test.go b/pkg/kv/kvserver/batch_spanset_test.go index 682bccad5a00..b6f697dba1ab 100644 --- a/pkg/kv/kvserver/batch_spanset_test.go +++ b/pkg/kv/kvserver/batch_spanset_test.go @@ -563,8 +563,8 @@ func TestSpanSetMVCCResolveWriteIntentRange(t *testing.T) { Txn: enginepb.TxnMeta{}, // unused Status: roachpb.PENDING, } - if _, _, err := storage.MVCCResolveWriteIntentRange( - ctx, batch, nil /* ms */, intent, 0, + if _, _, _, _, err := storage.MVCCResolveWriteIntentRange( + ctx, batch, nil /* ms */, intent, storage.MVCCResolveWriteIntentRangeOptions{}, ); err != nil { t.Fatal(err) } diff --git a/pkg/kv/kvserver/batcheval/cmd_end_transaction.go b/pkg/kv/kvserver/batcheval/cmd_end_transaction.go index 6238027e7972..5b5954ee2855 100644 --- a/pkg/kv/kvserver/batcheval/cmd_end_transaction.go +++ b/pkg/kv/kvserver/batcheval/cmd_end_transaction.go @@ -554,7 +554,8 @@ func resolveLocalLocks( // // Note that the underlying pebbleIterator will still be reused // since readWriter is a pebbleBatch in the typical case. - ok, err := storage.MVCCResolveWriteIntent(ctx, readWriter, ms, update) + ok, _, _, err := storage.MVCCResolveWriteIntent(ctx, readWriter, ms, update, + storage.MVCCResolveWriteIntentOptions{}) if err != nil { return err } @@ -579,15 +580,15 @@ func resolveLocalLocks( externalLocks = append(externalLocks, outSpans...) if inSpan != nil { update.Span = *inSpan - num, resumeSpan, err := storage.MVCCResolveWriteIntentRange( - ctx, readWriter, ms, update, resolveAllowance) + numKeys, _, resumeSpan, _, err := storage.MVCCResolveWriteIntentRange(ctx, readWriter, ms, update, + storage.MVCCResolveWriteIntentRangeOptions{MaxKeys: resolveAllowance}) if err != nil { return err } if evalCtx.EvalKnobs().NumKeysEvaluatedForRangeIntentResolution != nil { - atomic.AddInt64(evalCtx.EvalKnobs().NumKeysEvaluatedForRangeIntentResolution, num) + atomic.AddInt64(evalCtx.EvalKnobs().NumKeysEvaluatedForRangeIntentResolution, numKeys) } - resolveAllowance -= num + resolveAllowance -= numKeys if resumeSpan != nil { if resolveAllowance != 0 { log.Fatalf(ctx, "expected resolve allowance to be exactly 0 resolving %s; got %d", update.Span, resolveAllowance) diff --git a/pkg/kv/kvserver/batcheval/cmd_refresh_range_test.go b/pkg/kv/kvserver/batcheval/cmd_refresh_range_test.go index ac4bba3eebbb..7300c51955ff 100644 --- a/pkg/kv/kvserver/batcheval/cmd_refresh_range_test.go +++ b/pkg/kv/kvserver/batcheval/cmd_refresh_range_test.go @@ -173,7 +173,7 @@ func TestRefreshRangeTimeBoundIterator(t *testing.T) { // would not have any timestamp bounds and would be selected for every read. intent := roachpb.MakeLockUpdate(txn, roachpb.Span{Key: k}) intent.Status = roachpb.COMMITTED - if _, err := storage.MVCCResolveWriteIntent(ctx, db, nil, intent); err != nil { + if _, _, _, err := storage.MVCCResolveWriteIntent(ctx, db, nil, intent, storage.MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } if err := storage.MVCCPut(ctx, db, nil, roachpb.Key("unused2"), ts1, hlc.ClockTimestamp{}, v, nil); err != nil { @@ -272,7 +272,7 @@ func TestRefreshRangeError(t *testing.T) { if resolveIntent { intent := roachpb.MakeLockUpdate(txn, roachpb.Span{Key: k}) intent.Status = roachpb.COMMITTED - if _, err := storage.MVCCResolveWriteIntent(ctx, db, nil, intent); err != nil { + if _, _, _, err := storage.MVCCResolveWriteIntent(ctx, db, nil, intent, storage.MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } } diff --git a/pkg/kv/kvserver/batcheval/cmd_refresh_test.go b/pkg/kv/kvserver/batcheval/cmd_refresh_test.go index 56ee5ee25394..89a9a664633b 100644 --- a/pkg/kv/kvserver/batcheval/cmd_refresh_test.go +++ b/pkg/kv/kvserver/batcheval/cmd_refresh_test.go @@ -66,7 +66,7 @@ func TestRefreshError(t *testing.T) { if resolveIntent { intent := roachpb.MakeLockUpdate(txn, roachpb.Span{Key: k}) intent.Status = roachpb.COMMITTED - if _, err := storage.MVCCResolveWriteIntent(ctx, db, nil, intent); err != nil { + if _, _, _, err := storage.MVCCResolveWriteIntent(ctx, db, nil, intent, storage.MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } } diff --git a/pkg/kv/kvserver/batcheval/cmd_resolve_intent.go b/pkg/kv/kvserver/batcheval/cmd_resolve_intent.go index 9a7a8c02d2a9..fe5e255812e9 100644 --- a/pkg/kv/kvserver/batcheval/cmd_resolve_intent.go +++ b/pkg/kv/kvserver/batcheval/cmd_resolve_intent.go @@ -93,7 +93,8 @@ func ResolveIntent( // The observation was from the wrong node. Ignore. update.ClockWhilePending = roachpb.ObservedTimestamp{} } - ok, err := storage.MVCCResolveWriteIntent(ctx, readWriter, ms, update) + ok, _, _, err := storage.MVCCResolveWriteIntent(ctx, readWriter, ms, update, + storage.MVCCResolveWriteIntentOptions{}) if err != nil { return result.Result{}, err } diff --git a/pkg/kv/kvserver/batcheval/cmd_resolve_intent_range.go b/pkg/kv/kvserver/batcheval/cmd_resolve_intent_range.go index 1cf59d7f87ee..2e8d202b213f 100644 --- a/pkg/kv/kvserver/batcheval/cmd_resolve_intent_range.go +++ b/pkg/kv/kvserver/batcheval/cmd_resolve_intent_range.go @@ -52,8 +52,8 @@ func ResolveIntentRange( // The observation was from the wrong node. Ignore. update.ClockWhilePending = roachpb.ObservedTimestamp{} } - numKeys, resumeSpan, err := storage.MVCCResolveWriteIntentRange( - ctx, readWriter, ms, update, h.MaxSpanRequestKeys) + numKeys, _, resumeSpan, _, err := storage.MVCCResolveWriteIntentRange(ctx, readWriter, ms, update, + storage.MVCCResolveWriteIntentRangeOptions{MaxKeys: h.MaxSpanRequestKeys}) if err != nil { return result.Result{}, err } diff --git a/pkg/kv/kvserver/gc/gc_random_test.go b/pkg/kv/kvserver/gc/gc_random_test.go index f6b3d74fd97f..c87bfdf183c3 100644 --- a/pkg/kv/kvserver/gc/gc_random_test.go +++ b/pkg/kv/kvserver/gc/gc_random_test.go @@ -314,7 +314,7 @@ func TestNewVsInvariants(t *testing.T) { Txn: i.Txn, Status: roachpb.ABORTED, } - _, err := storage.MVCCResolveWriteIntent(ctx, eng, &stats, l) + _, _, _, err := storage.MVCCResolveWriteIntent(ctx, eng, &stats, l, storage.MVCCResolveWriteIntentOptions{}) require.NoError(t, err, "failed to resolve intent") } for _, cr := range gcer.clearRanges() { diff --git a/pkg/kv/kvserver/loqrecovery/apply.go b/pkg/kv/kvserver/loqrecovery/apply.go index 0dc1df5ed174..14d18e9de29e 100644 --- a/pkg/kv/kvserver/loqrecovery/apply.go +++ b/pkg/kv/kvserver/loqrecovery/apply.go @@ -264,7 +264,7 @@ func applyReplicaUpdate( Txn: intent.Txn, Status: roachpb.ABORTED, } - if _, err := storage.MVCCResolveWriteIntent(ctx, readWriter, &ms, update); err != nil { + if _, _, _, err := storage.MVCCResolveWriteIntent(ctx, readWriter, &ms, update, storage.MVCCResolveWriteIntentOptions{}); err != nil { return PrepareReplicaReport{}, err } report.AbortedTransaction = true diff --git a/pkg/storage/bench_data_test.go b/pkg/storage/bench_data_test.go index 033c9cef9dfc..6914cc0305d6 100644 --- a/pkg/storage/bench_data_test.go +++ b/pkg/storage/bench_data_test.go @@ -353,11 +353,11 @@ func (d mvccBenchData) Build(ctx context.Context, b *testing.B, eng Engine) erro key := keySlice[idx] txnMeta := txn.TxnMeta txnMeta.WriteTimestamp = hlc.Timestamp{WallTime: int64(counts[idx]) * 5} - if _, err := MVCCResolveWriteIntent(ctx, batch, nil /* ms */, roachpb.LockUpdate{ + if _, _, _, err := MVCCResolveWriteIntent(ctx, batch, nil /* ms */, roachpb.LockUpdate{ Span: roachpb.Span{Key: key}, Status: roachpb.COMMITTED, Txn: txnMeta, - }); err != nil { + }, MVCCResolveWriteIntentOptions{}); err != nil { b.Fatal(err) } } diff --git a/pkg/storage/bench_test.go b/pkg/storage/bench_test.go index 3c58af1bac28..5db7a6243aae 100644 --- a/pkg/storage/bench_test.go +++ b/pkg/storage/bench_test.go @@ -334,7 +334,7 @@ func setupKeysWithIntent( // is not one that should be resolved. continue } - found, err := MVCCResolveWriteIntent(context.Background(), batch, nil, lu) + found, _, _, err := MVCCResolveWriteIntent(context.Background(), batch, nil, lu, MVCCResolveWriteIntentOptions{}) require.Equal(b, true, found) require.NoError(b, err) } @@ -553,7 +553,7 @@ func BenchmarkIntentResolution(b *testing.B) { b.StartTimer() } lockUpdate.Key = keys[i%numIntentKeys] - found, err := MVCCResolveWriteIntent(context.Background(), batch, nil, lockUpdate) + found, _, _, err := MVCCResolveWriteIntent(context.Background(), batch, nil, lockUpdate, MVCCResolveWriteIntentOptions{}) if !found || err != nil { b.Fatalf("intent not found or err %s", err) } @@ -613,8 +613,9 @@ func BenchmarkIntentRangeResolution(b *testing.B) { rangeNum := i % numRanges lockUpdate.Key = keys[rangeNum*numKeysPerRange] lockUpdate.EndKey = keys[(rangeNum+1)*numKeysPerRange] - resolved, span, err := MVCCResolveWriteIntentRange( - context.Background(), batch, nil, lockUpdate, 1000 /* max */) + resolved, _, span, _, err := MVCCResolveWriteIntentRange( + context.Background(), batch, nil, lockUpdate, + MVCCResolveWriteIntentRangeOptions{MaxKeys: 1000}) if err != nil { b.Fatal(err) } diff --git a/pkg/storage/metamorphic/operations.go b/pkg/storage/metamorphic/operations.go index 0105dd19ee2e..8550a12bcecf 100644 --- a/pkg/storage/metamorphic/operations.go +++ b/pkg/storage/metamorphic/operations.go @@ -485,7 +485,7 @@ func (t txnCommitOp) run(ctx context.Context) string { for _, span := range txn.LockSpans { intent := roachpb.MakeLockUpdate(txn, span) intent.Status = roachpb.COMMITTED - _, err := storage.MVCCResolveWriteIntent(context.TODO(), t.m.engine, nil, intent) + _, _, _, err := storage.MVCCResolveWriteIntent(context.TODO(), t.m.engine, nil, intent, storage.MVCCResolveWriteIntentOptions{}) if err != nil { panic(err) } @@ -508,7 +508,7 @@ func (t txnAbortOp) run(ctx context.Context) string { for _, span := range txn.LockSpans { intent := roachpb.MakeLockUpdate(txn, span) intent.Status = roachpb.ABORTED - _, err := storage.MVCCResolveWriteIntent(context.TODO(), t.m.engine, nil, intent) + _, _, _, err := storage.MVCCResolveWriteIntent(context.TODO(), t.m.engine, nil, intent, storage.MVCCResolveWriteIntentOptions{}) if err != nil { panic(err) } diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go index 01978fe2329e..03435d2a2d4c 100644 --- a/pkg/storage/mvcc.go +++ b/pkg/storage/mvcc.go @@ -922,6 +922,21 @@ func (opts *MVCCGetOptions) errOnIntents() bool { return !opts.Inconsistent && !opts.SkipLocked } +// MVCCResolveWriteIntentOptions bundles options for the MVCCResolveWriteIntent +// function. +type MVCCResolveWriteIntentOptions struct { + // See the documentation for MVCCResolveWriteIntent for information on these + // parameters. +} + +// MVCCResolveWriteIntentRangeOptions bundles options for the +// MVCCResolveWriteIntentRange function. +type MVCCResolveWriteIntentRangeOptions struct { + // See the documentation for MVCCResolveWriteIntentRange for information on + // these parameters. + MaxKeys int64 +} + // newMVCCIterator sets up a suitable iterator for high-level MVCC operations // operating at the given timestamp. If timestamp is empty or if // `noInterleavedIntents` is set, the iterator is considered to be used for @@ -3973,7 +3988,9 @@ func MVCCIterate( // MVCCResolveWriteIntent either commits, aborts (rolls back), or moves forward // in time an extant write intent for a given txn according to commit parameter. // ResolveWriteIntent will skip write intents of other txns. It returns -// whether or not an intent was found to resolve. +// whether or not an intent was found to resolve. Note that the numBytes and +// resumeSpan return values are currently unused and serve as a placeholder in +// refactoring, but will be used in the future. // // Transaction epochs deserve a bit of explanation. The epoch for a // transaction is incremented on transaction retries. A transaction @@ -3991,13 +4008,17 @@ func MVCCIterate( // epoch matching the commit epoch), and which intents get aborted, // even if the transaction succeeds. func MVCCResolveWriteIntent( - ctx context.Context, rw ReadWriter, ms *enginepb.MVCCStats, intent roachpb.LockUpdate, -) (bool, error) { + ctx context.Context, + rw ReadWriter, + ms *enginepb.MVCCStats, + intent roachpb.LockUpdate, + opts MVCCResolveWriteIntentOptions, +) (ok bool, numBytes int64, resumeSpan *roachpb.Span, err error) { if len(intent.Key) == 0 { - return false, emptyKeyError() + return false, 0, nil, emptyKeyError() } if len(intent.EndKey) > 0 { - return false, errors.Errorf("can't resolve range intent as point intent") + return false, 0, nil, errors.Errorf("can't resolve range intent as point intent") } iterAndBuf := GetBufUsingIter(rw.NewMVCCIterator(MVCCKeyAndIntentsIterKind, IterOptions{ @@ -4005,10 +4026,10 @@ func MVCCResolveWriteIntent( Prefix: true, })) iterAndBuf.iter.SeekIntentGE(intent.Key, intent.Txn.ID) - ok, err := mvccResolveWriteIntent(ctx, rw, iterAndBuf.iter, ms, intent, iterAndBuf.buf) + ok, err = mvccResolveWriteIntent(ctx, rw, iterAndBuf.iter, ms, intent, iterAndBuf.buf) // Using defer would be more convenient, but it is measurably slower. iterAndBuf.Cleanup() - return ok, err + return ok, 0, nil, err } // iterForKeyVersions provides a subset of the functionality of MVCCIterator. @@ -4771,13 +4792,24 @@ func (b IterAndBuf) Cleanup() { // ResolveWriteIntentRange will skip write intents of other txns. A max of zero // means unbounded. A max of -1 means resolve nothing and returns the entire // intent span as the resume span. Returns the number of intents resolved and a -// resume span if the max keys limit was exceeded. +// resume span if the max keys limit was exceeded. Note that the numBytes and +// resumeReason return values are currently unused and serve as a placeholder +// in refactoring, but will be used in the future. func MVCCResolveWriteIntentRange( - ctx context.Context, rw ReadWriter, ms *enginepb.MVCCStats, intent roachpb.LockUpdate, max int64, -) (int64, *roachpb.Span, error) { - if max < 0 { + ctx context.Context, + rw ReadWriter, + ms *enginepb.MVCCStats, + intent roachpb.LockUpdate, + opts MVCCResolveWriteIntentRangeOptions, +) ( + numKeys, numBytes int64, + resumeSpan *roachpb.Span, + resumeReason roachpb.ResumeReason, + err error, +) { + if opts.MaxKeys < 0 { resumeSpan := intent.Span // don't inline or `intent` would escape to heap - return 0, &resumeSpan, nil + return 0, 0, &resumeSpan, 0, nil } ltStart, _ := keys.LockTableSingleKey(intent.Key, nil) ltEnd, _ := keys.LockTableSingleKey(intent.EndKey, nil) @@ -4814,25 +4846,24 @@ func MVCCResolveWriteIntentRange( intent.EndKey = nil var lastResolvedKey roachpb.Key - num := int64(0) for { if valid, err := sepIter.Valid(); err != nil { - return 0, nil, err + return 0, 0, nil, 0, err } else if !valid { // No more intents in the given range. break } - if max > 0 && num == max { + if opts.MaxKeys > 0 && numKeys == opts.MaxKeys { // We could also compute a tighter nextKey here if we wanted to. - return num, &roachpb.Span{Key: lastResolvedKey.Next(), EndKey: intentEndKey}, nil + return numKeys, 0, &roachpb.Span{Key: lastResolvedKey.Next(), EndKey: intentEndKey}, 0, nil } // Parse the MVCCMetadata to see if it is a relevant intent. meta := &putBuf.meta if err := sepIter.ValueProto(meta); err != nil { - return 0, nil, err + return 0, 0, nil, 0, err } if meta.Txn == nil { - return 0, nil, errors.Errorf("intent with no txn") + return 0, 0, nil, 0, errors.Errorf("intent with no txn") } if intent.Txn.ID != meta.Txn.ID { // Intent for a different txn, so ignore. @@ -4852,11 +4883,11 @@ func MVCCResolveWriteIntentRange( if err != nil { log.Warningf(ctx, "failed to resolve intent for key %q: %+v", lastResolvedKey, err) } else if ok { - num++ + numKeys++ } sepIter.nextEngineKey() } - return num, nil, nil + return numKeys, 0, nil, 0, nil } // MVCCGarbageCollect creates an iterator on the ReadWriter. In parallel diff --git a/pkg/storage/mvcc_history_test.go b/pkg/storage/mvcc_history_test.go index dc931c979183..b25bb0cbca38 100644 --- a/pkg/storage/mvcc_history_test.go +++ b/pkg/storage/mvcc_history_test.go @@ -890,7 +890,7 @@ func cmdResolveIntentRange(e *evalCtx) error { intent.Status = status return e.withWriter("resolve_intent_range", func(rw storage.ReadWriter) error { - _, _, err := storage.MVCCResolveWriteIntentRange(e.ctx, rw, e.ms, intent, 0) + _, _, _, _, err := storage.MVCCResolveWriteIntentRange(e.ctx, rw, e.ms, intent, storage.MVCCResolveWriteIntentRangeOptions{}) return err }) } @@ -905,7 +905,7 @@ func (e *evalCtx) resolveIntent( intent := roachpb.MakeLockUpdate(txn, roachpb.Span{Key: key}) intent.Status = resolveStatus intent.ClockWhilePending = roachpb.ObservedTimestamp{Timestamp: clockWhilePending} - _, err := storage.MVCCResolveWriteIntent(e.ctx, rw, e.ms, intent) + _, _, _, err := storage.MVCCResolveWriteIntent(e.ctx, rw, e.ms, intent, storage.MVCCResolveWriteIntentOptions{}) return err } diff --git a/pkg/storage/mvcc_incremental_iterator_test.go b/pkg/storage/mvcc_incremental_iterator_test.go index dddf6827cebf..296beb383444 100644 --- a/pkg/storage/mvcc_incremental_iterator_test.go +++ b/pkg/storage/mvcc_incremental_iterator_test.go @@ -1006,12 +1006,12 @@ func TestMVCCIncrementalIterator(t *testing.T) { intent1 := roachpb.MakeLockUpdate(&txn1, roachpb.Span{Key: testKey1}) intent1.Status = roachpb.COMMITTED - if _, err := MVCCResolveWriteIntent(ctx, e, nil, intent1); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, e, nil, intent1, MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } intent2 := roachpb.MakeLockUpdate(&txn2, roachpb.Span{Key: testKey2}) intent2.Status = roachpb.ABORTED - if _, err := MVCCResolveWriteIntent(ctx, e, nil, intent2); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, e, nil, intent2, MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } t.Run("intents-resolved", assertEqualKVs(e, localMax, keyMax, tsMin, tsMax, latest, kvs(kv1_4_4, kv2_2_2))) @@ -1073,12 +1073,12 @@ func TestMVCCIncrementalIterator(t *testing.T) { intent1 := roachpb.MakeLockUpdate(&txn1, roachpb.Span{Key: testKey1}) intent1.Status = roachpb.COMMITTED - if _, err := MVCCResolveWriteIntent(ctx, e, nil, intent1); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, e, nil, intent1, MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } intent2 := roachpb.MakeLockUpdate(&txn2, roachpb.Span{Key: testKey2}) intent2.Status = roachpb.ABORTED - if _, err := MVCCResolveWriteIntent(ctx, e, nil, intent2); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, e, nil, intent2, MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } t.Run("intents-resolved", assertEqualKVs(e, localMax, keyMax, tsMin, tsMax, all, kvs(kv1_4_4, kv1Deleted3, kv1_2_2, kv1_1_1, kv2_2_2))) @@ -1261,9 +1261,9 @@ func TestMVCCIncrementalIteratorIntentDeletion(t *testing.T) { require.NoError(t, MVCCPut(ctx, db, nil, kC, txnC1.ReadTimestamp, hlc.ClockTimestamp{}, vC1, txnC1)) require.NoError(t, db.Flush()) require.NoError(t, db.Compact()) - _, err := MVCCResolveWriteIntent(ctx, db, nil, intent(txnA1)) + _, _, _, err := MVCCResolveWriteIntent(ctx, db, nil, intent(txnA1), MVCCResolveWriteIntentOptions{}) require.NoError(t, err) - _, err = MVCCResolveWriteIntent(ctx, db, nil, intent(txnB1)) + _, _, _, err = MVCCResolveWriteIntent(ctx, db, nil, intent(txnB1), MVCCResolveWriteIntentOptions{}) require.NoError(t, err) require.NoError(t, MVCCPut(ctx, db, nil, kA, ts2, hlc.ClockTimestamp{}, vA2, nil)) require.NoError(t, MVCCPut(ctx, db, nil, kA, txnA3.WriteTimestamp, hlc.ClockTimestamp{}, vA3, txnA3)) diff --git a/pkg/storage/mvcc_logical_ops_test.go b/pkg/storage/mvcc_logical_ops_test.go index b045c1b3e3ed..537eb700d6f0 100644 --- a/pkg/storage/mvcc_logical_ops_test.go +++ b/pkg/storage/mvcc_logical_ops_test.go @@ -12,7 +12,6 @@ package storage import ( "context" - "math" "strings" "testing" @@ -73,18 +72,18 @@ func TestMVCCOpLogWriter(t *testing.T) { // Resolve all three intent. txn1CommitTS := *txn1Commit txn1CommitTS.WriteTimestamp = hlc.Timestamp{Logical: 4} - if _, _, err := MVCCResolveWriteIntentRange(ctx, ol, nil, + if _, _, _, _, err := MVCCResolveWriteIntentRange(ctx, ol, nil, roachpb.MakeLockUpdate( &txn1CommitTS, roachpb.Span{Key: testKey1, EndKey: testKey2.Next()}), - math.MaxInt64); err != nil { + MVCCResolveWriteIntentRangeOptions{}); err != nil { t.Fatal(err) } - if _, _, err := MVCCResolveWriteIntentRange(ctx, ol, nil, + if _, _, _, _, err := MVCCResolveWriteIntentRange(ctx, ol, nil, roachpb.MakeLockUpdate( &txn1CommitTS, roachpb.Span{Key: localKey, EndKey: localKey.Next()}), - math.MaxInt64); err != nil { + MVCCResolveWriteIntentRangeOptions{}); err != nil { t.Fatal(err) } @@ -95,15 +94,17 @@ func TestMVCCOpLogWriter(t *testing.T) { } txn2Pushed := *txn2 txn2Pushed.WriteTimestamp = hlc.Timestamp{Logical: 6} - if _, err := MVCCResolveWriteIntent(ctx, ol, nil, + if _, _, _, err := MVCCResolveWriteIntent(ctx, ol, nil, roachpb.MakeLockUpdate(&txn2Pushed, roachpb.Span{Key: testKey3}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } txn2Abort := txn2Pushed txn2Abort.Status = roachpb.ABORTED - if _, err := MVCCResolveWriteIntent(ctx, ol, nil, + if _, _, _, err := MVCCResolveWriteIntent(ctx, ol, nil, roachpb.MakeLockUpdate(&txn2Abort, roachpb.Span{Key: testKey3}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } diff --git a/pkg/storage/mvcc_stats_test.go b/pkg/storage/mvcc_stats_test.go index 64cf802693c1..66da5df50f8e 100644 --- a/pkg/storage/mvcc_stats_test.go +++ b/pkg/storage/mvcc_stats_test.go @@ -134,8 +134,9 @@ func TestMVCCStatsDeleteCommitMovesTimestamp(t *testing.T) { ts4 := hlc.Timestamp{WallTime: 4 * 1e9} txn.Status = roachpb.COMMITTED txn.WriteTimestamp.Forward(ts4) - if _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, roachpb.MakeLockUpdate(txn, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -223,8 +224,9 @@ func TestMVCCStatsPutCommitMovesTimestamp(t *testing.T) { ts4 := hlc.Timestamp{WallTime: 4 * 1e9} txn.Status = roachpb.COMMITTED txn.WriteTimestamp.Forward(ts4) - if _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, roachpb.MakeLockUpdate(txn, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -310,8 +312,9 @@ func TestMVCCStatsPutPushMovesTimestamp(t *testing.T) { // push as it would happen for a SNAPSHOT txn) ts4 := hlc.Timestamp{WallTime: 4 * 1e9} txn.WriteTimestamp.Forward(ts4) - if _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, roachpb.MakeLockUpdate(txn, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -687,8 +690,9 @@ func TestMVCCStatsDelDelCommitMovesTimestamp(t *testing.T) { txnCommit := txn.Clone() txnCommit.Status = roachpb.COMMITTED txnCommit.WriteTimestamp.Forward(ts3) - if _, err := MVCCResolveWriteIntent(ctx, engine, &aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, &aggMS, roachpb.MakeLockUpdate(txnCommit, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -723,8 +727,9 @@ func TestMVCCStatsDelDelCommitMovesTimestamp(t *testing.T) { txnAbort := txn.Clone() txnAbort.Status = roachpb.ABORTED txnAbort.WriteTimestamp.Forward(ts3) - if _, err := MVCCResolveWriteIntent(ctx, engine, &aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, &aggMS, roachpb.MakeLockUpdate(txnAbort, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -860,8 +865,9 @@ func TestMVCCStatsPutDelPutMovesTimestamp(t *testing.T) { txnAbort := txn.Clone() txnAbort.Status = roachpb.ABORTED // doesn't change m2ValSize, fortunately - if _, err := MVCCResolveWriteIntent(ctx, engine, &aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, &aggMS, roachpb.MakeLockUpdate(txnAbort, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -1375,8 +1381,9 @@ func TestMVCCStatsTxnSysPutAbort(t *testing.T) { // Now abort the intent. txn.Status = roachpb.ABORTED - if _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, aggMS, roachpb.MakeLockUpdate(txn, roachpb.Span{Key: key}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -1754,14 +1761,15 @@ func TestMVCCStatsRandomized(t *testing.T) { desc := fmt.Sprintf("ranged=%t", ranged) if s.Txn != nil { if !ranged { - if _, err := MVCCResolveWriteIntent(ctx, s.batch, s.MSDelta, s.intent(status)); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, s.batch, s.MSDelta, s.intent(status), MVCCResolveWriteIntentOptions{}); err != nil { return false, desc + ": " + err.Error() } } else { max := s.rng.Int63n(5) desc += fmt.Sprintf(", max=%d", max) - if _, _, err := MVCCResolveWriteIntentRange( - ctx, s.batch, s.MSDelta, s.intentRange(status), max); err != nil { + if _, _, _, _, err := MVCCResolveWriteIntentRange( + ctx, s.batch, s.MSDelta, s.intentRange(status), + MVCCResolveWriteIntentRangeOptions{}); err != nil { return false, desc + ": " + err.Error() } } diff --git a/pkg/storage/mvcc_test.go b/pkg/storage/mvcc_test.go index c1bf794c0716..948257e6b736 100644 --- a/pkg/storage/mvcc_test.go +++ b/pkg/storage/mvcc_test.go @@ -2317,8 +2317,9 @@ func TestMVCCInitPutWithTxn(t *testing.T) { txnCommit := txn txnCommit.Status = roachpb.COMMITTED txnCommit.WriteTimestamp = clock.Now().Add(1, 0) - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(&txnCommit, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(&txnCommit, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -2617,8 +2618,9 @@ func TestMVCCResolveTxn(t *testing.T) { } // Resolve will write with txn1's timestamp which is 0,1. - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -2656,8 +2658,9 @@ func TestMVCCResolveNewerIntent(t *testing.T) { } // Resolve will succeed but should remove the intent. - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -2696,7 +2699,7 @@ func TestMVCCResolveIntentTxnTimestampMismatch(t *testing.T) { // A bug (see #7654) caused intents to just stay where they were instead // of being moved forward in the situation set up above. - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, intent); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, intent, MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -2896,8 +2899,9 @@ func TestMVCCAbortTxn(t *testing.T) { txn1AbortWithTS := txn1Abort.Clone() txn1AbortWithTS.WriteTimestamp = hlc.Timestamp{Logical: 1} - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, roachpb.MakeLockUpdate(txn1AbortWithTS, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -2934,8 +2938,9 @@ func TestMVCCAbortTxnWithPreviousVersion(t *testing.T) { txn1AbortWithTS := txn1Abort.Clone() txn1AbortWithTS.WriteTimestamp = hlc.Timestamp{WallTime: 2} - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, roachpb.MakeLockUpdate(txn1AbortWithTS, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}, ); err != nil { t.Fatal(err) } @@ -3001,8 +3006,9 @@ func TestMVCCWriteWithDiffTimestampsAndEpochs(t *testing.T) { txne2Commit := txne2 txne2Commit.Status = roachpb.COMMITTED txne2Commit.WriteTimestamp = hlc.Timestamp{WallTime: 1} - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(&txne2Commit, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(&txne2Commit, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -3289,8 +3295,9 @@ func TestMVCCGetWithPushedTimestamp(t *testing.T) { } // Resolve the intent, pushing its timestamp forward. txn := makeTxn(*txn1, hlc.Timestamp{WallTime: 1}) - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } // Attempt to read using naive txn's previous timestamp. @@ -3316,14 +3323,14 @@ func TestMVCCResolveWithDiffEpochs(t *testing.T) { if err := MVCCPut(ctx, engine, nil, testKey2, txn1e2.ReadTimestamp, hlc.ClockTimestamp{}, value2, txn1e2); err != nil { t.Fatal(err) } - num, _, err := MVCCResolveWriteIntentRange(ctx, engine, nil, + numKeys, _, _, _, err := MVCCResolveWriteIntentRange(ctx, engine, nil, roachpb.MakeLockUpdate(txn1e2Commit, roachpb.Span{Key: testKey1, EndKey: testKey2.Next()}), - 2) + MVCCResolveWriteIntentRangeOptions{MaxKeys: 2}) if err != nil { t.Fatal(err) } - if num != 2 { - t.Errorf("expected 2 rows resolved; got %d", num) + if numKeys != 2 { + t.Errorf("expected 2 rows resolved; got %d", numKeys) } // Verify key1 is empty, as resolution with epoch 2 would have @@ -3370,8 +3377,9 @@ func TestMVCCResolveWithUpdatedTimestamp(t *testing.T) { // Resolve with a higher commit timestamp -- this should rewrite the // intent when making it permanent. txn := makeTxn(*txn1Commit, hlc.Timestamp{WallTime: 1}) - if _, err = MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err = MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -3418,8 +3426,9 @@ func TestMVCCResolveWithPushedTimestamp(t *testing.T) { // Resolve with a higher commit timestamp, but with still-pending transaction. // This represents a straightforward push (i.e. from a read/write conflict). txn := makeTxn(*txn1, hlc.Timestamp{WallTime: 1}) - if _, err = MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err = MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -3453,8 +3462,9 @@ func TestMVCCResolveTxnNoOps(t *testing.T) { defer engine.Close() // Resolve a non existent key; noop. - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -3462,8 +3472,9 @@ func TestMVCCResolveTxnNoOps(t *testing.T) { if err := MVCCPut(ctx, engine, nil, testKey1, hlc.Timestamp{Logical: 1}, hlc.ClockTimestamp{}, value1, nil); err != nil { t.Fatal(err) } - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn2Commit, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn2Commit, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } @@ -3474,8 +3485,9 @@ func TestMVCCResolveTxnNoOps(t *testing.T) { txn1CommitWithTS := txn2Commit.Clone() txn1CommitWithTS.WriteTimestamp = hlc.Timestamp{WallTime: 1} - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn1CommitWithTS, roachpb.Span{Key: testKey2})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn1CommitWithTS, roachpb.Span{Key: testKey2}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) } } @@ -3501,15 +3513,15 @@ func TestMVCCResolveTxnRange(t *testing.T) { t.Fatal(err) } - num, resumeSpan, err := MVCCResolveWriteIntentRange(ctx, engine, nil, + numKeys, _, resumeSpan, _, err := MVCCResolveWriteIntentRange(ctx, engine, nil, roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: testKey1, EndKey: testKey4.Next()}), - math.MaxInt64) + MVCCResolveWriteIntentRangeOptions{}) if err != nil { t.Fatal(err) } - if num != 2 || resumeSpan != nil { + if numKeys != 2 || resumeSpan != nil { t.Fatalf("expected all keys to process for resolution, even though 2 are noops; got %d, resume=%s", - num, resumeSpan) + numKeys, resumeSpan) } { @@ -3590,14 +3602,14 @@ func TestMVCCResolveTxnRangeResume(t *testing.T) { defer rw.Close() // Resolve up to 6 intents: the keys are 000, 033, 066, 099, 1212, 1515. - num, resumeSpan, err := MVCCResolveWriteIntentRange(ctx, rw, nil, + numKeys, _, resumeSpan, _, err := MVCCResolveWriteIntentRange(ctx, rw, nil, roachpb.MakeLockUpdate(txn1Commit, roachpb.Span{Key: roachpb.Key("00"), EndKey: roachpb.Key("33")}), - 6) + MVCCResolveWriteIntentRangeOptions{MaxKeys: 6}) if err != nil { t.Fatal(err) } - if num != 6 || resumeSpan == nil { - t.Errorf("expected resolution for only 6 keys; got %d, resume=%s", num, resumeSpan) + if numKeys != 6 || resumeSpan == nil { + t.Errorf("expected resolution for only 6 keys; got %d, resume=%s", numKeys, resumeSpan) } expResumeSpan := roachpb.Span{Key: roachpb.Key("1515").Next(), EndKey: roachpb.Key("33")} if !resumeSpan.Equal(expResumeSpan) { @@ -3635,15 +3647,15 @@ func TestMVCCResolveTxnRangeResumeWithManyVersions(t *testing.T) { i := 0 for { // Resolve up to 20 intents. - num, resumeSpan, err := MVCCResolveWriteIntentRange(ctx, engine, nil, lockUpdate, - 20) + numKeys, _, resumeSpan, _, err := MVCCResolveWriteIntentRange(ctx, engine, nil, lockUpdate, + MVCCResolveWriteIntentRangeOptions{MaxKeys: 20}) require.NoError(t, err) if resumeSpan == nil { // Last call resolves 0 intents. - require.Equal(t, int64(0), num) + require.Equal(t, int64(0), numKeys) break } - require.Equal(t, int64(20), num) + require.Equal(t, int64(20), numKeys) i++ expResumeSpan := roachpb.Span{ Key: makeKey(nil, (i*20-1)*10).Next(), @@ -3859,7 +3871,7 @@ func TestRandomizedMVCCResolveWriteIntentRange(t *testing.T) { func() { batch := engs[i].eng.NewBatch() defer batch.Close() - _, _, err := MVCCResolveWriteIntentRange(ctx, batch, &engs[i].stats, lu, 0) + _, _, _, _, err := MVCCResolveWriteIntentRange(ctx, batch, &engs[i].stats, lu, MVCCResolveWriteIntentRangeOptions{}) require.NoError(t, err) require.NoError(t, batch.Commit(false)) }() @@ -3877,7 +3889,7 @@ func TestRandomizedMVCCResolveWriteIntentRange(t *testing.T) { func() { batch := engs[i].eng.NewBatch() defer batch.Close() - _, _, err := MVCCResolveWriteIntentRange(ctx, batch, &engs[i].stats, lu, 0) + _, _, _, _, err := MVCCResolveWriteIntentRange(ctx, batch, &engs[i].stats, lu, MVCCResolveWriteIntentRangeOptions{}) require.NoError(t, err) require.NoError(t, batch.Commit(false)) }() @@ -3966,7 +3978,7 @@ func TestRandomizedSavepointRollbackAndIntentResolution(t *testing.T) { } // All the writes are ignored, so DEL is written for the intent. These // should be buffered in the memtable. - _, _, err = MVCCResolveWriteIntentRange(ctx, eng, nil, lu, 0) + _, _, _, _, err = MVCCResolveWriteIntentRange(ctx, eng, nil, lu, MVCCResolveWriteIntentRangeOptions{}) require.NoError(t, err) { iter := eng.NewMVCCIterator(MVCCKeyAndIntentsIterKind, @@ -3998,7 +4010,7 @@ func TestRandomizedSavepointRollbackAndIntentResolution(t *testing.T) { if debug { log.Infof(ctx, "LockUpdate: %s", lu.String()) } - _, _, err = MVCCResolveWriteIntentRange(ctx, eng, nil, lu, 0) + _, _, _, _, err = MVCCResolveWriteIntentRange(ctx, eng, nil, lu, MVCCResolveWriteIntentRangeOptions{}) require.NoError(t, err) // Compact the engine so that SINGLEDEL consumes the SETWITHDEL, becoming a // DEL. @@ -5864,8 +5876,9 @@ func TestResolveIntentWithLowerEpoch(t *testing.T) { t.Fatal(err) } // Resolve the intent with a low epoch. - if _, err := MVCCResolveWriteIntent(ctx, engine, nil, - roachpb.MakeLockUpdate(txn1, roachpb.Span{Key: testKey1})); err != nil { + if _, _, _, err := MVCCResolveWriteIntent(ctx, engine, nil, + roachpb.MakeLockUpdate(txn1, roachpb.Span{Key: testKey1}), + MVCCResolveWriteIntentOptions{}); err != nil { t.Fatal(err) }