Skip to content

Commit

Permalink
clusterversion,kvserver,sql: remove AutoStatsTableSettings
Browse files Browse the repository at this point in the history
Release note: None
  • Loading branch information
celiala committed Aug 11, 2022
1 parent 9fb8d36 commit a2a26ea
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 131 deletions.
7 changes: 0 additions & 7 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,6 @@ const (
// 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
// SuperRegions enables the usage on super regions.
SuperRegions
// EnableNewChangefeedOptions enables the usage of new changefeed options
Expand Down Expand Up @@ -489,10 +486,6 @@ var versionsSingleton = keyedVersions{
Key: ClusterLocksVirtualTable,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 98},
},
{
Key: AutoStatsTableSettings,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 100},
},
{
Key: SuperRegions,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 104},
Expand Down
63 changes: 31 additions & 32 deletions pkg/clusterversion/key_string.go

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

8 changes: 2 additions & 6 deletions pkg/kv/kvserver/allocator/allocatorimpl/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1605,12 +1605,8 @@ func (a *Allocator) leaseholderShouldMoveDueToPreferences(
// stores as targets) when enforcementLevel is set to storeHealthNoAction or
// storeHealthLogOnly. By default storeHealthBlockRebalanceTo is the action taken. When
// there is a mixed version cluster, storeHealthNoAction is set instead.
func (a *Allocator) StoreHealthOptions(ctx context.Context) StoreHealthOptions {
enforcementLevel := StoreHealthNoAction
if a.StorePool.St.Version.IsActive(ctx, clusterversion.AutoStatsTableSettings) {
enforcementLevel = StoreHealthEnforcement(l0SublevelsThresholdEnforce.Get(&a.StorePool.St.SV))
}

func (a *Allocator) StoreHealthOptions(_ context.Context) StoreHealthOptions {
enforcementLevel := StoreHealthEnforcement(l0SublevelsThresholdEnforce.Get(&a.StorePool.St.SV))
return StoreHealthOptions{
EnforcementLevel: enforcementLevel,
L0SublevelThreshold: l0SublevelsThreshold.Get(&a.StorePool.St.SV),
Expand Down
25 changes: 0 additions & 25 deletions pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,6 @@ func (n *alterTableNode) startExec(params runParams) error {
}

case *tree.AlterTableSetStorageParams:
oldTableHasAutoStatsSettings := n.tableDesc.GetAutoStatsSettings() != nil
var ttlBefore *catpb.RowLevelTTL
if ttl := n.tableDesc.GetRowLevelTTL(); ttl != nil {
ttlBefore = protoutil.Clone(ttl).(*catpb.RowLevelTTL)
Expand All @@ -757,15 +756,7 @@ func (n *alterTableNode) startExec(params runParams) error {
return err
}

newTableHasAutoStatsSettings := n.tableDesc.GetAutoStatsSettings() != nil
if err := checkDisallowedAutoStatsSettingChange(
params, oldTableHasAutoStatsSettings, newTableHasAutoStatsSettings,
); err != nil {
return err
}

case *tree.AlterTableResetStorageParams:
oldTableHasAutoStatsSettings := n.tableDesc.GetAutoStatsSettings() != nil
var ttlBefore *catpb.RowLevelTTL
if ttl := n.tableDesc.GetRowLevelTTL(); ttl != nil {
ttlBefore = protoutil.Clone(ttl).(*catpb.RowLevelTTL)
Expand All @@ -789,13 +780,6 @@ func (n *alterTableNode) startExec(params runParams) error {
return err
}

newTableHasAutoStatsSettings := n.tableDesc.GetAutoStatsSettings() != nil
if err := checkDisallowedAutoStatsSettingChange(
params, oldTableHasAutoStatsSettings, newTableHasAutoStatsSettings,
); err != nil {
return err
}

case *tree.AlterTableRenameColumn:
descChanged, err := params.p.renameColumn(params.ctx, n.tableDesc, t.Column, t.NewName)
if err != nil {
Expand Down Expand Up @@ -1991,15 +1975,6 @@ func handleTTLStorageParamChange(
func checkDisallowedAutoStatsSettingChange(
params runParams, oldTableHasAutoStatsSettings, newTableHasAutoStatsSettings bool,
) error {
if !oldTableHasAutoStatsSettings && !newTableHasAutoStatsSettings {
// Do not have to do anything here.
return nil
}

if err := checkAutoStatsTableSettingsEnabledForCluster(params.ctx, params.p.ExecCfg().Settings); err != nil {
return err
}

return nil
}

Expand Down
16 changes: 0 additions & 16 deletions pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1467,12 +1467,6 @@ func NewTableDesc(
primaryIndexColumnSet[string(regionalByRowCol)] = struct{}{}
}

if autoStatsSettings := desc.GetAutoStatsSettings(); autoStatsSettings != nil {
if err := checkAutoStatsTableSettingsEnabledForCluster(ctx, st); err != nil {
return nil, err
}
}

// Create the TTL automatic column (crdb_internal_expiration) if one does not already exist.
if ttl := desc.GetRowLevelTTL(); ttl != nil && ttl.HasDurationExpr() {
if err := checkTTLEnabledForCluster(ctx, st); err != nil {
Expand Down Expand Up @@ -2442,16 +2436,6 @@ func checkTTLEnabledForCluster(ctx context.Context, st *cluster.Settings) error
return nil
}

func checkAutoStatsTableSettingsEnabledForCluster(ctx context.Context, st *cluster.Settings) error {
if !st.Version.IsActive(ctx, clusterversion.AutoStatsTableSettings) {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"auto stats table settings are only available once the cluster is fully upgraded",
)
}
return nil
}

// CreateRowLevelTTLScheduledJob creates a new row-level TTL schedule.
func CreateRowLevelTTLScheduledJob(
ctx context.Context,
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 a2a26ea

Please sign in to comment.