Skip to content

Commit ed7f5cd

Browse files
iskettanehiskettaneh
committed
spanset: assert that batches don't access store local and unreplicated RangeID local keys
This commit adds the following test-only assertions: 1) Generated batches don't touch store-local keys. 2) Generated batches don't touch unreplicated RangeID local keys. We disable the check in exactly 3 locations we know that we currently touch those keys. Fixes: #156537 Release note: None
1 parent 9c63594 commit ed7f5cd

File tree

5 files changed

+346
-4
lines changed

5 files changed

+346
-4
lines changed

‎pkg/keys/constants.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ var (
163163
//
164164
// LocalStorePrefix is the prefix identifying per-store data.
165165
LocalStorePrefix = makeKey(LocalPrefix, roachpb.Key("s"))
166+
LocalStoreMax = roachpb.Key(LocalStorePrefix).PrefixEnd()
166167
// localStoreClusterVersionSuffix stores the cluster-wide version
167168
// information for this store, updated any time the operator
168169
// updates the minimum cluster version.

‎pkg/kv/kvserver/batcheval/cmd_end_transaction.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1328,8 +1328,10 @@ func splitTriggerHelper(
13281328
if err != nil {
13291329
return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to fetch last replica GC timestamp")
13301330
}
1331+
13311332
if err := storage.MVCCPutProto(
1332-
ctx, batch, keys.RangeLastReplicaGCTimestampKey(split.RightDesc.RangeID), hlc.Timestamp{},
1333+
ctx, spanset.DisableForbiddenSpanAssertionsOnBatch(batch),
1334+
keys.RangeLastReplicaGCTimestampKey(split.RightDesc.RangeID), hlc.Timestamp{},
13331335
&replicaGCTS, storage.MVCCWriteOptions{Category: fs.BatchEvalReadCategory}); err != nil {
13341336
return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to copy last replica GC timestamp")
13351337
}
@@ -1529,7 +1531,8 @@ func splitTriggerHelper(
15291531
// as all replicas will be responsible for writing it locally before
15301532
// applying the split.
15311533
if !rec.ClusterSettings().Version.IsActive(ctx, clusterversion.V25_4_WriteInitialTruncStateBeforeSplitApplication) {
1532-
if err := kvstorage.WriteInitialTruncState(ctx, batch, split.RightDesc.RangeID); err != nil {
1534+
if err := kvstorage.WriteInitialTruncState(ctx,
1535+
spanset.DisableForbiddenSpanAssertionsOnBatch(batch), split.RightDesc.RangeID); err != nil {
15331536
return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to write initial Replica state")
15341537
}
15351538
}

‎pkg/kv/kvserver/batcheval/cmd_truncate_log.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ func TruncateLog(
118118
// are not tracked in the raft log delta. The delta will be adjusted below
119119
// raft.
120120
// We can pass zero as nowNanos because we're only interested in SysBytes.
121-
ms, err := storage.ComputeStats(ctx, readWriter, start, end, 0 /* nowNanos */)
121+
ms, err := storage.ComputeStats(ctx,
122+
spanset.DisableForbiddenSpanAssertionsOnBatch(readWriter), start, end, 0 /* nowNanos */)
122123
if err != nil {
123124
return result.Result{}, errors.Wrap(err, "while computing stats of Raft log freed by truncation")
124125
}

‎pkg/kv/kvserver/replica_test.go‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15197,3 +15197,206 @@ func TestLeaderlessWatcherInit(t *testing.T) {
1519715197
t.Fatalf("expected LeaderlessWatcher channel to be closed")
1519815198
}
1519915199
}
15200+
15201+
// TestOverlapsUnreplicatedRangeIDLocalKeys verifies that the function
15202+
// overlapsUnreplicatedRangeIDLocalKeys() successfully catches any overlap with
15203+
// unreplicated rangeID local keys.
15204+
func TestOverlapsUnreplicatedRangeIDLocalKeys(t *testing.T) {
15205+
defer leaktest.AfterTest(t)()
15206+
defer log.Scope(t).Close(t)
15207+
testCases := []struct {
15208+
name string
15209+
span roachpb.Span
15210+
expectErr bool
15211+
}{
15212+
{
15213+
name: "point start key before unreplicated rangeID local span",
15214+
span: roachpb.Span{Key: keys.MakeRangeIDUnreplicatedPrefix(1).
15215+
Prevish(roachpb.PrevishKeyLength), EndKey: nil},
15216+
expectErr: false,
15217+
},
15218+
{
15219+
name: "point start key within unreplicated rangeID local span",
15220+
span: roachpb.Span{Key: keys.RaftTruncatedStateKey(1), EndKey: nil},
15221+
expectErr: true,
15222+
},
15223+
{
15224+
name: "point start key at unreplicated rangeID local span end",
15225+
span: roachpb.Span{Key: keys.MakeRangeIDUnreplicatedPrefix(1).
15226+
PrefixEnd(), EndKey: nil},
15227+
expectErr: false,
15228+
},
15229+
{
15230+
name: "point end key before unreplicated rangeID local span",
15231+
span: roachpb.Span{Key: nil, EndKey: keys.MakeRangeIDUnreplicatedPrefix(1)},
15232+
expectErr: false,
15233+
},
15234+
{
15235+
name: "point end key within unreplicated rangeID local span",
15236+
span: roachpb.Span{Key: nil, EndKey: keys.RaftTruncatedStateKey(1)},
15237+
expectErr: true,
15238+
},
15239+
{
15240+
name: "point end key at unreplicated rangeID local span end",
15241+
span: roachpb.Span{Key: nil, EndKey: keys.MakeRangeIDUnreplicatedPrefix(1).
15242+
PrefixEnd()},
15243+
expectErr: true,
15244+
},
15245+
{
15246+
name: "point end key after unreplicated rangeID local span",
15247+
span: roachpb.Span{Key: nil, EndKey: keys.MakeRangeIDUnreplicatedPrefix(1).
15248+
PrefixEnd().Next()},
15249+
expectErr: false,
15250+
},
15251+
{
15252+
name: "span fully before unreplicated rangeID local span",
15253+
span: roachpb.Span{Key: roachpb.KeyMin, EndKey: keys.MakeRangeIDUnreplicatedPrefix(0)},
15254+
expectErr: false,
15255+
},
15256+
{
15257+
name: "span fully after unreplicated rangeID local span",
15258+
span: roachpb.Span{Key: keys.LocalRangePrefix, EndKey: roachpb.KeyMax},
15259+
expectErr: false,
15260+
},
15261+
{
15262+
name: "span fully overlaps unreplicated rangeID local span",
15263+
span: roachpb.Span{Key: roachpb.KeyMin, EndKey: roachpb.KeyMax},
15264+
expectErr: true,
15265+
},
15266+
{
15267+
name: "span partially overlaps with start of unreplicated rangeID local span",
15268+
span: roachpb.Span{Key: roachpb.KeyMin, EndKey: keys.RaftTruncatedStateKey(1)},
15269+
expectErr: true,
15270+
},
15271+
{
15272+
name: "span partially overlaps with end of unreplicated rangeID local span",
15273+
span: roachpb.Span{Key: keys.RaftTruncatedStateKey(1), EndKey: roachpb.KeyMax},
15274+
expectErr: true,
15275+
},
15276+
{
15277+
name: "span within with unreplicated rangeID local span",
15278+
span: roachpb.Span{Key: keys.RaftTruncatedStateKey(1),
15279+
EndKey: keys.RangeLastReplicaGCTimestampKey(1)},
15280+
expectErr: true,
15281+
},
15282+
{
15283+
name: "span overlaps with multiple unreplicated range local span",
15284+
span: roachpb.Span{Key: keys.RaftTruncatedStateKey(1),
15285+
EndKey: keys.RaftTruncatedStateKey(3)},
15286+
expectErr: true,
15287+
},
15288+
{
15289+
name: "replicated range ID key",
15290+
span: roachpb.Span{Key: keys.RangeDescriptorKey(roachpb.RKey("a")),
15291+
EndKey: keys.RangeDescriptorKey(roachpb.RKey("a")).Next()},
15292+
expectErr: false,
15293+
},
15294+
}
15295+
15296+
for _, tc := range testCases {
15297+
t.Run(tc.name, func(t *testing.T) {
15298+
err := overlapsUnreplicatedRangeIDLocalKeys(tc.span)
15299+
if tc.expectErr {
15300+
require.Errorf(t, err, "expected error for span %s", tc.span)
15301+
} else {
15302+
require.NoErrorf(t, err, "expected no error for span %s", tc.span)
15303+
}
15304+
})
15305+
}
15306+
}
15307+
15308+
// TestOverlapsStoreLocalKeys verifies that the function
15309+
// overlapsStoreLocalKeys() successfully catches any overlap with
15310+
// store local keys.
15311+
func TestOverlapsStoreLocalKeys(t *testing.T) {
15312+
defer leaktest.AfterTest(t)()
15313+
defer log.Scope(t).Close(t)
15314+
testCases := []struct {
15315+
name string
15316+
span roachpb.Span
15317+
expectErr bool
15318+
}{
15319+
{
15320+
name: "point start key before store local span",
15321+
span: roachpb.Span{Key: roachpb.Key(keys.LocalStorePrefix).
15322+
Prevish(roachpb.PrevishKeyLength), EndKey: nil},
15323+
expectErr: false,
15324+
},
15325+
{
15326+
name: "point start key at store local span start",
15327+
span: roachpb.Span{Key: keys.LocalStorePrefix, EndKey: nil},
15328+
expectErr: true,
15329+
},
15330+
{
15331+
name: "point start key within store local span",
15332+
span: roachpb.Span{Key: keys.StoreIdentKey(), EndKey: nil},
15333+
expectErr: true,
15334+
},
15335+
{
15336+
name: "point start key at store local span end",
15337+
span: roachpb.Span{Key: keys.LocalStoreMax, EndKey: nil},
15338+
expectErr: false,
15339+
},
15340+
{
15341+
name: "point end key before store local span",
15342+
span: roachpb.Span{Key: nil, EndKey: keys.LocalStorePrefix},
15343+
expectErr: false,
15344+
},
15345+
{
15346+
name: "point end key at store local span start",
15347+
span: roachpb.Span{Key: nil, EndKey: roachpb.Key(keys.LocalStorePrefix).Next()},
15348+
expectErr: true,
15349+
},
15350+
{
15351+
name: "point end key within store local span",
15352+
span: roachpb.Span{Key: nil, EndKey: keys.StoreIdentKey()},
15353+
expectErr: true,
15354+
},
15355+
{
15356+
name: "point end key at store local span end",
15357+
span: roachpb.Span{Key: nil, EndKey: keys.LocalStoreMax},
15358+
expectErr: true,
15359+
},
15360+
{
15361+
name: "span fully before store local span",
15362+
span: roachpb.Span{Key: roachpb.KeyMin, EndKey: keys.LocalStorePrefix},
15363+
expectErr: false,
15364+
},
15365+
{
15366+
name: "span fully after store local span",
15367+
span: roachpb.Span{Key: keys.LocalStoreMax, EndKey: roachpb.KeyMax},
15368+
expectErr: false,
15369+
},
15370+
{
15371+
name: "span fully overlaps store local span",
15372+
span: roachpb.Span{Key: roachpb.KeyMin, EndKey: roachpb.KeyMax},
15373+
expectErr: true,
15374+
},
15375+
{
15376+
name: "span partially overlaps with start of store local span",
15377+
span: roachpb.Span{Key: roachpb.KeyMin, EndKey: keys.StoreIdentKey()},
15378+
expectErr: true,
15379+
},
15380+
{
15381+
name: "span partially overlaps with end of store local span",
15382+
span: roachpb.Span{Key: keys.StoreIdentKey(), EndKey: roachpb.KeyMax},
15383+
expectErr: true,
15384+
},
15385+
{
15386+
name: "span within store local span",
15387+
span: roachpb.Span{Key: keys.StoreGossipKey(), EndKey: keys.StoreIdentKey()},
15388+
expectErr: true,
15389+
},
15390+
}
15391+
15392+
for _, tc := range testCases {
15393+
t.Run(tc.name, func(t *testing.T) {
15394+
err := overlapsStoreLocalKeys(tc.span)
15395+
if tc.expectErr {
15396+
require.Errorf(t, err, "expected error for span %s", tc.span)
15397+
} else {
15398+
require.NoErrorf(t, err, "expected no error for span %s", tc.span)
15399+
}
15400+
})
15401+
}
15402+
}

0 commit comments

Comments
 (0)