storage: never set a too-old format version

This change asserts that we never set a pebble FormatVersion that is
older than the one minimum supported by our code.

Benchmarks that set a lower value are updated (and
BenchmarkTimeBoundIterate is reenabled). These benchmarks can fail
when the code exercises a feature like range keys that is not
supported by the older format.

Fixes #97061

Release note: None
Epic: none

pkg/ccl/storageccl/engineccl/BUILD.bazel

@@ -50,7 +50,6 @@ go_test(
         "//pkg/storage/enginepb",
         "//pkg/testutils",
         "//pkg/testutils/datapathutils",
-        "//pkg/testutils/skip",
         "//pkg/testutils/storageutils",
         "//pkg/util/encoding",
         "//pkg/util/hlc",

pkg/ccl/storageccl/engineccl/bench_test.go

@@ -21,7 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
-	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -167,8 +166,6 @@ func runIterate(
 }
 
 func BenchmarkTimeBoundIterate(b *testing.B) {
-	skip.WithIssue(b, 95530, "bump minBinary to 22.2. Skip 22.2 mixed-version tests for future cleanup")
-
 	for _, loadFactor := range []float32{1.0, 0.5, 0.1, 0.05, 0.0} {
 		b.Run(fmt.Sprintf("LoadFactor=%.2f", loadFactor), func(b *testing.B) {
 			b.Run("NormalIterator", func(b *testing.B) {

pkg/kv/kvserver/batcheval/BUILD.bazel

@@ -173,7 +173,6 @@ go_test(
         "//pkg/util/uuid",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_errors//oserror",
-        "@com_github_cockroachdb_pebble//:pebble",
         "@com_github_cockroachdb_pebble//vfs",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",

pkg/kv/kvserver/batcheval/cmd_refresh_range_bench_test.go

@@ -30,7 +30,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/errors/oserror"
-	"github.com/cockroachdb/pebble"
 	"github.com/cockroachdb/pebble/vfs"
 	"github.com/stretchr/testify/require"
 )
@@ -183,7 +182,6 @@ func setupMVCCPebble(b testing.TB, dir string, lBaseMaxBytes int64, readOnly boo
 	opts.FS = vfs.Default
 	opts.LBaseMaxBytes = lBaseMaxBytes
 	opts.ReadOnly = readOnly
-	opts.FormatMajorVersion = pebble.FormatBlockPropertyCollector
 	peb, err := storage.NewPebble(
 		context.Background(),
 		storage.PebbleConfig{

pkg/kv/kvserver/rangefeed/BUILD.bazel

@@ -75,7 +75,6 @@ go_test(
         "//pkg/util/uuid",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_errors//oserror",
-        "@com_github_cockroachdb_pebble//:pebble",
         "@com_github_cockroachdb_pebble//vfs",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",

pkg/kv/kvserver/rangefeed/catchup_scan_bench_test.go

@@ -30,7 +30,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/errors/oserror"
-	"github.com/cockroachdb/pebble"
 	"github.com/cockroachdb/pebble/vfs"
 	"github.com/stretchr/testify/require"
 )
@@ -202,7 +201,6 @@ func setupMVCCPebble(b testing.TB, dir string, lBaseMaxBytes int64, readOnly boo
 	opts.FS = vfs.Default
 	opts.LBaseMaxBytes = lBaseMaxBytes
 	opts.ReadOnly = readOnly
-	opts.FormatMajorVersion = pebble.FormatRangeKeys
 	peb, err := storage.NewPebble(
 		context.Background(),
 		storage.PebbleConfig{

pkg/storage/metamorphic/options.go

@@ -100,15 +100,11 @@ func standardOptions(i int) *pebble.Options {
 	if err := opts.Parse(stdOpts[i], nil); err != nil {
 		panic(err)
 	}
-	// Enable range keys in all options.
-	opts.FormatMajorVersion = pebble.FormatRangeKeys
 	return opts
 }
 
 func randomOptions() *pebble.Options {
 	opts := storage.DefaultPebbleOptions()
-	// Enable range keys in all options.
-	opts.FormatMajorVersion = pebble.FormatRangeKeys
 
 	rng, _ := randutil.NewTestRand()
 	opts.BytesPerSync = 1 << rngIntRange(rng, 8, 30)

pkg/storage/mvcc_incremental_iterator_test.go

@@ -33,7 +33,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
-	"github.com/cockroachdb/pebble"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
@@ -1568,7 +1567,7 @@ func BenchmarkMVCCIncrementalIteratorForOldData(b *testing.B) {
 	// day of keys. The old keys are uniformly distributed in the key space,
 	// which is the worst case for block property filters.
 	keyAgeInterval := 400
-	setupMVCCPebbleWithBlockProperties := func(b *testing.B) Engine {
+	setupMVCCPebble := func(b *testing.B) Engine {
 		eng, err := Open(
 			context.Background(),
 			InMemory(),
@@ -1577,10 +1576,7 @@ func BenchmarkMVCCIncrementalIteratorForOldData(b *testing.B) {
 			// will mostly miss the cache (especially since the block cache is meant
 			// to be scan resistant).
 			CacheSize(1<<10),
-			func(cfg *engineConfig) error {
-				cfg.Opts.FormatMajorVersion = pebble.FormatBlockPropertyCollector
-				return nil
-			})
+		)
 		if err != nil {
 			b.Fatal(err)
 		}
@@ -1618,7 +1614,7 @@ func BenchmarkMVCCIncrementalIteratorForOldData(b *testing.B) {
 	}
 
 	for _, valueSize := range []int{100, 500, 1000, 2000} {
-		eng := setupMVCCPebbleWithBlockProperties(b)
+		eng := setupMVCCPebble(b)
 		setupData(b, eng, valueSize)
 		b.Run(fmt.Sprintf("valueSize=%d", valueSize), func(b *testing.B) {
 			startKey := roachpb.Key(encoding.EncodeUvarintAscending([]byte("key-"), uint64(0)))

pkg/storage/pebble.go

@@ -533,6 +533,12 @@ var PebbleBlockPropertyCollectors = []func() pebble.BlockPropertyCollector{
 	},
 }
 
+// MinimumSupportedFormatVersion is the version that provides features that the
+// Cockroach code relies on unconditionally (like range keys). New stores are by
+// default created with this version. It should correspond to the minimum
+// supported binary version.
+const MinimumSupportedFormatVersion = pebble.FormatPrePebblev1Marked
+
 // DefaultPebbleOptions returns the default pebble options.
 func DefaultPebbleOptions() *pebble.Options {
 	// In RocksDB, the concurrency setting corresponds to both flushes and
@@ -556,7 +562,7 @@ func DefaultPebbleOptions() *pebble.Options {
 		Merger:                      MVCCMerger,
 		BlockPropertyCollectors:     PebbleBlockPropertyCollectors,
 		// Minimum supported format.
-		FormatMajorVersion: pebble.FormatPrePebblev1Marked,
+		FormatMajorVersion: MinimumSupportedFormatVersion,
 	}
 	// Automatically flush 10s after the first range tombstone is added to a
 	// memtable. This ensures that we can reclaim space even when there's no
@@ -870,6 +876,12 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) {
 		// Clone the given options so that we are free to modify them.
 		opts = cfg.Opts.Clone()
 	}
+	if opts.FormatMajorVersion < MinimumSupportedFormatVersion {
+		return nil, errors.AssertionFailedf(
+			"FormatMajorVersion is %d, should be at least %d",
+			opts.FormatMajorVersion, MinimumSupportedFormatVersion,
+		)
+	}
 
 	// pebble.Open also calls EnsureDefaults, but only after doing a clone. Call
 	// EnsureDefaults here to make sure we have a working FS.
@@ -1965,7 +1977,7 @@ func (p *Pebble) SetMinVersion(version roachpb.Version) error {
 		// This should never happen in production. But we tolerate tests creating
 		// imaginary older versions; we must still use the earliest supported
 		// format.
-		formatVers = pebble.FormatPrePebblev1Marked
+		formatVers = MinimumSupportedFormatVersion
 	}
 
 	if p.db.FormatMajorVersion() < formatVers {