kvserver: exclude stateEngine keys from overlapsUnreplicatedRangeIDLocalKeys

We previously made a simplified assumption in
overlapsUnreplicatedRangeIDLocalKeys where we treat
RangeTombstoneKey and RaftReplicaIDKey as LogEngine keys
(even though they are technically stateEngine keys). This
commit excludes RangeTombstoneKey and RaftReplicaIDKey from
that check. This will be useful when we add Engine assertions
(not only eval batch assertions).

Author: iskettaneh

pkg/kv/kvserver/replica_raftstorage.go

@@ -880,11 +880,11 @@ func (r *Replica) destroyInfoRaftMuLocked() kvstorage.DestroyReplicaInfo {
 	}
 }
 
-// overlapsUnreplicatedRangeIDLocalKeys checks if the provided span overlaps
-// with any unreplicated rangeID local keys.
+// overlapsLocalRaftKeys returns an error if the provided span overlaps with any
+// unreplicated rangeID local raft keys.
 // Note that we could receive the span with a nil startKey, which has a special
 // meaning that the span represents: [endKey.Prev(), endKey).
-func overlapsUnreplicatedRangeIDLocalKeys(span spanset.TrickySpan) error {
+func overlapsLocalRaftKeys(span spanset.TrickySpan) error {
 	fullRangeIDLocalSpans := roachpb.Span{
 		Key:    keys.LocalRangeIDPrefix.AsRawKey(),
 		EndKey: keys.LocalRangeIDPrefix.AsRawKey().PrefixEnd(),
@@ -916,14 +916,31 @@ func overlapsUnreplicatedRangeIDLocalKeys(span spanset.TrickySpan) error {
 		return errors.NewAssertionErrorWithWrappedErrf(err,
 			"could not decode range ID for span: %s", span)
 	}
-	if spanset.Overlaps(roachpb.Span{
+
+	// If the span is inside RangeIDLocalSpans but outside RangeIDUnreplicated,
+	// it cannot overlap local raft keys.
+	if !spanset.Overlaps(roachpb.Span{
 		Key:    keys.MakeRangeIDUnreplicatedPrefix(rangeID),
 		EndKey: keys.MakeRangeIDUnreplicatedPrefix(rangeID).PrefixEnd(),
 	}, span) {
-		return errors.Errorf("overlapping an unreplicated rangeID span")
+		return nil
 	}
 
-	return nil
+	// RangeTombstoneKey and RaftReplicaIDKey are considered part of the
+	// StateEngine keys. We need to exclude them from the check below.
+	if span.Key != nil && roachpb.Span(span).Equal(roachpb.Span{
+		Key: keys.RangeTombstoneKey(rangeID),
+	}) {
+		return nil
+	}
+
+	if span.Key != nil && roachpb.Span(span).Equal(roachpb.Span{
+		Key: keys.RaftReplicaIDKey(rangeID),
+	}) {
+		return nil
+	}
+
+	return errors.Errorf("overlapping an unreplicated rangeID span")
 }
 
 // overlapsStoreLocalKeys returns an error if the provided span overlaps

pkg/kv/kvserver/replica_read.go

@@ -94,7 +94,7 @@ func (r *Replica) executeReadOnlyBatch(
 	}
 	if util.RaceEnabled {
 		spans := g.LatchSpans()
-		spans.AddForbiddenMatcher(overlapsUnreplicatedRangeIDLocalKeys)
+		spans.AddForbiddenMatcher(overlapsLocalRaftKeys)
 		spans.AddForbiddenMatcher(overlapsStoreLocalKeys)
 		rw = spanset.NewReadWriterAt(rw, spans, ba.Timestamp)
 	}

pkg/kv/kvserver/replica_test.go

@@ -15202,10 +15202,9 @@ func TestLeaderlessWatcherInit(t *testing.T) {
 	}
 }
 
-// TestOverlapsUnreplicatedRangeIDLocalKeys verifies that the function
-// overlapsUnreplicatedRangeIDLocalKeys() successfully catches any overlap with
-// unreplicated rangeID local keys.
-func TestOverlapsUnreplicatedRangeIDLocalKeys(t *testing.T) {
+// TestOverlapsLocalRaftKeys verifies that the function overlapsLocalRaftKeys()
+// successfully catches any overlap with unreplicated rangeID local raft keys.
+func TestOverlapsLocalRaftKeys(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
@@ -15230,20 +15229,23 @@ func TestOverlapsUnreplicatedRangeIDLocalKeys(t *testing.T) {
 		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1).PrefixEnd(), roachpb.KeyMax), notOk: true}, // partial overlap
 		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1),
 			keys.MakeRangeIDUnreplicatedPrefix(1).PrefixEnd()), notOk: true},
+		{span: s(keys.RangeTombstoneKey(1), keys.RaftTruncatedStateKey(1)), notOk: true},
+		{span: s(keys.RaftReplicaIDKey(1), keys.RaftTruncatedStateKey(1)), notOk: true},
 		{span: s(keys.RaftTruncatedStateKey(1), roachpb.KeyMax), notOk: true},
 
 		// Point spans not overlapping with unreplicated local RangeID spans.
 		{span: s(roachpb.KeyMin, nil)},
 		{span: s(keys.LocalRangeIDPrefix.AsRawKey().Prevish(1), nil)},
-		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1).Prevish(1), nil)},
 		{span: s(keys.RangeForceFlushKey(1), nil)},
+		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1).Prevish(1), nil)},
+		{span: s(keys.RangeTombstoneKey(1), nil)},
+		{span: s(keys.RaftReplicaIDKey(1), nil)},
 		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1).PrefixEnd(), nil)},
 		{span: s(keys.LocalRangeIDPrefix.AsRawKey().PrefixEnd(), nil)},
 		{span: s(roachpb.KeyMax, nil)},
 
 		// Point spans overlapping with unreplicated local RangeID spans.
 		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1), nil), notOk: true},
-		{span: s(keys.RangeTombstoneKey(1), nil), notOk: true},
 		{span: s(keys.RaftTruncatedStateKey(1), nil), notOk: true},
 		{span: s(keys.RaftTruncatedStateKey(2), nil), notOk: true},
 		{span: s(keys.MakeRangeIDUnreplicatedPrefix(1).PrefixEnd().Prevish(1), nil), notOk: true},
@@ -15252,12 +15254,13 @@ func TestOverlapsUnreplicatedRangeIDLocalKeys(t *testing.T) {
 		{span: s(nil, keys.LocalRangeIDPrefix.AsRawKey())},
 		{span: s(nil, keys.MakeRangeIDUnreplicatedPrefix(1))},
 		{span: s(nil, keys.RangeForceFlushKey(1))},
+		{span: s(nil, keys.RangeTombstoneKey(1).Next())},
+		{span: s(nil, keys.RaftReplicaIDKey(1).Next())},
 		{span: s(nil, keys.MakeRangeIDUnreplicatedPrefix(1).PrefixEnd().Next())},
 		{span: s(nil, keys.LocalRangeIDPrefix.AsRawKey().PrefixEnd().Next())},
 
 		// Tricky spans overlapping with unreplicated local RangeID spans.
 		{span: s(nil, keys.MakeRangeIDUnreplicatedPrefix(1).Next()), notOk: true},
-		{span: s(nil, keys.RangeTombstoneKey(1).Next()), notOk: true},
 		{span: s(nil, keys.RaftTruncatedStateKey(1).Next()), notOk: true},
 		{span: s(nil, keys.RaftTruncatedStateKey(2).Next()), notOk: true},
 		{span: s(nil, keys.MakeRangeIDUnreplicatedPrefix(1).PrefixEnd()), notOk: true},
@@ -15266,7 +15269,7 @@ func TestOverlapsUnreplicatedRangeIDLocalKeys(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run("", func(t *testing.T) {
-			err := overlapsUnreplicatedRangeIDLocalKeys(spanset.TrickySpan(tc.span))
+			err := overlapsLocalRaftKeys(spanset.TrickySpan(tc.span))
 			require.Equal(t, tc.notOk, err != nil, tc.span)
 		})
 	}

pkg/kv/kvserver/replica_write.go

@@ -815,7 +815,7 @@ func (r *Replica) newBatchedEngine(g *concurrency.Guard) (storage.Batch, *storag
 		// any write latch would be declared at. But because of this, we don't
 		// assert on access timestamps using spanset.NewBatchAt.
 		spans := g.LatchSpans()
-		spans.AddForbiddenMatcher(overlapsUnreplicatedRangeIDLocalKeys)
+		spans.AddForbiddenMatcher(overlapsLocalRaftKeys)
 		spans.AddForbiddenMatcher(overlapsStoreLocalKeys)
 		batch = spanset.NewBatch(batch, spans)
 	}