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.

Sign up for GitHub

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

sql: add support for dropping the primary region from a multi-region db #60437

Merged
merged 1 commit into from
Feb 11, 2021
Merged

sql: add support for dropping the primary region from a multi-region db #60437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 133 additions & 35 deletions pkg/sql/alter_database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
Expand Down Expand Up @@ -249,8 +250,10 @@ func (n *alterDatabaseAddRegionNode) Values() tree.Datums { return tree
func (n *alterDatabaseAddRegionNode) Close(context.Context) {}

type alterDatabaseDropRegionNode struct {
n *tree.AlterDatabaseDropRegion
desc *dbdesc.Mutable
n *tree.AlterDatabaseDropRegion
desc *dbdesc.Mutable
removingPrimaryRegion bool
toDrop []*typedesc.Mutable
}

// AlterDatabaseDropRegion transforms a tree.AlterDatabaseDropRegion into a plan node.
Expand Down Expand Up @@ -280,15 +283,92 @@ func (p *planner) AlterDatabaseDropRegion(
return nil, pgerror.New(pgcode.InvalidDatabaseDefinition, "database has no regions to drop")
}

removingPrimaryRegion := false
var toDrop []*typedesc.Mutable

if dbDesc.RegionConfig.PrimaryRegion == descpb.RegionName(n.Region) {
return nil, errors.WithHintf(
errors.Newf("cannot drop region %q", dbDesc.RegionConfig.PrimaryRegion),
"You must designate another region as the primary region or remove all "+
"other regions before attempting to drop region %q", n.Region,
)
removingPrimaryRegion = true

typeID, err := dbDesc.MultiRegionEnumID()
if err != nil {
return nil, err
}
typeDesc, err := p.Descriptors().GetMutableTypeVersionByID(ctx, p.txn, typeID)
if err != nil {
return nil, err
}
regions, err := typeDesc.RegionNames()
if err != nil {
return nil, err
}
if len(regions) != 1 {
return nil, errors.WithHintf(
errors.Newf("cannot drop region %q", dbDesc.RegionConfig.PrimaryRegion),
"You must designate another region as the primary region using "+
"ALTER DATABASE %s SET PRIMARY REGION <region name> or remove all other regions before "+
"attempting to drop region %q", dbDesc.GetName(), n.Region,
)
}

// When the last region is removed from the database, we also clean up
// detritus multi-region type descriptor. This includes both the
// type descriptor and its array counterpart.
toDrop = append(toDrop, typeDesc)
arrayTypeDesc, err := p.Descriptors().GetMutableTypeVersionByID(ctx, p.txn, typeDesc.ArrayTypeID)
if err != nil {
return nil, err
}
toDrop = append(toDrop, arrayTypeDesc)
for _, desc := range toDrop {
// canDropTypeDesc ensures that there are no references to tables on the
// type descriptor. This is what we expect when dropping the final
// (primary) region from a database, as REGIONAL BY ROW tables and
// REGIONAL BY TABLE tables (homed explicitly in the final region) will
// store a reference on the type descriptor. It is sufficient to simply
// check for stored references and not go through the validation that
// happens in the type_schema changer in this scenario.
if err := p.canDropTypeDesc(ctx, desc, tree.DropRestrict); err != nil {
return nil, errors.Wrapf(
err, "error removing primary region from database %s", dbDesc.Name)
}
}
}

return &alterDatabaseDropRegionNode{n, dbDesc}, nil
return &alterDatabaseDropRegionNode{
n,
dbDesc,
removingPrimaryRegion,
toDrop,
}, nil
}

// removeLocalityConfigFromAllTablesInDB removes the locality config from all
// tables under the supplied database.
func removeLocalityConfigFromAllTablesInDB(
ctx context.Context, p *planner, desc *dbdesc.Immutable,
) error {
if !desc.IsMultiRegion() {
return errors.AssertionFailedf(
"cannot remove locality configs from tables in non multi-region database with ID %d",
desc.GetID(),
)
}
b := p.Txn().NewBatch()
if err := forEachTableDesc(ctx, p, desc, hideVirtual,
func(immutable *dbdesc.Immutable, _ string, desc catalog.TableDescriptor) error {
mutDesc, err := p.Descriptors().GetMutableTableByID(ctx, p.txn, desc.GetID(), tree.ObjectLookupFlags{})
if err != nil {
return err
}
mutDesc.LocalityConfig = nil
if err := p.writeSchemaChangeToBatch(ctx, mutDesc, b); err != nil {
return err
}
return nil
}); err != nil {
return err
}
return p.Txn().Run(ctx, b)
}

func (n *alterDatabaseDropRegionNode) startExec(params runParams) error {
Expand All @@ -301,36 +381,54 @@ func (n *alterDatabaseDropRegionNode) startExec(params runParams) error {
return err
}

// dropEnumValue tries to remove the region value from the multi-region type
// descriptor. Among other things, it validates that the region is not in
// use by any tables. A region is considered "in use" if either a REGIONAL BY
// TABLE table is explicitly homed in that region or a row in a REGIONAL BY
// ROW table is homed in that region. The type schema changer is responsible
// for all the requisite validation.
if err := params.p.dropEnumValue(params.ctx, typeDesc, tree.EnumValue(n.n.Region)); err != nil {
return err
}
if n.removingPrimaryRegion {
for _, desc := range n.toDrop {
jobDesc := fmt.Sprintf("drop multi-region enum with ID %d", desc.ID)
err := params.p.dropTypeImpl(params.ctx, desc, jobDesc, true /* queueJob */)
if err != nil {
return err
}
}

idx := 0
found := false
for i, region := range n.desc.RegionConfig.Regions {
if region.Name == descpb.RegionName(n.n.Region) {
idx = i
found = true
break
err = removeLocalityConfigFromAllTablesInDB(params.ctx, params.p, &n.desc.Immutable)
if err != nil {
return errors.Wrap(err, "error removing locality configs from tables")
}

n.desc.UnsetMultiRegionConfig()
} else {
// dropEnumValue tries to remove the region value from the multi-region type
// descriptor. Among other things, it validates that the region is not in
// use by any tables. A region is considered "in use" if either a REGIONAL BY
// TABLE table is explicitly homed in that region or a row in a REGIONAL BY
// ROW table is homed in that region. The type schema changer is responsible
// for all the requisite validation.
if err := params.p.dropEnumValue(params.ctx, typeDesc, tree.EnumValue(n.n.Region)); err != nil {
return err
}

// Remove the region from the database descriptor as well.
idx := 0
found := false
for i, region := range n.desc.RegionConfig.Regions {
if region.Name == descpb.RegionName(n.n.Region) {
idx = i
found = true
break
}
}
}
if !found {
// This shouldn't happen and is simply a sanity check to ensure the database
// descriptor regions and multi-region enum regions are indeed consistent.
return errors.AssertionFailedf(
"attempting to drop region %s not on database descriptor %d but found on type descriptor",
n.n.Region, n.desc.GetID(),
)
}

n.desc.RegionConfig.Regions = append(n.desc.RegionConfig.Regions[:idx],
n.desc.RegionConfig.Regions[idx+1:]...)
if !found {
// This shouldn't happen and is simply a sanity check to ensure the database
// descriptor regions and multi-region enum regions are indeed consistent.
return errors.AssertionFailedf(
"attempting to drop region %s not on database descriptor %d but found on type descriptor",
n.n.Region, n.desc.GetID(),
)
}
n.desc.RegionConfig.Regions = append(n.desc.RegionConfig.Regions[:idx],
n.desc.RegionConfig.Regions[idx+1:]...)
}

if err := params.p.writeNonDropDatabaseChange(
params.ctx,
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/catalog/dbdesc/database_desc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,3 +387,9 @@ func (desc *Mutable) SetOffline(reason string) {
func (desc *Mutable) AddDrainingName(name descpb.NameInfo) {
desc.DrainingNames = append(desc.DrainingNames, name)
}

// UnsetMultiRegionConfig removes the stored multi-region config from the
// database descriptor.
func (desc *Mutable) UnsetMultiRegionConfig() {
desc.RegionConfig = nil
}
82 changes: 79 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/multiregion
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ ALTER DATABASE non_multi_region_db DROP REGION "ca-central-1"
statement ok
CREATE DATABASE drop_region_db PRIMARY REGION "ca-central-1" REGIONS "ap-southeast-2", "us-east-1"

statement error pq: cannot drop region "ca-central-1"\nHINT: You must designate another region as the primary region or remove all other regions before attempting to drop region "ca-central-1"
statement error pq: cannot drop region "ca-central-1"\nHINT: You must designate another region as the primary region using ALTER DATABASE drop_region_db SET PRIMARY REGION <region name> or remove all other regions before attempting to drop region "ca-central-1"
ALTER DATABASE drop_region_db DROP REGION "ca-central-1"

statement ok
Expand Down Expand Up @@ -891,7 +891,7 @@ CREATE TABLE start_off_non_multi_region.public.t(a INT);
ALTER DATABASE start_off_non_multi_region PRIMARY REGION "ca-central-1";
ALTER DATABASE start_off_non_multi_region ADD REGION "ap-southeast-2"

statement error pq: cannot drop region "ca-central-1"\nHINT: You must designate another region as the primary region or remove all other regions before attempting to drop region "ca-central-1"
statement error pq: cannot drop region "ca-central-1"\nHINT: You must designate another region as the primary region using ALTER DATABASE start_off_non_multi_region SET PRIMARY REGION <region name> or remove all other regions before attempting to drop region "ca-central-1"
ALTER DATABASE start_off_non_multi_region DROP REGION "ca-central-1"

statement ok
Expand All @@ -914,7 +914,6 @@ txn_database_drop_regions ca-central-1 true {ca-az1,ca-az2,ca-az3}
txn_database_drop_regions ap-southeast-2 false {ap-az1,ap-az2,ap-az3}
txn_database_drop_regions us-east-1 false {us-az1,us-az2,us-az3}


statement ok
BEGIN;
ALTER DATABASE txn_database_drop_regions DROP REGION "us-east-1";
Expand Down Expand Up @@ -964,3 +963,80 @@ ALTER DATABASE drop_regions_alter_patterns DROP REGION "us-east-1"
statement ok
DROP TABLE east;
DROP TABLE southeast;

statement ok
CREATE DATABASE drop_primary_regions_db PRIMARY REGION "ca-central-1" REGIONS "ap-southeast-2", "us-east-1"

statement ok
CREATE TABLE drop_primary_regions_db.primary(k INT PRIMARY KEY) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION;
CREATE TABLE drop_primary_regions_db.east(k INT PRIMARY KEY) LOCALITY REGIONAL BY TABLE IN "us-east-1";
CREATE TABLE drop_primary_regions_db.ca(k INT PRIMARY KEY) LOCALITY REGIONAL BY TABLE IN "ca-central-1"

statement ok
ALTER DATABASE drop_primary_regions_db PRIMARY REGION "us-east-1"

statement error pq: could not remove enum value "ca-central-1" as it is the home region for table "ca"
ALTER DATABASE drop_primary_regions_db DROP REGION "ca-central-1"

statement ok
DROP TABLE drop_primary_regions_db.ca

# As the primary region has been changed to "us-east-1", this drop should succeed.
statement ok
ALTER DATABASE drop_primary_regions_db DROP REGION "ca-central-1"

# Cannot drop the primary region yet, as there are other regions in the db.
statement error pq: cannot drop region "us-east-1"\nHINT: You must designate another region as the primary region using ALTER DATABASE drop_primary_regions_db SET PRIMARY REGION <region name> or remove all other regions before attempting to drop region "us-east-1"
ALTER DATABASE drop_primary_regions_db DROP REGION "us-east-1"

statement ok
ALTER DATABASE drop_primary_regions_db DROP REGION "ap-southeast-2"

statement error pq: error removing primary region from database drop_primary_regions_db: cannot drop type "crdb_internal_region" because other objects \(\[drop_primary_regions_db.public.east\]\) still depend on it
ALTER DATABASE drop_primary_regions_db DROP REGION "us-east-1"

statement ok
DROP TABLE drop_primary_regions_db.east

# This should succeed, even though the table "primary" hasn't been dropped, as
# homing RTTs in primary regions does not block drops.
statement ok
ALTER DATABASE drop_primary_regions_db DROP REGION "us-east-1"

# Accessing the table should work without an issue even after the database is
# no longer multi-region.
statement ok
SELECT * FROM drop_primary_regions_db.primary

# Adding a region to the multi-region database should be possible as well,
# with no trace that this database was ever a multi-region db before.
statement ok
ALTER DATABASE drop_primary_regions_db PRIMARY REGION "ca-central-1"

query TT colnames
SHOW ZONE CONFIGURATION FOR TABLE drop_primary_regions_db.primary
----
target raw_config_sql
DATABASE drop_primary_regions_db ALTER DATABASE drop_primary_regions_db CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 90000,
num_replicas = 3,
num_voters = 3,
constraints = '{+region=ca-central-1: 1}',
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'



statement ok
ALTER TABLE drop_primary_regions_db.primary SET LOCALITY REGIONAL BY TABLE IN "ca-central-1"

statement error pq: error removing primary region from database drop_primary_regions_db: cannot drop type "crdb_internal_region" because other objects \(\[drop_primary_regions_db.public."primary"\]\) still depend on it
ALTER DATABASE drop_primary_regions_db DROP REGION "ca-central-1"

statement ok
ALTER TABLE drop_primary_regions_db.primary SET LOCALITY REGIONAL BY TABLE IN PRIMARY REGION

statement ok
ALTER DATABASE drop_primary_regions_db DROP REGION "ca-central-1"