Merge #86110

86110: storage: use `RangeKeyChanged()` in `pebbleMVCCScanner` r=nicktrav a=erikgrinaker

This patch uses `RangeKeyChanged()` in `pebbleMVCCScanner` to detect
MVCC range keys and enable point key synthesis. This shows a ~5% speedup
for large scans with no range keys, but a minor regression for point
gets with range keys -- the latter caused by having to call both
`HasPointAndRange()` and `RangeKeyChanged()`, while previously only the
first was called.

```
name                                                                  old time/op    new time/op    delta
MVCCScan_Pebble/rows=1/versions=1/valueSize=64/numRangeKeys=0-24        5.62µs ± 2%    5.68µs ± 1%    ~     (p=0.058 n=10+8)
MVCCScan_Pebble/rows=1/versions=1/valueSize=64/numRangeKeys=1-24        12.2µs ± 3%    12.3µs ± 1%    ~     (p=0.113 n=10+9)
MVCCScan_Pebble/rows=100/versions=1/valueSize=64/numRangeKeys=0-24      38.9µs ± 1%    38.5µs ± 1%  -0.78%  (p=0.019 n=9+9)
MVCCScan_Pebble/rows=100/versions=1/valueSize=64/numRangeKeys=1-24      72.1µs ± 2%    72.7µs ± 2%  +0.83%  (p=0.043 n=10+10)
MVCCScan_Pebble/rows=10000/versions=1/valueSize=64/numRangeKeys=0-24    2.85ms ± 1%    2.72ms ± 1%  -4.63%  (p=0.000 n=9+10)
MVCCScan_Pebble/rows=10000/versions=1/valueSize=64/numRangeKeys=1-24    5.46ms ± 1%    5.48ms ± 1%    ~     (p=0.065 n=9+10)
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=0-24      3.34µs ± 1%    3.33µs ± 2%    ~     (p=0.591 n=10+10)
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=1-24      6.84µs ± 2%    7.00µs ± 1%  +2.35%  (p=0.000 n=10+8)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=0-24     4.31µs ± 3%    4.28µs ± 2%    ~     (p=0.225 n=10+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=1-24     9.56µs ± 2%    9.83µs ± 3%  +2.82%  (p=0.000 n=10+10)
```

Touches #82559.

Release note: None

Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>

pkg/storage/pebble_mvcc_scanner.go

@@ -439,6 +439,15 @@ func (p *pebbleMVCCScanner) init(
 // get iterates exactly once and adds one KV to the result set.
 func (p *pebbleMVCCScanner) get(ctx context.Context) {
 	p.isGet = true
+
+	// The iterator may already be positioned on a range key, in which
+	// case RangeKeyChanged() wouldn't fire.
+	if ok, _ := p.parent.Valid(); ok {
+		if _, hasRange := p.parent.HasPointAndRange(); hasRange {
+			p.enablePointSynthesis()
+		}
+	}
+
 	p.parent.SeekGE(MVCCKey{Key: p.start})
 	if !p.updateCurrent() {
 		return
@@ -456,8 +465,16 @@ func (p *pebbleMVCCScanner) scan(
 	if p.wholeRows && !p.results.lastOffsetsEnabled {
 		return nil, 0, 0, errors.AssertionFailedf("cannot use wholeRows without trackLastOffsets")
 	}
-
 	p.isGet = false
+
+	// The iterator may already be positioned on a range key, in which
+	// case RangeKeyChanged() wouldn't fire.
+	if ok, _ := p.parent.Valid(); ok {
+		if _, hasRange := p.parent.HasPointAndRange(); hasRange {
+			p.enablePointSynthesis()
+		}
+	}
+
 	if p.reverse {
 		if !p.iterSeekReverse(MVCCKey{Key: p.end}) {
 			return nil, 0, 0, p.err
@@ -1153,9 +1170,6 @@ func (p *pebbleMVCCScanner) updateCurrent() bool {
 	if !p.iterValid() {
 		return false
 	}
-	if !p.maybeEnablePointSynthesis() {
-		return false
-	}
 
 	p.curRawKey = p.parent.UnsafeRawMVCCKey()
 
@@ -1175,20 +1189,31 @@ func (p *pebbleMVCCScanner) updateCurrent() bool {
 	return true
 }
 
-// maybeEnablePointSynthesis checks if p.parent is on an MVCC range tombstone.
-// If it is, it wraps and replaces p.parent with a pointSynthesizingIter, which
+// enablePointSynthesis wraps p.parent with a pointSynthesizingIter, which
 // synthesizes MVCC point tombstones for MVCC range tombstones and never emits
-// range keys itself.
+// range keys itself. p.parent must be valid.
 //
-// Returns the iterator validity after a switch, and otherwise assumes the
-// iterator was valid when called and returns true if there is no change.
-func (p *pebbleMVCCScanner) maybeEnablePointSynthesis() bool {
-	if _, hasRange := p.parent.HasPointAndRange(); hasRange {
-		p.pointIter = newPointSynthesizingIterAtParent(p.parent)
-		p.parent = p.pointIter
-		return p.iterValid()
+// gcassert:inline
+func (p *pebbleMVCCScanner) enablePointSynthesis() {
+	if util.RaceEnabled {
+		if ok, _ := p.parent.Valid(); !ok {
+			panic(errors.AssertionFailedf("enablePointSynthesis called with invalid iter"))
+		}
+		if _, ok := p.parent.(*pointSynthesizingIter); ok {
+			panic(errors.AssertionFailedf("enablePointSynthesis called when already enabled"))
+		}
+		if _, hasRange := p.parent.HasPointAndRange(); !hasRange {
+			panic(errors.AssertionFailedf("enablePointSynthesis called on non-range-key position %s",
+				p.parent.UnsafeKey()))
+		}
+	}
+	p.pointIter = newPointSynthesizingIterAtParent(p.parent)
+	p.parent = p.pointIter
+	if util.RaceEnabled {
+		if ok, _ := p.parent.Valid(); !ok {
+			panic(errors.AssertionFailedf("invalid pointSynthesizingIter with valid iter"))
+		}
 	}
-	return true
 }
 
 func (p *pebbleMVCCScanner) decodeCurrentMetadata() bool {
@@ -1226,6 +1251,11 @@ func (p *pebbleMVCCScanner) iterValid() bool {
 		}
 		return false
 	}
+	// Since iterValid() is called after every iterator positioning operation, it
+	// is convenient to check for any range keys and enable point synthesis here.
+	if p.parent.RangeKeyChanged() {
+		p.enablePointSynthesis()
+	}
 	return true
 }
 
@@ -1315,9 +1345,6 @@ func (p *pebbleMVCCScanner) iterPeekPrev() ([]byte, bool) {
 			// iterator at the first entry, and in the latter iteration will be done.
 			return nil, false
 		}
-		if !p.maybeEnablePointSynthesis() {
-			return nil, false
-		}
 	} else if !p.iterValid() {
 		return nil, false
 	}