storage: re-introduce dual buffer for iterator bounds

[erikgrinaker]

For the mvcc-range-tombstones branch. Undoes a change in #79291 and #79744. Will be squashed into the original commit once merged.

---

storage: re-introduce dual buffer for iterator bounds

This is necessary to avoid mutating the bounds in use by
pebble.Iterator, which can interfere with seek optimizations.

Release note: None

storage: simplify pebbleIterator options comparisons

Since we now use dual buffers for bounds encoding, we can fully generate
the new pebble.IterOptions during setOptions() and compare them
directly with the existing options, which simplifies the code.

Release note: None

[cockroach-teamcity]

This change is 

[erikgrinaker]

Performance diff when reading from a batch (reusing iterators) is negligible, and within error margins (c.f. benchmarks in #79291):

name                                        old time/op    new time/op    delta
MVCCGet_Pebble/versions=1/valueSize=8-24      2.60µs ± 4%    2.61µs ± 1%    ~     (p=0.132 n=19+19)
MVCCGet_Pebble/versions=10/valueSize=8-24     3.82µs ± 1%    3.88µs ± 1%  +1.54%  (p=0.000 n=20+19)
MVCCGet_Pebble/versions=100/valueSize=8-24    9.47µs ± 6%    9.59µs ± 3%    ~     (p=0.123 n=19+19)

[sumeerbhola]

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @jbowens, and @nicktrav)

---

-- commits, line 5 at r2:
Can you look into why TestPebbleIterBoundSliceStabilityAndNoop did not fail?
Is it because it was removed since SetBounds is no more? We need a test that would have caught this bug.

---

pkg/storage/pebble_iterator.go, line 214 at r2 (raw file):

	// require either Prefix, UpperBound, or LowerBound to be set, so one of these
	// won't match the zero value of a new iterator.
	optsChanged := opts.Prefix != p.prefix ||

I didn't spot a new test for this noop optimization. If there isn't one, please add.

From past experience, this is the kind of thing that results in extremely long and painful roachtest failure investigations.

[erikgrinaker]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens, @nicktrav, and @sumeerbhola)

---

-- commits, line 5 at r2:

Previously, sumeerbhola wrote…

Can you look into why TestPebbleIterBoundSliceStabilityAndNoop did not fail?
Is it because it was removed since SetBounds is no more? We need a test that would have caught this bug.

Ah, yes, it was removed. Sloppy of me not to look closer here.

---

pkg/storage/pebble_iterator.go, line 214 at r2 (raw file):

Previously, sumeerbhola wrote…

I didn't spot a new test for this noop optimization. If there isn't one, please add.

From past experience, this is the kind of thing that results in extremely long and painful roachtest failure investigations.

Let's see where we land on cockroachdb/pebble#1640 first. I think it'd be beneficial to move this into Pebble, since it'd allow for more fine-grained optimization and less coupling.

[erikgrinaker]

Superseded by #81242.