Skip to content

Commit

Permalink
Merge pull request #101893 from fqazi/backport22.2-101682
Browse files Browse the repository at this point in the history
release-22.2: restore: skip_localities_check no longer works
  • Loading branch information
fqazi authored Apr 21, 2023
2 parents 6dd7fd8 + 8abab82 commit fce5302
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 14 deletions.
1 change: 1 addition & 0 deletions pkg/ccl/backupccl/restore_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,7 @@ func createImportingDescriptors(
txn,
p.ExecCfg(),
descsCol,
!details.SkipLocalitiesCheck,
); err != nil {
return err
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1862,10 +1862,11 @@ func doRestorePlan(
// compatability.
//
// TODO(msbutler): Delete in 23.1
RestoreSystemUsers: restoreStmt.DescriptorCoverage == tree.SystemUsers,
PreRewriteTenantId: oldTenantID,
SchemaOnly: restoreStmt.Options.SchemaOnly,
VerifyData: restoreStmt.Options.VerifyData,
RestoreSystemUsers: restoreStmt.DescriptorCoverage == tree.SystemUsers,
PreRewriteTenantId: oldTenantID,
SchemaOnly: restoreStmt.Options.SchemaOnly,
VerifyData: restoreStmt.Options.VerifyData,
SkipLocalitiesCheck: restoreStmt.Options.SkipLocalitiesCheck,
}

jr := jobs.Record{
Expand Down
20 changes: 20 additions & 0 deletions pkg/ccl/backupccl/testdata/backup-restore/multiregion
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,26 @@ RESTORE FROM LATEST IN 'nodelocal://0/full_cluster_backup/';
pq: detected a mismatch in regions between the restore cluster and the backup cluster, missing regions detected: us-east-1, us-west-1.
HINT: there are two ways you can resolve this issue: 1) update the cluster to which you're restoring to ensure that the regions present on the nodes' --locality flags match those present in the backup image, or 2) restore with the "skip_localities_check" option

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/full_cluster_backup/' WITH skip_localities_check;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH skip_localities_check, new_db_name='d_new';
----

exec-sql
DROP DATABASE d_new;
----

exec-sql
DROP DATABASE d;
----

exec-sql
DROP DATABASE data;
----

# Create a database with no regions to check default primary regions.
exec-sql
CREATE DATABASE no_region_db;
Expand Down
21 changes: 16 additions & 5 deletions pkg/jobs/jobspb/jobs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,16 @@ message StreamIngestionDetails {
// StreamAddress locates the stream. It enables the client to find the
// addresses of the stream's partitions.
string stream_address = 1;

uint64 stream_id = 4 [(gogoproto.customname) = "StreamID"];

// Span is the keyspan into which this job will ingest KVs.
//
// The stream should emit all changes for a given span, and no changes outside
// a span. Note that KVs received from the stream may need to be re-keyed into
// this span.
roachpb.Span span = 2 [(gogoproto.nullable) = false];

reserved 5;
roachpb.TenantID tenant_id = 6 [(gogoproto.customname) = "TenantID", (gogoproto.nullable) = false];

Expand Down Expand Up @@ -267,7 +267,7 @@ message BackupProgress {
}

// DescriptorRewrite specifies a remapping from one descriptor ID to another for
// use in rewritting descriptors themselves or things that reference them such
// use in rewritting descriptors themselves or things that reference them such
// as is done during RESTORE or IMPORT.
message DescriptorRewrite {
uint32 id = 1 [
Expand Down Expand Up @@ -390,7 +390,18 @@ message RestoreDetails {

bool VerifyData = 26;

// NEXT ID: 28.

// ProtectedTimestampRecord is the ID of the protected timestamp record
// corresponding to this job.
bytes protected_timestamp_record = 28 [
(gogoproto.customname) = "ProtectedTimestampRecord",
(gogoproto.customtype) = "github.com/cockroachdb/cockroach/pkg/util/uuid.UUID"
];

// Disables loacality checking for zone configs.
bool SkipLocalitiesCheck = 29;

// NEXT ID: 30.
}


Expand Down
7 changes: 6 additions & 1 deletion pkg/sql/alter_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ func (n *alterDatabasePrimaryRegionNode) switchPrimaryRegion(params runParams) e
params.p.txn,
params.p.execCfg,
params.p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}
Expand Down Expand Up @@ -1165,6 +1166,7 @@ func (n *alterDatabaseSurvivalGoalNode) startExec(params runParams) error {
params.p.txn,
params.p.execCfg,
params.p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}
Expand Down Expand Up @@ -1295,6 +1297,7 @@ func (n *alterDatabasePlacementNode) startExec(params runParams) error {
params.p.txn,
params.p.execCfg,
params.p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}
Expand Down Expand Up @@ -1839,6 +1842,7 @@ func (n *alterDatabaseSecondaryRegion) startExec(params runParams) error {
params.p.txn,
params.p.execCfg,
params.p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}
Expand Down Expand Up @@ -1946,6 +1950,7 @@ func (n *alterDatabaseDropSecondaryRegion) startExec(params runParams) error {
params.p.txn,
params.p.execCfg,
params.p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}
Expand Down Expand Up @@ -2219,7 +2224,7 @@ func (n *alterDatabaseSetZoneConfigExtensionNode) startExec(params runParams) er

// Validate if the zone config extension is compatible with the database.
dbZoneConfig, err := generateAndValidateZoneConfigForMultiRegionDatabase(
params.ctx, params.ExecCfg(), updatedRegionConfig,
params.ctx, params.ExecCfg(), updatedRegionConfig, true, /* validateLocalities */
)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/database_region_change_finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ func (r *databaseRegionChangeFinalizer) updateDatabaseZoneConfig(
txn,
r.localPlanner.ExecCfg(),
r.localPlanner.Descriptors(),
true, /* validateLocalities */
)
}

Expand Down
22 changes: 18 additions & 4 deletions pkg/sql/region_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
"github.com/gogo/protobuf/proto"
)
Expand Down Expand Up @@ -877,7 +878,10 @@ func ApplyZoneConfigForMultiRegionTable(
// generateAndValidateZoneConfigForMultiRegionDatabase returns a validated
// zone config generated by the region config.
func generateAndValidateZoneConfigForMultiRegionDatabase(
ctx context.Context, execConfig *ExecutorConfig, regionConfig multiregion.RegionConfig,
ctx context.Context,
execConfig *ExecutorConfig,
regionConfig multiregion.RegionConfig,
validateLocalities bool,
) (zonepb.ZoneConfig, error) {
// Build a zone config based on the RegionConfig information.
dbZoneConfig := zonepb.ZoneConfig{}
Expand All @@ -898,7 +902,13 @@ func generateAndValidateZoneConfigForMultiRegionDatabase(
}

if err := validateZoneAttrsAndLocalities(ctx, execConfig, &dbZoneConfig); err != nil {
return zonepb.ZoneConfig{}, err
// If we are validating localities this is fatal, otherwise let's log any
// errors as warnings.
if validateLocalities {
return zonepb.ZoneConfig{}, err
}
log.Warningf(ctx, "ignoring locality validation error for DB zone config %v", err)
err = nil
}

return dbZoneConfig, nil
Expand All @@ -913,9 +923,10 @@ func ApplyZoneConfigFromDatabaseRegionConfig(
txn *kv.Txn,
execConfig *ExecutorConfig,
descriptors *descs.Collection,
validateLocalities bool,
) error {
// Build a zone config based on the RegionConfig information.
dbZoneConfig, err := generateAndValidateZoneConfigForMultiRegionDatabase(ctx, execConfig, regionConfig)
dbZoneConfig, err := generateAndValidateZoneConfigForMultiRegionDatabase(ctx, execConfig, regionConfig, validateLocalities)
if err != nil {
return err
}
Expand Down Expand Up @@ -1152,7 +1163,9 @@ func (p *planner) maybeInitializeMultiRegionDatabase(
*regionConfig,
p.txn,
p.execCfg,
p.Descriptors()); err != nil {
p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}

Expand Down Expand Up @@ -1293,6 +1306,7 @@ func (p *planner) ResetMultiRegionZoneConfigsForDatabase(ctx context.Context, id
p.txn,
p.execCfg,
p.Descriptors(),
true, /* validateLocalities */
); err != nil {
return err
}
Expand Down

0 comments on commit fce5302

Please sign in to comment.