Skip to content

Commit

Permalink
Merge #85938
Browse files Browse the repository at this point in the history
85938: clusterversion,sql: remove ClusterLocksVirtualTable r=AlexTalks a=celiala

This commit removes the 22.1 `ClusterLocksVirtualTable` version gate.

Cleanup was done following guidance from [21.2 cleanup](#74270 (comment)):

> For the most part, if the gates were just simple if !version.IsActive { return x } or something, I just removed the block, and even if it was a little more complicated, like args = [x]; if version { args = append(args, y) }; foo(args) I still tried to mostly inline it such that it looked natural (i.e. remove that append and make it args = [x, y]).

> However for just a couple more complicated cases that were referring to <21.2 versions that needed to be replaced when those were deleted, I added a placeholder clusterversion.TODOPre21_2 alias for 21.2. Replacing those calls with this alias shouldn't change their behavior -- it was already always true, since the code today should never run in a <21.2 cluster -- but means we can delete those older versions in the meantime and then the owners of these bits can decide how to update them.

Partially addresses #80663

Release note: none
Release justification: remove old version gates.

Co-authored-by: Celia La <[email protected]>
  • Loading branch information
craig[bot] and celiala committed Aug 19, 2022
2 parents 43eadea + 493f535 commit 9a1c3a9
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 94 deletions.
7 changes: 0 additions & 7 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,6 @@ const (
// EnableNewStoreRebalancer enables the new store rebalancer introduced in
// 22.1.
EnableNewStoreRebalancer
// ClusterLocksVirtualTable enables querying the crdb_internal.cluster_locks
// virtual table, which sends a QueryLocksRequest RPC to all cluster ranges.
ClusterLocksVirtualTable
// AutoStatsTableSettings is the version where we allow auto stats related
// table settings.
AutoStatsTableSettings
Expand Down Expand Up @@ -391,10 +388,6 @@ var versionsSingleton = keyedVersions{
Key: EnableNewStoreRebalancer,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 96},
},
{
Key: ClusterLocksVirtualTable,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 98},
},
{
Key: AutoStatsTableSettings,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 100},
Expand Down
71 changes: 35 additions & 36 deletions pkg/clusterversion/key_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -6055,12 +6055,6 @@ func genClusterLocksGenerator(
filters clusterLocksFilters,
) func(ctx context.Context, p *planner, db catalog.DatabaseDescriptor, stopper *stop.Stopper) (virtualTableGenerator, cleanupFunc, error) {
return func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, _ *stop.Stopper) (virtualTableGenerator, cleanupFunc, error) {
// TODO(sarkesian): remove gate for 22.2 release
if !p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.ClusterLocksVirtualTable) {
return nil, nil, pgerror.New(pgcode.FeatureNotSupported,
"table crdb_internal.cluster_locks is not supported on this version")
}

hasAdmin, err := p.HasAdminRole(ctx)
if err != nil {
return nil, nil, err
Expand Down
45 changes: 0 additions & 45 deletions pkg/sql/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils"
Expand Down Expand Up @@ -413,46 +411,3 @@ func TestSetUserPasswordInsecure(t *testing.T) {
})
}
}

func TestAutoStatsTableSettingsDisallowedOnOldCluster(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

serverArgs := base.TestServerArgs{
Insecure: true,
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: make(chan struct{}),
BinaryVersionOverride: clusterversion.ByKey(clusterversion.ClusterLocksVirtualTable),
},
},
}

var (
ctx = context.Background()
s, conn, _ = serverutils.StartServer(t, serverArgs)
sqlDB = sqlutils.MakeSQLRunner(conn)
)
defer conn.Close()
defer s.Stopper().Stop(ctx)

sqlDB.Exec(t,
`CREATE DATABASE t;`)

sqlDB.ExpectErr(t, "pq: auto stats table settings are only available once the cluster is fully upgraded", "CREATE TABLE t1 (a int) WITH (sql_stats_automatic_collection_enabled = true)")

sqlDB.Exec(t,
`CREATE TABLE t2 (a int)`)

sqlDB.ExpectErr(t, "pq: auto stats table settings are only available once the cluster is fully upgraded", "ALTER TABLE t2 SET (sql_stats_automatic_collection_enabled = true)")

// Run the upgrade.
sqlDB.Exec(t, "SET CLUSTER SETTING version = $1", clusterversion.ByKey(clusterversion.AutoStatsTableSettings).String())

sqlDB.Exec(t,
`CREATE TABLE t1 (a int) WITH (sql_stats_automatic_collection_enabled = true)`)

sqlDB.Exec(t,
`ALTER TABLE t2 SET (sql_stats_automatic_collection_enabled = true)`)

}

0 comments on commit 9a1c3a9

Please sign in to comment.