From 64b4fa9d93721f63dc0c10dad768e1dac4421036 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 14:54:08 +0800 Subject: [PATCH 01/34] done --- br/pkg/errors/errors.go | 1 + br/pkg/task/restore.go | 18 ++++++++++++++++++ errors.toml | 5 +++++ 3 files changed, 24 insertions(+) diff --git a/br/pkg/errors/errors.go b/br/pkg/errors/errors.go index 1b78a861fa668..3bd2ab776ccb3 100644 --- a/br/pkg/errors/errors.go +++ b/br/pkg/errors/errors.go @@ -70,6 +70,7 @@ var ( ErrRestoreIncompatibleSys = errors.Normalize("incompatible system table", errors.RFCCodeText("BR:Restore:ErrRestoreIncompatibleSys")) ErrUnsupportedSystemTable = errors.Normalize("the system table isn't supported for restoring yet", errors.RFCCodeText("BR:Restore:ErrUnsupportedSysTable")) ErrDatabasesAlreadyExisted = errors.Normalize("databases already existed in restored cluster", errors.RFCCodeText("BR:Restore:ErrDatabasesAlreadyExisted")) + ErrTablesAlreadyExisted = errors.Normalize("tables already existed in restored cluster", errors.RFCCodeText("BR:Restore:ErrTablesAlreadyExisted")) // ErrStreamLogTaskExist is the error when stream log task already exists, because of supporting single task currently. ErrStreamLogTaskExist = errors.Normalize("stream task already exists", errors.RFCCodeText("BR:Stream:ErrStreamLogTaskExist")) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 8bc6383be78b6..6dd619b971a6b 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -842,6 +842,9 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err := checkDiskSpace(ctx, mgr, files, tables); err != nil { return errors.Trace(err) } + if err := checkTableExistence(ctx, mgr, tables); err != nil { + return errors.Trace(err) + } } archiveSize := reader.ArchiveSize(ctx, files) @@ -1358,6 +1361,21 @@ func Exhaust(ec <-chan error) []error { } } +func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table) error { + message := "table already exists: " + allUnique := true + for _, table := range tables { + if _, err := mgr.GetDomain().InfoSchema().TableByName(ctx, table.DB.Name, table.Info.Name); err == nil { + message += fmt.Sprintf("%s.%s ", table.DB.Name, table.Info.Name) + allUnique = false + } + } + if !allUnique { + return errors.Annotate(berrors.ErrTablesAlreadyExisted, message) + } + return nil +} + // EstimateRangeSize estimates the total range count by file. func EstimateRangeSize(files []*backuppb.File) int { result := 0 diff --git a/errors.toml b/errors.toml index bde9ae2e18350..9c02f10d6ac04 100644 --- a/errors.toml +++ b/errors.toml @@ -291,6 +291,11 @@ error = ''' failed to write and ingest ''' +["BR:Restore:ErrTablesAlreadyExisted"] +error = ''' +tables already existed in restored cluster +''' + ["BR:Restore:ErrUnsupportedSysTable"] error = ''' the system table isn't supported for restoring yet From 03e7107116e000bd4a6a173c30e92db595c862ca Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 16:35:49 +0800 Subject: [PATCH 02/34] add integration test --- br/pkg/task/restore.go | 4 +++ br/tests/br_duplicate_tables/run.sh | 53 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 br/tests/br_duplicate_tables/run.sh diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 6dd619b971a6b..fad96d5dfb620 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1318,6 +1318,10 @@ func checkDiskSpace(ctx context.Context, mgr *conn.Mgr, files []*backuppb.File, } return base * uint64(ratio*10) / 10 } + + // The preserve rate is a magic number based on experience + // The actual usage should less than the preserve space, but it's better to be conservative. + // Restore to Tiflash involves a data form transformation, the estimation is not accurate. tikvUsage := preserve(EstimateTikvUsage(files, maxReplica, tikvCnt), 1.1) tiflashUsage := preserve(EstimateTiflashUsage(tables, tiflashCnt), 1.4) log.Info("preserved disk space", zap.Uint64("tikv", tikvUsage), zap.Uint64("tiflash", tiflashUsage)) diff --git a/br/tests/br_duplicate_tables/run.sh b/br/tests/br_duplicate_tables/run.sh new file mode 100644 index 0000000000000..927c168d7af18 --- /dev/null +++ b/br/tests/br_duplicate_tables/run.sh @@ -0,0 +1,53 @@ +#!/bin/sh +# +# Copyright 2019 PingCAP, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eu +DB="$TEST_NAME" +TABLE="duplicate" +DB_COUNT=3 +CUR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) + +for i in $(seq $DB_COUNT); do + run_sql "CREATE DATABASE $DB${i};" + run_sql "CREATE TABLE $DB${i}.$TABLE (id INT PRIMARY KEY, name VARCHAR(64));" + run_sql "INSERT INTO $DB${i}.$TABLE VALUES (1, 'hello'), (2, 'world');" +done + +# backup full +echo "backup start..." +run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/$DB" --concurrency 4 + + +# restore full +set +e +output=$(run_sql "restore database * from 'local://$TEST_DIR/$DB'" 2>&1) +exit_status=$? +set -e + +if [ "$exit_status" -eq 0 ]; then + echo "TEST: [$TEST_NAME] failed!" + exit 1 +fi + +if ! echo "$output" | grep -q "tables already existed"; then + echo "TEST: [$TEST_NAME] failed!" + exit 1 +fi + + +for i in $(seq $DB_COUNT); do + run_sql "DROP DATABASE $DB${i};" +done From 0c4445f948230e7f9b7f265b94e91dfd30bbaab4 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:09:21 +0800 Subject: [PATCH 03/34] Revert "add integration test" This reverts commit eabe9672ab8f8eec01874201f3b77cedbbb55701. --- br/pkg/task/restore.go | 4 --- br/tests/br_duplicate_tables/run.sh | 53 ----------------------------- 2 files changed, 57 deletions(-) delete mode 100644 br/tests/br_duplicate_tables/run.sh diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index fad96d5dfb620..6dd619b971a6b 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1318,10 +1318,6 @@ func checkDiskSpace(ctx context.Context, mgr *conn.Mgr, files []*backuppb.File, } return base * uint64(ratio*10) / 10 } - - // The preserve rate is a magic number based on experience - // The actual usage should less than the preserve space, but it's better to be conservative. - // Restore to Tiflash involves a data form transformation, the estimation is not accurate. tikvUsage := preserve(EstimateTikvUsage(files, maxReplica, tikvCnt), 1.1) tiflashUsage := preserve(EstimateTiflashUsage(tables, tiflashCnt), 1.4) log.Info("preserved disk space", zap.Uint64("tikv", tikvUsage), zap.Uint64("tiflash", tiflashUsage)) diff --git a/br/tests/br_duplicate_tables/run.sh b/br/tests/br_duplicate_tables/run.sh deleted file mode 100644 index 927c168d7af18..0000000000000 --- a/br/tests/br_duplicate_tables/run.sh +++ /dev/null @@ -1,53 +0,0 @@ -#!/bin/sh -# -# Copyright 2019 PingCAP, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -set -eu -DB="$TEST_NAME" -TABLE="duplicate" -DB_COUNT=3 -CUR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) - -for i in $(seq $DB_COUNT); do - run_sql "CREATE DATABASE $DB${i};" - run_sql "CREATE TABLE $DB${i}.$TABLE (id INT PRIMARY KEY, name VARCHAR(64));" - run_sql "INSERT INTO $DB${i}.$TABLE VALUES (1, 'hello'), (2, 'world');" -done - -# backup full -echo "backup start..." -run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/$DB" --concurrency 4 - - -# restore full -set +e -output=$(run_sql "restore database * from 'local://$TEST_DIR/$DB'" 2>&1) -exit_status=$? -set -e - -if [ "$exit_status" -eq 0 ]; then - echo "TEST: [$TEST_NAME] failed!" - exit 1 -fi - -if ! echo "$output" | grep -q "tables already existed"; then - echo "TEST: [$TEST_NAME] failed!" - exit 1 -fi - - -for i in $(seq $DB_COUNT); do - run_sql "DROP DATABASE $DB${i};" -done From 27e643b9d862155fab5c9611c17086265bb14332 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:09:43 +0800 Subject: [PATCH 04/34] add comments --- br/pkg/task/restore.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 6dd619b971a6b..a64cd73c8044c 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1318,6 +1318,10 @@ func checkDiskSpace(ctx context.Context, mgr *conn.Mgr, files []*backuppb.File, } return base * uint64(ratio*10) / 10 } + + // The preserve rate is a magic number based on experience + // The actual usage should less than the preserve space, but it's better to be conservative. + // Restore to Tiflash involves a data form transformation, the estimation is not accurate. tikvUsage := preserve(EstimateTikvUsage(files, maxReplica, tikvCnt), 1.1) tiflashUsage := preserve(EstimateTiflashUsage(tables, tiflashCnt), 1.4) log.Info("preserved disk space", zap.Uint64("tikv", tikvUsage), zap.Uint64("tiflash", tiflashUsage)) From 373ac6bd58de57ebf301d543ef6c84cdd27093b4 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 22:43:42 +0800 Subject: [PATCH 05/34] done --- tests/realtikvtest/brietest/brie_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index c72b164b66058..421743f4f88a0 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -15,6 +15,7 @@ package brietest import ( + "context" "fmt" "os" "strings" @@ -24,6 +25,7 @@ import ( "github.com/pingcap/failpoint" "github.com/pingcap/log" "github.com/pingcap/tidb/pkg/executor" + "github.com/pingcap/tidb/pkg/session" "github.com/pingcap/tidb/pkg/testkit" "github.com/stretchr/testify/require" "go.uber.org/zap/zapcore" @@ -126,3 +128,21 @@ func TestCancel(t *testing.T) { req.FailNow("the backup job doesn't be canceled") } } + +func TestExistedTables(t *testing.T) { + tk := initTestKit(t) + tmp := makeTempDirForBackup(t) + sqlTmp := strings.ReplaceAll(tmp, "'", "''") + executor.ResetGlobalBRIEQueueForTest() + tk.MustExec("use test;") + tk.MustExec("create table foo(pk int primary key auto_increment, v varchar(255));") + tk.MustExec("insert into foo(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") + backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) + tk.MustQuery(backupQuery) + restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) + res,err := tk.Exec(restoreQuery) + require.NoError(t, err) + _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) + require.ErrorContains(t,err,"table already exists") + tk.MustExec("drop table `foo`;") +} From 433e4cd91d2f14ea44cb5b7cc5183d4a59da5a24 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 22:44:58 +0800 Subject: [PATCH 06/34] lint --- br/pkg/task/restore.go | 2 +- tests/realtikvtest/brietest/brie_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index a64cd73c8044c..fad96d5dfb620 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1318,7 +1318,7 @@ func checkDiskSpace(ctx context.Context, mgr *conn.Mgr, files []*backuppb.File, } return base * uint64(ratio*10) / 10 } - + // The preserve rate is a magic number based on experience // The actual usage should less than the preserve space, but it's better to be conservative. // Restore to Tiflash involves a data form transformation, the estimation is not accurate. diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index 421743f4f88a0..9a459d9ebdd90 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -140,9 +140,9 @@ func TestExistedTables(t *testing.T) { backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) tk.MustQuery(backupQuery) restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) - res,err := tk.Exec(restoreQuery) + res, err := tk.Exec(restoreQuery) require.NoError(t, err) _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) - require.ErrorContains(t,err,"table already exists") + require.ErrorContains(t, err, "table already exists") tk.MustExec("drop table `foo`;") } From c6010655c4ef885b2756a0e3c165a4d9f306cc1e Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Tue, 30 Jul 2024 22:51:16 +0800 Subject: [PATCH 07/34] bazel --- tests/realtikvtest/brietest/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/realtikvtest/brietest/BUILD.bazel b/tests/realtikvtest/brietest/BUILD.bazel index f1a20bff34dff..70f784fb3d07e 100644 --- a/tests/realtikvtest/brietest/BUILD.bazel +++ b/tests/realtikvtest/brietest/BUILD.bazel @@ -18,6 +18,7 @@ go_test( "//pkg/config", "//pkg/executor", "//pkg/parser/mysql", + "//pkg/session", "//pkg/sessionctx/binloginfo", "//pkg/store/mockstore/mockcopr", "//pkg/testkit", From 01b22592ad28f7ea675fe7cd575de7aabebe1043 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:07:36 +0800 Subject: [PATCH 08/34] modify glue --- br/pkg/glue/glue.go | 3 +++ br/pkg/gluetidb/glue.go | 4 ++++ br/pkg/gluetidb/mock/mock.go | 4 ++++ br/pkg/gluetikv/glue.go | 4 ++++ br/pkg/task/restore.go | 4 ++-- pkg/executor/brie.go | 4 ++++ 6 files changed, 21 insertions(+), 2 deletions(-) diff --git a/br/pkg/glue/glue.go b/br/pkg/glue/glue.go index 5d5f611fa39e6..a72b8d0704a3d 100644 --- a/br/pkg/glue/glue.go +++ b/br/pkg/glue/glue.go @@ -36,6 +36,9 @@ type Glue interface { // we can close domain as soon as possible. // and we must reuse the exists session and don't close it in SQL backup job. UseOneShotSession(store kv.Storage, closeDomain bool, fn func(se Session) error) error + + // GetType returns the glue type + GetType() string } // Session is an abstraction of the session.Session interface. diff --git a/br/pkg/gluetidb/glue.go b/br/pkg/gluetidb/glue.go index d7d5c907fddff..30501e27cc8a4 100644 --- a/br/pkg/gluetidb/glue.go +++ b/br/pkg/gluetidb/glue.go @@ -183,6 +183,10 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue return nil } +func (g Glue) GetType() string { + return "TidbGlue" +} + // GetSessionCtx implements glue.Glue func (gs *tidbSession) GetSessionCtx() sessionctx.Context { return gs.se diff --git a/br/pkg/gluetidb/mock/mock.go b/br/pkg/gluetidb/mock/mock.go index 42cbd4a814657..e9c4715279c9d 100644 --- a/br/pkg/gluetidb/mock/mock.go +++ b/br/pkg/gluetidb/mock/mock.go @@ -159,3 +159,7 @@ func (m *MockGlue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func } return fn(glueSession) } + +func (m *MockGlue) GetType() string { + return "MockGlue" +} diff --git a/br/pkg/gluetikv/glue.go b/br/pkg/gluetikv/glue.go index ad3ae045f478f..aa6d0088635b7 100644 --- a/br/pkg/gluetikv/glue.go +++ b/br/pkg/gluetikv/glue.go @@ -73,3 +73,7 @@ func (Glue) GetVersion() string { func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue.Session) error) error { return nil } + +func (g Glue) GetType() string { + return "TikvGlue" +} diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index fad96d5dfb620..cfb8c33926b67 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -842,7 +842,7 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err := checkDiskSpace(ctx, mgr, files, tables); err != nil { return errors.Trace(err) } - if err := checkTableExistence(ctx, mgr, tables); err != nil { + if err := checkTableExistence(ctx, mgr, backupMeta); err != nil { return errors.Trace(err) } } @@ -1365,7 +1365,7 @@ func Exhaust(ec <-chan error) []error { } } -func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table) error { +func checkTableExistence(ctx context.Context, mgr *conn.Mgr, backupmeta *backuppb.BackupMeta) error { message := "table already exists: " allUnique := true for _, table := range tables { diff --git a/pkg/executor/brie.go b/pkg/executor/brie.go index 130f627bc6609..79951a490c081 100644 --- a/pkg/executor/brie.go +++ b/pkg/executor/brie.go @@ -753,6 +753,10 @@ func (gs *tidbGlue) UseOneShotSession(_ kv.Storage, _ bool, fn func(se glue.Sess return fn(glueSession) } +func (gs *tidbGlue) GetType() string { + return "ExecGlue" +} + type tidbGlueSession struct { // the session context of the brie task's subtask, such as `CREATE TABLE`. se sessionctx.Context From 8eb193f6c5ed41ee7467ad4a8830c5e4d14b4c45 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:14:08 +0800 Subject: [PATCH 09/34] revert param --- br/pkg/task/restore.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index cfb8c33926b67..fad96d5dfb620 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -842,7 +842,7 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err := checkDiskSpace(ctx, mgr, files, tables); err != nil { return errors.Trace(err) } - if err := checkTableExistence(ctx, mgr, backupMeta); err != nil { + if err := checkTableExistence(ctx, mgr, tables); err != nil { return errors.Trace(err) } } @@ -1365,7 +1365,7 @@ func Exhaust(ec <-chan error) []error { } } -func checkTableExistence(ctx context.Context, mgr *conn.Mgr, backupmeta *backuppb.BackupMeta) error { +func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table) error { message := "table already exists: " allUnique := true for _, table := range tables { From 253c7d66b29ce9123b88ecb0477fc115593747d7 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:21:07 +0800 Subject: [PATCH 10/34] sepecify glue --- br/pkg/task/restore.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index fad96d5dfb620..93cc189badf15 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -842,7 +842,7 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err := checkDiskSpace(ctx, mgr, files, tables); err != nil { return errors.Trace(err) } - if err := checkTableExistence(ctx, mgr, tables); err != nil { + if err := checkTableExistence(ctx, mgr, tables, g); err != nil { return errors.Trace(err) } } @@ -1365,7 +1365,10 @@ func Exhaust(ec <-chan error) []error { } } -func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table) error { +func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table, glue glue.Glue) error { + if glue.GetType() != "ExecGlue" { + return nil + } message := "table already exists: " allUnique := true for _, table := range tables { From a150f08b2aaf555ec957b16aa019e82e1a79122f Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Wed, 31 Jul 2024 14:46:01 +0800 Subject: [PATCH 11/34] update --- br/pkg/glue/glue.go | 9 +++++++-- br/pkg/gluetidb/glue.go | 4 ++-- br/pkg/gluetidb/mock/mock.go | 4 ++-- br/pkg/gluetikv/glue.go | 4 ++-- br/pkg/task/restore.go | 11 ++++++++--- pkg/executor/brie.go | 4 ++-- 6 files changed, 23 insertions(+), 13 deletions(-) diff --git a/br/pkg/glue/glue.go b/br/pkg/glue/glue.go index a72b8d0704a3d..7f1ec6ad50476 100644 --- a/br/pkg/glue/glue.go +++ b/br/pkg/glue/glue.go @@ -13,6 +13,11 @@ import ( pd "github.com/tikv/pd/client" ) +const ( + BrClient int = iota + SqlClient +) + // Glue is an abstraction of TiDB function calls used in BR. type Glue interface { GetDomain(store kv.Storage) (*domain.Domain, error) @@ -37,8 +42,8 @@ type Glue interface { // and we must reuse the exists session and don't close it in SQL backup job. UseOneShotSession(store kv.Storage, closeDomain bool, fn func(se Session) error) error - // GetType returns the glue type - GetType() string + // GetClient returns the client type of the glue + GetClient() int } // Session is an abstraction of the session.Session interface. diff --git a/br/pkg/gluetidb/glue.go b/br/pkg/gluetidb/glue.go index 30501e27cc8a4..435c202f8ca43 100644 --- a/br/pkg/gluetidb/glue.go +++ b/br/pkg/gluetidb/glue.go @@ -183,8 +183,8 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue return nil } -func (g Glue) GetType() string { - return "TidbGlue" +func (_ Glue) GetClient() int { + return glue.BrClient } // GetSessionCtx implements glue.Glue diff --git a/br/pkg/gluetidb/mock/mock.go b/br/pkg/gluetidb/mock/mock.go index e9c4715279c9d..cd2b956ca98b8 100644 --- a/br/pkg/gluetidb/mock/mock.go +++ b/br/pkg/gluetidb/mock/mock.go @@ -160,6 +160,6 @@ func (m *MockGlue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func return fn(glueSession) } -func (m *MockGlue) GetType() string { - return "MockGlue" +func (_ *MockGlue) GetClient() int { + return glue.BrClient } diff --git a/br/pkg/gluetikv/glue.go b/br/pkg/gluetikv/glue.go index aa6d0088635b7..c0a21bbddb8ca 100644 --- a/br/pkg/gluetikv/glue.go +++ b/br/pkg/gluetikv/glue.go @@ -74,6 +74,6 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue return nil } -func (g Glue) GetType() string { - return "TikvGlue" +func (_ Glue) GetClient() int { + return glue.BrClient } diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 93cc189badf15..18c2bd20b7229 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -34,6 +34,7 @@ import ( "github.com/pingcap/tidb/br/pkg/version" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/domain" + "github.com/pingcap/tidb/pkg/infoschema" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/parser/model" "github.com/pingcap/tidb/pkg/tablecodec" @@ -1365,16 +1366,20 @@ func Exhaust(ec <-chan error) []error { } } -func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table, glue glue.Glue) error { - if glue.GetType() != "ExecGlue" { +func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table, g glue.Glue) error { + // Tasks from br client use other checks to validate + if g.GetClient() != glue.SqlClient { return nil } message := "table already exists: " allUnique := true for _, table := range tables { - if _, err := mgr.GetDomain().InfoSchema().TableByName(ctx, table.DB.Name, table.Info.Name); err == nil { + _, err := mgr.GetDomain().InfoSchema().TableByName(ctx, table.DB.Name, table.Info.Name) + if err == nil { message += fmt.Sprintf("%s.%s ", table.DB.Name, table.Info.Name) allUnique = false + } else if errors.Cause(err) != infoschema.ErrTableNotExists { + return errors.Trace(err) } } if !allUnique { diff --git a/pkg/executor/brie.go b/pkg/executor/brie.go index 79951a490c081..202ded5ba5aab 100644 --- a/pkg/executor/brie.go +++ b/pkg/executor/brie.go @@ -753,8 +753,8 @@ func (gs *tidbGlue) UseOneShotSession(_ kv.Storage, _ bool, fn func(se glue.Sess return fn(glueSession) } -func (gs *tidbGlue) GetType() string { - return "ExecGlue" +func (_ *tidbGlue) GetClient() int { + return glue.SqlClient } type tidbGlueSession struct { From 43946c37a91b1738d950d8f8db3f1cf1058298ab Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Wed, 31 Jul 2024 15:04:18 +0800 Subject: [PATCH 12/34] code style --- br/pkg/glue/glue.go | 8 +++++--- br/pkg/gluetidb/glue.go | 4 ++-- br/pkg/gluetidb/mock/mock.go | 4 ++-- br/pkg/gluetikv/glue.go | 4 ++-- br/pkg/task/restore.go | 2 +- pkg/executor/brie.go | 4 ++-- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/br/pkg/glue/glue.go b/br/pkg/glue/glue.go index 7f1ec6ad50476..cb7e1cd9a2528 100644 --- a/br/pkg/glue/glue.go +++ b/br/pkg/glue/glue.go @@ -13,9 +13,11 @@ import ( pd "github.com/tikv/pd/client" ) +type GlueClient int + const ( - BrClient int = iota - SqlClient + ClientBr GlueClient = iota + ClientSql ) // Glue is an abstraction of TiDB function calls used in BR. @@ -43,7 +45,7 @@ type Glue interface { UseOneShotSession(store kv.Storage, closeDomain bool, fn func(se Session) error) error // GetClient returns the client type of the glue - GetClient() int + GetClient() GlueClient } // Session is an abstraction of the session.Session interface. diff --git a/br/pkg/gluetidb/glue.go b/br/pkg/gluetidb/glue.go index 435c202f8ca43..7c2397889ad85 100644 --- a/br/pkg/gluetidb/glue.go +++ b/br/pkg/gluetidb/glue.go @@ -183,8 +183,8 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue return nil } -func (_ Glue) GetClient() int { - return glue.BrClient +func (Glue) GetClient() glue.GlueClient { + return glue.ClientBr } // GetSessionCtx implements glue.Glue diff --git a/br/pkg/gluetidb/mock/mock.go b/br/pkg/gluetidb/mock/mock.go index cd2b956ca98b8..b9f23d3069fa1 100644 --- a/br/pkg/gluetidb/mock/mock.go +++ b/br/pkg/gluetidb/mock/mock.go @@ -160,6 +160,6 @@ func (m *MockGlue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func return fn(glueSession) } -func (_ *MockGlue) GetClient() int { - return glue.BrClient +func (*MockGlue) GetClient() glue.GlueClient { + return glue.ClientBr } diff --git a/br/pkg/gluetikv/glue.go b/br/pkg/gluetikv/glue.go index c0a21bbddb8ca..5fd9c2cfb308f 100644 --- a/br/pkg/gluetikv/glue.go +++ b/br/pkg/gluetikv/glue.go @@ -74,6 +74,6 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue return nil } -func (_ Glue) GetClient() int { - return glue.BrClient +func (Glue) GetClient() glue.GlueClient { + return glue.ClientBr } diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 18c2bd20b7229..60d23d28154fa 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1368,7 +1368,7 @@ func Exhaust(ec <-chan error) []error { func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table, g glue.Glue) error { // Tasks from br client use other checks to validate - if g.GetClient() != glue.SqlClient { + if g.GetClient() != glue.ClientSql { return nil } message := "table already exists: " diff --git a/pkg/executor/brie.go b/pkg/executor/brie.go index 202ded5ba5aab..dd019e30b67b3 100644 --- a/pkg/executor/brie.go +++ b/pkg/executor/brie.go @@ -753,8 +753,8 @@ func (gs *tidbGlue) UseOneShotSession(_ kv.Storage, _ bool, fn func(se glue.Sess return fn(glueSession) } -func (_ *tidbGlue) GetClient() int { - return glue.SqlClient +func (*tidbGlue) GetClient() glue.GlueClient { + return glue.ClientSql } type tidbGlueSession struct { From b53d590d9b2a1ef4e33d52644b16f0fbd1e0e079 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Thu, 1 Aug 2024 11:07:21 +0800 Subject: [PATCH 13/34] hide userFSR --- br/pkg/task/restore.go | 1 + 1 file changed, 1 insertion(+) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 60d23d28154fa..8379933a6d495 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -180,6 +180,7 @@ func DefineRestoreCommonFlags(flags *pflag.FlagSet) { _ = flags.MarkHidden(FlagStatsConcurrency) _ = flags.MarkHidden(FlagBatchFlushInterval) _ = flags.MarkHidden(FlagDdlBatchSize) + _ = flags.MarkHidden(flagUseFSR) } // ParseFromFlags parses the config from the flag set. From 7b2793eee832dd6b2455533c53ff6e30d3693c0d Mon Sep 17 00:00:00 2001 From: ris <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:10:03 +0800 Subject: [PATCH 14/34] Update br/pkg/glue/glue.go Co-authored-by: BornChanger <97348524+BornChanger@users.noreply.github.com> --- br/pkg/glue/glue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/br/pkg/glue/glue.go b/br/pkg/glue/glue.go index cb7e1cd9a2528..1895ee092cc78 100644 --- a/br/pkg/glue/glue.go +++ b/br/pkg/glue/glue.go @@ -16,7 +16,7 @@ import ( type GlueClient int const ( - ClientBr GlueClient = iota + ClientCLP GlueClient = iota ClientSql ) From 0281eafe882d2d65c7f2016bcdee56e6eec994c4 Mon Sep 17 00:00:00 2001 From: ris <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:10:13 +0800 Subject: [PATCH 15/34] Update br/pkg/task/restore.go Co-authored-by: BornChanger <97348524+BornChanger@users.noreply.github.com> --- br/pkg/task/restore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 8379933a6d495..43387beed644e 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1368,7 +1368,7 @@ func Exhaust(ec <-chan error) []error { } func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil.Table, g glue.Glue) error { - // Tasks from br client use other checks to validate + // Tasks from br clp client use other checks to validate if g.GetClient() != glue.ClientSql { return nil } From 632641b9c27e9e2088907644cfb2d52a1f831756 Mon Sep 17 00:00:00 2001 From: ris <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:10:26 +0800 Subject: [PATCH 16/34] Update errors.toml Co-authored-by: BornChanger <97348524+BornChanger@users.noreply.github.com> --- errors.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors.toml b/errors.toml index 9c02f10d6ac04..6951cb5ffe940 100644 --- a/errors.toml +++ b/errors.toml @@ -293,7 +293,7 @@ failed to write and ingest ["BR:Restore:ErrTablesAlreadyExisted"] error = ''' -tables already existed in restored cluster +tables already existed in target cluster ''' ["BR:Restore:ErrUnsupportedSysTable"] From 774a977458472361766b34bdbceaf6dbc48eeb8d Mon Sep 17 00:00:00 2001 From: ris <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:11:07 +0800 Subject: [PATCH 17/34] Update br/pkg/task/restore.go Co-authored-by: BornChanger <97348524+BornChanger@users.noreply.github.com> --- br/pkg/task/restore.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 43387beed644e..7f99200e4c7f2 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1321,9 +1321,9 @@ func checkDiskSpace(ctx context.Context, mgr *conn.Mgr, files []*backuppb.File, return base * uint64(ratio*10) / 10 } - // The preserve rate is a magic number based on experience - // The actual usage should less than the preserve space, but it's better to be conservative. - // Restore to Tiflash involves a data form transformation, the estimation is not accurate. + // The preserve rate for tikv is quite accurate, while rate for tiflash is a + // number calculated from tpcc testing with variable data sizes. 1.4 is a + // relative conservative value. tikvUsage := preserve(EstimateTikvUsage(files, maxReplica, tikvCnt), 1.1) tiflashUsage := preserve(EstimateTiflashUsage(tables, tiflashCnt), 1.4) log.Info("preserved disk space", zap.Uint64("tikv", tikvUsage), zap.Uint64("tiflash", tiflashUsage)) From 23179f3774744ac66772db5f8f9a5471b23ece5b Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 11:16:20 +0800 Subject: [PATCH 18/34] lint --- br/pkg/task/restore.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 7f99200e4c7f2..80c69c1f29d24 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1321,8 +1321,8 @@ func checkDiskSpace(ctx context.Context, mgr *conn.Mgr, files []*backuppb.File, return base * uint64(ratio*10) / 10 } - // The preserve rate for tikv is quite accurate, while rate for tiflash is a - // number calculated from tpcc testing with variable data sizes. 1.4 is a + // The preserve rate for tikv is quite accurate, while rate for tiflash is a + // number calculated from tpcc testing with variable data sizes. 1.4 is a // relative conservative value. tikvUsage := preserve(EstimateTikvUsage(files, maxReplica, tikvCnt), 1.1) tiflashUsage := preserve(EstimateTiflashUsage(tables, tiflashCnt), 1.4) From 59f1d03a6c2f79daee8b51323bcc83b41807e851 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 11:23:10 +0800 Subject: [PATCH 19/34] fix --- br/pkg/gluetidb/glue.go | 2 +- br/pkg/gluetidb/mock/mock.go | 2 +- br/pkg/gluetikv/glue.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/br/pkg/gluetidb/glue.go b/br/pkg/gluetidb/glue.go index 7c2397889ad85..7963c14b0f5bb 100644 --- a/br/pkg/gluetidb/glue.go +++ b/br/pkg/gluetidb/glue.go @@ -184,7 +184,7 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue } func (Glue) GetClient() glue.GlueClient { - return glue.ClientBr + return glue.ClientCLP } // GetSessionCtx implements glue.Glue diff --git a/br/pkg/gluetidb/mock/mock.go b/br/pkg/gluetidb/mock/mock.go index b9f23d3069fa1..dc54f48ebda3a 100644 --- a/br/pkg/gluetidb/mock/mock.go +++ b/br/pkg/gluetidb/mock/mock.go @@ -161,5 +161,5 @@ func (m *MockGlue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func } func (*MockGlue) GetClient() glue.GlueClient { - return glue.ClientBr + return glue.ClientCLP } diff --git a/br/pkg/gluetikv/glue.go b/br/pkg/gluetikv/glue.go index 5fd9c2cfb308f..2fd990e92d2a5 100644 --- a/br/pkg/gluetikv/glue.go +++ b/br/pkg/gluetikv/glue.go @@ -75,5 +75,5 @@ func (g Glue) UseOneShotSession(store kv.Storage, closeDomain bool, fn func(glue } func (Glue) GetClient() glue.GlueClient { - return glue.ClientBr + return glue.ClientCLP } From df3e089d459f9ea2d089ce7fc1a06561f56e4a4d Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 11:45:43 +0800 Subject: [PATCH 20/34] error code --- errors.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors.toml b/errors.toml index 6951cb5ffe940..9c02f10d6ac04 100644 --- a/errors.toml +++ b/errors.toml @@ -293,7 +293,7 @@ failed to write and ingest ["BR:Restore:ErrTablesAlreadyExisted"] error = ''' -tables already existed in target cluster +tables already existed in restored cluster ''' ["BR:Restore:ErrUnsupportedSysTable"] From 21c71e069dae5f2e78a99e3a1de245d8d0f09ab2 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 14:32:45 +0800 Subject: [PATCH 21/34] err.Equal --- br/pkg/task/restore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 80c69c1f29d24..bd9b404204daa 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1379,7 +1379,7 @@ func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil. if err == nil { message += fmt.Sprintf("%s.%s ", table.DB.Name, table.Info.Name) allUnique = false - } else if errors.Cause(err) != infoschema.ErrTableNotExists { + } else if infoschema.ErrTableNotExists.Equal(errors.Cause(err)) { return errors.Trace(err) } } From 5b04775f230926d603c1af5cd3c1626b823160e9 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 14:34:27 +0800 Subject: [PATCH 22/34] fix more --- br/pkg/task/restore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index bd9b404204daa..f71b96d4be79f 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1379,7 +1379,7 @@ func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil. if err == nil { message += fmt.Sprintf("%s.%s ", table.DB.Name, table.Info.Name) allUnique = false - } else if infoschema.ErrTableNotExists.Equal(errors.Cause(err)) { + } else if infoschema.ErrTableNotExists.Equal(err) { return errors.Trace(err) } } From 0e5e783a85c9a15bc60fbec5dbeea1eac0a49fee Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 15:08:17 +0800 Subject: [PATCH 23/34] add more tables --- tests/realtikvtest/brietest/brie_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index 9a459d9ebdd90..ae05c58956302 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -136,7 +136,9 @@ func TestExistedTables(t *testing.T) { executor.ResetGlobalBRIEQueueForTest() tk.MustExec("use test;") tk.MustExec("create table foo(pk int primary key auto_increment, v varchar(255));") + tk.MustExec("create table baa(pk int primary key auto_increment, v varchar(255));") tk.MustExec("insert into foo(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") + tk.MustExec("insert into baa(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) tk.MustQuery(backupQuery) restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) @@ -144,5 +146,4 @@ func TestExistedTables(t *testing.T) { require.NoError(t, err) _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) require.ErrorContains(t, err, "table already exists") - tk.MustExec("drop table `foo`;") } From f5f84e94767f4f3d2f2985f69e5cc85fda3fb36d Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 2 Aug 2024 16:40:43 +0800 Subject: [PATCH 24/34] fix missed unequal --- br/pkg/task/restore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index f71b96d4be79f..a014a5e1fa540 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -1379,7 +1379,7 @@ func checkTableExistence(ctx context.Context, mgr *conn.Mgr, tables []*metautil. if err == nil { message += fmt.Sprintf("%s.%s ", table.DB.Name, table.Info.Name) allUnique = false - } else if infoschema.ErrTableNotExists.Equal(err) { + } else if !infoschema.ErrTableNotExists.Equal(err) { return errors.Trace(err) } } From 5b80e154a2a89035dcc068d5ae28cc9083e38113 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 9 Aug 2024 11:48:07 +0800 Subject: [PATCH 25/34] add timeout --- tests/realtikvtest/brietest/brie_test.go | 47 ++++++++++++++++++++---- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index ae05c58956302..d99c0a7bce2f2 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -130,6 +130,10 @@ func TestCancel(t *testing.T) { } func TestExistedTables(t *testing.T) { + // Create a context with a 20-second timeout for the entire test. + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + tk := initTestKit(t) tmp := makeTempDirForBackup(t) sqlTmp := strings.ReplaceAll(tmp, "'", "''") @@ -139,11 +143,40 @@ func TestExistedTables(t *testing.T) { tk.MustExec("create table baa(pk int primary key auto_increment, v varchar(255));") tk.MustExec("insert into foo(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") tk.MustExec("insert into baa(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") - backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) - tk.MustQuery(backupQuery) - restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) - res, err := tk.Exec(restoreQuery) - require.NoError(t, err) - _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) - require.ErrorContains(t, err, "table already exists") + + // Record time for the backup operation + backupStartTime := time.Now() + done := make(chan struct{}) + go func() { + defer close(done) + backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) + _ = tk.MustQuery(backupQuery) + }() + select { + case <-done: + backupDuration := time.Since(backupStartTime) + t.Logf("Backup operation took: %s", backupDuration) + case <-ctx.Done(): + t.Fatal("Backup operation exceeded") + } + + // Record time for the restore operation + restoreStartTime := time.Now() + done = make(chan struct{}) + go func() { + defer close(done) + restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) + res, err := tk.Exec(restoreQuery) + require.NoError(t, err) + + _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) + require.ErrorContains(t, err, "table already exists") + }() + select { + case <-done: + restoreDuration := time.Since(restoreStartTime) + t.Logf("Restore operation took: %s", restoreDuration) + case <-ctx.Done(): + t.Fatal("Restore operation exceeded") + } } From 4581388c7499a3813dbf9b156df931119c9db0e4 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 9 Aug 2024 12:06:13 +0800 Subject: [PATCH 26/34] add more timeout --- tests/realtikvtest/brietest/brie_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index d99c0a7bce2f2..d77f8b2d40356 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -131,7 +131,7 @@ func TestCancel(t *testing.T) { func TestExistedTables(t *testing.T) { // Create a context with a 20-second timeout for the entire test. - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() tk := initTestKit(t) From c6c30b28215a8fd2bc9f4a3ea26f8f0cf7d741ff Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 9 Aug 2024 12:55:07 +0800 Subject: [PATCH 27/34] more protect --- tests/realtikvtest/brietest/brie_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index d77f8b2d40356..665e10b640f56 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -62,6 +62,7 @@ func TestShowBackupQuery(t *testing.T) { restoreQuery := fmt.Sprintf("RESTORE TABLE `test`.`foo` FROM 'local://%s'", sqlTmp) tk.MustQuery(restoreQuery) res = tk.MustQuery("show br job query 2;") + tk.MustExec("drop table foo;") res.CheckContain(restoreQuery) } From 447a9abf46e18fe26a59905e9636d1034c1b9ba7 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 9 Aug 2024 13:47:00 +0800 Subject: [PATCH 28/34] disable failpoint --- tests/realtikvtest/brietest/brie_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index 665e10b640f56..a3568619cc633 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -105,6 +105,7 @@ func TestCancel(t *testing.T) { executor.ResetGlobalBRIEQueueForTest() tk.MustExec("use test;") failpoint.Enable("github.com/pingcap/tidb/pkg/executor/block-on-brie", "return") + defer failpoint.Disable("github.com/pingcap/tidb/pkg/executor/block-on-brie") req := require.New(t) ch := make(chan struct{}) From 1710c132d7a166d008320b850c2e3dc01b5aea86 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:39:26 +0800 Subject: [PATCH 29/34] checkpointFirstRun --- br/pkg/task/restore.go | 7 ++--- tests/realtikvtest/brietest/brie_test.go | 33 ++++++++---------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index a014a5e1fa540..7f06703fd48cf 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -844,9 +844,6 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err := checkDiskSpace(ctx, mgr, files, tables); err != nil { return errors.Trace(err) } - if err := checkTableExistence(ctx, mgr, tables, g); err != nil { - return errors.Trace(err) - } } archiveSize := reader.ArchiveSize(ctx, files) @@ -915,6 +912,10 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if cfg.WithSysTable { client.InitFullClusterRestore(cfg.ExplicitFilter) } + } else if checkpointFirstRun && cfg.CheckRequirements { + if err := checkTableExistence(ctx, mgr, tables, g); err != nil { + return errors.Trace(err) + } } if client.IsFullClusterRestore() && client.HasBackedUpSysDB() { diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index a3568619cc633..903d2ff89f705 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -132,42 +132,33 @@ func TestCancel(t *testing.T) { } func TestExistedTables(t *testing.T) { - // Create a context with a 20-second timeout for the entire test. - ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) - defer cancel() - tk := initTestKit(t) tmp := makeTempDirForBackup(t) sqlTmp := strings.ReplaceAll(tmp, "'", "''") executor.ResetGlobalBRIEQueueForTest() tk.MustExec("use test;") - tk.MustExec("create table foo(pk int primary key auto_increment, v varchar(255));") - tk.MustExec("create table baa(pk int primary key auto_increment, v varchar(255));") - tk.MustExec("insert into foo(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") - tk.MustExec("insert into baa(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") + for i := 0; i < 10; i++ { + tableName := fmt.Sprintf("foo%d", i) + tk.MustExec(fmt.Sprintf("create table %s(pk int primary key auto_increment, v varchar(255));", tableName)) + tk.MustExec(fmt.Sprintf("insert into %s(v) values %s;", tableName, strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ","))) + } - // Record time for the backup operation - backupStartTime := time.Now() done := make(chan struct{}) go func() { defer close(done) - backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) + backupQuery := fmt.Sprintf("BACKUP DATABASE * TO 'local://%s'", sqlTmp) _ = tk.MustQuery(backupQuery) }() select { - case <-done: - backupDuration := time.Since(backupStartTime) - t.Logf("Backup operation took: %s", backupDuration) - case <-ctx.Done(): + case <-time.After(20 * time.Second): t.Fatal("Backup operation exceeded") + case <-done: } - // Record time for the restore operation - restoreStartTime := time.Now() done = make(chan struct{}) go func() { defer close(done) - restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) + restoreQuery := fmt.Sprintf("RESTORE DATABASE * FROM 'local://%s'", sqlTmp) res, err := tk.Exec(restoreQuery) require.NoError(t, err) @@ -175,10 +166,8 @@ func TestExistedTables(t *testing.T) { require.ErrorContains(t, err, "table already exists") }() select { - case <-done: - restoreDuration := time.Since(restoreStartTime) - t.Logf("Restore operation took: %s", restoreDuration) - case <-ctx.Done(): + case <-time.After(20 * time.Second): t.Fatal("Restore operation exceeded") + case <-done: } } From aa4c00876a87546d942e35f0c2658c72a884eeb7 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Fri, 9 Aug 2024 17:31:00 +0800 Subject: [PATCH 30/34] drop tables --- tests/realtikvtest/brietest/brie_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index 903d2ff89f705..a09aaac13eb7f 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -170,4 +170,9 @@ func TestExistedTables(t *testing.T) { t.Fatal("Restore operation exceeded") case <-done: } + + for i := 0; i < 10; i++ { + tableName := fmt.Sprintf("foo%d", i) + tk.MustExec(fmt.Sprintf("drop table %s;", tableName)) + } } From 77e6cdb3966e90e1dfef2cc40e17838804306196 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Mon, 19 Aug 2024 02:28:09 +0800 Subject: [PATCH 31/34] fix test --- tests/realtikvtest/brietest/brie_test.go | 49 ++++++------------------ 1 file changed, 11 insertions(+), 38 deletions(-) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index a09aaac13eb7f..2f9b0a4bc87f9 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -137,42 +137,15 @@ func TestExistedTables(t *testing.T) { sqlTmp := strings.ReplaceAll(tmp, "'", "''") executor.ResetGlobalBRIEQueueForTest() tk.MustExec("use test;") - for i := 0; i < 10; i++ { - tableName := fmt.Sprintf("foo%d", i) - tk.MustExec(fmt.Sprintf("create table %s(pk int primary key auto_increment, v varchar(255));", tableName)) - tk.MustExec(fmt.Sprintf("insert into %s(v) values %s;", tableName, strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ","))) - } - - done := make(chan struct{}) - go func() { - defer close(done) - backupQuery := fmt.Sprintf("BACKUP DATABASE * TO 'local://%s'", sqlTmp) - _ = tk.MustQuery(backupQuery) - }() - select { - case <-time.After(20 * time.Second): - t.Fatal("Backup operation exceeded") - case <-done: - } - - done = make(chan struct{}) - go func() { - defer close(done) - restoreQuery := fmt.Sprintf("RESTORE DATABASE * FROM 'local://%s'", sqlTmp) - res, err := tk.Exec(restoreQuery) - require.NoError(t, err) - - _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) - require.ErrorContains(t, err, "table already exists") - }() - select { - case <-time.After(20 * time.Second): - t.Fatal("Restore operation exceeded") - case <-done: - } - - for i := 0; i < 10; i++ { - tableName := fmt.Sprintf("foo%d", i) - tk.MustExec(fmt.Sprintf("drop table %s;", tableName)) - } + tk.MustExec("create table foo(pk int primary key auto_increment, v varchar(255));") + tk.MustExec("create table baa(pk int primary key auto_increment, v varchar(255));") + tk.MustExec("insert into foo(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") + tk.MustExec("insert into baa(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") + backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) + tk.MustQuery(backupQuery) + restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) + res, err := tk.Exec(restoreQuery) + require.NoError(t, err) + _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) + require.ErrorContains(t, err, "table already exists") } From b5ca87bdfae49a0c38da46c18e140f2eddb31462 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Mon, 19 Aug 2024 02:51:27 +0800 Subject: [PATCH 32/34] add disable --- tests/realtikvtest/brietest/brie_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index 2f9b0a4bc87f9..f1f18565f0b44 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -71,6 +71,7 @@ func TestShowBackupQueryRedact(t *testing.T) { executor.ResetGlobalBRIEQueueForTest() failpoint.Enable("github.com/pingcap/tidb/pkg/executor/block-on-brie", "return") + defer failpoint.Disable("github.com/pingcap/tidb/pkg/executor/block-on-brie") ch := make(chan any) go func() { tk := testkit.NewTestKit(t, tk.Session().GetStore()) From e943d57b7a3745486501168b17fa585753ce1627 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Mon, 19 Aug 2024 10:29:30 +0800 Subject: [PATCH 33/34] Revert "fix test" This reverts commit 77e6cdb3966e90e1dfef2cc40e17838804306196. --- tests/realtikvtest/brietest/brie_test.go | 49 ++++++++++++++++++------ 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/tests/realtikvtest/brietest/brie_test.go b/tests/realtikvtest/brietest/brie_test.go index f1f18565f0b44..e1c5abd475a54 100644 --- a/tests/realtikvtest/brietest/brie_test.go +++ b/tests/realtikvtest/brietest/brie_test.go @@ -138,15 +138,42 @@ func TestExistedTables(t *testing.T) { sqlTmp := strings.ReplaceAll(tmp, "'", "''") executor.ResetGlobalBRIEQueueForTest() tk.MustExec("use test;") - tk.MustExec("create table foo(pk int primary key auto_increment, v varchar(255));") - tk.MustExec("create table baa(pk int primary key auto_increment, v varchar(255));") - tk.MustExec("insert into foo(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") - tk.MustExec("insert into baa(v) values " + strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ",") + ";") - backupQuery := fmt.Sprintf("BACKUP DATABASE `test` TO 'local://%s'", sqlTmp) - tk.MustQuery(backupQuery) - restoreQuery := fmt.Sprintf("RESTORE DATABASE `test` FROM 'local://%s'", sqlTmp) - res, err := tk.Exec(restoreQuery) - require.NoError(t, err) - _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) - require.ErrorContains(t, err, "table already exists") + for i := 0; i < 10; i++ { + tableName := fmt.Sprintf("foo%d", i) + tk.MustExec(fmt.Sprintf("create table %s(pk int primary key auto_increment, v varchar(255));", tableName)) + tk.MustExec(fmt.Sprintf("insert into %s(v) values %s;", tableName, strings.TrimSuffix(strings.Repeat("('hello, world'),", 100), ","))) + } + + done := make(chan struct{}) + go func() { + defer close(done) + backupQuery := fmt.Sprintf("BACKUP DATABASE * TO 'local://%s'", sqlTmp) + _ = tk.MustQuery(backupQuery) + }() + select { + case <-time.After(20 * time.Second): + t.Fatal("Backup operation exceeded") + case <-done: + } + + done = make(chan struct{}) + go func() { + defer close(done) + restoreQuery := fmt.Sprintf("RESTORE DATABASE * FROM 'local://%s'", sqlTmp) + res, err := tk.Exec(restoreQuery) + require.NoError(t, err) + + _, err = session.ResultSetToStringSlice(context.Background(), tk.Session(), res) + require.ErrorContains(t, err, "table already exists") + }() + select { + case <-time.After(20 * time.Second): + t.Fatal("Restore operation exceeded") + case <-done: + } + + for i := 0; i < 10; i++ { + tableName := fmt.Sprintf("foo%d", i) + tk.MustExec(fmt.Sprintf("drop table %s;", tableName)) + } } From c858500d6d0a2f019e709dbec45312dc37dfb514 Mon Sep 17 00:00:00 2001 From: RidRisR <79858083+RidRisR@users.noreply.github.com> Date: Mon, 19 Aug 2024 11:33:50 +0800 Subject: [PATCH 34/34] fix scheduler removed --- br/pkg/task/restore.go | 1 + 1 file changed, 1 insertion(+) diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index 7f06703fd48cf..3c59650dcd7f0 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -914,6 +914,7 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf } } else if checkpointFirstRun && cfg.CheckRequirements { if err := checkTableExistence(ctx, mgr, tables, g); err != nil { + schedulersRemovable = true return errors.Trace(err) } }