Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zone: Fix zone configuration application bug #41089

Merged
merged 1 commit into from
Sep 26, 2019

Conversation

rohany
Copy link
Contributor

@rohany rohany commented Sep 25, 2019

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 #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.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei, @knz, and @rohany)


pkg/ccl/logictestccl/testdata/logic_test/zone, line 766 at r1 (raw file):

ALTER TABLE system.jobs CONFIGURE ZONE USING num_replicas = 3

# Test that index configurations don't infect partition configurations.

pls add more words here. Say something checking that num_replicas is not copied to p1's partition

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.
@rohany
Copy link
Contributor Author

rohany commented Sep 26, 2019

TFTR -- updated.

@rohany
Copy link
Contributor Author

rohany commented Sep 26, 2019

bors r=andreimatei

craig bot pushed a commit that referenced this pull request Sep 26, 2019
41089: zone: Fix zone configuration application bug r=andreimatei a=rohany

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 #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.

41130: roachtest: update hibernate blacklist after new syntax support r=jordanlewis a=rafiss

Some tests that used to fail pass now since we support SELECT FOR UPDATE
syntax.

relates to #40538 

Release justification: test only change

Release note: None

Co-authored-by: Rohan Yadav <rohany@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Sep 26, 2019

Build succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants