From 3f3ee25c22d470d9fe9e0013cc50543b77e6cdc2 Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Wed, 27 May 2020 12:31:34 +0800 Subject: [PATCH 1/6] add session config --- dm/config/subtask.go | 16 +++++++++++----- pkg/conn/basedb.go | 11 +++++++++++ tests/all_mode/conf/dm-task.yaml | 2 ++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/dm/config/subtask.go b/dm/config/subtask.go index 4bd1af4728..38557919f1 100644 --- a/dm/config/subtask.go +++ b/dm/config/subtask.go @@ -45,6 +45,11 @@ var ( defaultMaxIdleConns = 2 ) +// SessionConfig contains some session config +type SessionConfig struct { + SQLMode string `toml:"sql-mode" json:"sql-mode" yaml:"sql-mode"` +} + // RawDBConfig contains some low level database config type RawDBConfig struct { MaxIdleConns int @@ -81,11 +86,12 @@ func (c *RawDBConfig) SetMaxIdleConns(value int) *RawDBConfig { // DBConfig is the DB configuration. type DBConfig struct { - Host string `toml:"host" json:"host" yaml:"host"` - Port int `toml:"port" json:"port" yaml:"port"` - User string `toml:"user" json:"user" yaml:"user"` - Password string `toml:"password" json:"-" yaml:"password"` // omit it for privacy - MaxAllowedPacket *int `toml:"max-allowed-packet" json:"max-allowed-packet" yaml:"max-allowed-packet"` + Host string `toml:"host" json:"host" yaml:"host"` + Port int `toml:"port" json:"port" yaml:"port"` + User string `toml:"user" json:"user" yaml:"user"` + Password string `toml:"password" json:"-" yaml:"password"` // omit it for privacy + MaxAllowedPacket *int `toml:"max-allowed-packet" json:"max-allowed-packet" yaml:"max-allowed-packet"` + Session *SessionConfig `toml:"session" json:"session" yaml:"session"` RawDBCfg *RawDBConfig `toml:"-" json:"-" yaml:"-"` } diff --git a/pkg/conn/basedb.go b/pkg/conn/basedb.go index 3906c5dbca..2b98011646 100644 --- a/pkg/conn/basedb.go +++ b/pkg/conn/basedb.go @@ -60,6 +60,17 @@ func (d *DefaultDBProviderImpl) Apply(config config.DBConfig) (*BaseDB, error) { if err != nil { return nil, terror.DBErrorAdapt(err, terror.ErrDBDriverError) } + + sessionConfig := config.Session + if sessionConfig != nil { + if sessionConfig.SQLMode != "" { + _, err := db.Exec(fmt.Sprintf("set @@session.sql_mode = \"%s\";", sessionConfig.SQLMode)) + if err != nil { + return nil, terror.ErrDBExecuteFailed.Delegate(err) + } + } + } + db.SetMaxIdleConns(maxIdleConns) return NewBaseDB(db), nil diff --git a/tests/all_mode/conf/dm-task.yaml b/tests/all_mode/conf/dm-task.yaml index acef26f5bc..d190b1fbc2 100644 --- a/tests/all_mode/conf/dm-task.yaml +++ b/tests/all_mode/conf/dm-task.yaml @@ -13,6 +13,8 @@ target-database: port: 4000 user: "root" password: "" + session: + sql-mode: "ANSI,ANSI_QUOTES" mysql-instances: - source-id: "mysql-replica-01" From cd3bc526d862af6ce267e21da892904654758e18 Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Wed, 27 May 2020 14:05:37 +0800 Subject: [PATCH 2/6] change session config to map --- dm/config/subtask.go | 17 ++++++----------- pkg/conn/basedb.go | 11 ++++------- tests/all_mode/conf/dm-task.yaml | 5 ++++- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/dm/config/subtask.go b/dm/config/subtask.go index 38557919f1..198567a9d8 100644 --- a/dm/config/subtask.go +++ b/dm/config/subtask.go @@ -45,11 +45,6 @@ var ( defaultMaxIdleConns = 2 ) -// SessionConfig contains some session config -type SessionConfig struct { - SQLMode string `toml:"sql-mode" json:"sql-mode" yaml:"sql-mode"` -} - // RawDBConfig contains some low level database config type RawDBConfig struct { MaxIdleConns int @@ -86,12 +81,12 @@ func (c *RawDBConfig) SetMaxIdleConns(value int) *RawDBConfig { // DBConfig is the DB configuration. type DBConfig struct { - Host string `toml:"host" json:"host" yaml:"host"` - Port int `toml:"port" json:"port" yaml:"port"` - User string `toml:"user" json:"user" yaml:"user"` - Password string `toml:"password" json:"-" yaml:"password"` // omit it for privacy - MaxAllowedPacket *int `toml:"max-allowed-packet" json:"max-allowed-packet" yaml:"max-allowed-packet"` - Session *SessionConfig `toml:"session" json:"session" yaml:"session"` + Host string `toml:"host" json:"host" yaml:"host"` + Port int `toml:"port" json:"port" yaml:"port"` + User string `toml:"user" json:"user" yaml:"user"` + Password string `toml:"password" json:"-" yaml:"password"` // omit it for privacy + MaxAllowedPacket *int `toml:"max-allowed-packet" json:"max-allowed-packet" yaml:"max-allowed-packet"` + Session map[string]string `toml:"session" json:"session" yaml:"session"` RawDBCfg *RawDBConfig `toml:"-" json:"-" yaml:"-"` } diff --git a/pkg/conn/basedb.go b/pkg/conn/basedb.go index 2b98011646..c5ead77ad8 100644 --- a/pkg/conn/basedb.go +++ b/pkg/conn/basedb.go @@ -61,13 +61,10 @@ func (d *DefaultDBProviderImpl) Apply(config config.DBConfig) (*BaseDB, error) { return nil, terror.DBErrorAdapt(err, terror.ErrDBDriverError) } - sessionConfig := config.Session - if sessionConfig != nil { - if sessionConfig.SQLMode != "" { - _, err := db.Exec(fmt.Sprintf("set @@session.sql_mode = \"%s\";", sessionConfig.SQLMode)) - if err != nil { - return nil, terror.ErrDBExecuteFailed.Delegate(err) - } + for key, val := range config.Session { + _, err := db.Exec(fmt.Sprintf("set @@session.%s = \"%s\";", key, val)) + if err != nil { + return nil, terror.ErrDBExecuteFailed.Delegate(err) } } diff --git a/tests/all_mode/conf/dm-task.yaml b/tests/all_mode/conf/dm-task.yaml index d190b1fbc2..c77a413534 100644 --- a/tests/all_mode/conf/dm-task.yaml +++ b/tests/all_mode/conf/dm-task.yaml @@ -14,7 +14,10 @@ target-database: user: "root" password: "" session: - sql-mode: "ANSI,ANSI_QUOTES" + sql_mode: "ANSI,ANSI_QUOTES" + tidb_skip_utf8_check: 1 + tidb_disable_txn_auto_retry: off + tidb_retry_limit: "10" mysql-instances: - source-id: "mysql-replica-01" From 2ad6fc6a9c15f5ef20fae2ab5a79ab7864f388df Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Wed, 27 May 2020 18:03:54 +0800 Subject: [PATCH 3/6] change to DSN --- pkg/conn/basedb.go | 12 +++++------- tests/all_mode/conf/dm-task.yaml | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/conn/basedb.go b/pkg/conn/basedb.go index c5ead77ad8..b3d64cb056 100644 --- a/pkg/conn/basedb.go +++ b/pkg/conn/basedb.go @@ -56,18 +56,16 @@ func (d *DefaultDBProviderImpl) Apply(config config.DBConfig) (*BaseDB, error) { } maxIdleConns = rawCfg.MaxIdleConns } + + for key, val := range config.Session { + dsn += fmt.Sprintf("&%s=\"%s\"", key, val) + } + db, err := sql.Open("mysql", dsn) if err != nil { return nil, terror.DBErrorAdapt(err, terror.ErrDBDriverError) } - for key, val := range config.Session { - _, err := db.Exec(fmt.Sprintf("set @@session.%s = \"%s\";", key, val)) - if err != nil { - return nil, terror.ErrDBExecuteFailed.Delegate(err) - } - } - db.SetMaxIdleConns(maxIdleConns) return NewBaseDB(db), nil diff --git a/tests/all_mode/conf/dm-task.yaml b/tests/all_mode/conf/dm-task.yaml index c77a413534..42496fbcb2 100644 --- a/tests/all_mode/conf/dm-task.yaml +++ b/tests/all_mode/conf/dm-task.yaml @@ -14,7 +14,7 @@ target-database: user: "root" password: "" session: - sql_mode: "ANSI,ANSI_QUOTES" + sql_mode: "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION" tidb_skip_utf8_check: 1 tidb_disable_txn_auto_retry: off tidb_retry_limit: "10" From 36e81037be5c3fe4a71d6913c20fc020ebe20cc6 Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Thu, 28 May 2020 15:04:54 +0800 Subject: [PATCH 4/6] format dsn --- pkg/conn/basedb.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/conn/basedb.go b/pkg/conn/basedb.go index b3d64cb056..6b94afc461 100644 --- a/pkg/conn/basedb.go +++ b/pkg/conn/basedb.go @@ -17,6 +17,7 @@ import ( "context" "database/sql" "fmt" + "net/url" "sync" "github.com/pingcap/dm/dm/config" @@ -58,7 +59,7 @@ func (d *DefaultDBProviderImpl) Apply(config config.DBConfig) (*BaseDB, error) { } for key, val := range config.Session { - dsn += fmt.Sprintf("&%s=\"%s\"", key, val) + dsn += fmt.Sprintf("&%s='%s'", key, url.QueryEscape(val)) } db, err := sql.Open("mysql", dsn) From 9ec51cb2ace7e31d8c61112134f5b0a795bf938f Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Thu, 28 May 2020 15:05:39 +0800 Subject: [PATCH 5/6] add integration_test for session config --- tests/_utils/check_sync_diff | 12 ++++++-- tests/all_mode/run.sh | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/tests/_utils/check_sync_diff b/tests/_utils/check_sync_diff index ed3d324fa3..2b4a161227 100755 --- a/tests/_utils/check_sync_diff +++ b/tests/_utils/check_sync_diff @@ -2,6 +2,7 @@ # parameter 1: work directory # parameter 2: config file for sync_diff_inspector # parameter 3: max check times +# parameter 4: check diff should fail or not workdir=$1 conf=$2 @@ -23,6 +24,9 @@ do ret=$? if [ "$ret" == 0 ]; then echo "check diff successfully" + if [[ $4 = "fail" ]]; then + exit 1 + fi break fi ((i++)) @@ -32,8 +36,10 @@ done if [ $i -ge $check_time ]; then echo "check diff failed at last" - # show \n and other blanks - printf "$(cat $LOG)\n" - exit 1 + if [[ $4 != "fail" ]]; then + # show \n and other blanks + printf "$(cat $LOG)\n" + exit 1 + fi fi cd $PWD diff --git a/tests/all_mode/run.sh b/tests/all_mode/run.sh index fa6d22cc72..5de8f1e732 100755 --- a/tests/all_mode/run.sh +++ b/tests/all_mode/run.sh @@ -7,7 +7,61 @@ source $cur/../_utils/test_prepare WORK_DIR=$TEST_DIR/$TEST_NAME API_VERSION="v1alpha1" +function test_session_config(){ + run_sql_file $cur/data/db1.prepare.sql $MYSQL_HOST1 $MYSQL_PORT1 $MYSQL_PASSWORD1 + check_contains 'Query OK, 2 rows affected' + run_sql_file $cur/data/db2.prepare.sql $MYSQL_HOST2 $MYSQL_PORT2 $MYSQL_PASSWORD2 + check_contains 'Query OK, 3 rows affected' + + # start DM worker and master + run_dm_master $WORK_DIR/master $MASTER_PORT $cur/conf/dm-master.toml + check_rpc_alive $cur/../bin/check_master_online 127.0.0.1:$MASTER_PORT + run_dm_worker $WORK_DIR/worker1 $WORKER1_PORT $cur/conf/dm-worker1.toml + check_rpc_alive $cur/../bin/check_worker_online 127.0.0.1:$WORKER1_PORT + run_dm_worker $WORK_DIR/worker2 $WORKER2_PORT $cur/conf/dm-worker2.toml + check_rpc_alive $cur/../bin/check_worker_online 127.0.0.1:$WORKER2_PORT + # operate mysql config to worker + cp $cur/conf/source1.toml $WORK_DIR/source1.toml + cp $cur/conf/source2.toml $WORK_DIR/source2.toml + sed -i "/relay-binlog-name/i\relay-dir = \"$WORK_DIR/worker1/relay_log\"" $WORK_DIR/source1.toml + sed -i "/relay-binlog-name/i\relay-dir = \"$WORK_DIR/worker2/relay_log\"" $WORK_DIR/source2.toml + dmctl_operate_source create $WORK_DIR/source1.toml $SOURCE_ID1 + dmctl_operate_source create $WORK_DIR/source2.toml $SOURCE_ID2 + + cp $cur/conf/dm-task.yaml $WORK_DIR/dm-task.yaml + + # error config + sed -i 's/tidb_retry_limit: "10"/tidb_retry_limit: "fjs"/g' $WORK_DIR/dm-task.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "start-task $WORK_DIR/dm-task.yaml --remove-meta" \ + "'tidb_retry_limit' can't be set to the value" 1 + + # sql_mode: "ANSI_QUOTES" + sed -i 's/tidb_retry_limit: "fjs"/tidb_retry_limit: "10"/g' $WORK_DIR/dm-task.yaml + sed -i 's/sql_mode: ".*"/sql_mode: "ANSI_QUOTES"/g' $WORK_DIR/dm-task.yaml + dmctl_start_task "$WORK_DIR/dm-task.yaml" "--remove-meta" + + # dumpling works fine since it use single quote + check_sync_diff $WORK_DIR $cur/conf/diff_config.toml + + run_sql_file $cur/data/db1.increment.sql $MYSQL_HOST1 $MYSQL_PORT1 $MYSQL_PASSWORD1 + run_sql_file $cur/data/db2.increment.sql $MYSQL_HOST2 $MYSQL_PORT2 $MYSQL_PASSWORD2 + + # but syncer should fail + echo "check diff should fail" + check_sync_diff $WORK_DIR $cur/conf/diff_config.toml 10 "fail" + + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "stop-task test"\ + "\"result\": true" 3 + + cleanup_data all_mode + cleanup_process $* +} + function run() { + test_session_config + export GO_FAILPOINTS="github.com/pingcap/dm/dm/worker/TaskCheckInterval=return(\"500ms\")" run_sql_file $cur/data/db1.prepare.sql $MYSQL_HOST1 $MYSQL_PORT1 $MYSQL_PASSWORD1 From 5d57a5baeb498bc24517b97511c6f081af041515 Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Thu, 28 May 2020 21:59:01 +0800 Subject: [PATCH 6/6] add comment --- pkg/conn/basedb.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/conn/basedb.go b/pkg/conn/basedb.go index 6b94afc461..b933184e41 100644 --- a/pkg/conn/basedb.go +++ b/pkg/conn/basedb.go @@ -59,6 +59,9 @@ func (d *DefaultDBProviderImpl) Apply(config config.DBConfig) (*BaseDB, error) { } for key, val := range config.Session { + // for num such as 1/"1", format as key='1' + // for string, format as key='string' + // both are valid for mysql and tidb dsn += fmt.Sprintf("&%s='%s'", key, url.QueryEscape(val)) }