storage/engine: rename Iterator.{Seek,SeekReverse} to {SeekGE,SeekLT}

This addresses a comment from cockroachdb#42210.

The commit also adjusts the reverse seek (now SeekLT) to be exclusive instead of
inclusive. This matches the API exposed by Pebble and eliminates the need for
the bug fix in 79b4c77.

c-deps/libroach/db.cc

@@ -675,6 +675,12 @@ DBIterState DBIterSeek(DBIterator* iter, DBKey key) {
   return DBIterGetState(iter);
 }
 
+DBIterState DBIterSeekForPrev(DBIterator* iter, DBKey key) {
+  ScopedStats stats(iter);
+  iter->rep->SeekForPrev(EncodeKey(key));
+  return DBIterGetState(iter);
+}
+
 DBIterState DBIterSeekToFirst(DBIterator* iter) {
   ScopedStats stats(iter);
   iter->rep->SeekToFirst();

c-deps/libroach/include/libroach.h

@@ -225,6 +225,9 @@ void DBIterDestroy(DBIterator* iter);
 // Positions the iterator at the first key that is >= "key".
 DBIterState DBIterSeek(DBIterator* iter, DBKey key);
 
+// Positions the iterator at the first key that is <= "key".
+DBIterState DBIterSeekForPrev(DBIterator* iter, DBKey key);
+
 typedef struct {
   uint64_t internal_delete_skipped_count;
   // the number of SSTables touched (only for time bound iterators).

pkg/ccl/changefeedccl/poller.go

@@ -532,7 +532,7 @@ func (p *poller) slurpSST(ctx context.Context, sst []byte, schemaTimestamp hlc.T
 		return err
 	}
 	defer it.Close()
-	for it.Seek(engine.NilKey); ; it.Next() {
+	for it.SeekGE(engine.NilKey); ; it.Next() {
 		if ok, err := it.Valid(); err != nil {
 			return err
 		} else if !ok {

pkg/ccl/changefeedccl/table_history.go

@@ -254,7 +254,7 @@ func fetchTableDescriptorVersions(
 				return err
 			}
 			defer it.Close()
-			for it.Seek(engine.NilKey); ; it.Next() {
+			for it.SeekGE(engine.NilKey); ; it.Next() {
 				if ok, err := it.Valid(); err != nil {
 					return err
 				} else if !ok {

pkg/ccl/storageccl/engineccl/bench_test.go

@@ -149,7 +149,7 @@ func runIterate(
 		endTime := hlc.Timestamp{WallTime: int64(loadFactor * numBatches * batchTimeSpan)}
 		it := makeIterator(eng, startTime, endTime)
 		defer it.Close()
-		for it.Seek(engine.MVCCKey{}); ; it.Next() {
+		for it.SeekGE(engine.MVCCKey{}); ; it.Next() {
 			if ok, err := it.Valid(); !ok {
 				if err != nil {
 					b.Fatal(err)

pkg/ccl/storageccl/export_test.go

@@ -236,7 +236,7 @@ func exportUsingGoIterator(
 		EndTime:     endTime,
 	})
 	defer iter.Close()
-	for iter.Seek(engine.MakeMVCCMetadataKey(startKey)); ; iterFn(iter) {
+	for iter.SeekGE(engine.MakeMVCCMetadataKey(startKey)); ; iterFn(iter) {
 		ok, err := iter.Valid()
 		if err != nil {
 			// The error may be a WriteIntentError. In which case, returning it will

pkg/ccl/storageccl/import.go

@@ -202,7 +202,7 @@ func evalImport(ctx context.Context, cArgs batcheval.CommandArgs) (*roachpb.Impo
 	defer iter.Close()
 	var keyScratch, valueScratch []byte
 
-	for iter.Seek(startKeyMVCC); ; {
+	for iter.SeekGE(startKeyMVCC); ; {
 		ok, err := iter.Valid()
 		if err != nil {
 			return nil, err

pkg/ccl/storageccl/import_test.go

@@ -103,7 +103,7 @@ func slurpSSTablesLatestKey(
 	var kvs []engine.MVCCKeyValue
 	it := batch.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 	defer it.Close()
-	for it.Seek(start); ; it.NextKey() {
+	for it.SeekGE(start); ; it.NextKey() {
 		if ok, err := it.Valid(); err != nil {
 			t.Fatal(err)
 		} else if !ok || !it.UnsafeKey().Less(end) {

pkg/ccl/storageccl/writebatch.go

@@ -98,7 +98,7 @@ func clearExistingData(
 	iter := batch.NewIterator(engine.IterOptions{UpperBound: end})
 	defer iter.Close()
 
-	iter.Seek(engine.MakeMVCCMetadataKey(start))
+	iter.SeekGE(engine.MakeMVCCMetadataKey(start))
 	if ok, err := iter.Valid(); err != nil {
 		return enginepb.MVCCStats{}, err
 	} else if ok && !iter.UnsafeKey().Less(engine.MakeMVCCMetadataKey(end)) {

pkg/ccl/utilccl/sampledataccl/bankdata.go

@@ -130,7 +130,7 @@ func (b *Backup) NextKeyValues(
 
 		it := sst.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 		defer it.Close()
-		it.Seek(engine.MVCCKey{Key: file.Span.Key})
+		it.SeekGE(engine.MVCCKey{Key: file.Span.Key})
 
 		iterIdx := 0
 		for ; ; it.Next() {

pkg/sql/row/fetcher_mvcc_test.go

@@ -43,7 +43,7 @@ func slurpUserDataKVs(t testing.TB, e engine.Engine) []roachpb.KeyValue {
 		kvs = nil
 		it := e.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 		defer it.Close()
-		for it.Seek(engine.MVCCKey{Key: keys.UserTableDataMin}); ; it.NextKey() {
+		for it.SeekGE(engine.MVCCKey{Key: keys.UserTableDataMin}); ; it.NextKey() {
 			ok, err := it.Valid()
 			if err != nil {
 				t.Fatal(err)

pkg/sql/rowcontainer/disk_row_container.go

@@ -336,7 +336,7 @@ func (d *DiskRowContainer) NewFinalIterator(ctx context.Context) RowIterator {
 }
 
 func (r *diskRowFinalIterator) Rewind() {
-	r.Seek(r.diskRowIterator.rowContainer.lastReadKey)
+	r.SeekGE(r.diskRowIterator.rowContainer.lastReadKey)
 	if r.diskRowIterator.rowContainer.lastReadKey != nil {
 		r.Next()
 	}

pkg/sql/rowcontainer/hash_row_container.go

@@ -575,7 +575,7 @@ func (h *HashDiskRowContainer) NewBucketIterator(
 
 // Rewind implements the RowIterator interface.
 func (i *hashDiskRowBucketIterator) Rewind() {
-	i.Seek(i.encodedEqCols)
+	i.SeekGE(i.encodedEqCols)
 }
 
 // Valid implements the RowIterator interface.
@@ -1034,7 +1034,7 @@ func (h *HashDiskBackedRowContainer) recreateAllRowsIterators(ctx context.Contex
 			return errors.Errorf("the iterator is unexpectedly not hashMemRowIterator")
 		}
 		newIterator := h.NewUnmarkedIterator(ctx)
-		newIterator.(*diskRowIterator).Seek(oldIterator.curKey)
+		newIterator.(*diskRowIterator).SeekGE(oldIterator.curKey)
 		(*iterator).RowIterator.Close()
 		iterator.RowIterator = newIterator
 	}

pkg/storage/batch_spanset_test.go

@@ -39,6 +39,7 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
 	outsideKey3 := engine.MakeMVCCMetadataKey(roachpb.Key("m"))
 	insideKey := engine.MakeMVCCMetadataKey(roachpb.Key("c"))
 	insideKey2 := engine.MakeMVCCMetadataKey(roachpb.Key("d"))
+	insideKey3 := engine.MakeMVCCMetadataKey(roachpb.Key("f"))
 
 	// Write values outside the range that we can try to read later.
 	if err := eng.Put(outsideKey, []byte("value")); err != nil {
@@ -120,12 +121,12 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
 		defer iter.Close()
 
 		// Iterators check boundaries on seek and next/prev
-		iter.Seek(outsideKey)
+		iter.SeekGE(outsideKey)
 		if _, err := iter.Valid(); !isReadSpanErr(err) {
 			t.Fatalf("Seek: unexpected error %v", err)
 		}
 		// Seeking back in bounds restores validity.
-		iter.Seek(insideKey)
+		iter.SeekGE(insideKey)
 		if ok, err := iter.Valid(); !ok {
 			t.Fatalf("expected valid iterator, err=%v", err)
 		}
@@ -157,12 +158,12 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
 	}
 	iter := spanset.NewIterator(eng.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax}), &ss)
 	defer iter.Close()
-	iter.SeekReverse(outsideKey)
+	iter.SeekLT(outsideKey)
 	if _, err := iter.Valid(); !isReadSpanErr(err) {
-		t.Fatalf("SeekReverse: unexpected error %v", err)
+		t.Fatalf("SeekLT: unexpected error %v", err)
 	}
 	// Seeking back in bounds restores validity.
-	iter.SeekReverse(insideKey2)
+	iter.SeekLT(insideKey3)
 	if ok, err := iter.Valid(); !ok {
 		t.Fatalf("expected valid iterator, err=%v", err)
 	}
@@ -184,7 +185,7 @@ func TestSpanSetBatchBoundaries(t *testing.T) {
 		t.Errorf("unexpected error on iterator: %+v", err)
 	}
 	// Seeking back in bounds restores validity.
-	iter.SeekReverse(insideKey)
+	iter.SeekLT(insideKey)
 	if ok, err := iter.Valid(); !ok {
 		t.Fatalf("expected valid iterator, err=%v", err)
 	}
@@ -334,7 +335,7 @@ func TestSpanSetIteratorTimestamps(t *testing.T) {
 		iter := batchAt1.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 		defer iter.Close()
 
-		iter.Seek(k1)
+		iter.SeekGE(k1)
 		if ok, err := iter.Valid(); !ok {
 			t.Fatalf("expected valid iterator, err=%v", err)
 		}
@@ -356,12 +357,12 @@ func TestSpanSetIteratorTimestamps(t *testing.T) {
 		iter := batchAt2.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 		defer iter.Close()
 
-		iter.Seek(k1)
+		iter.SeekGE(k1)
 		if ok, _ := iter.Valid(); ok {
 			t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
 		}
 
-		iter.Seek(k2)
+		iter.SeekGE(k2)
 		if ok, err := iter.Valid(); !ok {
 			t.Fatalf("expected valid iterator, err=%v", err)
 		}
@@ -376,12 +377,12 @@ func TestSpanSetIteratorTimestamps(t *testing.T) {
 		iter := batch.NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 		defer iter.Close()
 
-		iter.Seek(k1)
+		iter.SeekGE(k1)
 		if ok, _ := iter.Valid(); ok {
 			t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
 		}
 
-		iter.Seek(k2)
+		iter.SeekGE(k2)
 		if ok, _ := iter.Valid(); ok {
 			t.Fatalf("expected invalid iterator; found valid at key %s", iter.Key())
 		}

pkg/storage/batcheval/cmd_add_sstable.go

@@ -65,7 +65,7 @@ func EvalAddSSTable(
 	defer dataIter.Close()
 
 	// Check that the first key is in the expected range.
-	dataIter.Seek(engine.MVCCKey{Key: keys.MinKey})
+	dataIter.SeekGE(engine.MVCCKey{Key: keys.MinKey})
 	ok, err := dataIter.Valid()
 	if err != nil {
 		return result.Result{}, err
@@ -107,7 +107,7 @@ func EvalAddSSTable(
 		stats = computed
 	}
 
-	dataIter.Seek(mvccEndKey)
+	dataIter.SeekGE(mvccEndKey)
 	ok, err = dataIter.Valid()
 	if err != nil {
 		return result.Result{}, err
@@ -181,7 +181,7 @@ func EvalAddSSTable(
 
 	if args.IngestAsWrites {
 		log.VEventf(ctx, 2, "ingesting SST (%d keys/%d bytes) via regular write batch", stats.KeyCount, len(args.Data))
-		dataIter.Seek(engine.MVCCKey{Key: keys.MinKey})
+		dataIter.SeekGE(engine.MVCCKey{Key: keys.MinKey})
 		for {
 			ok, err := dataIter.Valid()
 			if err != nil {
@@ -227,7 +227,7 @@ func checkForKeyCollisions(
 	// Create iterator over the existing data.
 	existingDataIter := dbEngine.NewIterator(engine.IterOptions{UpperBound: mvccEndKey.Key})
 	defer existingDataIter.Close()
-	existingDataIter.Seek(mvccStartKey)
+	existingDataIter.SeekGE(mvccStartKey)
 	if ok, err := existingDataIter.Valid(); err != nil {
 		return emptyMVCCStats, errors.Wrap(err, "checking for key collisions")
 	} else if !ok {

pkg/storage/batcheval/cmd_revert_range.go

@@ -50,7 +50,7 @@ func isEmptyKeyTimeRange(
 		MinTimestampHint: since.Next() /* make exclusive */, MaxTimestampHint: until,
 	})
 	defer iter.Close()
-	iter.Seek(engine.MVCCKey{Key: from})
+	iter.SeekGE(engine.MVCCKey{Key: from})
 	ok, err := iter.Valid()
 	return !ok, err
 }

pkg/storage/bulk/sst_batcher.go

@@ -478,7 +478,7 @@ func createSplitSSTable(
 	var first, last roachpb.Key
 	var left, right *sstSpan
 
-	iter.Seek(engine.MVCCKey{Key: start})
+	iter.SeekGE(engine.MVCCKey{Key: start})
 	for {
 		if ok, err := iter.Valid(); err != nil {
 			return nil, nil, err

pkg/storage/client_split_test.go

@@ -391,7 +391,7 @@ func TestStoreRangeSplitIntents(t *testing.T) {
 	iter := store.Engine().NewIterator(engine.IterOptions{UpperBound: roachpb.KeyMax})
 
 	defer iter.Close()
-	for iter.Seek(start); ; iter.Next() {
+	for iter.SeekGE(start); ; iter.Next() {
 		if ok, err := iter.Valid(); err != nil {
 			t.Fatal(err)
 		} else if !ok || !iter.UnsafeKey().Less(end) {

pkg/storage/compactor/compactor.go

@@ -309,7 +309,7 @@ func (c *Compactor) fetchSuggestions(
 			}
 
 			dataIter.SetUpperBound(sc.EndKey)
-			dataIter.Seek(engine.MakeMVCCMetadataKey(sc.StartKey))
+			dataIter.SeekGE(engine.MakeMVCCMetadataKey(sc.StartKey))
 			if ok, err := dataIter.Valid(); err != nil {
 				return false, err
 			} else if ok && dataIter.UnsafeKey().Less(engine.MakeMVCCMetadataKey(sc.EndKey)) {

pkg/storage/diskmap/disk_map.go

@@ -37,9 +37,9 @@ type Factory interface {
 //		// Do something.
 // 	}
 type SortedDiskMapIterator interface {
-	// Seek sets the iterator's position to the first key greater than or equal
+	// SeekGE sets the iterator's position to the first key greater than or equal
 	// to the provided key.
-	Seek(key []byte)
+	SeekGE(key []byte)
 	// Rewind seeks to the start key.
 	Rewind()
 	// Valid must be called after any call to Seek(), Rewind(), or Next(). It

pkg/storage/engine/batch_test.go

@@ -997,7 +997,7 @@ func TestBatchDistinct(t *testing.T) {
 			// Similarly, for distinct batch iterators we will see previous writes to the
 			// batch.
 			iter := distinct.NewIterator(IterOptions{UpperBound: roachpb.KeyMax})
-			iter.Seek(mvccKey("a"))
+			iter.SeekGE(mvccKey("a"))
 			if ok, err := iter.Valid(); !ok {
 				t.Fatalf("expected iterator to be valid; err=%v", err)
 			}
@@ -1064,7 +1064,7 @@ func TestWriteOnlyBatchDistinct(t *testing.T) {
 			// Verify that reads on the distinct batch go to the underlying engine, not
 			// to the write-only batch.
 			iter := distinct.NewIterator(IterOptions{UpperBound: roachpb.KeyMax})
-			iter.Seek(mvccKey("a"))
+			iter.SeekGE(mvccKey("a"))
 			if ok, err := iter.Valid(); !ok {
 				t.Fatalf("expected iterator to be valid, err=%v", err)
 			}
@@ -1158,11 +1158,12 @@ func TestBatchIteration(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			iter := b.NewIterator(IterOptions{UpperBound: k3.Key})
+			iterOpts := IterOptions{UpperBound: k3.Key}
+			iter := b.NewIterator(iterOpts)
 			defer iter.Close()
 
-			// Forward iteration
-			iter.Seek(k1)
+			// Forward iteration,
+			iter.SeekGE(k1)
 			if ok, err := iter.Valid(); !ok {
 				t.Fatal(err)
 			}
@@ -1189,22 +1190,22 @@ func TestBatchIteration(t *testing.T) {
 				t.Fatalf("expected invalid, got valid at key %s", iter.Key())
 			}
 
-			// SeekReverse works.
-			iter.SeekReverse(k2)
-			if ok, err := iter.Valid(); !ok {
-				t.Fatal(err)
-			}
-			if !reflect.DeepEqual(iter.Key(), k2) {
-				t.Fatalf("expected %s, got %s", k2, iter.Key())
-			}
-			if !reflect.DeepEqual(iter.Value(), v2) {
-				t.Fatalf("expected %s, got %s", v2, iter.Value())
-			}
-			iter.Prev()
-
+			// Reverse iteration.
 			switch engineImpl.name {
 			case "pebble":
 				// Reverse iteration in batches works on Pebble.
+				iter.SeekLT(k3)
+				if ok, err := iter.Valid(); !ok {
+					t.Fatal(err)
+				}
+				if !reflect.DeepEqual(iter.Key(), k2) {
+					t.Fatalf("expected %s, got %s", k2, iter.Key())
+				}
+				if !reflect.DeepEqual(iter.Value(), v2) {
+					t.Fatalf("expected %s, got %s", v2, iter.Value())
+				}
+
+				iter.Prev()
 				if ok, err := iter.Valid(); !ok || err != nil {
 					t.Fatalf("expected success, but got invalid: %v", err)
 				}
@@ -1216,6 +1217,14 @@ func TestBatchIteration(t *testing.T) {
 				}
 			default:
 				// Reverse iteration in batches is not supported with RocksDB.
+				iter.SeekLT(k3)
+				if ok, err := iter.Valid(); ok {
+					t.Fatalf("expected invalid, got valid at key %s", iter.Key())
+				} else if !testutils.IsError(err, "SeekForPrev\\(\\) not supported") {
+					t.Fatalf("expected 'SeekForPrev() not supported', got %s", err)
+				}
+
+				iter.Prev()
 				if ok, err := iter.Valid(); ok {
 					t.Fatalf("expected invalid, got valid at key %s", iter.Key())
 				} else if !testutils.IsError(err, "Prev\\(\\) not supported") {

pkg/storage/engine/bench_test.go

@@ -684,7 +684,7 @@ func runClearRange(
 	// first key and simply ClearRange(NilKey, MVCCKeyMax).
 	iter := eng.NewIterator(IterOptions{UpperBound: roachpb.KeyMax})
 	defer iter.Close()
-	iter.Seek(NilKey)
+	iter.SeekGE(NilKey)
 	if ok, err := iter.Valid(); !ok {
 		b.Fatalf("unable to find first key (err: %v)", err)
 	}