Skip to content

Commit 076a595

Browse files
craig[bot]pav-kv
craig[bot]
and
pav-kv
committed
Merge #158274
158274: kvserver: use StateEngine in a bunch of tests r=arulajmani a=pav-kv This PR is one in the series of PRs that clarify in code which of the two engines is being used. Scoped to a subset of `kvserver` tests, and includes places where it's clear that it is `StateEngine` (e.g. it only accesses MVCC keyspace or Range-local). It is ok if we mis-label at this point since both engines share the same underlying engine. Later, we should add an assertion (#158281) that the right kinds of keys are accessed. Part of #97627 Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
2 parents 92912e6 + 89bbf0a commit 076a595

File tree

7 files changed

+36
-43
lines changed

7 files changed

+36
-43
lines changed

‎pkg/kv/kvserver/addressing_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func TestUpdateRangeAddressing(t *testing.T) {
164164
// to RocksDB will be asynchronous.
165165
var kvs []roachpb.KeyValue
166166
testutils.SucceedsSoon(t, func() error {
167-
res, err := storage.MVCCScan(ctx, store.TODOEngine(), keys.MetaMin, keys.MetaMax,
167+
res, err := storage.MVCCScan(ctx, store.StateEngine(), keys.MetaMin, keys.MetaMax,
168168
hlc.MaxTimestamp, storage.MVCCScanOptions{})
169169
if err != nil {
170170
// Wait for the intent to be resolved.

‎pkg/kv/kvserver/client_merge_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func mergeWithData(t *testing.T, retries int64) {
360360
// Verify no intents remains on range descriptor keys.
361361
for _, key := range []roachpb.Key{keys.RangeDescriptorKey(lhsDesc.StartKey), keys.RangeDescriptorKey(rhsDesc.StartKey)} {
362362
if _, err := storage.MVCCGet(
363-
ctx, store.TODOEngine(), key, store.Clock().Now(), storage.MVCCGetOptions{},
363+
ctx, store.StateEngine(), key, store.Clock().Now(), storage.MVCCGetOptions{},
364364
); err != nil {
365365
t.Fatal(err)
366366
}
@@ -1365,7 +1365,7 @@ func TestStoreRangeMergeStats(t *testing.T) {
13651365
// merge below.
13661366

13671367
// Get the range stats for both ranges now that we have data.
1368-
snap := store.TODOEngine().NewSnapshot()
1368+
snap := store.StateEngine().NewSnapshot()
13691369
defer snap.Close()
13701370
msA, err := kvstorage.MakeStateLoader(lhsDesc.RangeID).LoadMVCCStats(ctx, snap)
13711371
require.NoError(t, err)
@@ -1383,7 +1383,7 @@ func TestStoreRangeMergeStats(t *testing.T) {
13831383
replMerged := store.LookupReplica(lhsDesc.StartKey)
13841384

13851385
// Get the range stats for the merged range and verify.
1386-
snap = store.TODOEngine().NewSnapshot()
1386+
snap = store.StateEngine().NewSnapshot()
13871387
defer snap.Close()
13881388
msMerged, err := kvstorage.MakeStateLoader(replMerged.RangeID).LoadMVCCStats(ctx, snap)
13891389
require.NoError(t, err)
@@ -4043,8 +4043,8 @@ func TestStoreRangeMergeRaftSnapshot(t *testing.T) {
40434043
})
40444044
defer tc.Stopper().Stop(ctx)
40454045
store0, store2 := tc.GetFirstStoreFromServer(t, 0), tc.GetFirstStoreFromServer(t, 2)
4046-
sendingEng = store0.TODOEngine()
4047-
receivingEng = store2.TODOEngine()
4046+
sendingEng = store0.StateEngine()
4047+
receivingEng = store2.StateEngine()
40484048
distSender := tc.Servers[0].DistSenderI().(kv.Sender)
40494049

40504050
// This test works across 5 ranges in total. We start with a scratch

‎pkg/kv/kvserver/client_raft_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ func TestReplicateRange(t *testing.T) {
304304
// Verify no intent remains on range descriptor key.
305305
key := keys.RangeDescriptorKey(rhsDesc.StartKey)
306306
desc := roachpb.RangeDescriptor{}
307-
if ok, err := storage.MVCCGetProto(ctx, store.TODOEngine(), key,
307+
if ok, err := storage.MVCCGetProto(ctx, store.StateEngine(), key,
308308
store.Clock().Now(), &desc, storage.MVCCGetOptions{}); err != nil {
309309
t.Fatal(err)
310310
} else if !ok {
@@ -317,7 +317,7 @@ func TestReplicateRange(t *testing.T) {
317317
meta1 := keys.RangeMetaKey(meta2)
318318
for _, key := range []roachpb.RKey{meta2, meta1} {
319319
metaDesc := roachpb.RangeDescriptor{}
320-
if ok, err := storage.MVCCGetProto(ctx, store.TODOEngine(), key.AsRawKey(),
320+
if ok, err := storage.MVCCGetProto(ctx, store.StateEngine(), key.AsRawKey(),
321321
store.Clock().Now(), &metaDesc, storage.MVCCGetOptions{}); err != nil {
322322
return err
323323
} else if !ok {
@@ -5930,12 +5930,12 @@ func TestRaftSnapshotsWithMVCCRangeKeys(t *testing.T) {
59305930
rangeKVWithTS("a", "b", ts1, storage.MVCCValue{}),
59315931
rangeKVWithTS("b", "c", ts2, storage.MVCCValue{}),
59325932
rangeKVWithTS("b", "c", ts1, storage.MVCCValue{}),
5933-
}, storageutils.ScanRange(t, store.TODOEngine(), descA))
5933+
}, storageutils.ScanRange(t, store.StateEngine(), descA))
59345934
require.Equal(t, kvs{
59355935
rangeKVWithTS("c", "d", ts2, storage.MVCCValue{}),
59365936
rangeKVWithTS("c", "d", ts1, storage.MVCCValue{}),
59375937
rangeKVWithTS("d", "e", ts2, storage.MVCCValue{}),
5938-
}, storageutils.ScanRange(t, store.TODOEngine(), descC))
5938+
}, storageutils.ScanRange(t, store.StateEngine(), descC))
59395939
}
59405940

59415941
// Quick check of MVCC stats.
@@ -5978,7 +5978,7 @@ func TestRaftSnapshotsWithMVCCRangeKeysEverywhere(t *testing.T) {
59785978
defer tc.Stopper().Stop(ctx)
59795979
srv := tc.Server(0)
59805980
store := tc.GetFirstStoreFromServer(t, 0)
5981-
engine := store.TODOEngine()
5981+
engine := store.StateEngine()
59825982
sender := srv.DB().NonTransactionalSender()
59835983

59845984
// Split off ranges at "a" and "b".
@@ -6029,7 +6029,7 @@ func TestRaftSnapshotsWithMVCCRangeKeysEverywhere(t *testing.T) {
60296029

60306030
// Look for the range keys on the other servers.
60316031
for _, srvIdx := range []int{1, 2} {
6032-
e := tc.GetFirstStoreFromServer(t, srvIdx).TODOEngine()
6032+
e := tc.GetFirstStoreFromServer(t, srvIdx).StateEngine()
60336033
for _, desc := range descs {
60346034
for _, span := range rditer.MakeReplicatedKeySpans(&desc) {
60356035
prefix := append(span.Key.Clone(), ':')

‎pkg/kv/kvserver/client_replica_gc_test.go‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,6 @@ func TestReplicaGCQueueDropReplicaDirect(t *testing.T) {
9595
repl1 := store.LookupReplica(roachpb.RKey(k))
9696
require.NotNil(t, repl1)
9797

98-
eng := store.TODOEngine()
99-
10098
// Put some bogus sideloaded data on the replica which we're about to
10199
// remove. Then, at the end of the test, check that that sideloaded
102100
// storage is now empty (in other words, GC'ing the Replica took care of
@@ -105,15 +103,10 @@ func TestReplicaGCQueueDropReplicaDirect(t *testing.T) {
105103
dir := repl1.SideloadedRaftMuLocked().Dir()
106104
repl1.RaftUnlock()
107105

108-
if dir == "" {
109-
t.Fatal("no sideloaded directory")
110-
}
111-
if err := eng.Env().MkdirAll(dir, os.ModePerm); err != nil {
112-
t.Fatal(err)
113-
}
114-
if err := fs.WriteFile(eng.Env(), filepath.Join(dir, "i1000000.t100000"), []byte("foo"), fs.UnspecifiedWriteCategory); err != nil {
115-
t.Fatal(err)
116-
}
106+
require.NotEmpty(t, dir, "no sideloaded directory")
107+
eng := store.StateEngine()
108+
require.NoError(t, eng.Env().MkdirAll(dir, os.ModePerm))
109+
require.NoError(t, fs.WriteFile(eng.Env(), filepath.Join(dir, "i1000000.t100000"), []byte("foo"), fs.UnspecifiedWriteCategory))
117110

118111
defer func() {
119112
if !t.Failed() {

‎pkg/kv/kvserver/client_replica_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func TestLeaseholdersRejectClockUpdateWithJump(t *testing.T) {
213213
// The clock did not advance and the final command was not executed.
214214
ts3 := s.Clock().Now()
215215
require.Zero(t, ts3.GoTime().Sub(ts2.GoTime()))
216-
valRes, err := storage.MVCCGet(context.Background(), store.TODOEngine(), key, ts3,
216+
valRes, err := storage.MVCCGet(context.Background(), store.StateEngine(), key, ts3,
217217
storage.MVCCGetOptions{})
218218
require.NoError(t, err)
219219
require.Equal(t, incArgs.Increment*numCmds, mustGetInt(valRes.Value))
@@ -3199,7 +3199,7 @@ func TestClearRange(t *testing.T) {
31993199
t.Helper()
32003200
start := prefix
32013201
end := prefix.PrefixEnd()
3202-
kvs, err := storage.Scan(context.Background(), store.TODOEngine(), start, end, 0)
3202+
kvs, err := storage.Scan(context.Background(), store.StateEngine(), start, end, 0)
32033203
if err != nil {
32043204
t.Fatal(err)
32053205
}

‎pkg/kv/kvserver/client_split_test.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func TestStoreSplitAbortSpan(t *testing.T) {
235235

236236
collect := func(as *abortspan.AbortSpan) []roachpb.AbortSpanEntry {
237237
var results []roachpb.AbortSpanEntry
238-
if err := as.Iterate(ctx, store.TODOEngine(), func(_ roachpb.Key, entry roachpb.AbortSpanEntry) error {
238+
if err := as.Iterate(ctx, store.StateEngine(), func(_ roachpb.Key, entry roachpb.AbortSpanEntry) error {
239239
entry.Priority = 0 // don't care about that
240240
results = append(results, entry)
241241
return nil
@@ -376,7 +376,7 @@ func TestStoreRangeSplitIntents(t *testing.T) {
376376
}
377377
for _, key := range []roachpb.Key{keys.RangeDescriptorKey(roachpb.RKeyMin), keys.RangeDescriptorKey(splitKeyAddr)} {
378378
if _, err := storage.MVCCGet(
379-
ctx, store.TODOEngine(), key, store.Clock().Now(), storage.MVCCGetOptions{},
379+
ctx, store.StateEngine(), key, store.Clock().Now(), storage.MVCCGetOptions{},
380380
); err != nil {
381381
t.Errorf("failed to read consistent range descriptor for key %s: %+v", key, err)
382382
}
@@ -392,7 +392,7 @@ func TestStoreRangeSplitIntents(t *testing.T) {
392392
// Verify the transaction record is gone.
393393
start := storage.MakeMVCCMetadataKey(keys.MakeRangeKeyPrefix(roachpb.RKeyMin))
394394
end := storage.MakeMVCCMetadataKey(keys.MakeRangeKeyPrefix(roachpb.RKeyMax))
395-
iter, err := store.TODOEngine().NewMVCCIterator(context.Background(), storage.MVCCKeyAndIntentsIterKind, storage.IterOptions{UpperBound: end.Key})
395+
iter, err := store.StateEngine().NewMVCCIterator(context.Background(), storage.MVCCKeyAndIntentsIterKind, storage.IterOptions{UpperBound: end.Key})
396396
if err != nil {
397397
t.Fatal(err)
398398
}
@@ -666,7 +666,7 @@ func TestStoreRangeSplitIdempotency(t *testing.T) {
666666
originalRepl := store.LookupReplica(roachpb.RKey(splitKey))
667667
require.NotNil(t, originalRepl)
668668
// Get the original stats for key and value bytes.
669-
ms, err := kvstorage.MakeStateLoader(originalRepl.RangeID).LoadMVCCStats(ctx, store.TODOEngine())
669+
ms, err := kvstorage.MakeStateLoader(originalRepl.RangeID).LoadMVCCStats(ctx, store.StateEngine())
670670
if err != nil {
671671
t.Fatal(err)
672672
}
@@ -685,7 +685,7 @@ func TestStoreRangeSplitIdempotency(t *testing.T) {
685685
}
686686
for _, key := range []roachpb.Key{keys.RangeDescriptorKey(roachpb.RKeyMin), keys.RangeDescriptorKey(splitKeyAddr)} {
687687
if _, err := storage.MVCCGet(
688-
context.Background(), store.TODOEngine(), key, store.Clock().Now(), storage.MVCCGetOptions{},
688+
context.Background(), store.StateEngine(), key, store.Clock().Now(), storage.MVCCGetOptions{},
689689
); err != nil {
690690
t.Fatal(err)
691691
}
@@ -739,12 +739,12 @@ func TestStoreRangeSplitIdempotency(t *testing.T) {
739739

740740
// Compare stats of split ranges to ensure they are non zero and
741741
// exceed the original range when summed.
742-
left, err := kvstorage.MakeStateLoader(originalRepl.RangeID).LoadMVCCStats(ctx, store.TODOEngine())
742+
left, err := kvstorage.MakeStateLoader(originalRepl.RangeID).LoadMVCCStats(ctx, store.StateEngine())
743743
if err != nil {
744744
t.Fatal(err)
745745
}
746746
lKeyBytes, lValBytes := left.KeyBytes, left.ValBytes
747-
right, err := kvstorage.MakeStateLoader(newRng.RangeID).LoadMVCCStats(ctx, store.TODOEngine())
747+
right, err := kvstorage.MakeStateLoader(newRng.RangeID).LoadMVCCStats(ctx, store.StateEngine())
748748
if err != nil {
749749
t.Fatal(err)
750750
}
@@ -797,7 +797,7 @@ func TestStoreRangeSplitMergeStats(t *testing.T) {
797797

798798
// Verify empty range has empty stats.
799799
repl := store.LookupReplica(roachpb.RKey(keyPrefix))
800-
assertRangeStats(t, "empty stats", store.TODOEngine(), repl.RangeID, enginepb.MVCCStats{})
800+
assertRangeStats(t, "empty stats", store.StateEngine(), repl.RangeID, enginepb.MVCCStats{})
801801

802802
// Write random data.
803803
splitKey := kvserver.WriteRandomDataToRange(t, store, repl.RangeID, keyPrefix)
@@ -809,7 +809,7 @@ func TestStoreRangeSplitMergeStats(t *testing.T) {
809809
// See: https://github.com/cockroachdb/cockroach/issues/129601#issuecomment-2309865742
810810
repl.RaftLock()
811811
replMS := repl.GetMVCCStats()
812-
snap := store.TODOEngine().NewSnapshot()
812+
snap := store.StateEngine().NewSnapshot()
813813
defer snap.Close()
814814
repl.RaftUnlock()
815815

@@ -825,7 +825,7 @@ func TestStoreRangeSplitMergeStats(t *testing.T) {
825825
_, pErr = repl.AdminSplit(ctx, *adminSplitArgs(splitKey), "test")
826826
require.NoError(t, pErr.GoError())
827827

828-
snap = store.TODOEngine().NewSnapshot()
828+
snap = store.StateEngine().NewSnapshot()
829829
defer snap.Close()
830830
msLeft, err := kvstorage.MakeStateLoader(repl.RangeID).LoadMVCCStats(ctx, snap)
831831
require.NoError(t, err)
@@ -886,7 +886,7 @@ func TestStoreRangeSplitMergeStats(t *testing.T) {
886886
require.NoError(t, pErr.GoError())
887887

888888
repl = store.LookupReplica(roachpb.RKey(keyPrefix))
889-
snap = store.TODOEngine().NewSnapshot()
889+
snap = store.StateEngine().NewSnapshot()
890890
defer snap.Close()
891891

892892
msMerged, err := kvstorage.MakeStateLoader(repl.RangeID).LoadMVCCStats(ctx, snap)
@@ -1006,7 +1006,7 @@ func TestStoreRangeSplitWithConcurrentWrites(t *testing.T) {
10061006
// Wait for the split to complete.
10071007
require.Nil(t, g.Wait())
10081008

1009-
snap := store.TODOEngine().NewSnapshot()
1009+
snap := store.StateEngine().NewSnapshot()
10101010
defer snap.Close()
10111011
lhsStats, err := kvstorage.MakeStateLoader(lhsRepl.RangeID).LoadMVCCStats(ctx, snap)
10121012
require.NoError(t, err)
@@ -1042,7 +1042,7 @@ func TestStoreRangeSplitWithConcurrentWrites(t *testing.T) {
10421042
_, pErr = rhsRepl.AdminSplit(ctx, *adminSplitArgs(splitKeyRight), "test")
10431043
require.NoError(t, pErr.GoError())
10441044

1045-
snap = store.TODOEngine().NewSnapshot()
1045+
snap = store.StateEngine().NewSnapshot()
10461046
defer snap.Close()
10471047
lhs1Stats, err := kvstorage.MakeStateLoader(lhsRepl.RangeID).LoadMVCCStats(ctx, snap)
10481048
require.NoError(t, err)
@@ -1416,7 +1416,7 @@ func TestStoreRangeSplitStatsWithMerges(t *testing.T) {
14161416
// NOTE that this value is expected to change over time, depending on what
14171417
// we store in the sys-local keyspace. Update it accordingly for this test.
14181418
empty := enginepb.MVCCStats{LastUpdateNanos: start.WallTime}
1419-
assertRangeStats(t, "empty stats", store.TODOEngine(), repl.RangeID, empty)
1419+
assertRangeStats(t, "empty stats", store.StateEngine(), repl.RangeID, empty)
14201420

14211421
// Write random TimeSeries data.
14221422
midKey := writeRandomTimeSeriesDataToRange(t, store, repl.RangeID, keyPrefix)
@@ -1427,7 +1427,7 @@ func TestStoreRangeSplitStatsWithMerges(t *testing.T) {
14271427
_, pErr = repl.AdminSplit(ctx, *adminSplitArgs(midKey), "test")
14281428
require.NoError(t, pErr.GoError())
14291429

1430-
snap := store.TODOEngine().NewSnapshot()
1430+
snap := store.StateEngine().NewSnapshot()
14311431
defer snap.Close()
14321432
msLeft, err := kvstorage.MakeStateLoader(repl.RangeID).LoadMVCCStats(ctx, snap)
14331433
require.NoError(t, err)
@@ -1463,7 +1463,7 @@ func fillRange(
14631463
src := rand.New(rand.NewSource(0))
14641464
var key []byte
14651465
for {
1466-
ms, err := kvstorage.MakeStateLoader(rangeID).LoadMVCCStats(context.Background(), store.TODOEngine())
1466+
ms, err := kvstorage.MakeStateLoader(rangeID).LoadMVCCStats(context.Background(), store.StateEngine())
14671467
if err != nil {
14681468
t.Fatal(err)
14691469
}
@@ -4506,7 +4506,7 @@ func TestSplitWithExternalFilesFastStats(t *testing.T) {
45064506

45074507
originalRepl := store.LookupReplica(roachpb.RKey(splitKey))
45084508
require.NotNil(t, originalRepl)
4509-
origStats, err := kvstorage.MakeStateLoader(originalRepl.RangeID).LoadMVCCStats(ctx, store.TODOEngine())
4509+
origStats, err := kvstorage.MakeStateLoader(originalRepl.RangeID).LoadMVCCStats(ctx, store.StateEngine())
45104510
require.NoError(t, err)
45114511
require.Greater(t, origStats.ContainsEstimates, int64(0), "range expected to have estimated stats")
45124512

‎pkg/kv/kvserver/split_queue_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func TestSplitQueueShouldQueue(t *testing.T) {
107107
cpy.EndKey = test.end
108108
replicaID := cpy.Replicas().VoterDescriptors()[0].ReplicaID
109109
require.NoError(t,
110-
kvstorage.MakeStateLoader(cpy.RangeID).SetRaftReplicaID(ctx, tc.store.TODOEngine(), replicaID))
110+
kvstorage.MakeStateLoader(cpy.RangeID).SetRaftReplicaID(ctx, tc.store.StateEngine(), replicaID))
111111
repl, err := loadInitializedReplicaForTesting(ctx, tc.store, &cpy, replicaID)
112112
if err != nil {
113113
t.Fatal(err)

0 commit comments

Comments
 (0)