From bad74fab393995a7a150ef36a4849bcefe7fadc0 Mon Sep 17 00:00:00 2001
From: Chunzhu Li <lichunzhu@pingcap.com>
Date: Tue, 22 Oct 2019 10:17:06 +0800
Subject: [PATCH] cherry-pick simplifyFlavor (#328)

---
 dm/worker/config.go                           | 50 ++++++++++++++++++-
 dm/worker/config_test.go                      | 44 ++++++++++++++++
 pkg/utils/db.go                               | 15 ++++++
 tests/all_mode/conf/dm-worker1.toml           |  2 +-
 tests/all_mode/conf/dm-worker2.toml           |  2 +-
 tests/compatibility/conf/dm-worker1.toml      |  2 +-
 tests/compatibility/conf/dm-worker2.toml      |  2 +-
 tests/dmctl_basic/conf/dm-worker1.toml        |  2 +-
 tests/dmctl_basic/conf/dm-worker2.toml        |  2 +-
 tests/http_apis/conf/dm-worker1.toml          |  2 +-
 tests/incremental_mode/conf/dm-worker1.toml   |  2 +-
 tests/incremental_mode/conf/dm-worker2.toml   |  2 +-
 tests/initial_unit/conf/dm-worker1.toml       |  2 +-
 tests/load_interrupt/conf/dm-worker1.toml     |  2 +-
 tests/load_interrupt/conf/dm-worker2.toml     |  2 +-
 tests/online_ddl/conf/dm-worker1.toml         |  2 +-
 tests/online_ddl/conf/dm-worker2.toml         |  2 +-
 tests/print_status/conf/dm-worker1.toml       |  2 +-
 tests/relay_interrupt/conf/dm-worker1.toml    |  2 +-
 tests/safe_mode/conf/dm-worker1.toml          |  2 +-
 tests/safe_mode/conf/dm-worker2.toml          |  2 +-
 tests/sequence_safe_mode/conf/dm-worker1.toml |  2 +-
 tests/sequence_safe_mode/conf/dm-worker2.toml |  2 +-
 tests/sequence_sharding/conf/dm-worker1.toml  |  2 +-
 tests/sequence_sharding/conf/dm-worker2.toml  |  2 +-
 tests/sharding/conf/dm-worker1.toml           |  2 +-
 tests/sharding/conf/dm-worker2.toml           |  2 +-
 tests/start_task/conf/dm-worker1.toml         |  2 +-
 28 files changed, 133 insertions(+), 26 deletions(-)

diff --git a/dm/worker/config.go b/dm/worker/config.go
index 632c504844..ca16b891ab 100644
--- a/dm/worker/config.go
+++ b/dm/worker/config.go
@@ -15,17 +15,21 @@ package worker
 
 import (
 	"bytes"
+	"context"
 	"encoding/base64"
 	"encoding/json"
 	"flag"
 	"fmt"
 	"io/ioutil"
 	"strings"
+	"time"
 
 	"github.com/BurntSushi/toml"
+	"github.com/siddontang/go-mysql/mysql"
 
 	"github.com/pingcap/dm/dm/config"
 	"github.com/pingcap/dm/pkg/binlog"
+	"github.com/pingcap/dm/pkg/conn"
 	"github.com/pingcap/dm/pkg/gtid"
 	"github.com/pingcap/dm/pkg/log"
 	"github.com/pingcap/dm/pkg/terror"
@@ -34,11 +38,20 @@ import (
 	"github.com/pingcap/dm/relay/purger"
 )
 
+const (
+	// flavorReadTimeout is readTimeout for DB connection in adjustFlavor
+	flavorReadTimeout = "30s"
+	// flavorGetTimeout is timeout for getting version info from DB
+	flavorGetTimeout = 30 * time.Second
+)
+
 // SampleConfigFile is sample config file of dm-worker
 // later we can read it from dm/worker/dm-worker.toml
 // and assign it to SampleConfigFile while we build dm-worker
 var SampleConfigFile string
 
+var applyNewBaseDB = conn.DefaultDBProvider.Apply
+
 // NewConfig creates a new base config for worker.
 func NewConfig() *Config {
 	cfg := &Config{}
@@ -188,6 +201,10 @@ func (c *Config) Parse(arguments []string) error {
 
 	c.From.Adjust()
 	c.Checker.adjust()
+	err = c.adjustFlavor()
+	if err != nil {
+		return err
+	}
 	return c.verify()
 }
 
@@ -239,6 +256,38 @@ func (c *Config) configFromFile(path string) error {
 	return c.verify()
 }
 
+// adjustFlavor adjusts flavor through querying from given database
+func (c *Config) adjustFlavor() error {
+	if c.Flavor != "" {
+		switch c.Flavor {
+		case mysql.MariaDBFlavor, mysql.MySQLFlavor:
+			return nil
+		default:
+			return terror.ErrNotSupportedFlavor.Generate(c.Flavor)
+		}
+	}
+	// decrypt password
+	clone, err := c.DecryptPassword()
+	if err != nil {
+		return err
+	}
+	from := clone.From
+	from.RawDBCfg = config.DefaultRawDBConfig().SetReadTimeout(flavorReadTimeout)
+	fromDB, err := applyNewBaseDB(from)
+	if err != nil {
+		return terror.WithScope(err, terror.ScopeUpstream)
+	}
+	defer fromDB.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), flavorGetTimeout)
+	defer cancel()
+	c.Flavor, err = utils.GetFlavor(ctx, fromDB.DB)
+	if ctx.Err() != nil {
+		err = terror.Annotatef(err, "time cost to get flavor info exceeds %s", flavorGetTimeout)
+	}
+	return terror.WithScope(err, terror.ScopeUpstream)
+}
+
 // UpdateConfigFile write configure to local file
 func (c *Config) UpdateConfigFile(content string) error {
 	if c.ConfigFile == "" {
@@ -279,6 +328,5 @@ func (c *Config) DecryptPassword() (*Config, error) {
 		}
 	}
 	clone.From.Password = pswdFrom
-
 	return clone, nil
 }
diff --git a/dm/worker/config_test.go b/dm/worker/config_test.go
index 5449ec73b8..0890508cba 100644
--- a/dm/worker/config_test.go
+++ b/dm/worker/config_test.go
@@ -19,9 +19,12 @@ import (
 	"path"
 	"strings"
 
+	"github.com/DATA-DOG/go-sqlmock"
 	. "github.com/pingcap/check"
+	"github.com/siddontang/go-mysql/mysql"
 
 	"github.com/pingcap/dm/dm/config"
+	"github.com/pingcap/dm/pkg/conn"
 )
 
 func (t *testServer) TestConfig(c *C) {
@@ -160,3 +163,44 @@ func (t *testServer) TestConfigVerify(c *C) {
 	}
 
 }
+
+func subtestFlavor(c *C, cfg *Config, sqlInfo, expectedFlavor, expectedError string) {
+	cfg.Flavor = ""
+	db, mock, err := sqlmock.New()
+	c.Assert(err, IsNil)
+	mock.ExpectQuery("SHOW GLOBAL VARIABLES LIKE 'version';").
+		WillReturnRows(sqlmock.NewRows([]string{"Variable_name", "Value"}).
+			AddRow("version", sqlInfo))
+	mock.ExpectClose()
+	applyNewBaseDB = func(config config.DBConfig) (*conn.BaseDB, error) {
+		return &conn.BaseDB{DB: db}, nil
+	}
+	err = cfg.adjustFlavor()
+	if expectedError == "" {
+		c.Assert(err, IsNil)
+		c.Assert(cfg.Flavor, Equals, expectedFlavor)
+	} else {
+		c.Assert(err, ErrorMatches, expectedError)
+	}
+}
+
+func (t *testServer) TestAdjustFlavor(c *C) {
+	cfg := NewConfig()
+	c.Assert(cfg.Parse([]string{"-config=./dm-worker.toml", "-relay-dir=./xx"}), IsNil)
+
+	cfg.Flavor = "mariadb"
+	err := cfg.adjustFlavor()
+	c.Assert(err, IsNil)
+	c.Assert(cfg.Flavor, Equals, mysql.MariaDBFlavor)
+	cfg.Flavor = "MongoDB"
+	err = cfg.adjustFlavor()
+	c.Assert(err, ErrorMatches, ".*flavor MongoDB not supported")
+
+	var origApplyNewBaseDB = applyNewBaseDB
+	defer func() {
+		applyNewBaseDB = origApplyNewBaseDB
+	}()
+
+	subtestFlavor(c, cfg, "10.4.8-MariaDB-1:10.4.8+maria~bionic", mysql.MariaDBFlavor, "")
+	subtestFlavor(c, cfg, "5.7.26-log", mysql.MySQLFlavor, "")
+}
diff --git a/pkg/utils/db.go b/pkg/utils/db.go
index 768fe8e9f0..3f3b9ee7df 100644
--- a/pkg/utils/db.go
+++ b/pkg/utils/db.go
@@ -14,6 +14,7 @@
 package utils
 
 import (
+	"context"
 	"database/sql"
 	"fmt"
 	"strconv"
@@ -28,6 +29,8 @@ import (
 	"github.com/pingcap/failpoint"
 	"github.com/pingcap/parser"
 	tmysql "github.com/pingcap/parser/mysql"
+	"github.com/pingcap/tidb-tools/pkg/check"
+	"github.com/pingcap/tidb-tools/pkg/dbutil"
 	gmysql "github.com/siddontang/go-mysql/mysql"
 	"go.uber.org/zap"
 )
@@ -37,6 +40,18 @@ var (
 	domainServerIDSeparator = "-"
 )
 
+// GetFlavor gets flavor from DB
+func GetFlavor(ctx context.Context, db *sql.DB) (string, error) {
+	value, err := dbutil.ShowVersion(ctx, db)
+	if err != nil {
+		return "", terror.DBErrorAdapt(err, terror.ErrDBDriverError)
+	}
+	if check.IsMariaDB(value) {
+		return gmysql.MariaDBFlavor, nil
+	}
+	return gmysql.MySQLFlavor, nil
+}
+
 // GetMasterStatus gets status from master
 func GetMasterStatus(db *sql.DB, flavor string) (gmysql.Position, gtid.Set, error) {
 	var (
diff --git a/tests/all_mode/conf/dm-worker1.toml b/tests/all_mode/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/all_mode/conf/dm-worker1.toml
+++ b/tests/all_mode/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/all_mode/conf/dm-worker2.toml b/tests/all_mode/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/all_mode/conf/dm-worker2.toml
+++ b/tests/all_mode/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/compatibility/conf/dm-worker1.toml b/tests/compatibility/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/compatibility/conf/dm-worker1.toml
+++ b/tests/compatibility/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/compatibility/conf/dm-worker2.toml b/tests/compatibility/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/compatibility/conf/dm-worker2.toml
+++ b/tests/compatibility/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/dmctl_basic/conf/dm-worker1.toml b/tests/dmctl_basic/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/dmctl_basic/conf/dm-worker1.toml
+++ b/tests/dmctl_basic/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/dmctl_basic/conf/dm-worker2.toml b/tests/dmctl_basic/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/dmctl_basic/conf/dm-worker2.toml
+++ b/tests/dmctl_basic/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/http_apis/conf/dm-worker1.toml b/tests/http_apis/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/http_apis/conf/dm-worker1.toml
+++ b/tests/http_apis/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/incremental_mode/conf/dm-worker1.toml b/tests/incremental_mode/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/incremental_mode/conf/dm-worker1.toml
+++ b/tests/incremental_mode/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/incremental_mode/conf/dm-worker2.toml b/tests/incremental_mode/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/incremental_mode/conf/dm-worker2.toml
+++ b/tests/incremental_mode/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/initial_unit/conf/dm-worker1.toml b/tests/initial_unit/conf/dm-worker1.toml
index b75fa9429d..61bbb8ee31 100644
--- a/tests/initial_unit/conf/dm-worker1.toml
+++ b/tests/initial_unit/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/load_interrupt/conf/dm-worker1.toml b/tests/load_interrupt/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/load_interrupt/conf/dm-worker1.toml
+++ b/tests/load_interrupt/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/load_interrupt/conf/dm-worker2.toml b/tests/load_interrupt/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/load_interrupt/conf/dm-worker2.toml
+++ b/tests/load_interrupt/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/online_ddl/conf/dm-worker1.toml b/tests/online_ddl/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/online_ddl/conf/dm-worker1.toml
+++ b/tests/online_ddl/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/online_ddl/conf/dm-worker2.toml b/tests/online_ddl/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/online_ddl/conf/dm-worker2.toml
+++ b/tests/online_ddl/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/print_status/conf/dm-worker1.toml b/tests/print_status/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/print_status/conf/dm-worker1.toml
+++ b/tests/print_status/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/relay_interrupt/conf/dm-worker1.toml b/tests/relay_interrupt/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/relay_interrupt/conf/dm-worker1.toml
+++ b/tests/relay_interrupt/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/safe_mode/conf/dm-worker1.toml b/tests/safe_mode/conf/dm-worker1.toml
index 8f30970f16..423e65ea43 100644
--- a/tests/safe_mode/conf/dm-worker1.toml
+++ b/tests/safe_mode/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/safe_mode/conf/dm-worker2.toml b/tests/safe_mode/conf/dm-worker2.toml
index cc661ff390..2ead050b79 100644
--- a/tests/safe_mode/conf/dm-worker2.toml
+++ b/tests/safe_mode/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/sequence_safe_mode/conf/dm-worker1.toml b/tests/sequence_safe_mode/conf/dm-worker1.toml
index 8f30970f16..423e65ea43 100644
--- a/tests/sequence_safe_mode/conf/dm-worker1.toml
+++ b/tests/sequence_safe_mode/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/sequence_safe_mode/conf/dm-worker2.toml b/tests/sequence_safe_mode/conf/dm-worker2.toml
index cc661ff390..2ead050b79 100644
--- a/tests/sequence_safe_mode/conf/dm-worker2.toml
+++ b/tests/sequence_safe_mode/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/sequence_sharding/conf/dm-worker1.toml b/tests/sequence_sharding/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/sequence_sharding/conf/dm-worker1.toml
+++ b/tests/sequence_sharding/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/sequence_sharding/conf/dm-worker2.toml b/tests/sequence_sharding/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/sequence_sharding/conf/dm-worker2.toml
+++ b/tests/sequence_sharding/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/sharding/conf/dm-worker1.toml b/tests/sharding/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/sharding/conf/dm-worker1.toml
+++ b/tests/sharding/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/sharding/conf/dm-worker2.toml b/tests/sharding/conf/dm-worker2.toml
index 315251f5e7..115a17459d 100644
--- a/tests/sharding/conf/dm-worker2.toml
+++ b/tests/sharding/conf/dm-worker2.toml
@@ -2,7 +2,7 @@
 
 server-id = 102
 source-id = "mysql-replica-02"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""
diff --git a/tests/start_task/conf/dm-worker1.toml b/tests/start_task/conf/dm-worker1.toml
index b8963aa04d..0657e62c30 100644
--- a/tests/start_task/conf/dm-worker1.toml
+++ b/tests/start_task/conf/dm-worker1.toml
@@ -2,7 +2,7 @@
 
 server-id = 101
 source-id = "mysql-replica-01"
-flavor = "mysql"
+flavor = ""
 enable-gtid = false
 relay-binlog-name = ""
 relay-binlog-gtid = ""