Merge pull request #79744 from erikgrinaker/mvcc-range-tombstone-iterfix

storage: minor iterator tweaks

pkg/storage/intent_interleaving_iter.go

@@ -233,7 +233,7 @@ func newIntentInterleavingIterator(reader Reader, opts IterOptions) MVCCIterator
 	if reader.ConsistentIterators() {
 		iter = reader.NewMVCCIterator(MVCCKeyIterKind, opts)
 	} else {
-		iter = newMVCCIteratorByCloningEngineIter(intentIter, opts)
+		iter = newPebbleIterator(nil, intentIter.GetRawIter(), opts, StandardDurability)
 	}
 
 	*iiIter = intentInterleavingIter{
@@ -973,13 +973,6 @@ func (i *intentInterleavingIter) SupportsPrev() bool {
 	return true
 }
 
-// newMVCCIteratorByCloningEngineIter assumes MVCCKeyIterKind and no timestamp
-// hints. It uses pebble.Iterator.Clone to ensure that the two iterators see
-// the identical engine state.
-func newMVCCIteratorByCloningEngineIter(iter EngineIterator, opts IterOptions) MVCCIterator {
-	return newPebbleIterator(nil, iter.GetRawIter(), opts, StandardDurability)
-}
-
 // unsageMVCCIterator is used in RaceEnabled test builds to randomly inject
 // changes to unsafe keys retrieved from MVCCIterators.
 type unsafeMVCCIterator struct {

pkg/storage/pebble_batch.go

@@ -215,18 +215,18 @@ func (p *pebbleBatch) NewMVCCIterator(iterKind MVCCIterKind, opts IterOptions) M
 	if opts.Prefix {
 		iter = &p.prefixIter
 	}
+	handle := pebble.Reader(p.batch)
+	if !p.batch.Indexed() {
+		handle = p.db
+	}
 	if iter.inuse {
-		return newPebbleIterator(p.db, p.iter, opts, StandardDurability)
+		return newPebbleIterator(handle, p.iter, opts, StandardDurability)
 	}
 
 	if iter.iter != nil {
 		iter.setOptions(opts, StandardDurability)
 	} else {
-		if p.batch.Indexed() {
-			iter.init(p.batch, p.iter, p.iterUnused, opts, StandardDurability)
-		} else {
-			iter.init(p.db, p.iter, p.iterUnused, opts, StandardDurability)
-		}
+		iter.init(handle, p.iter, p.iterUnused, opts, StandardDurability)
 		if p.iter == nil {
 			// For future cloning.
 			p.iter = iter.iter
@@ -252,18 +252,18 @@ func (p *pebbleBatch) NewEngineIterator(opts IterOptions) EngineIterator {
 	if opts.Prefix {
 		iter = &p.prefixEngineIter
 	}
+	handle := pebble.Reader(p.batch)
+	if !p.batch.Indexed() {
+		handle = p.db
+	}
 	if iter.inuse {
-		return newPebbleIterator(p.db, p.iter, opts, StandardDurability)
+		return newPebbleIterator(handle, p.iter, opts, StandardDurability)
 	}
 
 	if iter.iter != nil {
 		iter.setOptions(opts, StandardDurability)
 	} else {
-		if p.batch.Indexed() {
-			iter.init(p.batch, p.iter, p.iterUnused, opts, StandardDurability)
-		} else {
-			iter.init(p.db, p.iter, p.iterUnused, opts, StandardDurability)
-		}
+		iter.init(handle, p.iter, p.iterUnused, opts, StandardDurability)
 		if p.iter == nil {
 			// For future cloning.
 			p.iter = iter.iter

pkg/storage/pebble_iterator.go

@@ -35,12 +35,9 @@ type pebbleIterator struct {
 	// Reusable buffer for MVCCKey or EngineKey encoding.
 	keyBuf []byte
 	// Buffers for copying iterator bounds to. Note that the underlying memory
-	// is not GCed upon Close(), to reduce the number of overall allocations. We
-	// use two slices for each of the bounds since this caller should not change
-	// the slice holding the current bounds, that the callee (pebble.MVCCIterator)
-	// is currently using, until after the caller has made the SetBounds call.
-	lowerBoundBuf [2][]byte
-	upperBoundBuf [2][]byte
+	// is not GCed upon Close(), to reduce the number of overall allocations.
+	lowerBoundBuf []byte
+	upperBoundBuf []byte
 
 	// Set to true to govern whether to call SeekPrefixGE or SeekGE. Skips
 	// SSTables based on MVCC/Engine key when true.
@@ -184,15 +181,15 @@ func (p *pebbleIterator) setOptions(opts IterOptions, durability DurabilityRequi
 		// Since we are encoding keys with an empty version anyway, we can just
 		// append the NUL byte instead of calling the above encode functions which
 		// will do the same thing.
-		p.lowerBoundBuf[0] = append(p.lowerBoundBuf[0][:0], opts.LowerBound...)
-		p.lowerBoundBuf[0] = append(p.lowerBoundBuf[0], 0x00)
-		p.options.LowerBound = p.lowerBoundBuf[0]
+		p.lowerBoundBuf = append(p.lowerBoundBuf[:0], opts.LowerBound...)
+		p.lowerBoundBuf = append(p.lowerBoundBuf, 0x00)
+		p.options.LowerBound = p.lowerBoundBuf
 	}
 	if opts.UpperBound != nil {
 		// Same as above.
-		p.upperBoundBuf[0] = append(p.upperBoundBuf[0][:0], opts.UpperBound...)
-		p.upperBoundBuf[0] = append(p.upperBoundBuf[0], 0x00)
-		p.options.UpperBound = p.upperBoundBuf[0]
+		p.upperBoundBuf = append(p.upperBoundBuf[:0], opts.UpperBound...)
+		p.upperBoundBuf = append(p.upperBoundBuf, 0x00)
+		p.options.UpperBound = p.upperBoundBuf
 	}
 
 	if opts.MaxTimestampHint.IsSet() {