Skip to content

Commit

Permalink
storage: remove pre-22.2 code
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@RaduBerinde
RaduBerinde committed Feb 8, 2023
1 parent 87d1fed commit 3fd2977
Show file tree
Hide file tree
Showing 19 changed files with 102 additions and 522 deletions.
5 changes: 3 additions & 2 deletions pkg/ccl/storageccl/engineccl/encrypted_fs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
42 changes: 20 additions & 22 deletions pkg/kv/kvserver/kvstorage/cluster_version_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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: `version .* is no longer supported`,
},
} {
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)
}
})
Expand Down
5 changes: 0 additions & 5 deletions pkg/kv/kvserver/spanset/batch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 0 additions & 5 deletions pkg/sql/gcjob_test/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ go_test(
args = ["-test.timeout=295s"],
deps = [
"//pkg/base",
"//pkg/clusterversion",
"//pkg/config",
"//pkg/config/zonepb",
"//pkg/jobs",
"//pkg/jobs/jobspb",
"//pkg/keys",
Expand All @@ -33,7 +31,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",
Expand All @@ -46,10 +43,8 @@ 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",
],
)
Expand Down
88 changes: 0 additions & 88 deletions pkg/sql/gcjob_test/gc_job_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ 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"
Expand All @@ -30,7 +28,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"
Expand All @@ -39,7 +36,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"
Expand All @@ -51,10 +47,8 @@ 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"
)

Expand Down Expand Up @@ -664,88 +658,6 @@ SELECT descriptor_id, index_id
})
}

// 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
Expand Down
5 changes: 0 additions & 5 deletions pkg/storage/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 3fd2977

Please sign in to comment.