storage: remove pre-22.2 code

This change removes any code that handles pre-22.2 versions. Features that are in 22.2 (like range keys) are now always enabled. Any new store always uses at least the 22.2 pebble format version. Release note: None Epic: none Fixes: cockroachdb#96764
RaduBerinde · Feb 9, 2023 · 06edc6a · 06edc6a
1 parent 732c3a7
commit 06edc6a
Show file tree

Hide file tree

Showing 21 changed files with 127 additions and 603 deletions.
diff --git a/pkg/ccl/cliccl/ear_test.go b/pkg/ccl/cliccl/ear_test.go
@@ -156,13 +156,13 @@ func TestList(t *testing.T) {
 000004.log:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: 80 18 c0 79 61 c7 cf ef b4 25 4e 78
-  counter: 1483615076
+  nonce: 31 d3 cd 5a 69 e2 13 64 21 53 57 64
+  counter: 3952287331
 000005.sst:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: 71 12 f7 22 9a fb 90 24 4e 58 27 01
-  counter: 3082989236
+  nonce: 23 d9 b2 e1 39 b0 87 ed f9 6d 49 20
+  counter: 3481614039
 COCKROACHDB_DATA_KEYS_000001_monolith:
   env type: Store, AES128_CTR
   keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
@@ -171,8 +171,8 @@ COCKROACHDB_DATA_KEYS_000001_monolith:
 CURRENT:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: 18 c2 a6 23 cc 6e 2e 7c 8e bf 84 77
-  counter: 3159373900
+  nonce: 71 12 f7 22 9a fb 90 24 4e 58 27 01
+  counter: 3082989236
 MANIFEST-000001:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
@@ -181,14 +181,25 @@ MANIFEST-000001:
 OPTIONS-000003:
   env type: Data, AES128_CTR
   keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
-  nonce: d3 97 11 b3 1a ed 22 2b 74 fb 02 0c
-  counter: 1229228536
+  nonce: c3 6d b2 7b 3f 3e 67 b9 28 b9 81 b1
+  counter: 3050109376
 marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith:
   env type: Store, AES128_CTR
   keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
   nonce: 55 d7 d4 27 6c 97 9b dd f1 5d 40 c8
   counter: 467030050
+marker.format-version.000009.010:
+  env type: Data, AES128_CTR
+  keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
+  nonce: 6e 34 f4 3c 11 43 1a f5 69 ce 33 f1
+  counter: 2398097086
+marker.manifest.000001.MANIFEST-000001:
+  env type: Data, AES128_CTR
+  keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
+  nonce: d3 97 11 b3 1a ed 22 2b 74 fb 02 0c
+  counter: 1229228536
 `
+
 	require.Equal(t, want, b.String())
 }
 

diff --git a/pkg/ccl/storageccl/engineccl/encrypted_fs_test.go b/pkg/ccl/storageccl/engineccl/encrypted_fs_test.go
@@ -268,9 +268,10 @@ func TestPebbleEncryption(t *testing.T) {
 	stats, err := db.GetEnvStats()
 	require.NoError(t, err)
 	// Opening the DB should've created OPTIONS, CURRENT, MANIFEST and the
-	// WAL, all under the active key.
+	// WAL.
 	require.Equal(t, uint64(4), stats.TotalFiles)
-	require.Equal(t, uint64(4), stats.ActiveKeyFiles)
+	// We also created markers for the format version and the manifest.
+	require.Equal(t, uint64(6), stats.ActiveKeyFiles)
 	var s enginepbccl.EncryptionStatus
 	require.NoError(t, protoutil.Unmarshal(stats.EncryptionStatus, &s))
 	require.Equal(t, "16.key", s.ActiveStoreKey.Source)

diff --git a/pkg/kv/kvserver/kvstorage/cluster_version_test.go b/pkg/kv/kvserver/kvstorage/cluster_version_test.go
@@ -31,8 +31,14 @@ func TestStoresClusterVersionIncompatible(t *testing.T) {
 
 	ctx := context.Background()
 
-	vOneDashOne := roachpb.Version{Major: 1, Internal: 1}
-	vOne := roachpb.Version{Major: 1}
+	current := clusterversion.ByKey(clusterversion.BinaryVersionKey)
+	minSupported := clusterversion.ByKey(clusterversion.BinaryMinSupportedVersionKey)
+
+	future := current
+	future.Major++
+
+	tooOld := minSupported
+	tooOld.Major--
 
 	type testCase struct {
 		binV, minV roachpb.Version // binary version and min supported version
@@ -42,41 +48,33 @@ func TestStoresClusterVersionIncompatible(t *testing.T) {
 	for name, tc := range map[string]testCase{
 		"StoreTooNew": {
 			// This is what the node is running.
-			binV: vOneDashOne,
+			binV: current,
 			// This is what the running node requires from its stores.
-			minV: vOne,
+			minV: minSupported,
 			// Version is way too high for this node.
-			engV:   roachpb.Version{Major: 9},
-			expErr: `cockroach version v1\.0-1 is incompatible with data in store <no-attributes>=<in-mem>; use version v9\.0 or later`,
+			engV:   future,
+			expErr: `cockroach version .* is incompatible with data in store <no-attributes>=<in-mem>; use version .* or later`,
 		},
 		"StoreTooOldVersion": {
 			// This is what the node is running.
-			binV: roachpb.Version{Major: 9},
+			binV: current,
 			// This is what the running node requires from its stores.
-			minV: roachpb.Version{Major: 5},
+			minV: minSupported,
 			// Version is way too low.
-			engV:   roachpb.Version{Major: 4},
-			expErr: `store <no-attributes>=<in-mem>, last used with cockroach version v4\.0, is too old for running version v9\.0 \(which requires data from v5\.0 or later\)`,
-		},
-		"StoreTooOldMinVersion": {
-			// Like the previous test case, but this time cv.MinimumVersion is the culprit.
-			binV:   roachpb.Version{Major: 9},
-			minV:   roachpb.Version{Major: 5},
-			engV:   roachpb.Version{Major: 4},
-			expErr: `store <no-attributes>=<in-mem>, last used with cockroach version v4\.0, is too old for running version v9\.0 \(which requires data from v5\.0 or later\)`,
+			engV:   tooOld,
+			expErr: `store <no-attributes>=<in-mem>, last used with cockroach version .*, is too old for running version .* \(which requires data from .* or later\)`,
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
 			engs := []storage.Engine{storage.NewDefaultInMemForTesting()}
 			defer engs[0].Close()
 			// Configure versions and write.
 			cv := clusterversion.ClusterVersion{Version: tc.engV}
-			if err := WriteClusterVersionToEngines(ctx, engs, cv); err != nil {
-				t.Fatal(err)
+			err := WriteClusterVersionToEngines(ctx, engs, cv)
+			if err == nil {
+				cv, err = SynthesizeClusterVersionFromEngines(ctx, engs, tc.binV, tc.minV)
 			}
-			if cv, err := SynthesizeClusterVersionFromEngines(
-				ctx, engs, tc.binV, tc.minV,
-			); !testutils.IsError(err, tc.expErr) {
+			if !testutils.IsError(err, tc.expErr) {
 				t.Fatalf("unexpected error: %+v, got version %v", err, cv)
 			}
 		})

diff --git a/pkg/kv/kvserver/spanset/batch.go b/pkg/kv/kvserver/spanset/batch.go
@@ -475,11 +475,6 @@ func (s spanSetReader) ConsistentIterators() bool {
 	return s.r.ConsistentIterators()
 }
 
-// SupportsRangeKeys implements the storage.Reader interface.
-func (s spanSetReader) SupportsRangeKeys() bool {
-	return s.r.SupportsRangeKeys()
-}
-
 // PinEngineStateForIterators implements the storage.Reader interface.
 func (s spanSetReader) PinEngineStateForIterators() error {
 	return s.r.PinEngineStateForIterators()

diff --git a/pkg/sql/gcjob_test/BUILD.bazel b/pkg/sql/gcjob_test/BUILD.bazel
@@ -11,9 +11,6 @@ go_test(
     args = ["-test.timeout=295s"],
     deps = [
         "//pkg/base",
-        "//pkg/clusterversion",
-        "//pkg/config",
-        "//pkg/config/zonepb",
         "//pkg/jobs",
         "//pkg/jobs/jobspb",
         "//pkg/keys",
@@ -33,7 +30,6 @@ go_test(
         "//pkg/sql/catalog/descs",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/gcjob",
-        "//pkg/sql/gcjob/gcjobnotifier",
         "//pkg/sql/isql",
         "//pkg/sql/sem/catid",
         "//pkg/storage",
@@ -46,10 +42,7 @@ go_test(
         "//pkg/util/leaktest",
         "//pkg/util/log",
         "//pkg/util/randutil",
-        "//pkg/util/stop",
-        "//pkg/util/syncutil",
         "//pkg/util/timeutil",
-        "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//require",
     ],
 )

diff --git a/pkg/sql/gcjob_test/gc_job_test.go b/pkg/sql/gcjob_test/gc_job_test.go
@@ -19,9 +19,6 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
-	"github.com/cockroachdb/cockroach/pkg/config"
-	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
@@ -30,7 +27,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
@@ -39,7 +35,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/gcjob"
-	"github.com/cockroachdb/cockroach/pkg/sql/gcjob/gcjobnotifier"
 	"github.com/cockroachdb/cockroach/pkg/sql/isql"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
 	"github.com/cockroachdb/cockroach/pkg/storage"
@@ -51,10 +46,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
-	"github.com/cockroachdb/cockroach/pkg/util/stop"
-	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
-	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -663,149 +655,3 @@ SELECT descriptor_id, index_id
 		require.NoError(t, jr.WaitForJobs(ctx, []jobspb.JobID{jobID}))
 	})
 }
-
-// TestGCJobNoSystemConfig tests that the GC job is robust to running with
-// no system config provided by the SystemConfigProvider. It is a regression
-// test for a panic which could occur due to a slow systemconfigwatcher
-// initialization.
-//
-// TODO(ajwerner): Remove this test in 23.1.
-func TestGCJobNoSystemConfig(t *testing.T) {
-	defer leaktest.AfterTest(t)()
-
-	provider := fakeSystemConfigProvider{}
-	var (
-		v0 = clusterversion.ByKey(clusterversion.TODODelete_V22_2UseDelRangeInGCJob - 1)
-		v1 = clusterversion.ByKey(clusterversion.TODODelete_V22_2UseDelRangeInGCJob)
-	)
-	settings := cluster.MakeTestingClusterSettingsWithVersions(v1, v0, false /* initializeVersion */)
-	ctx := context.Background()
-	require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV))
-	stopper := stop.NewStopper()
-	gcKnobs := &sql.GCJobTestingKnobs{}
-	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{
-		Settings: settings,
-		Stopper:  stopper,
-		Knobs: base.TestingKnobs{
-			GCJob: gcKnobs,
-			Server: &server.TestingKnobs{
-				DisableAutomaticVersionUpgrade: make(chan struct{}),
-				BinaryVersionOverride:          v0,
-			},
-		},
-	})
-	defer stopper.Stop(ctx)
-	codec := s.ExecutorConfig().(sql.ExecutorConfig).Codec
-	n := gcjobnotifier.New(settings, &provider, codec, stopper)
-	n.Start(ctx)
-	gcKnobs.Notifier = n
-
-	tdb := sqlutils.MakeSQLRunner(sqlDB)
-	tdb.Exec(t, "CREATE TABLE foo (i INT PRIMARY KEY)")
-	tdb.Exec(t, "SET CLUSTER SETTING kv.protectedts.poll_interval = '10ms'")
-	var id uint32
-	tdb.QueryRow(t, "SELECT 'foo'::regclass::int").Scan(&id)
-	tdb.Exec(t, "DROP TABLE foo")
-	// We want to make sure there's a notifyee and that the job attempted
-	// to read the status twice. We expect it once for the notifier and
-	// once for the job itself.
-	testutils.SucceedsSoon(t, func() error {
-		if n := provider.numNotifyees(); n == 0 {
-			return errors.Errorf("expected 1 notifyee, got %d", n)
-		}
-		if n := provider.numCalls(); n < 2 {
-			return errors.Errorf("expected at least 2 calls, got %d", n)
-		}
-		return nil
-	})
-	cfgProto := &zonepb.ZoneConfig{
-		GC: &zonepb.GCPolicy{TTLSeconds: 0},
-	}
-	cfg := config.NewSystemConfig(cfgProto)
-	descKV, err := kvDB.Get(ctx, codec.DescMetadataKey(id))
-	require.NoError(t, err)
-	var zoneKV roachpb.KeyValue
-	zoneKV.Key = config.MakeZoneKey(codec, descpb.ID(id))
-	require.NoError(t, zoneKV.Value.SetProto(cfgProto))
-	defaultKV := zoneKV
-	defaultKV.Key = config.MakeZoneKey(codec, 0)
-	// We need to put in an entry for the descriptor both so that the notifier
-	// fires and so that we don't think the descriptor is missing. We also
-	// need a zone config KV to make the delta filter happy.
-	cfg.Values = []roachpb.KeyValue{
-		{Key: descKV.Key, Value: *descKV.Value},
-		defaultKV,
-		zoneKV,
-	}
-
-	provider.setConfig(cfg)
-	tdb.CheckQueryResultsRetry(t, `
-SELECT status
-  FROM crdb_internal.jobs
- WHERE description = 'GC for DROP TABLE defaultdb.public.foo'`,
-		[][]string{{"succeeded"}})
-}
-
-type fakeSystemConfigProvider struct {
-	mu struct {
-		syncutil.Mutex
-
-		calls     int
-		n         int
-		config    *config.SystemConfig
-		notifyees map[int]chan struct{}
-	}
-}
-
-func (f *fakeSystemConfigProvider) GetSystemConfig() *config.SystemConfig {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	f.mu.calls++
-	return f.mu.config
-}
-
-func (f *fakeSystemConfigProvider) RegisterSystemConfigChannel() (
-	_ <-chan struct{},
-	unregister func(),
-) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	ch := make(chan struct{}, 1)
-	n := f.mu.n
-	f.mu.n++
-	if f.mu.notifyees == nil {
-		f.mu.notifyees = map[int]chan struct{}{}
-	}
-	f.mu.notifyees[n] = ch
-	return ch, func() {
-		f.mu.Lock()
-		defer f.mu.Unlock()
-		delete(f.mu.notifyees, n)
-	}
-}
-
-func (f *fakeSystemConfigProvider) setConfig(c *config.SystemConfig) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	f.mu.config = c
-	for _, ch := range f.mu.notifyees {
-		select {
-		case ch <- struct{}{}:
-		default:
-		}
-	}
-}
-
-func (f *fakeSystemConfigProvider) numNotifyees() int {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	return len(f.mu.notifyees)
-}
-
-func (f *fakeSystemConfigProvider) numCalls() int {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	return f.mu.calls
-}
-
-var _ config.SystemConfigProvider = (*fakeSystemConfigProvider)(nil)
diff --git a/pkg/storage/engine.go b/pkg/storage/engine.go
@@ -591,11 +591,6 @@ type Reader interface {
 	// underlying Engine state. This is not true about Batch writes: new iterators
 	// will see new writes made to the batch, existing iterators won't.
 	ConsistentIterators() bool
-	// SupportsRangeKeys returns true if the Reader implementation supports
-	// range keys.
-	//
-	// TODO(erikgrinaker): Remove this after 22.2.
-	SupportsRangeKeys() bool
 
 	// PinEngineStateForIterators ensures that the state seen by iterators
 	// without timestamp hints (see IterOptions) is pinned and will not see