Skip to content

Commit 3f879d2

Browse files
iskettanehiskettaneh
committed
spanset: split the SpanSet checking functionality into SpanChecker.
1 parent 51d7523 commit 3f879d2

File tree

7 files changed

+162
-128
lines changed

7 files changed

+162
-128
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ func TestRequestsSerializeWithAllKeys(t *testing.T) {
8585
if err != nil {
8686
t.Error(err)
8787
}
88-
if !allLatchSpans.Intersects(&otherLatchSpans) {
88+
checker := spanset.NewSpanChecker(&allLatchSpans)
89+
if !checker.Intersects(&otherLatchSpans) {
8990
t.Errorf("%s does not serialize with declareAllKeys", method)
9091
}
9192
})

‎pkg/kv/kvserver/replica_eval_context.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ func NewReplicaEvalContext(
109109
rec = newEvalContextImpl(ctx, r, requiresClosedTSOlderThanStorageSnap, ah)
110110
if util.RaceEnabled {
111111
return &SpanSetReplicaEvalContext{
112-
i: rec,
113-
ss: *ss,
112+
i: rec,
113+
checker: *spanset.NewSpanChecker(ss),
114114
}
115115
}
116116
return rec

‎pkg/kv/kvserver/replica_eval_context_span.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ import (
2929
// ReplicaEvalContext which verifies that access to state is registered in the
3030
// SpanSet if one is given.
3131
type SpanSetReplicaEvalContext struct {
32-
i batcheval.EvalContext
33-
ss spanset.SpanSet
32+
i batcheval.EvalContext
33+
checker spanset.SpanChecker
3434
}
3535

3636
var _ batcheval.EvalContext = &SpanSetReplicaEvalContext{}
@@ -103,7 +103,7 @@ func (rec *SpanSetReplicaEvalContext) IsFirstRange() bool {
103103
// Desc returns the Replica's RangeDescriptor.
104104
func (rec SpanSetReplicaEvalContext) Desc() *roachpb.RangeDescriptor {
105105
desc := rec.i.Desc()
106-
rec.ss.AssertAllowed(spanset.SpanReadOnly,
106+
rec.checker.AssertAllowed(spanset.SpanReadOnly,
107107
roachpb.Span{Key: keys.RangeDescriptorKey(desc.StartKey)},
108108
)
109109
return desc
@@ -144,7 +144,7 @@ func (rec SpanSetReplicaEvalContext) GetMaxSplitCPU(ctx context.Context) (float6
144144
func (rec SpanSetReplicaEvalContext) CanCreateTxnRecord(
145145
ctx context.Context, txnID uuid.UUID, txnKey []byte, txnMinTS hlc.Timestamp,
146146
) (bool, kvpb.TransactionAbortedReason) {
147-
rec.ss.AssertAllowed(spanset.SpanReadOnly,
147+
rec.checker.AssertAllowed(spanset.SpanReadOnly,
148148
roachpb.Span{Key: keys.TransactionKey(txnKey, txnID)},
149149
)
150150
return rec.i.CanCreateTxnRecord(ctx, txnID, txnKey, txnMinTS)
@@ -156,7 +156,7 @@ func (rec SpanSetReplicaEvalContext) CanCreateTxnRecord(
156156
func (rec SpanSetReplicaEvalContext) MinTxnCommitTS(
157157
ctx context.Context, txnID uuid.UUID, txnKey []byte,
158158
) hlc.Timestamp {
159-
rec.ss.AssertAllowed(spanset.SpanReadOnly,
159+
rec.checker.AssertAllowed(spanset.SpanReadOnly,
160160
roachpb.Span{Key: keys.TransactionKey(txnKey, txnID)},
161161
)
162162
return rec.i.MinTxnCommitTS(ctx, txnID, txnKey)
@@ -166,7 +166,7 @@ func (rec SpanSetReplicaEvalContext) MinTxnCommitTS(
166166
// keys are garbage collected. Reads and writes at timestamps <= this time will
167167
// not be served.
168168
func (rec SpanSetReplicaEvalContext) GetGCThreshold() hlc.Timestamp {
169-
rec.ss.AssertAllowed(spanset.SpanReadOnly,
169+
rec.checker.AssertAllowed(spanset.SpanReadOnly,
170170
roachpb.Span{Key: keys.RangeGCThresholdKey(rec.GetRangeID())},
171171
)
172172
return rec.i.GetGCThreshold()
@@ -190,7 +190,7 @@ func (rec SpanSetReplicaEvalContext) String() string {
190190
func (rec SpanSetReplicaEvalContext) GetLastReplicaGCTimestamp(
191191
ctx context.Context,
192192
) (hlc.Timestamp, error) {
193-
if err := rec.ss.CheckAllowed(spanset.SpanReadOnly,
193+
if err := rec.checker.CheckAllowed(spanset.SpanReadOnly,
194194
roachpb.Span{Key: keys.RangeLastReplicaGCTimestampKey(rec.GetRangeID())},
195195
); err != nil {
196196
return hlc.Timestamp{}, err
@@ -222,11 +222,11 @@ func (rec *SpanSetReplicaEvalContext) GetCurrentReadSummary(ctx context.Context)
222222
// To capture a read summary over the range, all keys must be latched for
223223
// writing to prevent any concurrent reads or writes.
224224
desc := rec.i.Desc()
225-
rec.ss.AssertAllowed(spanset.SpanReadWrite, roachpb.Span{
225+
rec.checker.AssertAllowed(spanset.SpanReadWrite, roachpb.Span{
226226
Key: keys.MakeRangeKeyPrefix(desc.StartKey),
227227
EndKey: keys.MakeRangeKeyPrefix(desc.EndKey),
228228
})
229-
rec.ss.AssertAllowed(spanset.SpanReadWrite, roachpb.Span{
229+
rec.checker.AssertAllowed(spanset.SpanReadWrite, roachpb.Span{
230230
Key: desc.StartKey.AsRawKey(),
231231
EndKey: desc.EndKey.AsRawKey(),
232232
})

‎pkg/kv/kvserver/replica_test.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2722,6 +2722,7 @@ func TestReplicaLatchingSplitDeclaresWrites(t *testing.T) {
27222722
0,
27232723
)
27242724
require.NoError(t, err)
2725+
checker := spanset.NewSpanChecker(&spans)
27252726
for _, tc := range []struct {
27262727
access spanset.SpanAccess
27272728
key roachpb.Key
@@ -2732,7 +2733,7 @@ func TestReplicaLatchingSplitDeclaresWrites(t *testing.T) {
27322733
{spanset.SpanReadWrite, roachpb.Key("b"), false},
27332734
{spanset.SpanReadWrite, roachpb.Key("d"), true},
27342735
} {
2735-
err := spans.CheckAllowed(tc.access, roachpb.Span{Key: tc.key})
2736+
err := checker.CheckAllowed(tc.access, roachpb.Span{Key: tc.key})
27362737
if tc.expectAccess {
27372738
require.NoError(t, err)
27382739
} else {
@@ -5344,7 +5345,11 @@ func TestAbortSpanError(t *testing.T) {
53445345
}
53455346

53465347
ec := newEvalContextImpl(ctx, tc.repl, false /* requireClosedTS */, kvpb.AdmissionHeader{})
5347-
rec := &SpanSetReplicaEvalContext{ec, *allSpans()}
5348+
ss := allSpans()
5349+
rec := &SpanSetReplicaEvalContext{
5350+
i: ec,
5351+
checker: *spanset.NewSpanChecker(ss),
5352+
}
53485353
pErr := checkIfTxnAborted(ctx, rec, tc.engine, txn)
53495354
if _, ok := pErr.GetDetail().(*kvpb.TransactionAbortedError); ok {
53505355
expected := txn.Clone()

‎pkg/kv/kvserver/spanset/batch.go‎

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import (
2323
// MVCCIterator wraps an storage.MVCCIterator and ensures that it can
2424
// only be used to access spans in a SpanSet.
2525
type MVCCIterator struct {
26-
i storage.MVCCIterator
27-
spans *SpanSet
26+
i storage.MVCCIterator
27+
checker *SpanChecker
2828

2929
// spansOnly controls whether or not timestamps associated with the
3030
// spans are considered when ensuring access. If set to true,
@@ -48,13 +48,13 @@ var _ storage.MVCCIterator = &MVCCIterator{}
4848
// iterator against the given SpanSet. Timestamps associated with the spans
4949
// in the spanset are not considered, only the span boundaries are checked.
5050
func NewIterator(iter storage.MVCCIterator, spans *SpanSet) *MVCCIterator {
51-
return &MVCCIterator{i: iter, spans: spans, spansOnly: true}
51+
return &MVCCIterator{i: iter, checker: NewSpanChecker(spans), spansOnly: true}
5252
}
5353

5454
// NewIteratorAt constructs an iterator that verifies access of the underlying
5555
// iterator against the given SpanSet at the given timestamp.
5656
func NewIteratorAt(iter storage.MVCCIterator, spans *SpanSet, ts hlc.Timestamp) *MVCCIterator {
57-
return &MVCCIterator{i: iter, spans: spans, ts: ts}
57+
return &MVCCIterator{i: iter, checker: NewSpanChecker(spans), ts: ts}
5858
}
5959

6060
// Close is part of the storage.MVCCIterator interface.
@@ -137,9 +137,9 @@ func (i *MVCCIterator) checkAllowed(span roachpb.Span, errIfDisallowed bool) {
137137
func (i *MVCCIterator) checkAllowedValidPos(span roachpb.Span, errIfDisallowed bool) {
138138
var err error
139139
if i.spansOnly {
140-
err = i.spans.CheckAllowed(SpanReadOnly, span)
140+
err = i.checker.CheckAllowed(SpanReadOnly, span)
141141
} else {
142-
err = i.spans.CheckAllowedAt(SpanReadOnly, span, i.ts)
142+
err = i.checker.CheckAllowedAt(SpanReadOnly, span, i.ts)
143143
}
144144
if errIfDisallowed {
145145
i.err = err
@@ -213,11 +213,11 @@ func (i *MVCCIterator) FindSplitKey(
213213
start, end, minSplitKey roachpb.Key, targetSize int64,
214214
) (storage.MVCCKey, error) {
215215
if i.spansOnly {
216-
if err := i.spans.CheckAllowed(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}); err != nil {
216+
if err := i.checker.CheckAllowed(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}); err != nil {
217217
return storage.MVCCKey{}, err
218218
}
219219
} else {
220-
if err := i.spans.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}, i.ts); err != nil {
220+
if err := i.checker.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}, i.ts); err != nil {
221221
return storage.MVCCKey{}, err
222222
}
223223
}
@@ -242,8 +242,9 @@ func (i *MVCCIterator) UnsafeLazyValue() pebble.LazyValue {
242242
// EngineIterator wraps a storage.EngineIterator and ensures that it can
243243
// only be used to access spans in a SpanSet.
244244
type EngineIterator struct {
245-
i storage.EngineIterator
246-
spans *SpanSet
245+
i storage.EngineIterator
246+
//spans *SpanSet
247+
checker *SpanChecker
247248
spansOnly bool
248249
ts hlc.Timestamp
249250
}
@@ -261,10 +262,10 @@ func (i *EngineIterator) SeekEngineKeyGE(key storage.EngineKey) (valid bool, err
261262
}
262263
if key.IsMVCCKey() && !i.spansOnly {
263264
mvccKey, _ := key.ToMVCCKey()
264-
if err := i.spans.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: mvccKey.Key}, i.ts); err != nil {
265+
if err := i.checker.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: mvccKey.Key}, i.ts); err != nil {
265266
return false, err
266267
}
267-
} else if err = i.spans.CheckAllowed(SpanReadOnly, roachpb.Span{Key: key.Key}); err != nil {
268+
} else if err = i.checker.CheckAllowed(SpanReadOnly, roachpb.Span{Key: key.Key}); err != nil {
268269
return false, err
269270
}
270271
return valid, err
@@ -278,10 +279,10 @@ func (i *EngineIterator) SeekEngineKeyLT(key storage.EngineKey) (valid bool, err
278279
}
279280
if key.IsMVCCKey() && !i.spansOnly {
280281
mvccKey, _ := key.ToMVCCKey()
281-
if err := i.spans.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: mvccKey.Key}, i.ts); err != nil {
282+
if err := i.checker.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: mvccKey.Key}, i.ts); err != nil {
282283
return false, err
283284
}
284-
} else if err = i.spans.CheckAllowed(SpanReadOnly, roachpb.Span{EndKey: key.Key}); err != nil {
285+
} else if err = i.checker.CheckAllowed(SpanReadOnly, roachpb.Span{EndKey: key.Key}); err != nil {
285286
return false, err
286287
}
287288
return valid, err
@@ -313,7 +314,7 @@ func (i *EngineIterator) SeekEngineKeyGEWithLimit(
313314
if state != pebble.IterValid {
314315
return state, err
315316
}
316-
if err = i.spans.CheckAllowed(SpanReadOnly, roachpb.Span{Key: key.Key}); err != nil {
317+
if err = i.checker.CheckAllowed(SpanReadOnly, roachpb.Span{Key: key.Key}); err != nil {
317318
return pebble.IterExhausted, err
318319
}
319320
return state, err
@@ -327,7 +328,7 @@ func (i *EngineIterator) SeekEngineKeyLTWithLimit(
327328
if state != pebble.IterValid {
328329
return state, err
329330
}
330-
if err = i.spans.CheckAllowed(SpanReadOnly, roachpb.Span{EndKey: key.Key}); err != nil {
331+
if err = i.checker.CheckAllowed(SpanReadOnly, roachpb.Span{EndKey: key.Key}); err != nil {
331332
return pebble.IterExhausted, err
332333
}
333334
return state, err
@@ -354,11 +355,11 @@ func (i *EngineIterator) checkKeyAllowed() (valid bool, err error) {
354355
}
355356
if key.IsMVCCKey() && !i.spansOnly {
356357
mvccKey, _ := key.ToMVCCKey()
357-
if err := i.spans.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: mvccKey.Key}, i.ts); err != nil {
358+
if err := i.checker.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: mvccKey.Key}, i.ts); err != nil {
358359
// Invalid, but no error.
359360
return false, nil // nolint:returnerrcheck
360361
}
361-
} else if err = i.spans.CheckAllowed(SpanReadOnly, roachpb.Span{Key: key.Key}); err != nil {
362+
} else if err = i.checker.CheckAllowed(SpanReadOnly, roachpb.Span{Key: key.Key}); err != nil {
362363
// Invalid, but no error.
363364
return false, nil // nolint:returnerrcheck
364365
}
@@ -431,8 +432,8 @@ func (i *EngineIterator) Stats() storage.IteratorStats {
431432
}
432433

433434
type spanSetReader struct {
434-
r storage.Reader
435-
spans *SpanSet
435+
r storage.Reader
436+
checker *SpanChecker
436437

437438
spansOnly bool
438439
ts hlc.Timestamp
@@ -469,11 +470,11 @@ func (s spanSetReader) MVCCIterate(
469470
f func(storage.MVCCKeyValue, storage.MVCCRangeKeyStack) error,
470471
) error {
471472
if s.spansOnly {
472-
if err := s.spans.CheckAllowed(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}); err != nil {
473+
if err := s.checker.CheckAllowed(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}); err != nil {
473474
return err
474475
}
475476
} else {
476-
if err := s.spans.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}, s.ts); err != nil {
477+
if err := s.checker.CheckAllowedAt(SpanReadOnly, roachpb.Span{Key: start, EndKey: end}, s.ts); err != nil {
477478
return err
478479
}
479480
}
@@ -488,9 +489,9 @@ func (s spanSetReader) NewMVCCIterator(
488489
return nil, err
489490
}
490491
if s.spansOnly {
491-
return NewIterator(mvccIter, s.spans), nil
492+
return NewIterator(mvccIter, s.checker.spans), nil
492493
}
493-
return NewIteratorAt(mvccIter, s.spans, s.ts), nil
494+
return NewIteratorAt(mvccIter, s.checker.spans, s.ts), nil
494495
}
495496

496497
func (s spanSetReader) NewEngineIterator(
@@ -501,8 +502,9 @@ func (s spanSetReader) NewEngineIterator(
501502
return nil, err
502503
}
503504
return &EngineIterator{
504-
i: engineIter,
505-
spans: s.spans,
505+
i: engineIter,
506+
//spans: s.checker.spans,
507+
checker: s.checker,
506508
spansOnly: s.spansOnly,
507509
ts: s.ts,
508510
}, nil
@@ -519,8 +521,8 @@ func (s spanSetReader) PinEngineStateForIterators(readCategory fs.ReadCategory)
519521
}
520522

521523
type spanSetWriter struct {
522-
w storage.Writer
523-
spans *SpanSet
524+
w storage.Writer
525+
checker *SpanChecker
524526

525527
spansOnly bool
526528
ts hlc.Timestamp
@@ -535,11 +537,11 @@ func (s spanSetWriter) ApplyBatchRepr(repr []byte, sync bool) error {
535537

536538
func (s spanSetWriter) checkAllowed(key roachpb.Key) error {
537539
if s.spansOnly {
538-
if err := s.spans.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key}); err != nil {
540+
if err := s.checker.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key}); err != nil {
539541
return err
540542
}
541543
} else {
542-
if err := s.spans.CheckAllowedAt(SpanReadWrite, roachpb.Span{Key: key}, s.ts); err != nil {
544+
if err := s.checker.CheckAllowedAt(SpanReadWrite, roachpb.Span{Key: key}, s.ts); err != nil {
543545
return err
544546
}
545547
}
@@ -561,7 +563,7 @@ func (s spanSetWriter) ClearUnversioned(key roachpb.Key, opts storage.ClearOptio
561563
}
562564

563565
func (s spanSetWriter) ClearEngineKey(key storage.EngineKey, opts storage.ClearOptions) error {
564-
if err := s.spans.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
566+
if err := s.checker.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
565567
return err
566568
}
567569
return s.w.ClearEngineKey(key, opts)
@@ -575,11 +577,11 @@ func (s spanSetWriter) SingleClearEngineKey(key storage.EngineKey) error {
575577

576578
func (s spanSetWriter) checkAllowedRange(start, end roachpb.Key) error {
577579
if s.spansOnly {
578-
if err := s.spans.CheckAllowed(SpanReadWrite, roachpb.Span{Key: start, EndKey: end}); err != nil {
580+
if err := s.checker.CheckAllowed(SpanReadWrite, roachpb.Span{Key: start, EndKey: end}); err != nil {
579581
return err
580582
}
581583
} else {
582-
if err := s.spans.CheckAllowedAt(SpanReadWrite, roachpb.Span{Key: start, EndKey: end}, s.ts); err != nil {
584+
if err := s.checker.CheckAllowedAt(SpanReadWrite, roachpb.Span{Key: start, EndKey: end}, s.ts); err != nil {
583585
return err
584586
}
585587
}
@@ -661,11 +663,11 @@ func (s spanSetWriter) ClearMVCCRangeKey(rangeKey storage.MVCCRangeKey) error {
661663

662664
func (s spanSetWriter) Merge(key storage.MVCCKey, value []byte) error {
663665
if s.spansOnly {
664-
if err := s.spans.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
666+
if err := s.checker.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
665667
return err
666668
}
667669
} else {
668-
if err := s.spans.CheckAllowedAt(SpanReadWrite, roachpb.Span{Key: key.Key}, s.ts); err != nil {
670+
if err := s.checker.CheckAllowedAt(SpanReadWrite, roachpb.Span{Key: key.Key}, s.ts); err != nil {
669671
return err
670672
}
671673
}
@@ -697,7 +699,7 @@ func (s spanSetWriter) PutEngineKey(key storage.EngineKey, value []byte) error {
697699
if !s.spansOnly {
698700
panic("cannot do timestamp checking for putting EngineKey")
699701
}
700-
if err := s.spans.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
702+
if err := s.checker.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
701703
return err
702704
}
703705
return s.w.PutEngineKey(key, value)
@@ -731,17 +733,19 @@ var _ storage.ReadWriter = ReadWriter{}
731733

732734
func makeSpanSetReadWriter(rw storage.ReadWriter, spans *SpanSet) ReadWriter {
733735
spans = addLockTableSpans(spans)
736+
checker := NewSpanChecker(spans)
734737
return ReadWriter{
735-
spanSetReader: spanSetReader{r: rw, spans: spans, spansOnly: true},
736-
spanSetWriter: spanSetWriter{w: rw, spans: spans, spansOnly: true},
738+
spanSetReader: spanSetReader{r: rw, checker: checker, spansOnly: true},
739+
spanSetWriter: spanSetWriter{w: rw, checker: checker, spansOnly: true},
737740
}
738741
}
739742

740743
func makeSpanSetReadWriterAt(rw storage.ReadWriter, spans *SpanSet, ts hlc.Timestamp) ReadWriter {
741744
spans = addLockTableSpans(spans)
745+
checker := NewSpanChecker(spans)
742746
return ReadWriter{
743-
spanSetReader: spanSetReader{r: rw, spans: spans, ts: ts},
744-
spanSetWriter: spanSetWriter{w: rw, spans: spans, ts: ts},
747+
spanSetReader: spanSetReader{r: rw, checker: checker, ts: ts},
748+
spanSetWriter: spanSetWriter{w: rw, checker: checker, ts: ts},
745749
}
746750
}
747751

@@ -752,7 +756,7 @@ func makeSpanSetReadWriterAt(rw storage.ReadWriter, spans *SpanSet, ts hlc.Times
752756
// NewReader clones and does not retain the provided span set.
753757
func NewReader(r storage.Reader, spans *SpanSet, ts hlc.Timestamp) storage.Reader {
754758
spans = addLockTableSpans(spans)
755-
return spanSetReader{r: r, spans: spans, ts: ts}
759+
return spanSetReader{r: r, checker: NewSpanChecker(spans), ts: ts}
756760
}
757761

758762
// NewReadWriterAt returns a storage.ReadWriter that asserts access of the

0 commit comments

Comments
 (0)