Skip to content

Commit

Permalink
config: introduce num_voters and voter_constraints
Browse files Browse the repository at this point in the history 
This commit introduces two new attributes to zone configs: `num_voters`
and `voter_constraints`. These attributes semantically act as subsets of
the existing `num_replicas` and `constraints` attributes, in that they
apply _only_ to voting replicas.

After this change, when `num_voters` is explicitly specified,
`num_replicas` indicates the sum of voting and non-voting replicas.
Likewise, the existing `constraints` field governs the placement
constraints for all replicas (similarly defaulting to just voting
replicas if non-voting replicas aren't used), whereas the new
`voter_constraints` field governs placement constraints for just the
voting replicas. *This retains backwards compatibility* for existing
zone configs that do not use these newly added attributes.

Note that the set of nodes satisfying `voter_constraints` may not
necessarily be a subset of the set of nodes satisfying `constraints`.
`voter_constraints` are simply required to be compatible with
`constraints`. They can be totally disjoint from the `constraints` (i.e.
talk about completely different set of attributes).

Release note (sql change): Zone configs now support new attributes
`num_voters` and `voter_constraints`. `num_voters` will specify the
number of voting replicas. When `num_voters` is explicitly specified,
`num_replicas` will be the sum of voting and non-voting replicas.
`voter_constraints` will specify the constraints that govern the
placement of just the voting replicas, whereas the existing
`constraints` attributes will govern the placement of all replicas
(voting as well as non-voting).
  • Loading branch information
@aayushshah15
aayushshah15 committed Jan 28, 2021
1 parent ba82208 commit 836f39c
Show file tree
Hide file tree
Showing 12 changed files with 875 additions and 143 deletions.
149 changes: 146 additions & 3 deletions pkg/config/zonepb/zone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ func (c *Constraint) FromString(short string) error {
func NewZoneConfig() *ZoneConfig {
return &ZoneConfig{
InheritedConstraints: true,
InheritedVoterConstraints: true,
InheritedLeasePreferences: true,
}
}
Expand All @@ -184,10 +185,12 @@ func NewZoneConfig() *ZoneConfig {
func EmptyCompleteZoneConfig() *ZoneConfig {
return &ZoneConfig{
NumReplicas: proto.Int32(0),
NumVoters: proto.Int32(0),
RangeMinBytes: proto.Int64(0),
RangeMaxBytes: proto.Int64(0),
GC: &GCPolicy{TTLSeconds: 0},
InheritedConstraints: true,
InheritedVoterConstraints: true,
InheritedLeasePreferences: true,
}
}
Expand Down Expand Up @@ -240,26 +243,43 @@ func DefaultSystemZoneConfigRef() *ZoneConfig {
func (z *ZoneConfig) IsComplete() bool {
return ((z.NumReplicas != nil) && (z.RangeMinBytes != nil) &&
(z.RangeMaxBytes != nil) && (z.GC != nil) &&
(!z.InheritedConstraints) && (!z.InheritedLeasePreferences))
(!z.InheritedVoterConstraints) && (!z.InheritedConstraints) &&
(!z.InheritedLeasePreferences))
}

// ValidateTandemFields returns an error if the ZoneConfig to be written
// specifies a configuration that could cause problems with the introduction
// of cascading zone configs.
func (z *ZoneConfig) ValidateTandemFields() error {
var numConstrainedRepls int32
numVotersExplicit := z.NumVoters != nil && *z.NumVoters > 0
for _, constraint := range z.Constraints {
numConstrainedRepls += constraint.NumReplicas
}

if numConstrainedRepls > 0 && z.NumReplicas == nil {
return fmt.Errorf("when per-replica constraints are set, num_replicas must be set as well")
}

var numConstrainedVoters int32
for _, constraint := range z.VoterConstraints {
numConstrainedVoters += constraint.NumReplicas
}

if (numConstrainedVoters > 0 && z.NumVoters == nil) ||
(!numVotersExplicit && len(z.VoterConstraints) > 0) {
return fmt.Errorf("when voter_constraints are set, num_voters must be set as well")
}

if (z.RangeMinBytes != nil || z.RangeMaxBytes != nil) &&
(z.RangeMinBytes == nil || z.RangeMaxBytes == nil) {
return fmt.Errorf("range_min_bytes and range_max_bytes must be set together")
}
if !z.InheritedLeasePreferences && z.InheritedConstraints {
if numVotersExplicit {
if !z.InheritedLeasePreferences && z.InheritedVoterConstraints {
return fmt.Errorf("lease preferences can not be set unless the voter_constraints are explicitly set as well")
}
} else if !z.InheritedLeasePreferences && z.InheritedConstraints {
return fmt.Errorf("lease preferences can not be set unless the constraints are explicitly set as well")
}
return nil
Expand All @@ -286,7 +306,23 @@ func (z *ZoneConfig) Validate() error {
}
return fmt.Errorf("at least one replica is required")
case *z.NumReplicas == 2:
return fmt.Errorf("at least 3 replicas are required for multi-replica configurations")
if !(z.NumVoters != nil && *z.NumVoters > 0) {
return fmt.Errorf("at least 3 replicas are required for multi-replica configurations")
}
}
}

var numVotersExplicit bool
if z.NumVoters != nil {
numVotersExplicit = true
switch {
case *z.NumVoters <= 0:
return fmt.Errorf("at least one voting replica is required")
case *z.NumVoters == 2:
return fmt.Errorf("at least 3 voting replicas are required for multi-replica configurations")
}
if z.NumReplicas != nil && *z.NumVoters > *z.NumReplicas {
return fmt.Errorf("num_voters cannot be greater than num_replicas")
}
}

Expand Down Expand Up @@ -318,6 +354,19 @@ func (z *ZoneConfig) Validate() error {
}
}

for _, constraints := range z.VoterConstraints {
for _, constraint := range constraints.Constraints {
if constraint.Type == Constraint_DEPRECATED_POSITIVE {
return fmt.Errorf("voter_constraints must be of type 'required' (prefixed with a '+')")
}
// TODO(aayush): Allowing these makes validating `voter_constraints`
// against `constraints` harder. Revisit this decision if need be.
if constraint.Type == Constraint_PROHIBITED {
return fmt.Errorf("voter_constraints cannot contain prohibitive constraints")
}
}
}

// We only need to further validate constraints if per-replica constraints
// are in use. The old style of constraints that apply to all replicas don't
// require validation.
Expand All @@ -344,6 +393,48 @@ func (z *ZoneConfig) Validate() error {
}
}

// If we have per replica constraints inside voter_constraints, make sure
// that the number of replicas adds up to less than the number of voters.
//
// NB: We intentionally allow the number of replicas constrained by
// `constraints` plus the number of voters constrained by `voter_constraints`
// to exceed num_voters.
// For instance, the following would be a valid zone configuration:
// num_replicas = 3
// num_voters = 3
// constraints = {"+region=A": 1, "+region=B": 1, "+region=C": 1}
// voter_constraints = {"+ssd": 3}
// In the current state of our zone config validation logic, allowing examples
// like the one shown above also allows the user to walk themselves into
// unsatisfiable zone configurations like the following:
// num_replicas = 3
// num_voters = 3
// constraints = {"+region=A": 2, "+region=B": 1}
// voter_constraints = {"+region=C": 2, "+region=D": 1}
if numVotersExplicit {
if len(z.VoterConstraints) > 1 || (len(z.VoterConstraints) == 1 && z.VoterConstraints[0].NumReplicas != 0) {
var numConstrainedRepls int64
for _, constraints := range z.VoterConstraints {
if constraints.NumReplicas <= 0 {
return fmt.Errorf("constraints must apply to at least one replica")
}
numConstrainedRepls += int64(constraints.NumReplicas)
}
// NB: These nil checks are not required in production code but they are
// for testing as some tests run `Validate()` on incomplete zone configs.
if z.NumVoters != nil && numConstrainedRepls > int64(*z.NumVoters) {
return fmt.Errorf("the number of replicas specified in voter_constraints (%d) cannot be greater "+
"than the number of voters configured for the zone (%d)",
numConstrainedRepls, *z.NumVoters)
}
}
}

// Validate that `constraints` aren't incompatible with `voter_constraints`.
if err := validateVoterConstraintsCompatibility(z.VoterConstraints, z.Constraints); err != nil {
return err
}

for _, leasePref := range z.LeasePreferences {
if len(leasePref.Constraints) == 0 {
return fmt.Errorf("every lease preference must include at least one constraint")
Expand All @@ -359,6 +450,35 @@ func (z *ZoneConfig) Validate() error {
return nil
}

// validateVoterConstraintsCompatibility cross-validates `voter_constraints`
// against `constraints` and ensures that nothing that is prohibited at the
// overall `constraints` level is required at the `voter_constraints` level,
// since this sort of incongruity will lead to an unsatisfiable zone
// configuration.
func validateVoterConstraintsCompatibility(
voterConstraints, overallConstraints []ConstraintsConjunction,
) error {
// We know that prohibitive constraints are not allowed under
// `voter_constraints`. Walk through overallConstraints to ensure that none of
// the prohibitive constraints conflict with the `required` constraints in
// voterConstraints.
for _, constraints := range overallConstraints {
for _, constraint := range constraints.Constraints {
if constraint.Type == Constraint_PROHIBITED {
for _, otherConstraints := range voterConstraints {
for _, otherConstraint := range otherConstraints.Constraints {
conflicting := otherConstraint.Value == constraint.Value && otherConstraint.Key == constraint.Key
if conflicting {
return fmt.Errorf("prohibitive constraint %s conflicts with voter_constraint %s", constraint, otherConstraint)
}
}
}
}
}
}
return nil
}

// InheritFromParent hydrates a zones missing fields from its parent.
func (z *ZoneConfig) InheritFromParent(parent *ZoneConfig) {
// Allow for subzonePlaceholders to inherit fields from parents if needed.
Expand All @@ -367,6 +487,11 @@ func (z *ZoneConfig) InheritFromParent(parent *ZoneConfig) {
z.NumReplicas = proto.Int32(*parent.NumReplicas)
}
}
if z.NumVoters == nil || (z.NumVoters != nil && *z.NumVoters == 0) {
if parent.NumVoters != nil {
z.NumVoters = proto.Int32(*parent.NumVoters)
}
}
if z.RangeMinBytes == nil {
if parent.RangeMinBytes != nil {
z.RangeMinBytes = proto.Int64(*parent.RangeMinBytes)
Expand All @@ -389,6 +514,12 @@ func (z *ZoneConfig) InheritFromParent(parent *ZoneConfig) {
z.InheritedConstraints = false
}
}
if z.InheritedVoterConstraints {
if !parent.InheritedVoterConstraints {
z.VoterConstraints = parent.VoterConstraints
z.InheritedVoterConstraints = false
}
}
if z.InheritedLeasePreferences {
if !parent.InheritedLeasePreferences {
z.LeasePreferences = parent.LeasePreferences
Expand All @@ -406,6 +537,12 @@ func (z *ZoneConfig) CopyFromZone(other ZoneConfig, fieldList []tree.Name) {
z.NumReplicas = proto.Int32(*other.NumReplicas)
}
}
if fieldName == "num_voters" {
z.NumVoters = nil
if other.NumVoters != nil {
z.NumVoters = proto.Int32(*other.NumVoters)
}
}
if fieldName == "range_min_bytes" {
z.RangeMinBytes = nil
if other.RangeMinBytes != nil {
Expand Down Expand Up @@ -435,6 +572,10 @@ func (z *ZoneConfig) CopyFromZone(other ZoneConfig, fieldList []tree.Name) {
z.Constraints = other.Constraints
z.InheritedConstraints = other.InheritedConstraints
}
if fieldName == "voter_constraints" {
z.VoterConstraints = other.VoterConstraints
z.InheritedVoterConstraints = other.InheritedVoterConstraints
}
if fieldName == "lease_preferences" {
z.LeasePreferences = other.LeasePreferences
z.InheritedLeasePreferences = other.InheritedLeasePreferences
Expand Down Expand Up @@ -500,6 +641,8 @@ func (z *ZoneConfig) IsSubzonePlaceholder() bool {
// A ZoneConfig with zero replicas is otherwise invalid, so we repurpose it to
// indicate that a ZoneConfig is a placeholder for subzones rather than
// introducing a dedicated IsPlaceholder flag.
// TODO(aayush): Decide whether its worth introducing a isPlaceholder flag to
// clean this up after num_voters is introduced.
return z.NumReplicas != nil && *z.NumReplicas == 0
}

Expand Down
Loading

0 comments on commit 836f39c

Please sign in to comment.