Skip to content

Commit

Permalink
storage: hide raw iterator from EngineIterator interface
Browse files Browse the repository at this point in the history
Previously, the EngineIterator interface exposed a GetRawIter method to
retrieve the underlying Pebble iterator. This existed only to facilitate
iterator clones and was intended to only ever be consumed by the storage
package itself.

This commit replaces GetRawIter with a CloneContext method that returns an
opaque CloneContext type that contains the raw iterator. This prevents external
packages from directly using the raw Pebble iterator. It also prepares for the
introduction of aggregation of iterator stats, providing an envelope for
propagating information on where to aggregate iterator stats when the clonee
iterator closes.

Epic: None
Release note: None
  • Loading branch information
jbowens committed Mar 27, 2023
1 parent 20e41ff commit 7657c00
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 12 deletions.
7 changes: 3 additions & 4 deletions pkg/kv/kvserver/spanset/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/pebbleiter"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
Expand Down Expand Up @@ -421,9 +420,9 @@ func (i *EngineIterator) UnsafeRawEngineKey() []byte {
return i.i.UnsafeRawEngineKey()
}

// GetRawIter is part of the storage.EngineIterator interface.
func (i *EngineIterator) GetRawIter() pebbleiter.Iterator {
return i.i.GetRawIter()
// CloneContext is part of the storage.EngineIterator interface.
func (i *EngineIterator) CloneContext() storage.CloneContext {
return i.i.CloneContext()
}

// Stats is part of the storage.EngineIterator interface.
Expand Down
12 changes: 9 additions & 3 deletions pkg/storage/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ type EngineIterator interface {
// Value returns the current value as a byte slice.
// REQUIRES: latest positioning function returned valid=true.
Value() ([]byte, error)
// GetRawIter is a low-level method only for use in the storage package,
// that returns the underlying pebble Iterator.
GetRawIter() pebbleiter.Iterator
// CloneContext is a low-level method only for use in the storage package,
// that provides sufficient context that the iterator may be cloned.
CloneContext() CloneContext
// SeekEngineKeyGEWithLimit is similar to SeekEngineKeyGE, but takes an
// additional exclusive upper limit parameter. The limit is semantically
// best-effort, and is an optimization to avoid O(n^2) iteration behavior in
Expand All @@ -388,6 +388,12 @@ type EngineIterator interface {
Stats() IteratorStats
}

// CloneContext is an opaque type encapsulating sufficient context to construct
// a clone of an existing iterator.
type CloneContext struct {
rawIter pebbleiter.Iterator
}

// IterOptions contains options used to create an {MVCC,Engine}Iterator.
//
// For performance, every {MVCC,Engine}Iterator must specify either Prefix or
Expand Down
3 changes: 2 additions & 1 deletion pkg/storage/intent_interleaving_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ func newIntentInterleavingIterator(reader Reader, opts IterOptions) MVCCIterator
if reader.ConsistentIterators() {
iter = maybeUnwrapUnsafeIter(reader.NewMVCCIterator(MVCCKeyIterKind, opts)).(*pebbleIterator)
} else {
iter = newPebbleIteratorByCloning(intentIter.GetRawIter(), opts, StandardDurability)
cloneCtx := intentIter.CloneContext()
iter = newPebbleIteratorByCloning(cloneCtx.rawIter, opts, StandardDurability)
}

*iiIter = intentInterleavingIter{
Expand Down
3 changes: 2 additions & 1 deletion pkg/storage/mvcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -5052,7 +5052,8 @@ func MVCCResolveWriteIntentRange(
mvccIter = rw.NewMVCCIterator(MVCCKeyIterKind, iterOpts)
} else {
// For correctness, we need mvccIter to be consistent with engineIter.
mvccIter = newPebbleIteratorByCloning(engineIter.GetRawIter(), iterOpts, StandardDurability)
cloneCtx := engineIter.CloneContext()
mvccIter = newPebbleIteratorByCloning(cloneCtx.rawIter, iterOpts, StandardDurability)
}
iterAndBuf := GetBufUsingIter(mvccIter)
defer func() {
Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/pebble_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,9 @@ func (p *pebbleIterator) IsPrefix() bool {
return p.prefix
}

// GetRawIter is part of the EngineIterator interface.
func (p *pebbleIterator) GetRawIter() pebbleiter.Iterator {
return p.iter
// CloneContext is part of the EngineIterator interface.
func (p *pebbleIterator) CloneContext() CloneContext {
return CloneContext{rawIter: p.iter}
}

func (p *pebbleIterator) getBlockPropertyFilterMask() pebble.BlockPropertyFilterMask {
Expand Down

0 comments on commit 7657c00

Please sign in to comment.