spanset: assert read-only batches don't access store local and unreplicated RangeID local keys

Author: iskettaneh

pkg/kv/kvserver/replica.go

@@ -36,6 +36,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/logstore"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rafttrace"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rangefeed"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/split"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/tenantrate"
 	"github.com/cockroachdb/cockroach/pkg/raft"
@@ -2984,3 +2985,66 @@ func (r *Replica) SendStreamStats(stats *rac2.RangeSendStreamStats) {
 		r.flowControlV2.SendStreamStats(stats)
 	}
 }
+
+// overlapsUnreplicatedRangeIDLocalKeys checks if the provided span overlaps
+// with any unreplicated rangeID local 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 {
+	fullRangeIDLocalSpans := roachpb.Span{
+		Key:    keys.LocalRangeIDPrefix.AsRawKey(),
+		EndKey: keys.LocalRangeIDPrefix.AsRawKey().PrefixEnd(),
+	}
+
+	// If the provided span is completely outside the rangeID local spans for any
+	// rangeID, then there is no overlap with any rangeID local keys.
+	if !spanset.Overlaps(fullRangeIDLocalSpans, span) {
+		return nil
+	}
+
+	// At this point, we know that we overlap with fullRangeIDLocalSpans. If we
+	// are not completely within fullRangeIDLocalSpans, return an error as we
+	// make an assumption that spans should respect the local RangeID tree
+	// structure, and that spans that partially overlaps with
+	// fullRangeIDLocalSpans don't make logical sense.
+	if !spanset.Contains(fullRangeIDLocalSpans, span) {
+		return errors.Errorf("overlapping an unreplicated rangeID key")
+	}
+
+	// If the span in inside fullRangeIDLocalSpans, we expect that both start and
+	// end keys should be in the same rangeID.
+	rangeIDKey := span.Key
+	if rangeIDKey == nil {
+		rangeIDKey = span.EndKey
+	}
+	rangeID, err := keys.DecodeRangeIDPrefix(rangeIDKey)
+	if err != nil {
+		return errors.NewAssertionErrorWithWrappedErrf(err,
+			"could not decode range ID for span: %s", span)
+	}
+	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
+}
+
+// overlapsStoreLocalKeys returns an error if the provided span overlaps
+// with any store local 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 overlapsStoreLocalKeys(span spanset.TrickySpan) error {
+	localStoreSpan := roachpb.Span{
+		Key:    keys.LocalStorePrefix,
+		EndKey: keys.LocalStoreMax,
+	}
+
+	if spanset.Overlaps(localStoreSpan, span) {
+		return errors.Errorf("overlaps with store local keys")
+	}
+
+	return nil
+}

pkg/kv/kvserver/replica_read.go

@@ -24,7 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/fs"
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
@@ -92,8 +92,11 @@ func (r *Replica) executeReadOnlyBatch(
 	if err := rw.PinEngineStateForIterators(readCategory); err != nil {
 		return nil, g, nil, kvpb.NewError(err)
 	}
-	if util.RaceEnabled {
-		rw = spanset.NewReadWriterAt(rw, g.LatchSpans(), ba.Timestamp)
+	if buildutil.CrdbTestBuild {
+		spans := g.LatchSpans()
+		spans.AddForbiddenMatcher(overlapsUnreplicatedRangeIDLocalKeys)
+		spans.AddForbiddenMatcher(overlapsStoreLocalKeys)
+		rw = spanset.NewReadWriterAt(rw, spans, ba.Timestamp)
 	}
 	defer rw.Close()
 

pkg/kv/kvserver/replica_write.go

@@ -10,7 +10,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
@@ -889,66 +888,3 @@ func releaseMVCCStats(ms *enginepb.MVCCStats) {
 	ms.Reset()
 	mvccStatsPool.Put(ms)
 }
-
-// overlapsUnreplicatedRangeIDLocalKeys checks if the provided span overlaps
-// with any unreplicated rangeID local 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 {
-	fullRangeIDLocalSpans := roachpb.Span{
-		Key:    keys.LocalRangeIDPrefix.AsRawKey(),
-		EndKey: keys.LocalRangeIDPrefix.AsRawKey().PrefixEnd(),
-	}
-
-	// If the provided span is completely outside the rangeID local spans for any
-	// rangeID, then there is no overlap with any rangeID local keys.
-	if !spanset.Overlaps(fullRangeIDLocalSpans, span) {
-		return nil
-	}
-
-	// At this point, we know that we overlap with fullRangeIDLocalSpans. If we
-	// are not completely within fullRangeIDLocalSpans, return an error as we
-	// make an assumption that spans should respect the local RangeID tree
-	// structure, and that spans that partially overlaps with
-	// fullRangeIDLocalSpans don't make logical sense.
-	if !spanset.Contains(fullRangeIDLocalSpans, span) {
-		return errors.Errorf("overlapping an unreplicated rangeID key")
-	}
-
-	// If the span in inside fullRangeIDLocalSpans, we expect that both start and
-	// end keys should be in the same rangeID.
-	rangeIDKey := span.Key
-	if rangeIDKey == nil {
-		rangeIDKey = span.EndKey
-	}
-	rangeID, err := keys.DecodeRangeIDPrefix(rangeIDKey)
-	if err != nil {
-		return errors.NewAssertionErrorWithWrappedErrf(err,
-			"could not decode range ID for span: %s", span)
-	}
-	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
-}
-
-// overlapsStoreLocalKeys returns an error if the provided span overlaps
-// with any store local 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 overlapsStoreLocalKeys(span spanset.TrickySpan) error {
-	localStoreSpan := roachpb.Span{
-		Key:    keys.LocalStorePrefix,
-		EndKey: keys.LocalStoreMax,
-	}
-
-	if spanset.Overlaps(localStoreSpan, span) {
-		return errors.Errorf("overlaps with store local keys")
-	}
-
-	return nil
-}