pebble: fix incorrect TrySeekUsingNext across an indexed-batch refresh

When an indexed-batch-backed `Iterator` observes that its batch has been
mutated (e.g. on the first seek after `SetOptions`), it sets
`batchJustRefreshed`, which intentionally keeps `TrySeekUsingNext` enabled
for the re-seek of the internal iterator while disabling the top-level
no-op seek optimization.

This is unsafe when the internal iterator was left advanced *past* the
last key returned to the user — most commonly `i.pos == iterPosNext`,
which happens while collecting `MERGE` operands (the iterator steps to the
next user key to confirm there are no more operands). In that state, the
no-op optimization is what normally re-returns the cached result; with it
disabled by the refresh, the re-seek falls through to the internal
iterator with `TrySeekUsingNext`, which resumes from the advanced position
and skips the last-returned key whenever the new seek key is at or before
it. The result is a visible key being silently skipped.

This was found by the metamorphic test as a non-deterministic divergence
between two configurations: it requires the internal iterator to be parked
past the key (layout-dependent) and `TrySeekUsingNext` to not have been
randomly disabled that run.

Fix it by disabling `TrySeekUsingNext` on a `BatchJustRefreshed` seek
unless the internal iterator is positioned exactly at the last-returned
key (`iterPosCurForward`). The common case (no merge advance) keeps the
optimization. The fix lives in `Iterator.SeekGEWithLimit` so it is
iterator-stack agnostic.

A datadriven regression test is added in
`testdata/iter_histories/set_options_batch_refresh`. This required
`combined-iter` to support a `key-types` argument and the test harness to
propagate `forceEnableSeekOpt` to the merging iterator, so the
seek-using-next path is exercised deterministically.

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>

Author: RaduBerinde

iterator.go

@@ -1381,6 +1381,21 @@ func (i *Iterator) SeekGEWithLimit(key []byte, limit []byte) IterValidityState {
 			if testingDisableSeekOpt(key, uintptr(unsafe.Pointer(i))) && !i.forceEnableSeekOpt {
 				flags = flags.DisableTrySeekUsingNext()
 			}
+			// When the batch was just refreshed, the top-level no-op seek
+			// optimization above is skipped (it requires !BatchJustRefreshed),
+			// so we will re-seek the internal iterator below. TrySeekUsingNext
+			// is only safe to pass to the internal iterator when it is
+			// positioned exactly at the last key we returned
+			// (iterPosCurForward). If instead it was advanced past that key
+			// (most commonly iterPosNext, while consuming MERGE operands; also
+			// the paused positions), resuming with TrySeekUsingNext would start
+			// from that advanced position and skip the last-returned key — which
+			// this seek may still need to return, since BatchJustRefreshed seeks
+			// can be to a key at or before it. Disable TrySeekUsingNext in that
+			// case and fall back to a full re-seek.
+			if flags.BatchJustRefreshed() && i.pos != iterPosCurForward {
+				flags = flags.DisableTrySeekUsingNext()
+			}
 			if !flags.BatchJustRefreshed() && i.pos == iterPosCurForwardPaused && i.cmp(key, i.iterKV.K.UserKey) <= 0 {
 				// Have some work to do, but don't need to seek, and we can
 				// start doing findNextEntry from i.iterKey.

iterator_histories_test.go

@@ -47,7 +47,20 @@ func TestIterHistories(t *testing.T) {
 		parser := itertest.NewParser()
 		newIter := func(name string, reader Reader, o *IterOptions) *Iterator {
 			it, _ := reader.NewIter(o)
-			iters[name] = it
+			// These tests assert deterministic optimization behavior across
+			// SetOptions/seek calls; opt out of invariant-only randomization that
+			// suppresses seek-using-next or forces SetOptions reconstruction. The
+			// merging iterator has its own seek-using-next randomization, so it
+			// must be opted out as well.
+			it.forceEnableSeekOpt = true
+			if it.merging != nil {
+				it.merging.forceEnableSeekOpt = true
+			}
+			// Only track named iterators; unnamed ones are closed inline by
+			// runIterCmd and would otherwise be double-closed by cleanup.
+			if name != "" {
+				iters[name] = it
+			}
 			return it
 		}
 		var opts *Options
@@ -309,6 +322,17 @@ func TestIterHistories(t *testing.T) {
 						o.LowerBound = []byte(arg.Vals[0])
 					case "upper":
 						o.UpperBound = []byte(arg.Vals[0])
+					case "key-types":
+						switch arg.Vals[0] {
+						case "point":
+							o.KeyTypes = IterKeyTypePointsOnly
+						case "range":
+							o.KeyTypes = IterKeyTypeRangesOnly
+						case "both":
+							o.KeyTypes = IterKeyTypePointsAndRanges
+						default:
+							return fmt.Sprintf("unknown key-type %q", arg.Vals[0])
+						}
 					case "name":
 						name = arg.Vals[0]
 					case "reader":

testdata/iter_histories/set_options_batch_refresh

@@ -0,0 +1,46 @@
+# Regression test: TrySeekUsingNext must not be passed to the internal iterator
+# across an indexed-batch refresh when the internal iterator has been advanced
+# past the last-returned key (e.g. iterPosNext while collecting MERGE operands).
+#
+# Setup: the DB holds a MERGE at "c" and a SET at "e". A points-only iterator is
+# opened over an indexed batch (whose keys are all > c, e). The first SeekGE("a")
+# returns the merge at "c"; collecting its operand advances the internal memtable
+# iterator past "c" to "e" (leaving the iterator at iterPosNext). The batch is
+# then mutated and SetOptions refreshes the batch view, which disables the
+# top-level no-op seek optimization. The subsequent SeekGE("b") is eligible for
+# TrySeekUsingNext (b > the previous seek key "a") but b <= the cached result
+# "c"; it must re-find "c" rather than resuming from "e" and skipping it.
+
+reset
+----
+
+batch commit
+merge c v1
+set e ve
+----
+committed 2 keys
+
+batch name=b
+set z vz
+----
+wrote 1 keys to batch "b"
+
+combined-iter name=i reader=b key-types=point
+seek-ge a
+----
+c: (v1, .)
+
+# Mutate the batch so the following SetOptions detects a changed batch view and
+# sets batchJustRefreshed.
+mutate batch=b
+set y vy
+----
+
+# SetOptions (unchanged options) refreshes the batch view and disables the
+# no-op seek optimization, then SeekGE("b") must still return "c".
+iter iter=i
+set-options
+seek-ge b
+----
+.
+c: (v1, .)