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

Fixes #61039.

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 30, 2021
1 parent a9cff79 commit 42f7f08
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 42 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 = ''
29 changes: 29 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/zone
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1126,3 +1126,32 @@ ALTER PARTITION p3 OF INDEX test.public.t@i2 CONFIGURE ZONE USING
gc.ttlseconds = 15411;
ALTER PARTITION p4 OF INDEX test.public.t@i2 CONFIGURE ZONE USING
gc.ttlseconds = 15418

# Test that the global_reads attribute can be set with an enterprise license.
# This same test fails in pkg/sql/logictest/testdata/logic_test/zone_config.
statement ok
CREATE TABLE global (id INT PRIMARY KEY)

statement ok
ALTER TABLE global CONFIGURE ZONE USING global_reads = true

# This should reflect in the metrics.
query T
SELECT feature_name FROM crdb_internal.feature_usage
WHERE feature_name IN (
'sql.schema.zone_config.table.global_reads'
) AND usage_count > 0 ORDER BY feature_name
----
sql.schema.zone_config.table.global_reads

query TT
SHOW ZONE CONFIGURATION FOR TABLE global
----
TABLE global ALTER TABLE global CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
global_reads = true,
num_replicas = 7,
constraints = '[]',
lease_preferences = '[]'
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, 1 /* 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)
}
10 changes: 4 additions & 6 deletions pkg/sql/logictest/testdata/logic_test/zone_config
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ ALTER TABLE a CONFIGURE ZONE USING
range_min_bytes = 200000 + 1,
range_max_bytes = 300000 + 1,
gc.ttlseconds = 3000 + 600,
global_reads = true,
num_replicas = floor(1.2)::int,
constraints = '[+region=test]',
lease_preferences = '[[+region=test]]'
Expand All @@ -87,14 +86,12 @@ WHERE feature_name IN (
'sql.schema.zone_config.table.range_min_bytes',
'sql.schema.zone_config.table.range_max_bytes',
'sql.schema.zone_config.table.gc.ttlseconds',
'sql.schema.zone_config.table.global_reads',
'sql.schema.zone_config.table.num_replicas',
'sql.schema.zone_config.table.constraints'
) AND usage_count > 0 ORDER BY feature_name
----
sql.schema.zone_config.table.constraints
sql.schema.zone_config.table.gc.ttlseconds
sql.schema.zone_config.table.global_reads
sql.schema.zone_config.table.num_replicas
sql.schema.zone_config.table.range_max_bytes
sql.schema.zone_config.table.range_min_bytes
Expand All @@ -106,7 +103,6 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
range_min_bytes = 200001,
range_max_bytes = 300001,
gc.ttlseconds = 3600,
global_reads = true,
num_replicas = 1,
constraints = '[+region=test]',
lease_preferences = '[[+region=test]]'
Expand All @@ -122,7 +118,6 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
range_min_bytes = 200001,
range_max_bytes = 400000,
gc.ttlseconds = 3600,
global_reads = true,
num_replicas = 1,
constraints = '[+region=test]',
lease_preferences = '[[+region=test]]'
Expand Down Expand Up @@ -159,7 +154,6 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
range_min_bytes = 200001,
range_max_bytes = 400000,
gc.ttlseconds = 3600,
global_reads = true,
num_replicas = 1,
constraints = '[+region=test]',
lease_preferences = '[[+region=test]]'
Expand Down Expand Up @@ -266,6 +260,10 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
voter_constraints = '{+region=test: 1}',
lease_preferences = '[]'

# Check that global_reads cannot be set without a CCL binary and enterprise license.
statement error OSS binaries do not include enterprise features
ALTER TABLE a CONFIGURE ZONE USING global_reads = true

# Check entities for which we can set zone configs.
subtest test_entity_validity

Expand Down
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 42f7f08

Please sign in to comment.