Skip to content

Commit

Permalink
zone: Fix zone configuration application bug
Browse files Browse the repository at this point in the history
There was a bug that allowed zone configuration application on indexes
to leak into the zone configurations for partitions, due to a subtlety in
ZoneConfig.GetSubzone. This PR fixes the bug with zone configuration
application and adds a test.

This PR is necessary for cockroachdb#40493 to land.

An example of this is as follows:

```
CREATE TABLE infect (x INT PRIMARY KEY);
ALTER TABLE infect PARTITION BY LIST (x) ( PARTITION p1 VALUES IN (1));
ALTER INDEX infect@primary CONFIGURE ZONE USING num_replicas=5;
ALTER PARTITION p1 OF TABLE infect CONFIGURE ZONE USING
constraints='[+dc=dc1]';
```
Before, the zone configuration for p1 would *also have* num_replicas=5
set, which should not be the case. This PR ensures that the zone
configuration for p1 only has constraints set.

Release Justification: Important bug fix.

Release note (bug fix): Fixing bug where zone configuration application
on indexes could leak into configurations on partitions.
  • Loading branch information
Rohan Yadav committed Oct 21, 2019
1 parent d022c05 commit c4cf201
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/cliccl/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func Example_cclzone() {
// gc:
// ttlseconds: 90000
// num_replicas: 3
// constraints: [+zone=us-east-1a, +ssd]
// constraints: []
// lease_preferences: []
// zone ls
// .default
Expand Down
15 changes: 15 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/zone
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,21 @@ test.t38074@i ALTER INDEX t38074@i CONFIGURE ZONE USING
constraints = '[]',
lease_preferences = '[]'

# Test that index configurations don't infect partition configurations.
# Specifically we are testing that values written to infect@primary's
# zone configuration does not appear in partition p1 of infect@primary's zone config.
statement ok
CREATE TABLE infect (x INT PRIMARY KEY);
ALTER TABLE infect PARTITION BY LIST (x) ( PARTITION p1 VALUES IN (1));
ALTER INDEX infect@primary CONFIGURE ZONE USING num_replicas=5;
ALTER PARTITION p1 OF TABLE infect CONFIGURE ZONE USING constraints='[+dc=dc1]'

query T
SELECT config_sql FROM [SHOW ZONE CONFIGURATIONS] WHERE zone_name = 'test.infect.p1'
----
ALTER PARTITION p1 OF INDEX test.public.infect@primary CONFIGURE ZONE USING
constraints = '[+dc=dc1]'

# ------------------------------------------------------------------------------
# Regression test for #36348; place this at the bottom of this file.
# ------------------------------------------------------------------------------
Expand Down
13 changes: 13 additions & 0 deletions pkg/config/zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,19 @@ func (z *ZoneConfig) GetSubzone(indexID uint32, partition string) *Subzone {
return nil
}

// GetSubzoneExact is similar to GetSubzone but does not find the most specific
// subzone that applies to a specified index and partition, as it finds either the
// exact config that applies, or returns nil.
func (z *ZoneConfig) GetSubzoneExact(indexID uint32, partition string) *Subzone {
for _, s := range z.Subzones {
if s.IndexID == indexID && s.PartitionName == partition {
copySubzone := s
return &copySubzone
}
}
return nil
}

// GetSubzoneForKeySuffix returns the ZoneConfig for the subzone that contains
// keySuffix, if it exists.
func (z ZoneConfig) GetSubzoneForKeySuffix(keySuffix []byte) *Subzone {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/set_zone_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func (n *setZoneConfigNode) startExec(params runParams) error {

var partialSubzone *config.Subzone
if index != nil {
partialSubzone = partialZone.GetSubzone(uint32(index.ID), partition)
partialSubzone = partialZone.GetSubzoneExact(uint32(index.ID), partition)
if partialSubzone == nil {
partialSubzone = &config.Subzone{Config: *config.NewZoneConfig()}
}
Expand Down

0 comments on commit c4cf201

Please sign in to comment.