Skip to content

Commit

Permalink
sql: gate global_reads zone attribute on cluster version and enterpri…
Browse files Browse the repository at this point in the history 
…se license

This commit gates the use of the global_reads zone configuration
attribute on the cluster version and on the use of an enterprise
license.

The first half of this is necessary for correctness. The second half is
not, because follower reads won't be served from global read followers
without an enterprise license, but a check when the zone configs are set
improves UX.
  • Loading branch information
@nvanbenschoten
nvanbenschoten committed Mar 29, 2021
1 parent 0dd303e commit 0ee4687
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 36 deletions.
18 changes: 18 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,21 @@ CREATE TABLE t()

statement error cannot alter a table's LOCALITY if its database is not multi-region enabled
ALTER TABLE t SET LOCALITY REGIONAL BY ROW

statement error pgcode 0A000 global_reads cannot be used until cluster version is finalized
ALTER TABLE t CONFIGURE ZONE USING global_reads = true

statement error pgcode 0A000 global_reads cannot be used until cluster version is finalized
ALTER TABLE t CONFIGURE ZONE USING global_reads = false

statement error pgcode 0A000 num_voters cannot be used until cluster version is finalized
ALTER TABLE t CONFIGURE ZONE USING num_voters = 3

statement error pgcode 0A000 num_voters cannot be used until cluster version is finalized
ALTER TABLE t CONFIGURE ZONE USING num_voters = 0

statement error pgcode 0A000 voter_constraints cannot be used until cluster version is finalized
ALTER TABLE t CONFIGURE ZONE USING voter_constraints = '[+region=us-east-1]'

statement error pgcode 0A000 voter_constraints cannot be used until cluster version is finalized
ALTER TABLE t CONFIGURE ZONE USING voter_constraints = ''
43 changes: 43 additions & 0 deletions pkg/ccl/multiregionccl/multiregion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,46 @@ func TestMultiRegionAfterEnterpriseDisabled(t *testing.T) {
}
})
}

func TestGlobalReadsAfterEnterpriseDisabled(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
defer utilccl.TestingEnableEnterprise()()

_, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
t, 3 /* numServers */, base.TestingKnobs{}, nil, /* baseDir */
)
defer cleanup()

for _, setupQuery := range []string{
`CREATE DATABASE test`,
`USE test`,
`CREATE TABLE t1 ()`,
`CREATE TABLE t2 ()`,
} {
_, err := sqlDB.Exec(setupQuery)
require.NoError(t, err)
}

// Can set global_reads with enterprise license enabled.
_, err := sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = true`)
require.NoError(t, err)

_, err = sqlDB.Exec(`ALTER TABLE t2 CONFIGURE ZONE USING global_reads = true`)
require.NoError(t, err)

// Can unset global_reads with enterprise license enabled.
_, err = sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = false`)
require.NoError(t, err)

defer utilccl.TestingDisableEnterprise()()

// Cannot set global_reads with enterprise license disabled.
_, err = sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = true`)
require.Error(t, err)
require.Regexp(t, "use of global_reads requires an enterprise license", err)

// Can unset global_reads with enterprise license disabled.
_, err = sqlDB.Exec(`ALTER TABLE t2 CONFIGURE ZONE USING global_reads = false`)
require.NoError(t, err)
}
133 changes: 97 additions & 36 deletions pkg/sql/set_zone_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"sort"
"strings"

"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"
Expand Down Expand Up @@ -63,37 +64,86 @@ type setZoneConfigNode struct {
var supportedZoneConfigOptions = map[tree.Name]struct {
requiredType *types.T
setter func(*zonepb.ZoneConfig, tree.Datum)
checkAllowed func(context.Context, *ExecutorConfig, tree.Datum) error // optional
}{
"range_min_bytes": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMinBytes = proto.Int64(int64(tree.MustBeDInt(d))) }},
"range_max_bytes": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMaxBytes = proto.Int64(int64(tree.MustBeDInt(d))) }},
"global_reads": {types.Bool, func(c *zonepb.ZoneConfig, d tree.Datum) { c.GlobalReads = proto.Bool(bool(tree.MustBeDBool(d))) }},
"num_replicas": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumReplicas = proto.Int32(int32(tree.MustBeDInt(d))) }},
"num_voters": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumVoters = proto.Int32(int32(tree.MustBeDInt(d))) }},
"gc.ttlseconds": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) {
c.GC = &zonepb.GCPolicy{TTLSeconds: int32(tree.MustBeDInt(d))}
}},
"constraints": {types.String, func(c *zonepb.ZoneConfig, d tree.Datum) {
constraintsList := zonepb.ConstraintsList{
Constraints: c.Constraints,
Inherited: c.InheritedConstraints,
}
loadYAML(&constraintsList, string(tree.MustBeDString(d)))
c.Constraints = constraintsList.Constraints
c.InheritedConstraints = false
}},
"voter_constraints": {types.String, func(c *zonepb.ZoneConfig, d tree.Datum) {
voterConstraintsList := zonepb.ConstraintsList{
Constraints: c.VoterConstraints,
Inherited: c.InheritedVoterConstraints,
}
loadYAML(&voterConstraintsList, string(tree.MustBeDString(d)))
c.VoterConstraints = voterConstraintsList.Constraints
c.InheritedVoterConstraints = false
}},
"lease_preferences": {types.String, func(c *zonepb.ZoneConfig, d tree.Datum) {
loadYAML(&c.LeasePreferences, string(tree.MustBeDString(d)))
c.InheritedLeasePreferences = false
}},
"range_min_bytes": {
requiredType: types.Int,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMinBytes = proto.Int64(int64(tree.MustBeDInt(d))) },
},
"range_max_bytes": {
requiredType: types.Int,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMaxBytes = proto.Int64(int64(tree.MustBeDInt(d))) },
},
"global_reads": {
requiredType: types.Bool,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) { c.GlobalReads = proto.Bool(bool(tree.MustBeDBool(d))) },
checkAllowed: func(ctx context.Context, execCfg *ExecutorConfig, d tree.Datum) error {
if err := checkVersionActive(ctx, execCfg, clusterversion.NonVotingReplicas, "global_reads"); err != nil {
return err
}
if !tree.MustBeDBool(d) {
// Always allow the value to be unset.
return nil
}
return base.CheckEnterpriseEnabled(
execCfg.Settings,
execCfg.ClusterID(),
execCfg.Organization(),
"global_reads",
)
},
},
"num_replicas": {
requiredType: types.Int,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumReplicas = proto.Int32(int32(tree.MustBeDInt(d))) },
},
"num_voters": {
requiredType: types.Int,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumVoters = proto.Int32(int32(tree.MustBeDInt(d))) },
checkAllowed: func(ctx context.Context, execCfg *ExecutorConfig, _ tree.Datum) error {
return checkVersionActive(ctx, execCfg, clusterversion.NonVotingReplicas, "num_voters")
},
},
"gc.ttlseconds": {
requiredType: types.Int,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
c.GC = &zonepb.GCPolicy{TTLSeconds: int32(tree.MustBeDInt(d))}
},
},
"constraints": {
requiredType: types.String,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
constraintsList := zonepb.ConstraintsList{
Constraints: c.Constraints,
Inherited: c.InheritedConstraints,
}
loadYAML(&constraintsList, string(tree.MustBeDString(d)))
c.Constraints = constraintsList.Constraints
c.InheritedConstraints = false
},
},
"voter_constraints": {
requiredType: types.String,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
voterConstraintsList := zonepb.ConstraintsList{
Constraints: c.VoterConstraints,
Inherited: c.InheritedVoterConstraints,
}
loadYAML(&voterConstraintsList, string(tree.MustBeDString(d)))
c.VoterConstraints = voterConstraintsList.Constraints
c.InheritedVoterConstraints = false
},
checkAllowed: func(ctx context.Context, execCfg *ExecutorConfig, _ tree.Datum) error {
return checkVersionActive(ctx, execCfg, clusterversion.NonVotingReplicas, "voter_constraints")
},
},
"lease_preferences": {
requiredType: types.String,
setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
loadYAML(&c.LeasePreferences, string(tree.MustBeDString(d)))
c.InheritedLeasePreferences = false
},
},
}

// zoneOptionKeys contains the keys from suportedZoneConfigOptions in
Expand All @@ -114,6 +164,16 @@ func loadYAML(dst interface{}, yamlString string) {
}
}

func checkVersionActive(
ctx context.Context, execCfg *ExecutorConfig, minVersion clusterversion.Key, option string,
) error {
if !execCfg.Settings.Version.IsActive(ctx, minVersion) {
return pgerror.Newf(pgcode.FeatureNotSupported,
"%s cannot be used until cluster version is finalized", option)
}
return nil
}

func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (planNode, error) {
if err := checkSchemaChangeEnabled(
ctx,
Expand Down Expand Up @@ -182,11 +242,6 @@ func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (pla
return nil, pgerror.Newf(pgcode.InvalidParameterValue,
"unsupported zone config parameter: %q", tree.ErrString(&opt.Key))
}
if (opt.Key == "num_voters" || opt.Key == "voter_constraints") &&
!p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.NonVotingReplicas) {
return nil, pgerror.Newf(pgcode.FeatureNotSupported,
"num_voters and voter_constraints cannot be used until cluster version is finalized")
}
telemetry.Inc(
sqltelemetry.SchemaSetZoneConfigCounter(
n.ZoneSpecifier.TelemetryName(),
Expand Down Expand Up @@ -334,7 +389,13 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
return pgerror.Newf(pgcode.InvalidParameterValue,
"unsupported NULL value for %q", tree.ErrString(name))
}
setter := supportedZoneConfigOptions[*name].setter
opt := supportedZoneConfigOptions[*name]
if opt.checkAllowed != nil {
if err := opt.checkAllowed(params.ctx, params.ExecCfg(), datum); err != nil {
return err
}
}
setter := opt.setter
setters = append(setters, func(c *zonepb.ZoneConfig) { setter(c, datum) })
optionsStr = append(optionsStr, fmt.Sprintf("%s = %s", name, datum))
}
Expand Down

0 comments on commit 0ee4687

Please sign in to comment.