Skip to content

Commit

Permalink
sql: add support for dropping the primary region from a multi-region db
Browse files Browse the repository at this point in the history 
This patch adds support for dropping the primary region from a
multi-region database. Dropping the primary region is allowed only if
all other regions have been removed from the database. Put another way,
the primary region is always the last region to go and the last region
to go is always the primary region.

As part of removing the last region from the database, all of the
static sql state is also reverted, leaving no detritus from
multi-region ways of old. In particular, this means that all GLOBAL
and REGIONAL BY TABLE tables lose the locality config from their
descriptor and the multi-region type descriptor is removed from the
database.

Dropping the last region can fail for a few of reasons:
- A REGIONAL BY TABLE table is homed explicitly in the region being
dropped.
- A REGIONAL BY ROW table exists in the database (which must write the
last region in all rows of its region column).
- A table in the database uses the multi-region enum.

All of these fail scenarios can be ascertained by checking if
references on the type descriptor exist, without having to go through
the general case of enum value removal that happens in the type schema
changer. This is what we do.

Lastly, it's worth noting this patch doesn't modify zone configurations.

Closes #57391
Informs #58333

Release note (sql change): `ALTER DATABASE ... DROP REGION` now allows
for dropping the primary region.
  • Loading branch information
@arulajmani
arulajmani committed Feb 11, 2021
1 parent 952af3c commit 14af57d
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 38 deletions.
168 changes: 133 additions & 35 deletions pkg/sql/alter_database.go
View file
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
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
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"

0 comments on commit 14af57d

Please sign in to comment.