Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: remove MVCCIterator.Key method #98542

Merged
merged 1 commit into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/full_cluster_backup_restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ CREATE TABLE data2.foo (a int);
it.SeekGE(storage.MVCCKey{Key: startKey})
hasKey, err := it.Valid()
require.NoError(t, err)
require.False(t, hasKey, "did not expect to find a key, found %s", it.Key())
require.False(t, hasKey, "did not expect to find a key, found %s", it.UnsafeKey())
})

// Allow the restore to make progress after we've checked the pre-restore
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/restore_data_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func slurpSSTablesLatestKey(
if err != nil {
t.Fatal(err)
}
kvs = append(kvs, storage.MVCCKeyValue{Key: it.Key(), Value: val.Value.RawBytes})
kvs = append(kvs, storage.MVCCKeyValue{Key: it.UnsafeKey().Clone(), Value: val.Value.RawBytes})
}
return kvs
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/streamingccl/replicationutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func ScanSST(
return err
}
if err = mvccKeyValOp(storage.MVCCKeyValue{
Key: pointIter.Key(),
Key: pointIter.UnsafeKey().Clone(),
Value: v,
}); err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,18 @@ func assertExactlyEqualKVs(
}
break
}
if maxKVTimestampSeen.Less(it.Key().Timestamp) {
maxKVTimestampSeen = it.Key().Timestamp
if maxKVTimestampSeen.Less(it.UnsafeKey().Timestamp) {
maxKVTimestampSeen = it.UnsafeKey().Timestamp
}
newKey := (prevKey != nil && !it.Key().Key.Equal(prevKey)) || prevKey == nil
prevKey = it.Key().Key
newKey := (prevKey != nil && !it.UnsafeKey().Key.Equal(prevKey)) || prevKey == nil
prevKey = it.UnsafeKey().Clone().Key

if newKey {
// All value ts should have been drained at this point, otherwise there is
// a mismatch between the streamed and ingested data.
require.Equal(t, 0, len(valueTimestampTuples))
valueTimestampTuples, err = streamValidator.getValuesForKeyBelowTimestamp(
string(it.Key().Key), frontierTimestamp)
string(it.UnsafeKey().Key), frontierTimestamp)
require.NoError(t, err)
}

Expand All @@ -357,10 +357,10 @@ func assertExactlyEqualKVs(
v, err := it.Value()
require.NoError(t, err)
require.Equal(t, roachpb.KeyValue{
Key: it.Key().Key,
Key: it.UnsafeKey().Key,
Value: roachpb.Value{
RawBytes: v,
Timestamp: it.Key().Timestamp,
Timestamp: it.UnsafeKey().Timestamp,
},
}, latestVersionInChain)
matchingKVs++
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,19 +385,19 @@ func assertEqualKVs(

// We only want to process MVCC KVs with a ts less than or equal to the max
// resolved ts for this partition.
if partitionTimestamp.Less(it.Key().Timestamp) {
if partitionTimestamp.Less(it.UnsafeKey().Timestamp) {
continue
}

newKey := (prevKey != nil && !it.Key().Key.Equal(prevKey)) || prevKey == nil
prevKey = it.Key().Key
newKey := (prevKey != nil && !it.UnsafeKey().Key.Equal(prevKey)) || prevKey == nil
prevKey = it.UnsafeKey().Clone().Key

if newKey {
// All value ts should have been drained at this point, otherwise there is
// a mismatch between the streamed and ingested data.
require.Equal(t, 0, len(valueTimestampTuples))
valueTimestampTuples, err = streamValidator.getValuesForKeyBelowTimestamp(
string(it.Key().Key), partitionTimestamp)
string(it.UnsafeKey().Key), partitionTimestamp)
require.NoError(t, err)
}

Expand All @@ -408,10 +408,10 @@ func assertEqualKVs(
v, err := it.Value()
require.NoError(t, err)
require.Equal(t, roachpb.KeyValue{
Key: it.Key().Key,
Key: it.UnsafeKey().Key,
Value: roachpb.Value{
RawBytes: v,
Timestamp: it.Key().Timestamp,
Timestamp: it.UnsafeKey().Timestamp,
},
}, latestVersionInChain)
// Truncate the latest version which we just checked against in preparation
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvnemesis/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ func (v *validator) processOp(op Operation) {
}
}

key := iter.Key().Key
key := iter.UnsafeKey().Clone().Key
rawValue, err := iter.Value()
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvnemesis/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func (w *Watcher) handleSSTable(ctx context.Context, data []byte) error {
}

// Add point keys.
key := iter.Key()
key := iter.UnsafeKey().Clone()
rawValue, err := iter.Value()
if err != nil {
return err
Expand Down
40 changes: 20 additions & 20 deletions pkg/kv/kvserver/batch_spanset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,20 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
if ok, err := iter.Valid(); !ok || err != nil {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), insideKey) {
t.Fatalf("expected key %s, got %s", insideKey, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), insideKey) {
t.Fatalf("expected key %s, got %s", insideKey, iter.UnsafeKey())
}
iter.Next()
if ok, err := iter.Valid(); !ok || err != nil {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), insideKey2) {
t.Fatalf("expected key %s, got %s", insideKey2, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), insideKey2) {
t.Fatalf("expected key %s, got %s", insideKey2, iter.UnsafeKey())
}
// Scan out of bounds.
iter.Next()
if ok, err := iter.Valid(); ok {
t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
t.Fatalf("expected invalid iterator; found valid at key %s", iter.UnsafeKey())
} else if err != nil {
// Scanning out of bounds sets Valid() to false but is not an error.
t.Errorf("unexpected error on iterator: %+v", err)
Expand All @@ -210,20 +210,20 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
if ok, err := iter.Valid(); !ok || err != nil {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), insideKey2) {
t.Fatalf("expected key %s, got %s", insideKey2, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), insideKey2) {
t.Fatalf("expected key %s, got %s", insideKey2, iter.UnsafeKey())
}
iter.Prev()
if ok, err := iter.Valid(); !ok || err != nil {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), insideKey) {
t.Fatalf("expected key %s, got %s", insideKey, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), insideKey) {
t.Fatalf("expected key %s, got %s", insideKey, iter.UnsafeKey())
}
// Scan out of bounds.
iter.Prev()
if ok, err := iter.Valid(); ok {
t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
t.Fatalf("expected invalid iterator; found valid at key %s", iter.UnsafeKey())
} else if err != nil {
t.Errorf("unexpected error on iterator: %+v", err)
}
Expand All @@ -236,7 +236,7 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
// SeekLT to the lower bound is invalid.
iter.SeekLT(insideKey)
if ok, err := iter.Valid(); ok {
t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
t.Fatalf("expected invalid iterator; found valid at key %s", iter.UnsafeKey())
} else if !isReadSpanErr(err) {
t.Fatalf("SeekLT: unexpected error %v", err)
}
Expand Down Expand Up @@ -390,16 +390,16 @@ func TestSpanSetIteratorTimestamps(t *testing.T) {
if ok, err := iter.Valid(); !ok {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), k1) {
t.Fatalf("expected key %s, got %s", k1, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), k1) {
t.Fatalf("expected key %s, got %s", k1, iter.UnsafeKey())
}

iter.Next()
if ok, err := iter.Valid(); !ok {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), k2) {
t.Fatalf("expected key %s, got %s", k2, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), k2) {
t.Fatalf("expected key %s, got %s", k2, iter.UnsafeKey())
}
}()

Expand All @@ -410,15 +410,15 @@ func TestSpanSetIteratorTimestamps(t *testing.T) {

iter.SeekGE(k1)
if ok, _ := iter.Valid(); ok {
t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
t.Fatalf("expected invalid iterator; found valid at key %s", iter.UnsafeKey())
}

iter.SeekGE(k2)
if ok, err := iter.Valid(); !ok {
t.Fatalf("expected valid iterator, err=%v", err)
}
if !reflect.DeepEqual(iter.Key(), k2) {
t.Fatalf("expected key %s, got %s", k2, iter.Key())
if !reflect.DeepEqual(iter.UnsafeKey(), k2) {
t.Fatalf("expected key %s, got %s", k2, iter.UnsafeKey())
}
}()

Expand All @@ -430,12 +430,12 @@ func TestSpanSetIteratorTimestamps(t *testing.T) {

iter.SeekGE(k1)
if ok, _ := iter.Valid(); ok {
t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
t.Fatalf("expected invalid iterator; found valid at key %s", iter.UnsafeKey())
}

iter.SeekGE(k2)
if ok, _ := iter.Valid(); ok {
t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
t.Fatalf("expected invalid iterator; found valid at key %s", iter.UnsafeKey())
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_add_sstable.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func EvalAddSSTable(
if ok, err := existingIter.Valid(); err != nil {
return result.Result{}, errors.Wrap(err, "error while searching for non-empty span start")
} else if ok {
reply.FollowingLikelyNonEmptySpanStart = existingIter.Key().Key
reply.FollowingLikelyNonEmptySpanStart = existingIter.UnsafeKey().Key.Clone()
}
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/client_split_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,9 @@ func TestStoreRangeSplitIntents(t *testing.T) {
break
}

if bytes.HasPrefix([]byte(iter.Key().Key), txnPrefix(roachpb.KeyMin)) ||
bytes.HasPrefix([]byte(iter.Key().Key), txnPrefix(splitKey)) {
t.Errorf("unexpected system key: %s; txn record should have been cleaned up", iter.Key())
if bytes.HasPrefix([]byte(iter.UnsafeKey().Key), txnPrefix(roachpb.KeyMin)) ||
bytes.HasPrefix([]byte(iter.UnsafeKey().Key), txnPrefix(splitKey)) {
t.Errorf("unexpected system key: %s; txn record should have been cleaned up", iter.UnsafeKey())
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/gc/gc_old_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,13 @@ func runGCOld(
// Stop iterating if our context has expired.
return Info{}, err
}
iterKey := iter.Key()
iterKey := iter.UnsafeKey().Clone()
if !iterKey.IsValue() || !iterKey.Key.Equal(expBaseKey) {
// Moving to the next key (& values).
processKeysAndValues()
expBaseKey = iterKey.Key
if !iterKey.IsValue() {
keys = []storage.MVCCKey{iter.Key()}
keys = []storage.MVCCKey{iter.UnsafeKey().Clone()}
v, err := iter.Value()
if err != nil {
return Info{}, err
Expand All @@ -212,7 +212,7 @@ func runGCOld(
if err != nil {
return Info{}, err
}
keys = append(keys, iter.Key())
keys = append(keys, iter.UnsafeKey().Clone())
vals = append(vals, v)
}
// Handle last collected set of keys/vals.
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/gc/gc_random_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func getExpectationsGenerator(
}
p, r := it.HasPointAndRange()
if p {
k := it.Key()
k := it.UnsafeKey().Clone()
v, err := it.Value()
require.NoError(t, err)
if len(baseKey) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/gc/gc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,7 @@ func engineData(t *testing.T, r storage.Reader, desc roachpb.RangeDescriptor) []
v = prefix + string(b)
}
result = append(result, tableCell{
key: it.Key(),
key: it.UnsafeKey().Clone(),
value: v,
})
prefix = ""
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/loqrecovery/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func RegisterOfflineRecoveryEvents(
record := loqrecoverypb.ReplicaRecoveryRecord{}
if err := iter.ValueProto(&record); err != nil {
processingErrors = errors.CombineErrors(processingErrors, errors.Wrapf(err,
"failed to deserialize replica recovery event at key %s", iter.Key()))
"failed to deserialize replica recovery event at key %s", iter.UnsafeKey()))
continue
}
removeEvent, err := registerEvent(ctx, record)
Expand All @@ -109,7 +109,7 @@ func RegisterOfflineRecoveryEvents(
if removeEvent {
if err := readWriter.ClearUnversioned(iter.UnsafeKey().Key); err != nil {
processingErrors = errors.CombineErrors(processingErrors, errors.Wrapf(
err, "failed to delete replica recovery record at key %s", iter.Key()))
err, "failed to delete replica recovery record at key %s", iter.UnsafeKey()))
continue
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/raft_log_truncator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func (r *replicaTruncatorTest) printEngine(t *testing.T, eng storage.Engine) {
fmt.Fprintf(r.buf, "log entries:")
printPrefixStr := ""
for valid {
key := iter.Key()
key := iter.UnsafeKey()
_, index, err := encoding.DecodeUint64Ascending(key.Key[len(prefix):])
require.NoError(t, err)
fmt.Fprintf(r.buf, "%s %d", printPrefixStr, index)
Expand Down
5 changes: 0 additions & 5 deletions pkg/kv/kvserver/rditer/replica_data_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,6 @@ func (ri *ReplicaMVCCDataIterator) Valid() (bool, error) {
return true, nil
}

// Key returns the current key. Only called in tests.
func (ri *ReplicaMVCCDataIterator) Key() storage.MVCCKey {
return ri.it.Key()
}

// Value returns the current value. Only called in tests.
func (ri *ReplicaMVCCDataIterator) Value() ([]byte, error) {
return ri.it.Value()
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/rditer/replica_data_iter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,9 @@ func TestIterateMVCCReplicaKeySpansSpansSet(t *testing.T) {
p, r := iter.HasPointAndRange()
if p {
if !reverse {
actualKeys = append(actualKeys, iter.Key())
actualKeys = append(actualKeys, iter.UnsafeKey().Clone())
} else {
actualKeys = append([]storage.MVCCKey{iter.Key()}, actualKeys...)
actualKeys = append([]storage.MVCCKey{iter.UnsafeKey().Clone()}, actualKeys...)
}
}
if r {
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/replica_evaluate.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ func optimizePuts(
// running without the optimization?
log.Errorf(context.TODO(), "Seek returned error; disabling blind-put optimization: %+v", err)
return origReqs
} else if ok && bytes.Compare(iter.Key().Key, maxKey) <= 0 {
iterKey = iter.Key().Key
} else if ok && bytes.Compare(iter.UnsafeKey().Key, maxKey) <= 0 {
iterKey = iter.UnsafeKey().Key.Clone()
}
// Set the prefix of the run which is being written to virgin
// keyspace to "blindly" put values.
Expand Down
5 changes: 0 additions & 5 deletions pkg/kv/kvserver/spanset/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,6 @@ func (i *MVCCIterator) checkAllowedValidPos(span roachpb.Span, errIfDisallowed b
}
}

// Key is part of the storage.MVCCIterator interface.
func (i *MVCCIterator) Key() storage.MVCCKey {
return i.i.Key()
}

// Value is part of the storage.MVCCIterator interface.
func (i *MVCCIterator) Value() ([]byte, error) {
return i.i.Value()
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/lease/kv_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func getRawHistoryKVs(
} else if !ok {
return nil
}
k := it.Key()
k := it.UnsafeKey().Clone()
suffix, _, err := codec.DecodeTablePrefix(k.Key)
require.NoError(t, err)
v, err := it.Value()
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/row/fetcher_mvcc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func slurpUserDataKVs(t testing.TB, e storage.Engine) []roachpb.KeyValue {
}
value := mvccValue.Value
value.Timestamp = it.UnsafeKey().Timestamp
kv := roachpb.KeyValue{Key: it.Key().Key, Value: value}
kv := roachpb.KeyValue{Key: it.UnsafeKey().Key.Clone(), Value: value}
kvs = append(kvs, kv)
}
return nil
Expand Down
Loading