Skip to content

Commit

Permalink
restore: precheck cluster is empty when first time full restore (#45014
Browse files Browse the repository at this point in the history
…) (#58774)

close #35744
  • Loading branch information
ti-chi-bot authored Feb 17, 2025
1 parent 5526691 commit 9752961
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 14 deletions.
6 changes: 3 additions & 3 deletions br/cmd/br/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ func printWorkaroundOnFullRestoreError(command *cobra.Command, err error) {
fmt.Println("#######################################################################")
switch {
case errors.ErrorEqual(err, berrors.ErrRestoreNotFreshCluster):
fmt.Println("# the target cluster is not fresh, br cannot restore system tables.")
fmt.Println("# the target cluster is not fresh, cannot restore.")
fmt.Println("# you can drop existing databases and tables and start restore again")
case errors.ErrorEqual(err, berrors.ErrRestoreIncompatibleSys):
fmt.Println("# the target cluster is not compatible with the backup data,")
fmt.Println("# br cannot restore system tables.")
fmt.Println("# you can remove 'with-sys-table' flag to skip restoring system tables")
}
fmt.Println("# you can remove 'with-sys-table' flag to skip restoring system tables")
fmt.Println("#######################################################################")
}

Expand Down
2 changes: 1 addition & 1 deletion br/pkg/backup/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (push *pushDown) pushBackup(
}
failpoint.Inject("backup-timeout-error", func(val failpoint.Value) {
msg := val.(string)
logutil.CL(ctx).Debug("failpoint backup-timeout-error injected.", zap.String("msg", msg))
logutil.CL(ctx).Info("failpoint backup-timeout-error injected.", zap.String("msg", msg))
resp.Error = &backuppb.Error{
Msg: msg,
}
Expand Down
5 changes: 5 additions & 0 deletions br/pkg/restore/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1756,6 +1756,11 @@ func (rc *Client) IsFull() bool {
return !rc.IsIncremental()
}

// NeedCheckFreshCluster is every time. except user has not set filter argument.
func (rc *Client) NeedCheckFreshCluster(ExplicitFilter bool) bool {
return rc.IsFull() && !ExplicitFilter
}

// IsIncremental returns whether this backup is incremental.
func (rc *Client) IsIncremental() bool {
return !(rc.backupMeta.StartVersion == rc.backupMeta.EndVersion ||
Expand Down
23 changes: 14 additions & 9 deletions br/pkg/task/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,17 +613,22 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf
return errors.Trace(err)
}

// todo: move this check into InitFullClusterRestore, we should move restore config into a separate package
// to avoid import cycle problem which we won't do it in this pr, then refactor this
//
// if it's point restore and reached here, then cmdName=FullRestoreCmd and len(cfg.FullBackupStorage) > 0
if cmdName == FullRestoreCmd && cfg.WithSysTable {
client.InitFullClusterRestore(cfg.ExplicitFilter)
if isFullRestore(cmdName) {
if client.NeedCheckFreshCluster(cfg.ExplicitFilter) {
if err = client.CheckTargetClusterFresh(ctx); err != nil {
return errors.Trace(err)
}
}
// todo: move this check into InitFullClusterRestore, we should move restore config into a separate package
// to avoid import cycle problem which we won't do it in this pr, then refactor this
//
// if it's point restore and reached here, then cmdName=FullRestoreCmd and len(cfg.FullBackupStorage) > 0
if cfg.WithSysTable {
client.InitFullClusterRestore(cfg.ExplicitFilter)
}
}

if client.IsFullClusterRestore() && client.HasBackedUpSysDB() {
if err = client.CheckTargetClusterFresh(ctx); err != nil {
return errors.Trace(err)
}
if err = client.CheckSysTableCompatibility(mgr.GetDomain(), tables); err != nil {
return errors.Trace(err)
}
Expand Down
3 changes: 3 additions & 0 deletions br/tests/br_backup_empty/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ if [ $? -ne 0 ]; then
exit 1
fi

i=1
while [ $i -le $DB_COUNT ]; do
run_sql "DROP DATABASE $DB$i;"
i=$(($i+1))
Expand All @@ -71,6 +72,7 @@ run_sql "CREATE TABLE ${DB}1.usertable1 ( \
echo "backup empty table start..."
run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/empty_table"

i=1
while [ $i -le $DB_COUNT ]; do
run_sql "DROP DATABASE $DB$i;"
i=$(($i+1))
Expand All @@ -83,6 +85,7 @@ run_br --pd $PD_ADDR restore full -s "local://$TEST_DIR/empty_table"
# insert one row to make sure table is restored.
run_sql "INSERT INTO ${DB}1.usertable1 VALUES (\"a\", \"b\");"

i=1
while [ $i -le $DB_COUNT ]; do
run_sql "DROP DATABASE $DB$i;"
i=$(($i+1))
Expand Down
3 changes: 3 additions & 0 deletions br/tests/br_file_corruption/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ if [ $restore_fail -ne 1 ]; then
exit 1
fi
run_sql "DROP DATABASE IF EXISTS $DB;"
run_sql "DROP DATABASE __tidb_br_temporary_mysql;"

# file corruption
for filename in $(find $TEST_DIR/$DB -name "*.sst_temp"); do
Expand All @@ -62,6 +63,7 @@ if [ $restore_fail -ne 1 ]; then
exit 1
fi
run_sql "DROP DATABASE IF EXISTS $DB;"
run_sql "DROP DATABASE __tidb_br_temporary_mysql;"

# verify validating checksum is still performed even backup didn't enable it
for filename in $(find $TEST_DIR/$DB -name "*.sst_bak"); do
Expand All @@ -77,6 +79,7 @@ if [ $restore_fail -ne 1 ]; then
exit 1
fi
run_sql "DROP DATABASE IF EXISTS $DB;"
run_sql "DROP DATABASE __tidb_br_temporary_mysql;"

# sanity check restore can succeed
run_br --pd $PD_ADDR restore full -s "local://$TEST_DIR/$DB" --checksum=true
Expand Down
1 change: 1 addition & 0 deletions br/tests/br_full_ddl/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ fi

# clear restore environment
run_sql "DROP DATABASE $DB;"
run_sql "DROP DATABASE __tidb_br_temporary_mysql;"
# restore full
echo "restore start..."
export GO_FAILPOINTS="github.com/pingcap/tidb/br/pkg/pdutil/PDEnabledPauseConfig=return(true)"
Expand Down
2 changes: 1 addition & 1 deletion br/tests/br_systables/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ add_test_data() {
}

delete_test_data() {
run_sql "DROP TABLE usertest.test;"
run_sql "DROP DATABASE usertest;"
}

rollback_modify() {
Expand Down

0 comments on commit 9752961

Please sign in to comment.