Skip to content

Commit

Permalink
sql: add scaffolding for version upgrade hook
Browse files Browse the repository at this point in the history
This callback will be called after validating a `SET CLUSTER SETTING
version` but before executing it. It will be used in future PRs to
execute arbitrary migrations to allow us to eventually remove code to
support legacy behavior.

This diff was pulled out of the long-running migrations prototype
(cockroachdb#56107). For more details, see the RFC (cockroachdb#48843).

Release note: None
  • Loading branch information
irfansharif committed Nov 6, 2020
1 parent 5c2b08d commit b83f71f
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
4 changes: 4 additions & 0 deletions pkg/server/server_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,10 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*sqlServer, error) {
RangeDescriptorCache: cfg.distSender.RangeDescriptorCache(),
RoleMemberCache: &sql.MembershipCache{},
TestingKnobs: sqlExecutorTestingKnobs,
VersionUpgradeHook: func(ctx context.Context, to roachpb.Version) error {
// TODO(irfansharif): Do something real here.
return nil
},

DistSQLPlanner: sql.NewDistSQLPlanner(
ctx,
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,11 @@ type ExecutorConfig struct {
HydratedTables *hydratedtables.Cache

GCJobNotifier *gcjobnotifier.Notifier

// VersionUpgradeHook is called after validating a `SET CLUSTER SETTING
// version` but before executing it. It can carry out arbitrary migrations
// that allow us to eventually remove legacy code.
VersionUpgradeHook func(ctx context.Context, to roachpb.Version) error
}

// Organization returns the value of cluster.organization.
Expand Down
26 changes: 24 additions & 2 deletions pkg/sql/set_cluster_setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/settings"
Expand Down Expand Up @@ -47,6 +48,10 @@ type setClusterSettingNode struct {
setting settings.WritableSetting
// If value is nil, the setting should be reset.
value tree.TypedExpr
// versionUpgradeHook is called after validating a `SET CLUSTER SETTING
// version` but before executing it. It can carry out arbitrary migrations
// that allow us to eventually remove legacy code.
versionUpgradeHook func(ctx context.Context, to roachpb.Version) error
}

func checkPrivilegesForSetting(ctx context.Context, p *planner, name string, action string) error {
Expand Down Expand Up @@ -156,7 +161,11 @@ func (p *planner) SetClusterSetting(
}
}

return &setClusterSettingNode{name: name, st: st, setting: setting, value: value}, nil
csNode := setClusterSettingNode{
name: name, st: st, setting: setting, value: value,
versionUpgradeHook: p.execCfg.VersionUpgradeHook,
}
return &csNode, nil
}

func (n *setClusterSettingNode) startExec(params runParams) error {
Expand Down Expand Up @@ -209,7 +218,8 @@ func (n *setClusterSettingNode) startExec(params runParams) error {
}
reportedValue = tree.AsStringWithFlags(value, tree.FmtBareStrings)
var prev tree.Datum
if _, ok := n.setting.(*settings.VersionSetting); ok {
_, isSetVersion := n.setting.(*settings.VersionSetting)
if isSetVersion {
datums, err := execCfg.InternalExecutor.QueryRowEx(
ctx, "retrieve-prev-setting", txn,
sessiondata.InternalExecutorOverride{User: security.RootUserName()},
Expand All @@ -232,6 +242,18 @@ func (n *setClusterSettingNode) startExec(params runParams) error {
if err != nil {
return err
}

if isSetVersion {
// toSettingString already validated the input, and checked to
// see that we are allowed to transition. Let's call into our
// upgrade hook to run migrations, if any.
versionStr := string(*value.(*tree.DString))
targetVersion := roachpb.MustParseVersion(versionStr)
if err := n.versionUpgradeHook(ctx, targetVersion); err != nil {
return err
}
}

if _, err = execCfg.InternalExecutor.ExecEx(
ctx, "update-setting", txn,
sessiondata.InternalExecutorOverride{User: security.RootUserName()},
Expand Down

0 comments on commit b83f71f

Please sign in to comment.