Skip to content

Commit

Permalink
storage: rename Writer.Clear*Range methods
Browse files Browse the repository at this point in the history
This renames the `Clear*Range` methods on `Writer` in an attempt to
clarify their semantics, keeping their upcoming range key effects in
mind.

* `ClearMVCCRange` → `ClearMVCCVersions` (only point key versions)
* `ClearMVCCRangeAndIntents` → `ClearMVCCRange` (everything in span)
* `ClearIterRange` → `ClearMVCCIteratorRange` (uses `MVCCIterator`)

Release note: None
  • Loading branch information
erikgrinaker committed May 1, 2022
1 parent 905b1df commit 54b5232
Show file tree
Hide file tree
Showing 16 changed files with 84 additions and 91 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/abortspan/abortspan.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (sc *AbortSpan) max() roachpb.Key {
func (sc *AbortSpan) ClearData(e storage.Engine) error {
b := e.NewUnindexedBatch(false /* writeOnly */)
defer b.Close()
if err := b.ClearIterRange(sc.min(), sc.max()); err != nil {
if err := b.ClearMVCCIteratorRange(sc.min(), sc.max()); err != nil {
return err
}
return b.Commit(false /* sync */)
Expand Down
28 changes: 14 additions & 14 deletions pkg/kv/kvserver/batch_spanset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,43 +72,43 @@ func TestSpanSetBatchBoundaries(t *testing.T) {

t.Run("writes before range", func(t *testing.T) {
if err := batch.ClearUnversioned(outsideKey.Key); !isWriteSpanErr(err) {
t.Errorf("Clear: unexpected error %v", err)
t.Errorf("ClearUnversioned: unexpected error %v", err)
}
if err := batch.ClearRawRange(outsideKey.Key, outsideKey2.Key); !isWriteSpanErr(err) {
t.Errorf("ClearRange: unexpected error %v", err)
t.Errorf("ClearRawRange: unexpected error %v", err)
}
{
err := batch.ClearIterRange(outsideKey.Key, outsideKey2.Key)
err := batch.ClearMVCCIteratorRange(outsideKey.Key, outsideKey2.Key)
if !isWriteSpanErr(err) {
t.Errorf("ClearIterRange: unexpected error %v", err)
t.Errorf("ClearMVCCIteratorRange: unexpected error %v", err)
}
}
if err := batch.Merge(outsideKey, nil); !isWriteSpanErr(err) {
t.Errorf("Merge: unexpected error %v", err)
}
if err := batch.PutUnversioned(outsideKey.Key, nil); !isWriteSpanErr(err) {
t.Errorf("Put: unexpected error %v", err)
t.Errorf("PutUnversioned: unexpected error %v", err)
}
})

t.Run("writes after range", func(t *testing.T) {
if err := batch.ClearUnversioned(outsideKey3.Key); !isWriteSpanErr(err) {
t.Errorf("Clear: unexpected error %v", err)
t.Errorf("ClearUnversioned: unexpected error %v", err)
}
if err := batch.ClearRawRange(insideKey2.Key, outsideKey4.Key); !isWriteSpanErr(err) {
t.Errorf("ClearRange: unexpected error %v", err)
t.Errorf("ClearRawRange: unexpected error %v", err)
}
{
err := batch.ClearIterRange(outsideKey2.Key, outsideKey4.Key)
err := batch.ClearMVCCIteratorRange(outsideKey2.Key, outsideKey4.Key)
if !isWriteSpanErr(err) {
t.Errorf("ClearIterRange: unexpected error %v", err)
t.Errorf("ClearMVCCIteratorRange: unexpected error %v", err)
}
}
if err := batch.Merge(outsideKey3, nil); !isWriteSpanErr(err) {
t.Errorf("Merge: unexpected error %v", err)
}
if err := batch.PutUnversioned(outsideKey3.Key, nil); !isWriteSpanErr(err) {
t.Errorf("Put: unexpected error %v", err)
t.Errorf("PutUnversioned: unexpected error %v", err)
}
})

Expand Down Expand Up @@ -318,19 +318,19 @@ func TestSpanSetBatchTimestamps(t *testing.T) {

for _, batch := range []storage.Batch{batchBefore, batchNonMVCC} {
if err := batch.ClearUnversioned(wkey.Key); !isWriteSpanErr(err) {
t.Errorf("Clear: unexpected error %v", err)
t.Errorf("ClearUnversioned: unexpected error %v", err)
}
{
err := batch.ClearIterRange(wkey.Key, wkey.Key)
err := batch.ClearMVCCIteratorRange(wkey.Key, wkey.Key)
if !isWriteSpanErr(err) {
t.Errorf("ClearIterRange: unexpected error %v", err)
t.Errorf("ClearMVCCIteratorRange: unexpected error %v", err)
}
}
if err := batch.Merge(wkey, nil); !isWriteSpanErr(err) {
t.Errorf("Merge: unexpected error %v", err)
}
if err := batch.PutUnversioned(wkey.Key, nil); !isWriteSpanErr(err) {
t.Errorf("Put: unexpected error %v", err)
t.Errorf("PutUnversioned: unexpected error %v", err)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/batcheval/cmd_clear_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ func ClearRange(
if statsDelta.ContainsEstimates == 0 && statsDelta.Total() < ClearRangeBytesThreshold {
log.VEventf(ctx, 2, "delta=%d < threshold=%d; using non-range clear",
statsDelta.Total(), ClearRangeBytesThreshold)
if err = readWriter.ClearIterRange(from, to); err != nil {
if err = readWriter.ClearMVCCIteratorRange(from, to); err != nil {
return result.Result{}, err
}
return pd, nil
}

if err := readWriter.ClearMVCCRangeAndIntents(from, to); err != nil {
if err := readWriter.ClearMVCCRange(from, to); err != nil {
return result.Result{}, err
}
return pd, nil
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/batcheval/cmd_clear_range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ type wrappedBatch struct {
clearRangeCount int
}

func (wb *wrappedBatch) ClearIterRange(start, end roachpb.Key) error {
func (wb *wrappedBatch) ClearMVCCIteratorRange(start, end roachpb.Key) error {
wb.clearIterCount++
return wb.Batch.ClearIterRange(start, end)
return wb.Batch.ClearMVCCIteratorRange(start, end)
}

func (wb *wrappedBatch) ClearMVCCRangeAndIntents(start, end roachpb.Key) error {
func (wb *wrappedBatch) ClearMVCCRange(start, end roachpb.Key) error {
wb.clearRangeCount++
return wb.Batch.ClearMVCCRangeAndIntents(start, end)
return wb.Batch.ClearMVCCRange(start, end)
}

// TestCmdClearRangeBytesThreshold verifies that clear range resorts to
Expand Down
12 changes: 6 additions & 6 deletions pkg/kv/kvserver/spanset/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,25 +588,25 @@ func (s spanSetWriter) ClearRawRange(start, end roachpb.Key) error {
return s.w.ClearRawRange(start, end)
}

func (s spanSetWriter) ClearMVCCRangeAndIntents(start, end roachpb.Key) error {
func (s spanSetWriter) ClearMVCCRange(start, end roachpb.Key) error {
if err := s.checkAllowedRange(start, end); err != nil {
return err
}
return s.w.ClearMVCCRangeAndIntents(start, end)
return s.w.ClearMVCCRange(start, end)
}

func (s spanSetWriter) ClearMVCCRange(start, end storage.MVCCKey) error {
func (s spanSetWriter) ClearMVCCVersions(start, end storage.MVCCKey) error {
if err := s.checkAllowedRange(start.Key, end.Key); err != nil {
return err
}
return s.w.ClearMVCCRange(start, end)
return s.w.ClearMVCCVersions(start, end)
}

func (s spanSetWriter) ClearIterRange(start, end roachpb.Key) error {
func (s spanSetWriter) ClearMVCCIteratorRange(start, end roachpb.Key) error {
if err := s.checkAllowedRange(start, end); err != nil {
return err
}
return s.w.ClearIterRange(start, end)
return s.w.ClearMVCCIteratorRange(start, end)
}

func (s spanSetWriter) ExperimentalPutMVCCRangeKey(rangeKey storage.MVCCRangeKey) error {
Expand Down
8 changes: 4 additions & 4 deletions pkg/storage/bench_pebble_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,18 @@ func BenchmarkMVCCDeleteRange_Pebble(b *testing.B) {
}
}

func BenchmarkClearMVCCRange_Pebble(b *testing.B) {
func BenchmarkClearMVCCVersions_Pebble(b *testing.B) {
skip.UnderShort(b)
ctx := context.Background()
runClearRange(ctx, b, setupMVCCPebble, func(eng Engine, batch Batch, start, end MVCCKey) error {
return batch.ClearMVCCRange(start, end)
return batch.ClearMVCCVersions(start, end)
})
}

func BenchmarkClearIterRange_Pebble(b *testing.B) {
func BenchmarkClearMVCCIteratorRange_Pebble(b *testing.B) {
ctx := context.Background()
runClearRange(ctx, b, setupMVCCPebble, func(eng Engine, batch Batch, start, end MVCCKey) error {
return batch.ClearIterRange(start.Key, end.Key)
return batch.ClearMVCCIteratorRange(start.Key, end.Key)
})
}

Expand Down
33 changes: 15 additions & 18 deletions pkg/storage/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,32 +612,29 @@ type Writer interface {
//
// It is safe to modify the contents of the arguments after it returns.
ClearRawRange(start, end roachpb.Key) error
// ClearMVCCRangeAndIntents removes MVCC keys and intents from start (inclusive)
// to end (exclusive). This is a higher-level method that handles both
// interleaved and separated intents. Similar to the other Clear* methods,
// this method actually removes entries from the storage engine.
// ClearMVCCRange removes MVCC keys from start (inclusive) to end (exclusive),
// including intents. Similar to the other Clear* methods, this method
// actually removes entries from the storage engine.
//
// It is safe to modify the contents of the arguments after it returns.
//
// TODO(erikgrinaker): This should clear range keys too.
ClearMVCCRangeAndIntents(start, end roachpb.Key) error
// ClearMVCCRange removes MVCC keys from start (inclusive) to end
// (exclusive). It should not be expected to clear intents, though may clear
// interleaved intents that it encounters. It is meant for efficiently
// clearing a subset of versions of a key, since the parameters are MVCCKeys
// and not roachpb.Keys. Similar to the other Clear* methods, this method
// actually removes entries from the storage engine.
ClearMVCCRange(start, end roachpb.Key) error
// ClearMVCCVersions removes MVCC point key versions from start (inclusive) to
// end (exclusive). It does not affect intents nor range keys. It is meant for
// efficiently clearing a subset of versions of a key, since the parameters
// are MVCCKeys and not roachpb.Keys. Similar to the other Clear* methods,
// this method actually removes entries from the storage engine.
//
// It is safe to modify the contents of the arguments after it returns.
ClearMVCCRange(start, end MVCCKey) error

// ClearIterRange removes all keys in the given span using an iterator to
// iterate over point keys and remove them from the storage engine using
// per-key storage tombstones (not MVCC tombstones). Any separated
// intents/locks will also be cleared.
ClearMVCCVersions(start, end MVCCKey) error
// ClearMVCCIteratorRange removes all keys in the given span using an MVCC
// iterator and clearing individual point keys (including intents). Similar to
// the other Clear* methods, this method actually removes entries from the
// storage engine.
//
// TODO(erikgrinaker): This should clear range keys too.
ClearIterRange(start, end roachpb.Key) error
ClearMVCCIteratorRange(start, end roachpb.Key) error

// ExperimentalClearMVCCRangeKey deletes an MVCC range key from start
// (inclusive) to end (exclusive) at the given timestamp. For any range key
Expand Down
8 changes: 4 additions & 4 deletions pkg/storage/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ func TestEngineDeleteRange(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
testEngineDeleteRange(t, func(engine Engine, start, end MVCCKey) error {
return engine.ClearMVCCRange(start, end)
return engine.ClearMVCCVersions(start, end)
})
}

Expand All @@ -998,7 +998,7 @@ func TestEngineDeleteRangeBatch(t *testing.T) {
testEngineDeleteRange(t, func(engine Engine, start, end MVCCKey) error {
batch := engine.NewUnindexedBatch(true /* writeOnly */)
defer batch.Close()
if err := batch.ClearMVCCRange(start, end); err != nil {
if err := batch.ClearMVCCVersions(start, end); err != nil {
return err
}
batch2 := engine.NewUnindexedBatch(true /* writeOnly */)
Expand All @@ -1010,11 +1010,11 @@ func TestEngineDeleteRangeBatch(t *testing.T) {
})
}

func TestEngineDeleteIterRange(t *testing.T) {
func TestEngineDeleteRangeIterator(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
testEngineDeleteRange(t, func(engine Engine, start, end MVCCKey) error {
return engine.ClearIterRange(start.Key, end.Key)
return engine.ClearMVCCIteratorRange(start.Key, end.Key)
})
}

Expand Down
11 changes: 4 additions & 7 deletions pkg/storage/intent_reader_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,10 @@ func (idw intentDemuxWriter) PutIntent(
return buf, idw.w.PutEngineKey(engineKey, value)
}

// ClearMVCCRangeAndIntents has the same behavior as
// Writer.ClearMVCCRangeAndIntents. buf is used as scratch-space to avoid
// allocations -- its contents will be overwritten and not appended to, and a
// possibly different buf returned.
func (idw intentDemuxWriter) ClearMVCCRangeAndIntents(
start, end roachpb.Key, buf []byte,
) ([]byte, error) {
// ClearMVCCRange has the same behavior as Writer.ClearMVCCRange. buf is used as
// scratch-space to avoid allocations -- its contents will be overwritten and
// not appended to, and a possibly different buf returned.
func (idw intentDemuxWriter) ClearMVCCRange(start, end roachpb.Key, buf []byte) ([]byte, error) {
err := idw.w.ClearRawRange(start, end)
if err != nil {
return buf, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/intent_reader_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func TestIntentDemuxWriter(t *testing.T) {
pw.reset()
start := scanRoachKey(t, d, "start")
end := scanRoachKey(t, d, "end")
if scratch, err = w.ClearMVCCRangeAndIntents(start, end, scratch); err != nil {
if scratch, err = w.ClearMVCCRange(start, end, scratch); err != nil {
return err.Error()
}
printEngContents(&pw.b, eng)
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/metamorphic/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ func (c clearRangeOp) run(ctx context.Context) string {
// Empty range. No-op.
return "no-op due to no non-conflicting key range"
}
err := c.m.engine.ClearMVCCRangeAndIntents(c.key, c.endKey)
err := c.m.engine.ClearMVCCRange(c.key, c.endKey)
if err != nil {
return fmt.Sprintf("error: %s", err.Error())
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/mvcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2036,7 +2036,7 @@ func MVCCClearTimeRange(

flushClearedKeys := func(nonMatch MVCCKey) error {
if len(clearRangeStart.Key) != 0 {
if err := rw.ClearMVCCRange(clearRangeStart, nonMatch); err != nil {
if err := rw.ClearMVCCVersions(clearRangeStart, nonMatch); err != nil {
return err
}
batchByteSize += int64(clearRangeStart.EncodedSize() + nonMatch.EncodedSize())
Expand All @@ -2051,7 +2051,7 @@ func MVCCClearTimeRange(
// clearrange, the byte size of the keys we did get is now too large to
// encode them all within the byte size limit, so use clearrange anyway.
if batchByteSize+encodedBufSize >= maxBatchByteSize {
if err := rw.ClearMVCCRange(buf[0], nonMatch); err != nil {
if err := rw.ClearMVCCVersions(buf[0], nonMatch); err != nil {
return err
}
batchByteSize += int64(buf[0].EncodedSize() + nonMatch.EncodedSize())
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/mvcc_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ func cmdCheckIntent(e *evalCtx) error {

func cmdClearRange(e *evalCtx) error {
key, endKey := e.getKeyRange()
return e.engine.ClearMVCCRangeAndIntents(key, endKey)
return e.engine.ClearMVCCRange(key, endKey)
}

func cmdCPut(e *evalCtx) error {
Expand Down
23 changes: 11 additions & 12 deletions pkg/storage/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -1213,15 +1213,14 @@ func (p *Pebble) ClearRawRange(start, end roachpb.Key) error {
return p.clearRange(MVCCKey{Key: start}, MVCCKey{Key: end})
}

// ClearMVCCRangeAndIntents implements the Engine interface.
func (p *Pebble) ClearMVCCRangeAndIntents(start, end roachpb.Key) error {
_, err := p.wrappedIntentWriter.ClearMVCCRangeAndIntents(start, end, nil)
// ClearMVCCRange implements the Engine interface.
func (p *Pebble) ClearMVCCRange(start, end roachpb.Key) error {
_, err := p.wrappedIntentWriter.ClearMVCCRange(start, end, nil)
return err

}

// ClearMVCCRange implements the Engine interface.
func (p *Pebble) ClearMVCCRange(start, end MVCCKey) error {
// ClearMVCCVersions implements the Engine interface.
func (p *Pebble) ClearMVCCVersions(start, end MVCCKey) error {
return p.clearRange(start, end)
}

Expand All @@ -1231,13 +1230,13 @@ func (p *Pebble) clearRange(start, end MVCCKey) error {
return p.db.DeleteRange(bufStart, bufEnd, pebble.Sync)
}

// ClearIterRange implements the Engine interface.
func (p *Pebble) ClearIterRange(start, end roachpb.Key) error {
// ClearMVCCIteratorRange implements the Engine interface.
func (p *Pebble) ClearMVCCIteratorRange(start, end roachpb.Key) error {
// Write all the tombstones in one batch.
batch := p.NewUnindexedBatch(false /* writeOnly */)
defer batch.Close()

if err := batch.ClearIterRange(start, end); err != nil {
if err := batch.ClearMVCCIteratorRange(start, end); err != nil {
return err
}
return batch.Commit(true)
Expand Down Expand Up @@ -2091,15 +2090,15 @@ func (p *pebbleReadOnly) ClearRawRange(start, end roachpb.Key) error {
panic("not implemented")
}

func (p *pebbleReadOnly) ClearMVCCRangeAndIntents(start, end roachpb.Key) error {
func (p *pebbleReadOnly) ClearMVCCRange(start, end roachpb.Key) error {
panic("not implemented")
}

func (p *pebbleReadOnly) ClearMVCCRange(start, end MVCCKey) error {
func (p *pebbleReadOnly) ClearMVCCVersions(start, end MVCCKey) error {
panic("not implemented")
}

func (p *pebbleReadOnly) ClearIterRange(start, end roachpb.Key) error {
func (p *pebbleReadOnly) ClearMVCCIteratorRange(start, end roachpb.Key) error {
panic("not implemented")
}

Expand Down
Loading

0 comments on commit 54b5232

Please sign in to comment.