Skip to content

Commit bbd252d

Browse files
committed
kvserver/spanset: introduce forbidden spans matchers
This commit introduces the concept of forbidden spans, which will allow us to assert our batches don't modify Raft's engine keys. References: #156537 Release note: None
1 parent 1befb1f commit bbd252d

File tree

7 files changed

+113
-22
lines changed

7 files changed

+113
-22
lines changed

pkg/kv/kvserver/batcheval/cmd_delete.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func Delete(
5858
// If requested, replace point tombstones with range tombstones.
5959
if cArgs.EvalCtx.EvalKnobs().UseRangeTombstonesForPointDeletes && h.Txn == nil {
6060
if err := storage.ReplacePointTombstonesWithRangeTombstones(
61-
ctx, spanset.DisableReadWriterAssertions(readWriter),
61+
ctx, spanset.DisableLatchAssertions(readWriter),
6262
cArgs.Stats, args.Key, args.EndKey); err != nil {
6363
return result.Result{}, err
6464
}

pkg/kv/kvserver/batcheval/cmd_delete_range.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ func deleteRangeTransactional(
302302
// If requested, replace point tombstones with range tombstones.
303303
if cArgs.EvalCtx.EvalKnobs().UseRangeTombstonesForPointDeletes && h.Txn == nil {
304304
if err := storage.ReplacePointTombstonesWithRangeTombstones(
305-
ctx, spanset.DisableReadWriterAssertions(readWriter),
305+
ctx, spanset.DisableLatchAssertions(readWriter),
306306
cArgs.Stats, args.Key, args.EndKey); err != nil {
307307
return result.Result{}, err
308308
}

pkg/kv/kvserver/batcheval/cmd_end_transaction.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ func resolveLocalLocksWithPagination(
719719
// If requested, replace point tombstones with range tombstones.
720720
if ok && evalCtx.EvalKnobs().UseRangeTombstonesForPointDeletes {
721721
if err := storage.ReplacePointTombstonesWithRangeTombstones(
722-
ctx, spanset.DisableReadWriterAssertions(readWriter),
722+
ctx, spanset.DisableLatchAssertions(readWriter),
723723
ms, update.Key, update.EndKey); err != nil {
724724
return 0, 0, 0, errors.Wrapf(err,
725725
"replacing point tombstones with range tombstones for write intent at %s on end transaction [%s]",
@@ -757,7 +757,7 @@ func resolveLocalLocksWithPagination(
757757
// If requested, replace point tombstones with range tombstones.
758758
if evalCtx.EvalKnobs().UseRangeTombstonesForPointDeletes {
759759
if err := storage.ReplacePointTombstonesWithRangeTombstones(
760-
ctx, spanset.DisableReadWriterAssertions(readWriter),
760+
ctx, spanset.DisableLatchAssertions(readWriter),
761761
ms, update.Key, update.EndKey); err != nil {
762762
return 0, 0, 0, errors.Wrapf(err,
763763
"replacing point tombstones with range tombstones for write intent range at %s on end transaction [%s]",

pkg/kv/kvserver/batcheval/cmd_resolve_intent.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func ResolveIntent(
129129
// If requested, replace point tombstones with range tombstones.
130130
if ok && cArgs.EvalCtx.EvalKnobs().UseRangeTombstonesForPointDeletes {
131131
if err := storage.ReplacePointTombstonesWithRangeTombstones(ctx,
132-
spanset.DisableReadWriterAssertions(readWriter), ms, update.Key, update.EndKey); err != nil {
132+
spanset.DisableLatchAssertions(readWriter), ms, update.Key, update.EndKey); err != nil {
133133
return result.Result{}, err
134134
}
135135
}

pkg/kv/kvserver/batcheval/cmd_resolve_intent_range.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func ResolveIntentRange(
9999
// If requested, replace point tombstones with range tombstones.
100100
if cArgs.EvalCtx.EvalKnobs().UseRangeTombstonesForPointDeletes {
101101
if err := storage.ReplacePointTombstonesWithRangeTombstones(ctx,
102-
spanset.DisableReadWriterAssertions(readWriter), ms, args.Key, args.EndKey); err != nil {
102+
spanset.DisableLatchAssertions(readWriter), ms, args.Key, args.EndKey); err != nil {
103103
return result.Result{}, err
104104
}
105105
}

pkg/kv/kvserver/spanset/batch.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,64 @@ func DisableReadWriterAssertions(rw storage.ReadWriter) storage.ReadWriter {
888888
}
889889
}
890890

891+
// DisableLatchAssertions returns a new batch wrapper with latch assertions
892+
// disabled. It does not modify the original batch. The returned batch shares
893+
// the same underlying storage.Batch but has its own SpanSet copies with
894+
// assertions disabled.
895+
func DisableLatchAssertions(rw storage.ReadWriter) storage.ReadWriter {
896+
switch v := rw.(type) {
897+
case *spanSetBatch:
898+
// Clone the span sets and disable assertions on the copies
899+
readSpans := v.ReadWriter.spanSetReader.spans.Copy()
900+
writeSpans := v.ReadWriter.spanSetWriter.spans.Copy()
901+
readSpans.DisableUndeclaredAccessAssertions()
902+
writeSpans.DisableUndeclaredAccessAssertions()
903+
904+
// Create a new spanSetBatch with the modified span sets.
905+
return &spanSetBatch{
906+
ReadWriter: ReadWriter{
907+
spanSetReader: spanSetReader{r: v.b, spans: readSpans, spansOnly: v.spansOnly, ts: v.ts},
908+
spanSetWriter: spanSetWriter{w: v.b, spans: writeSpans, spansOnly: v.spansOnly, ts: v.ts},
909+
},
910+
b: v.b,
911+
spans: readSpans, // Use the read spans as the main spans
912+
spansOnly: v.spansOnly,
913+
ts: v.ts,
914+
}
915+
default:
916+
return rw
917+
}
918+
}
919+
920+
// DisableForbiddenSpanAssertionsOnBatch returns a new batch wrapper with
921+
// forbidden span assertions disabled. It does not modify the original batch.
922+
// The returned batch shares the same underlying storage.Batch but has its own
923+
// SpanSet copies with assertions disabled.
924+
func DisableForbiddenSpanAssertionsOnBatch(rw storage.ReadWriter) storage.ReadWriter {
925+
switch v := rw.(type) {
926+
case *spanSetBatch:
927+
// Clone the span sets and disable assertions on the copies
928+
readSpans := v.ReadWriter.spanSetReader.spans.Copy()
929+
writeSpans := v.ReadWriter.spanSetWriter.spans.Copy()
930+
readSpans.DisableForbiddenSpansAssertions()
931+
writeSpans.DisableForbiddenSpansAssertions()
932+
933+
// Create a new spanSetBatch with the modified span sets.
934+
return &spanSetBatch{
935+
ReadWriter: ReadWriter{
936+
spanSetReader: spanSetReader{r: v.b, spans: readSpans, spansOnly: v.spansOnly, ts: v.ts},
937+
spanSetWriter: spanSetWriter{w: v.b, spans: writeSpans, spansOnly: v.spansOnly, ts: v.ts},
938+
},
939+
b: v.b,
940+
spans: readSpans, // Use the read spans as the main spans
941+
spansOnly: v.spansOnly,
942+
ts: v.ts,
943+
}
944+
default:
945+
return rw
946+
}
947+
}
948+
891949
// addLockTableSpans adds corresponding lock table spans for the declared
892950
// spans. This is to implicitly allow raw access to separated intents in the
893951
// lock table for any declared keys. Explicitly declaring lock table spans is

pkg/kv/kvserver/spanset/spanset.go

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,14 @@ type Span struct {
8383
// The Span slice for a particular access and scope contains non-overlapping
8484
// spans in increasing key order after calls to SortAndDedup.
8585
type SpanSet struct {
86-
spans [NumSpanAccess][NumSpanScope][]Span
87-
allowUndeclared bool
86+
spans [NumSpanAccess][NumSpanScope][]Span
87+
// forbiddenSpansMatchers are functions that return an error if the given span
88+
// shouldn't be accessed (forbidden). This allows for complex pattern matching
89+
// like forbidding specific keys across all range IDs without enumerating them
90+
// explicitly.
91+
forbiddenSpansMatchers []func(roachpb.Span) error
92+
allowUndeclared bool
93+
allowForbidden bool
8894
}
8995

9096
var spanSetPool = sync.Pool{
@@ -113,6 +119,8 @@ func (s *SpanSet) Release() {
113119
s.spans[sa][ss] = recycle
114120
}
115121
}
122+
s.forbiddenSpansMatchers = nil
123+
s.allowForbidden = false
116124
s.allowUndeclared = false
117125
spanSetPool.Put(s)
118126
}
@@ -155,7 +163,9 @@ func (s *SpanSet) Copy() *SpanSet {
155163
n.spans[sa][ss] = append(n.spans[sa][ss], s.spans[sa][ss]...)
156164
}
157165
}
166+
n.forbiddenSpansMatchers = append(n.forbiddenSpansMatchers, s.forbiddenSpansMatchers...)
158167
n.allowUndeclared = s.allowUndeclared
168+
n.allowForbidden = s.allowForbidden
159169
return n
160170
}
161171

@@ -201,14 +211,22 @@ func (s *SpanSet) AddMVCC(access SpanAccess, span roachpb.Span, timestamp hlc.Ti
201211
s.spans[access][scope] = append(s.spans[access][scope], Span{Span: span, Timestamp: timestamp})
202212
}
203213

214+
// AddForbiddenMatcher adds a forbidden span matcher. The matcher is a function
215+
// that is called for each span access to check if it should be forbidden.
216+
func (s *SpanSet) AddForbiddenMatcher(matcher func(roachpb.Span) error) {
217+
s.forbiddenSpansMatchers = append(s.forbiddenSpansMatchers, matcher)
218+
}
219+
204220
// Merge merges all spans in s2 into s. s2 is not modified.
205221
func (s *SpanSet) Merge(s2 *SpanSet) {
206222
for sa := SpanAccess(0); sa < NumSpanAccess; sa++ {
207223
for ss := SpanScope(0); ss < NumSpanScope; ss++ {
208224
s.spans[sa][ss] = append(s.spans[sa][ss], s2.spans[sa][ss]...)
209225
}
210226
}
227+
s.forbiddenSpansMatchers = append(s.forbiddenSpansMatchers, s2.forbiddenSpansMatchers...)
211228
s.allowUndeclared = s2.allowUndeclared
229+
s.allowForbidden = s2.allowForbidden
212230
s.SortAndDedup()
213231
}
214232

@@ -331,27 +349,37 @@ func (s *SpanSet) CheckAllowedAt(
331349
func (s *SpanSet) checkAllowed(
332350
access SpanAccess, span roachpb.Span, check func(SpanAccess, Span) bool,
333351
) error {
334-
if s.allowUndeclared {
335-
// If the request has specified that undeclared spans are allowed, do
336-
// nothing.
337-
return nil
352+
// Unless explicitly disabled, check if we access any forbidden spans.
353+
if !s.allowForbidden {
354+
// Check if the span is forbidden.
355+
for _, matcher := range s.forbiddenSpansMatchers {
356+
if err := matcher(span); err != nil {
357+
return errors.Errorf("cannot %s span %s: matches forbidden pattern",
358+
access, span)
359+
}
360+
}
338361
}
339362

340-
scope := SpanGlobal
341-
if (span.Key != nil && keys.IsLocal(span.Key)) ||
342-
(span.EndKey != nil && keys.IsLocal(span.EndKey)) {
343-
scope = SpanLocal
344-
}
363+
// Unless explicitly disabled, check if we access any undeclared spans.
364+
if !s.allowUndeclared {
365+
scope := SpanGlobal
366+
if (span.Key != nil && keys.IsLocal(span.Key)) ||
367+
(span.EndKey != nil && keys.IsLocal(span.EndKey)) {
368+
scope = SpanLocal
369+
}
345370

346-
for ac := access; ac < NumSpanAccess; ac++ {
347-
for _, cur := range s.spans[ac][scope] {
348-
if contains(cur.Span, span) && check(ac, cur) {
349-
return nil
371+
for ac := access; ac < NumSpanAccess; ac++ {
372+
for _, cur := range s.spans[ac][scope] {
373+
if contains(cur.Span, span) && check(ac, cur) {
374+
return nil
375+
}
350376
}
351377
}
378+
379+
return errors.Errorf("cannot %s undeclared span %s\ndeclared:\n%s\nstack:\n%s", access, span, s, debugutil.Stack())
352380
}
353381

354-
return errors.Errorf("cannot %s undeclared span %s\ndeclared:\n%s\nstack:\n%s", access, span, s, debugutil.Stack())
382+
return nil
355383
}
356384

357385
// contains returns whether s1 contains s2. Unlike Span.Contains, this function
@@ -396,3 +424,8 @@ func (s *SpanSet) Validate() error {
396424
func (s *SpanSet) DisableUndeclaredAccessAssertions() {
397425
s.allowUndeclared = true
398426
}
427+
428+
// DisableForbiddenAssertions disables forbidden spans assertions.
429+
func (s *SpanSet) DisableForbiddenSpansAssertions() {
430+
s.allowForbidden = true
431+
}

0 commit comments

Comments
 (0)