Skip to content

Commit cf33d36

Browse files
committed
sql: add setting for constraint validation for secondary tenants
This patch adds a cluster setting, ``sql.zone_configs.max_replicas_per_region, that helps prevent virtual clusters from setting undesired zone configs. This setting defines the max number of replicas that can be configured for a single region (0 for unlimited). Epic: none Informs: #142856 Release note: None
1 parent a66d2fb commit cf33d36

File tree

7 files changed

+165
-1
lines changed

7 files changed

+165
-1
lines changed
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# LogicTest: multiregion-9node-3region-3azs-tenant
2+
# tenant-cluster-setting-override-opt: sql.virtual_cluster.feature_access.multiregion.enabled=true
3+
4+
statement ok
5+
CREATE DATABASE db PRIMARY REGION "us-east-1"
6+
7+
statement ok
8+
ALTER DATABASE db ADD REGION "ca-central-1"
9+
10+
statement ok
11+
SET override_multi_region_zone_config = true
12+
13+
statement ok
14+
ALTER DATABASE db CONFIGURE ZONE USING constraints = '{+region=us-east-1: 4}', voter_constraints = '{+region=us-east-1: 3}'
15+
16+
subtest constrained_replicas
17+
18+
user root
19+
20+
statement ok
21+
SET CLUSTER SETTING sql.zone_configs.max_replicas_per_region = 10
22+
23+
user host-cluster-root
24+
25+
# Ensure that the override from the system tenant takes precedence over anything
26+
# set from within the application tenant.
27+
statement ok
28+
ALTER TENANT ALL SET CLUSTER SETTING sql.zone_configs.max_replicas_per_region = 4
29+
30+
user root
31+
32+
# Our setting should not affect non-constraint related zone config updates.
33+
statement ok
34+
ALTER DATABASE db CONFIGURE ZONE USING gc.ttlseconds = 1
35+
36+
statement error pgcode 23514 constraint for "us-east-1" exceeds the configured maximum of 4 replicas
37+
ALTER DATABASE db CONFIGURE ZONE USING constraints = '{+region=us-east-1: 5}'
38+
39+
statement ok
40+
ALTER DATABASE db CONFIGURE ZONE USING constraints = '{+region=us-east-1: 1, +region=ca-central-1: 1}', voter_constraints = '{+region=us-east-1: 3}'
41+
42+
statement error pgcode 23514 voter constraint for "us-east-1" exceeds the configured maximum of 4 replicas
43+
ALTER DATABASE db CONFIGURE ZONE USING voter_constraints = '{+region=us-east-1: 5}'
44+
45+
user host-cluster-root
46+
47+
statement ok
48+
ALTER TENANT ALL SET CLUSTER SETTING sql.zone_configs.max_replicas_per_region = 3
49+
50+
user root
51+
52+
statement ok
53+
ALTER DATABASE db CONFIGURE ZONE USING gc.ttlseconds = 1
54+
55+
statement ok
56+
ALTER DATABASE db CONFIGURE ZONE USING constraints = '{+region=us-east-1: 3, +region=ca-central-1: 1}', voter_constraints = '{+region=us-east-1: 2}'
57+
58+
# Ensure that prohibited constraints don't trigger a validation error.
59+
statement ok
60+
ALTER DATABASE test CONFIGURE ZONE USING num_replicas = 4, constraints = '{-region=us-east-1: 4}'
61+
62+
subtest end

pkg/ccl/logictestccl/tests/multiregion-9node-3region-3azs-tenant/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ go_test(
99
"//pkg/ccl/logictestccl:testdata", # keep
1010
],
1111
exec_properties = {"test.Pool": "large"},
12-
shard_count = 17,
12+
shard_count = 18,
1313
tags = ["cpu:4"],
1414
deps = [
1515
"//pkg/base",

pkg/ccl/logictestccl/tests/multiregion-9node-3region-3azs-tenant/generated_test.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/config/zonepb/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ go_library(
1515
"//pkg/clusterversion",
1616
"//pkg/keys",
1717
"//pkg/roachpb",
18+
"//pkg/settings",
1819
"//pkg/sql/pgwire/pgcode",
1920
"//pkg/sql/pgwire/pgerror",
2021
"//pkg/sql/sem/tree",
2122
"//pkg/util/debugutil",
2223
"//pkg/util/envutil",
2324
"//pkg/util/log",
2425
"@com_github_cockroachdb_errors//:errors",
26+
"@com_github_cockroachdb_redact//:redact",
2527
"@com_github_gogo_protobuf//proto",
2628
"@in_gopkg_yaml_v2//:yaml_v2",
2729
],

pkg/config/zonepb/zone.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/clusterversion"
1818
"github.com/cockroachdb/cockroach/pkg/keys"
1919
"github.com/cockroachdb/cockroach/pkg/roachpb"
20+
"github.com/cockroachdb/cockroach/pkg/settings"
2021
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
2122
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
2223
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
2324
"github.com/cockroachdb/cockroach/pkg/util/envutil"
2425
"github.com/cockroachdb/cockroach/pkg/util/log"
2526
"github.com/cockroachdb/errors"
27+
"github.com/cockroachdb/redact"
2628
"github.com/gogo/protobuf/proto"
2729
)
2830

@@ -91,6 +93,16 @@ var MultiRegionZoneConfigFields = []tree.Name{
9193
"lease_preferences",
9294
}
9395

96+
var MaxReplicasPerRegion = settings.RegisterIntSetting(
97+
settings.ApplicationLevel,
98+
"sql.zone_configs.max_replicas_per_region",
99+
"the maximum number of replicas that can be "+
100+
"configured per region (0 for unlimited); this is only enforced on "+
101+
"new zone config modifications",
102+
0,
103+
settings.WithVisibility(settings.Reserved),
104+
settings.NonNegativeInt)
105+
94106
// MultiRegionZoneConfigFieldsSet contain the items in
95107
// MultiRegionZoneConfigFields but in a set form for fast lookup.
96108
var MultiRegionZoneConfigFieldsSet = func() map[tree.Name]struct{} {
@@ -1474,3 +1486,78 @@ func (v ReplaceMinMaxValVisitor) VisitPre(expr tree.Expr) (recurse bool, newExpr
14741486

14751487
// VisitPost satisfies the Visitor interface.
14761488
func (ReplaceMinMaxValVisitor) VisitPost(expr tree.Expr) tree.Expr { return expr }
1489+
1490+
// ValidateNewUniqueConstraintsForSecondaryTenants validates that none of our
1491+
// zonepb.ConstraintsConjunction violate the given max replicas per region
1492+
// set by sql.zone_configs.max_replicas_per_region.
1493+
func ValidateNewUniqueConstraintsForSecondaryTenants(
1494+
sv *settings.Values, currentZone, newZone *ZoneConfig,
1495+
) error {
1496+
maxReplicas := MaxReplicasPerRegion.Get(sv)
1497+
if maxReplicas == 0 {
1498+
return nil
1499+
}
1500+
1501+
getRequiredConstraintMap := func(
1502+
constraintsConj []ConstraintsConjunction,
1503+
) map[Constraint]int32 {
1504+
constraintsMap := make(map[Constraint]int32)
1505+
for _, constraints := range constraintsConj {
1506+
for _, constraint := range constraints.Constraints {
1507+
if constraint.Type == Constraint_REQUIRED {
1508+
constraintsMap[constraint] = constraints.NumReplicas
1509+
}
1510+
}
1511+
}
1512+
return constraintsMap
1513+
}
1514+
1515+
validateConstraints := func(
1516+
constraintType redact.SafeString,
1517+
currentConstraints map[Constraint]int32,
1518+
newConstraints ConstraintsConjunction,
1519+
) error {
1520+
for _, constraint := range newConstraints.Constraints {
1521+
if constraint.Type != Constraint_REQUIRED {
1522+
continue
1523+
}
1524+
newReplicasConstrained := newConstraints.NumReplicas
1525+
// If the user has not explicitly changed what replicas are
1526+
// constrained to a region, bypass this validation.
1527+
if currentReplicasConstrained, ok := currentConstraints[constraint]; ok {
1528+
if currentReplicasConstrained == newReplicasConstrained {
1529+
continue
1530+
}
1531+
}
1532+
// If the amount of replicas we have constrained for this region
1533+
// surpasses the limit we have set, error out early.
1534+
if int64(newReplicasConstrained) > maxReplicas {
1535+
return pgerror.Newf(
1536+
pgcode.CheckViolation,
1537+
"%sconstraint for %q exceeds the configured "+
1538+
"maximum of %d replicas",
1539+
constraintType,
1540+
constraint.Value,
1541+
maxReplicas,
1542+
)
1543+
}
1544+
}
1545+
return nil
1546+
}
1547+
1548+
for _, newConstraints := range newZone.Constraints {
1549+
err := validateConstraints("",
1550+
getRequiredConstraintMap(currentZone.Constraints), newConstraints)
1551+
if err != nil {
1552+
return err
1553+
}
1554+
}
1555+
for _, newVoterConstraints := range newZone.VoterConstraints {
1556+
err := validateConstraints("voter ",
1557+
getRequiredConstraintMap(currentZone.VoterConstraints), newVoterConstraints)
1558+
if err != nil {
1559+
return err
1560+
}
1561+
}
1562+
return nil
1563+
}

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,9 @@ func validateZoneLocalitiesForSecondaryTenants(
601601
settings *cluster.Settings,
602602
) error {
603603
toValidate := accumulateNewUniqueConstraints(currentZone, newZone)
604+
if err := zonepb.ValidateNewUniqueConstraintsForSecondaryTenants(&settings.SV, currentZone, newZone); err != nil {
605+
return err
606+
}
604607

605608
// rs and zs will be lazily populated with regions and zones, respectively.
606609
// These should not be accessed directly - use getRegionsAndZones helper

pkg/sql/set_zone_config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,9 @@ func validateZoneLocalitiesForSecondaryTenants(
941941
settings *cluster.Settings,
942942
) error {
943943
toValidate := accumulateNewUniqueConstraints(currentZone, newZone)
944+
if err := zonepb.ValidateNewUniqueConstraintsForSecondaryTenants(&settings.SV, currentZone, newZone); err != nil {
945+
return err
946+
}
944947

945948
// rs and zs will be lazily populated with regions and zones, respectively.
946949
// These should not be accessed directly - use getRegionsAndZones helper

0 commit comments

Comments
 (0)