From 0ee46876f58a25e4e03924e7b183cbada31cf532 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 29 Mar 2021 18:51:50 -0400 Subject: [PATCH] sql: gate global_reads zone attribute on cluster version and enterprise 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. --- .../logic_test/multi_region_mixed_20.2_21.1 | 18 +++ pkg/ccl/multiregionccl/multiregion_test.go | 43 ++++++ pkg/sql/set_zone_config.go | 133 +++++++++++++----- 3 files changed, 158 insertions(+), 36 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1 b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1 index 1d344181e1ff..21f37f972e27 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1 +++ b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1 @@ -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 = '' diff --git a/pkg/ccl/multiregionccl/multiregion_test.go b/pkg/ccl/multiregionccl/multiregion_test.go index 85710015de04..e4ed2392828a 100644 --- a/pkg/ccl/multiregionccl/multiregion_test.go +++ b/pkg/ccl/multiregionccl/multiregion_test.go @@ -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) +} diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index 88276b855908..a5e250fdb3a4 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -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" @@ -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 @@ -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, @@ -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(), @@ -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)) }