diff --git a/pkg/kv/kvserver/batcheval/cmd_clear_range.go b/pkg/kv/kvserver/batcheval/cmd_clear_range.go index 75de88967e74..735c63dd3e0a 100644 --- a/pkg/kv/kvserver/batcheval/cmd_clear_range.go +++ b/pkg/kv/kvserver/batcheval/cmd_clear_range.go @@ -82,23 +82,16 @@ func ClearRange( cArgs.Stats.Subtract(statsDelta) // If the total size of data to be cleared is less than - // clearRangeBytesThreshold, clear the individual values manually, + // clearRangeBytesThreshold, clear the individual values with an iterator, // instead of using a range tombstone (inefficient for small ranges). if total := statsDelta.Total(); total < ClearRangeBytesThreshold { log.VEventf(ctx, 2, "delta=%d < threshold=%d; using non-range clear", total, ClearRangeBytesThreshold) - iter := readWriter.NewEngineIterator(storage.IterOptions{UpperBound: to}) - valid, err := iter.SeekEngineKeyGE(storage.EngineKey{Key: from}) - for ; valid; valid, err = iter.NextEngineKey() { - var k storage.EngineKey - if k, err = iter.UnsafeEngineKey(); err != nil { - break - } - if err = readWriter.ClearEngineKey(k); err != nil { - return result.Result{}, err - } - } - iter.Close() - if err != nil { + iter := readWriter.NewMVCCIterator(storage.MVCCKeyAndIntentsIterKind, storage.IterOptions{ + LowerBound: from, + UpperBound: to, + }) + defer iter.Close() + if err = readWriter.ClearIterRange(iter, from, to); err != nil { return result.Result{}, err } return pd, nil diff --git a/pkg/kv/kvserver/batcheval/cmd_clear_range_test.go b/pkg/kv/kvserver/batcheval/cmd_clear_range_test.go index 924e79b26a00..143439f084bd 100644 --- a/pkg/kv/kvserver/batcheval/cmd_clear_range_test.go +++ b/pkg/kv/kvserver/batcheval/cmd_clear_range_test.go @@ -29,13 +29,13 @@ import ( type wrappedBatch struct { storage.Batch - clearCount int + clearIterCount int clearRangeCount int } -func (wb *wrappedBatch) ClearEngineKey(key storage.EngineKey) error { - wb.clearCount++ - return wb.Batch.ClearEngineKey(key) +func (wb *wrappedBatch) ClearIterRange(iter storage.MVCCIterator, start, end roachpb.Key) error { + wb.clearIterCount++ + return wb.Batch.ClearIterRange(iter, start, end) } func (wb *wrappedBatch) ClearMVCCRangeAndIntents(start, end roachpb.Key) error { @@ -64,24 +64,24 @@ func TestCmdClearRangeBytesThreshold(t *testing.T) { overFull := ClearRangeBytesThreshold/len(valueStr) + 1 tests := []struct { keyCount int - expClearCount int + expClearIterCount int expClearRangeCount int }{ { keyCount: 1, - expClearCount: 1, + expClearIterCount: 1, expClearRangeCount: 0, }, // More than a single key, but not enough to use ClearRange. { keyCount: halfFull, - expClearCount: halfFull, + expClearIterCount: 1, expClearRangeCount: 0, }, // With key sizes requiring additional space, this will overshoot. { keyCount: overFull, - expClearCount: 0, + expClearIterCount: 0, expClearRangeCount: 1, }, } @@ -131,8 +131,8 @@ func TestCmdClearRangeBytesThreshold(t *testing.T) { } // Verify we see the correct counts for Clear and ClearRange. - if a, e := batch.clearCount, test.expClearCount; a != e { - t.Errorf("expected %d clears; got %d", e, a) + if a, e := batch.clearIterCount, test.expClearIterCount; a != e { + t.Errorf("expected %d iter range clears; got %d", e, a) } if a, e := batch.clearRangeCount, test.expClearRangeCount; a != e { t.Errorf("expected %d clear ranges; got %d", e, a) diff --git a/pkg/kv/kvserver/txn_recovery_integration_test.go b/pkg/kv/kvserver/txn_recovery_integration_test.go index 344363247e3c..47b10c23fac0 100644 --- a/pkg/kv/kvserver/txn_recovery_integration_test.go +++ b/pkg/kv/kvserver/txn_recovery_integration_test.go @@ -274,24 +274,12 @@ func TestTxnClearRangeIntents(t *testing.T) { // implicitly committed above. 😱 get := getArgs(keyA) reply, pErr = kv.SendWrappedWith(ctx, store.TestSender(), roachpb.Header{}, &get) + require.Nil(t, pErr, "error: %s", pErr) + require.Nil(t, reply.(*roachpb.GetResponse).Value, "unexpected value for key %q", keyA) - // When separated intents are enabled, ClearRange will only remove the B key - // but not its intent (stored in a separate lock table). This sometimes (but - // not always) causes an iterator error during txn recovery, and is a - // separate bug in ClearRange: - // https://github.com/cockroachdb/cockroach/issues/61606 - if pErr != nil { - require.True(t, store.Engine().IsSeparatedIntentsEnabledForTesting(), - "unexpected error with separated intents disabled: %s", pErr.String()) - require.Contains(t, pErr.String(), "intentIter at intent, but iter not at provisional value") - - } else { - require.Nil(t, reply.(*roachpb.GetResponse).Value, "unexpected value for key %q", keyA) - - // Query the original transaction, which should now be aborted. - queryTxn := queryTxnArgs(txn.TxnMeta, false) - reply, pErr = kv.SendWrappedWith(ctx, store.TestSender(), roachpb.Header{}, &queryTxn) - require.Nil(t, pErr, "error: %s", pErr) - require.Equal(t, roachpb.ABORTED, reply.(*roachpb.QueryTxnResponse).QueriedTxn.Status) - } + // Query the original transaction, which should now be aborted. + queryTxn := queryTxnArgs(txn.TxnMeta, false) + reply, pErr = kv.SendWrappedWith(ctx, store.TestSender(), roachpb.Header{}, &queryTxn) + require.Nil(t, pErr, "error: %s", pErr) + require.Equal(t, roachpb.ABORTED, reply.(*roachpb.QueryTxnResponse).QueriedTxn.Status) }