From 4277d904524ff3bcb0f449845e5b7801c3d840e8 Mon Sep 17 00:00:00 2001 From: luancheng Date: Fri, 27 Mar 2020 09:57:53 +0800 Subject: [PATCH 1/5] add skip create sqls --- pkg/restore/client.go | 25 ++++++++++++++++++++++--- pkg/task/restore.go | 19 ++++++++++++++++--- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/pkg/restore/client.go b/pkg/restore/client.go index 56bb83233..39e4e0819 100644 --- a/pkg/restore/client.go +++ b/pkg/restore/client.go @@ -64,6 +64,7 @@ type Client struct { db *DB rateLimit uint64 isOnline bool + isSkipCreateSQL bool hasSpeedLimited bool restoreStores []uint64 @@ -305,6 +306,10 @@ func (rc *Client) GetTableSchema( // CreateDatabase creates a database. func (rc *Client) CreateDatabase(db *model.DBInfo) error { + if rc.IsSkipCreateSQL() { + log.Info("skip create database", zap.Stringer("database", db.Name)) + return nil + } return rc.db.CreateDatabase(rc.ctx, db) } @@ -320,9 +325,13 @@ func (rc *Client) CreateTables( } newTables := make([]*model.TableInfo, 0, len(tables)) for _, table := range tables { - err := rc.db.CreateTable(rc.ctx, table) - if err != nil { - return nil, nil, err + if rc.IsSkipCreateSQL() { + log.Info("skip create table and alter autoIncID", zap.Stringer("table", table.Info.Name)) + } else { + err := rc.db.CreateTable(rc.ctx, table) + if err != nil { + return nil, nil, err + } } newTableInfo, err := rc.GetTableSchema(dom, table.Db.Name, table.Info.Name) if err != nil { @@ -847,3 +856,13 @@ func (rc *Client) IsIncremental() bool { return !(rc.backupMeta.StartVersion == rc.backupMeta.EndVersion || rc.backupMeta.StartVersion == 0) } + +// EnableSkipCreateSQL sets switch of skip create schema and tables +func (rc *Client) EnableSkipCreateSQL() { + rc.isSkipCreateSQL = true +} + +// IsSkipCreateSQL returns whether we need skip create schema and tables in restore +func (rc *Client) IsSkipCreateSQL() bool { + return rc.isSkipCreateSQL +} diff --git a/pkg/task/restore.go b/pkg/task/restore.go index 33bac7a6c..1825fa111 100644 --- a/pkg/task/restore.go +++ b/pkg/task/restore.go @@ -23,7 +23,8 @@ import ( ) const ( - flagOnline = "online" + flagOnline = "online" + flagSkipCreateSQL = "skip-create-sql" ) var schedulers = map[string]struct{}{ @@ -45,13 +46,18 @@ const ( type RestoreConfig struct { Config - Online bool `json:"online" toml:"online"` + Online bool `json:"online" toml:"online"` + SkipCreateSQL bool `json:"skip-create-sql" toml:"skip-create-sql"` } // DefineRestoreFlags defines common flags for the restore command. func DefineRestoreFlags(flags *pflag.FlagSet) { // TODO remove experimental tag if it's stable - flags.Bool("online", false, "(experimental) Whether online when restore") + flags.Bool(flagOnline, false, "(experimental) Whether online when restore") + flags.Bool(flagSkipCreateSQL, false, "Only allow when restore cluster has already use br create all schema and tables before") + + // Do not expose this flag + flags.MarkHidden(flagSkipCreateSQL) } // ParseFromFlags parses the restore-related flags from the flag set. @@ -61,6 +67,10 @@ func (cfg *RestoreConfig) ParseFromFlags(flags *pflag.FlagSet) error { if err != nil { return errors.Trace(err) } + cfg.SkipCreateSQL, err = flags.GetBool(flagSkipCreateSQL) + if err != nil { + return errors.Trace(err) + } err = cfg.Config.ParseFromFlags(flags) if err != nil { return errors.Trace(err) @@ -101,6 +111,9 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if cfg.Online { client.EnableOnline() } + if cfg.SkipCreateSQL { + client.EnableSkipCreateSQL() + } err = client.LoadRestoreStores(ctx) if err != nil { return err From 7e4d4c61ef964990620aa771a0568f4a154a4c89 Mon Sep 17 00:00:00 2001 From: 3pointer Date: Fri, 27 Mar 2020 10:19:02 +0800 Subject: [PATCH 2/5] Update pkg/task/restore.go Co-Authored-By: kennytm --- pkg/task/restore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/task/restore.go b/pkg/task/restore.go index 1825fa111..7ba049fe7 100644 --- a/pkg/task/restore.go +++ b/pkg/task/restore.go @@ -54,7 +54,7 @@ type RestoreConfig struct { func DefineRestoreFlags(flags *pflag.FlagSet) { // TODO remove experimental tag if it's stable flags.Bool(flagOnline, false, "(experimental) Whether online when restore") - flags.Bool(flagSkipCreateSQL, false, "Only allow when restore cluster has already use br create all schema and tables before") + flags.Bool(flagSkipCreateSQL, false, "skip creating schemas and tables, reuse existing empty ones") // Do not expose this flag flags.MarkHidden(flagSkipCreateSQL) From 36ee0f6d5e5f850008ee34bb95e0ea4c902b8dbe Mon Sep 17 00:00:00 2001 From: luancheng Date: Fri, 27 Mar 2020 10:22:32 +0800 Subject: [PATCH 3/5] address comment --- pkg/restore/client.go | 6 +++--- pkg/task/restore.go | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/restore/client.go b/pkg/restore/client.go index 39e4e0819..76dbf5066 100644 --- a/pkg/restore/client.go +++ b/pkg/restore/client.go @@ -64,7 +64,7 @@ type Client struct { db *DB rateLimit uint64 isOnline bool - isSkipCreateSQL bool + noSchema bool hasSpeedLimited bool restoreStores []uint64 @@ -859,10 +859,10 @@ func (rc *Client) IsIncremental() bool { // EnableSkipCreateSQL sets switch of skip create schema and tables func (rc *Client) EnableSkipCreateSQL() { - rc.isSkipCreateSQL = true + rc.noSchema = true } // IsSkipCreateSQL returns whether we need skip create schema and tables in restore func (rc *Client) IsSkipCreateSQL() bool { - return rc.isSkipCreateSQL + return rc.noSchema } diff --git a/pkg/task/restore.go b/pkg/task/restore.go index 7ba049fe7..336758dd7 100644 --- a/pkg/task/restore.go +++ b/pkg/task/restore.go @@ -23,8 +23,8 @@ import ( ) const ( - flagOnline = "online" - flagSkipCreateSQL = "skip-create-sql" + flagOnline = "online" + flagNoSchema = "no-schema" ) var schedulers = map[string]struct{}{ @@ -46,18 +46,18 @@ const ( type RestoreConfig struct { Config - Online bool `json:"online" toml:"online"` - SkipCreateSQL bool `json:"skip-create-sql" toml:"skip-create-sql"` + Online bool `json:"online" toml:"online"` + NoSchema bool `json:"no-schema" toml:"no-schema"` } // DefineRestoreFlags defines common flags for the restore command. func DefineRestoreFlags(flags *pflag.FlagSet) { // TODO remove experimental tag if it's stable flags.Bool(flagOnline, false, "(experimental) Whether online when restore") - flags.Bool(flagSkipCreateSQL, false, "skip creating schemas and tables, reuse existing empty ones") + flags.Bool(flagNoSchema, false, "skip creating schemas and tables, reuse existing empty ones") // Do not expose this flag - flags.MarkHidden(flagSkipCreateSQL) + _ = flags.MarkHidden(flagNoSchema) } // ParseFromFlags parses the restore-related flags from the flag set. @@ -67,7 +67,7 @@ func (cfg *RestoreConfig) ParseFromFlags(flags *pflag.FlagSet) error { if err != nil { return errors.Trace(err) } - cfg.SkipCreateSQL, err = flags.GetBool(flagSkipCreateSQL) + cfg.NoSchema, err = flags.GetBool(flagNoSchema) if err != nil { return errors.Trace(err) } @@ -111,7 +111,7 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if cfg.Online { client.EnableOnline() } - if cfg.SkipCreateSQL { + if cfg.NoSchema { client.EnableSkipCreateSQL() } err = client.LoadRestoreStores(ctx) From 6cb48c25c4ecc6d0fbf76d575bab3422ae73bbcc Mon Sep 17 00:00:00 2001 From: luancheng Date: Fri, 27 Mar 2020 10:52:43 +0800 Subject: [PATCH 4/5] add test --- tests/br_db_skip/run.sh | 71 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100755 tests/br_db_skip/run.sh diff --git a/tests/br_db_skip/run.sh b/tests/br_db_skip/run.sh new file mode 100755 index 000000000..0a1be1c00 --- /dev/null +++ b/tests/br_db_skip/run.sh @@ -0,0 +1,71 @@ +#!/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, +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eu +DB="$TEST_NAME" + +run_sql "CREATE DATABASE $DB;" + +run_sql "CREATE TABLE $DB.usertable1 ( \ + YCSB_KEY varchar(64) NOT NULL, \ + FIELD0 varchar(1) DEFAULT NULL, \ + PRIMARY KEY (YCSB_KEY) \ +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;" + +run_sql "INSERT INTO $DB.usertable1 VALUES (\"a\", \"b\");" +run_sql "INSERT INTO $DB.usertable1 VALUES (\"aa\", \"b\");" + +# backup db +echo "backup start..." +run_br --pd $PD_ADDR backup db --db "$DB" -s "local://$TEST_DIR/$DB" --ratelimit 5 --concurrency 4 + +run_sql "DROP DATABASE $DB;" + +# restore db with skip-create-sql must failed +echo "restore start but must failed" +fail=false +run_br restore db --db $DB -s "local://$TEST_DIR/$DB" --pd $PD_ADDR --no-schema=true || fail=true +if $fail; then + echo "TEST: [$TEST_NAME] restore $DB with no-schema must failed" +else + echo "TEST: [$TEST_NAME] restore $DB with no-schema not failed" + exit 1 +fi + +run_sql "CREATE DATABASE $DB;" + +run_sql "CREATE TABLE $DB.usertable1 ( \ + YCSB_KEY varchar(64) NOT NULL, \ + FIELD0 varchar(1) DEFAULT NULL, \ + PRIMARY KEY (YCSB_KEY) \ +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;" + +echo "restore start must succeed" +fail=false +run_br restore db --db $DB -s "local://$TEST_DIR/$DB" --pd $PD_ADDR --no-schema=true || fail=true +if $fail; then + echo "TEST: [$TEST_NAME] restore $DB with no-schema failed" + exit 1 +else + echo "TEST: [$TEST_NAME] restore $DB with no-schema succeed" +fi + +table_count=$(run_sql "use $DB; show tables;" | grep "Tables_in" | wc -l) +if [ "$table_count" -ne "2" ];then + echo "TEST: [$TEST_NAME] failed!" + exit 1 +fi + +run_sql "DROP DATABASE $DB;" From 946fa865ad98b738bfa4cd595dfe4d9d6c5243ce Mon Sep 17 00:00:00 2001 From: luancheng Date: Fri, 27 Mar 2020 13:10:28 +0800 Subject: [PATCH 5/5] fix test --- tests/br_db_skip/run.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/br_db_skip/run.sh b/tests/br_db_skip/run.sh index 0a1be1c00..e126447c6 100755 --- a/tests/br_db_skip/run.sh +++ b/tests/br_db_skip/run.sh @@ -33,18 +33,19 @@ run_br --pd $PD_ADDR backup db --db "$DB" -s "local://$TEST_DIR/$DB" --ratelimit run_sql "DROP DATABASE $DB;" +run_sql "CREATE DATABASE $DB;" # restore db with skip-create-sql must failed echo "restore start but must failed" fail=false run_br restore db --db $DB -s "local://$TEST_DIR/$DB" --pd $PD_ADDR --no-schema=true || fail=true if $fail; then + # Error: [schema:1146]Table 'br_db_skip.usertable1' doesn't exist echo "TEST: [$TEST_NAME] restore $DB with no-schema must failed" else echo "TEST: [$TEST_NAME] restore $DB with no-schema not failed" exit 1 fi -run_sql "CREATE DATABASE $DB;" run_sql "CREATE TABLE $DB.usertable1 ( \ YCSB_KEY varchar(64) NOT NULL, \ @@ -63,7 +64,7 @@ else fi table_count=$(run_sql "use $DB; show tables;" | grep "Tables_in" | wc -l) -if [ "$table_count" -ne "2" ];then +if [ "$table_count" -ne "1" ];then echo "TEST: [$TEST_NAME] failed!" exit 1 fi