From 70682b1c35b822e4e5990a62b0ba3fc841a5a550 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 11 Dec 2022 19:23:26 +0100 Subject: [PATCH 1/5] Optimize base.ValidateConnection --- go/base/utils.go | 38 ++++++++++++++++++-------------------- go/cmd/gh-ost/main.go | 2 +- go/mysql/instance_key.go | 4 ++-- go/mysql/utils.go | 2 +- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/go/base/utils.go b/go/base/utils.go index e3950f2bd..7e972bef2 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -62,34 +62,32 @@ func StringContainsAll(s string, substrings ...string) bool { } func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, migrationContext *MigrationContext, name string) (string, error) { - versionQuery := `select @@global.version` - var port, extraPort int + query := `select @@global.version, @@global.port` var version string - if err := db.QueryRow(versionQuery).Scan(&version); err != nil { + var port gosql.NullInt64 + if err := db.QueryRow(query).Scan(&version, &port); err != nil { return "", err } + extraPortQuery := `select @@global.extra_port` - if err := db.QueryRow(extraPortQuery).Scan(&extraPort); err != nil { // nolint:staticcheck - // swallow this error. not all servers support extra_port - } + var extraPort int64 + // swallow possible error. not all servers support extra_port + _ = db.QueryRow(extraPortQuery).Scan(&extraPort) + // AliyunRDS set users port to "NULL", replace it by gh-ost param // GCP set users port to "NULL", replace it by gh-ost param - // Azure MySQL set users port to a different value by design, replace it by gh-ost para + // Azure MySQL set users port to a different value by design, replace it by gh-ost param if migrationContext.AliyunRDS || migrationContext.GoogleCloudPlatform || migrationContext.AzureMySQL { - port = connectionConfig.Key.Port - } else { - portQuery := `select @@global.port` - if err := db.QueryRow(portQuery).Scan(&port); err != nil { - return "", err - } + port.Int64 = connectionConfig.Key.Port + port.Valid = true } - if connectionConfig.Key.Port == port || (extraPort > 0 && connectionConfig.Key.Port == extraPort) { - migrationContext.Log.Infof("%s connection validated on %+v", name, connectionConfig.Key) - return version, nil - } else if extraPort == 0 { - return "", fmt.Errorf("Unexpected database port reported: %+v", port) - } else { - return "", fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port, extraPort) + if !port.Valid || extraPort == 0 { + return "", fmt.Errorf("Unexpected database port reported: %+v", port.Int64) + } else if connectionConfig.Key.Port != port.Int64 || extraPort == 0 || connectionConfig.Key.Port != extraPort { + return "", fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port.Int64, extraPort) } + + migrationContext.Log.Infof("%s connection validated on %+v", name, connectionConfig.Key) + return version, nil } diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 3daf24441..2a2322163 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -49,7 +49,7 @@ func main() { migrationContext := base.NewMigrationContext() flag.StringVar(&migrationContext.InspectorConnectionConfig.Key.Hostname, "host", "127.0.0.1", "MySQL hostname (preferably a replica, not the master)") flag.StringVar(&migrationContext.AssumeMasterHostname, "assume-master-host", "", "(optional) explicitly tell gh-ost the identity of the master. Format: some.host.com[:port] This is useful in master-master setups where you wish to pick an explicit master, or in a tungsten-replicator where gh-ost is unable to determine the master") - flag.IntVar(&migrationContext.InspectorConnectionConfig.Key.Port, "port", 3306, "MySQL port (preferably a replica, not the master)") + flag.Int64Var(&migrationContext.InspectorConnectionConfig.Key.Port, "port", 3306, "MySQL port (preferably a replica, not the master)") flag.Float64Var(&migrationContext.InspectorConnectionConfig.Timeout, "mysql-timeout", 0.0, "Connect, read and write timeout for MySQL") flag.StringVar(&migrationContext.CliUser, "user", "", "MySQL user") flag.StringVar(&migrationContext.CliPassword, "password", "", "MySQL password") diff --git a/go/mysql/instance_key.go b/go/mysql/instance_key.go index 3d2bff114..80ae29ec5 100644 --- a/go/mysql/instance_key.go +++ b/go/mysql/instance_key.go @@ -28,7 +28,7 @@ var ( // InstanceKey is an instance indicator, identified by hostname and port type InstanceKey struct { Hostname string - Port int + Port int64 } const detachHint = "//" @@ -52,7 +52,7 @@ func NewRawInstanceKey(hostPort string) (*InstanceKey, error) { instanceKey := &InstanceKey{Hostname: hostname, Port: DefaultInstancePort} if port != "" { var err error - if instanceKey.Port, err = strconv.Atoi(port); err != nil { + if instanceKey.Port, err = strconv.ParseInt(port, 10, 64); err != nil { return instanceKey, fmt.Errorf("Invalid port: %s", port) } } diff --git a/go/mysql/utils.go b/go/mysql/utils.go index c69a3f255..1e56fcbb0 100644 --- a/go/mysql/utils.go +++ b/go/mysql/utils.go @@ -107,7 +107,7 @@ func GetMasterKeyFromSlaveStatus(connectionConfig *ConnectionConfig) (masterKey masterKey = &InstanceKey{ Hostname: rowMap.GetString("Master_Host"), - Port: rowMap.GetInt("Master_Port"), + Port: rowMap.GetInt64("Master_Port"), } return nil }) From 48f7467a3c60331ea3badc0b80a532bbbd866016 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 11 Dec 2022 19:26:14 +0100 Subject: [PATCH 2/5] Tweaks --- go/base/utils.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/go/base/utils.go b/go/base/utils.go index 7e972bef2..e5367300b 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -61,11 +61,10 @@ func StringContainsAll(s string, substrings ...string) bool { return nonEmptyStringsFound } -func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, migrationContext *MigrationContext, name string) (string, error) { +func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, migrationContext *MigrationContext, name string) (version string, err error) { query := `select @@global.version, @@global.port` - var version string var port gosql.NullInt64 - if err := db.QueryRow(query).Scan(&version, &port); err != nil { + if err = db.QueryRow(query).Scan(&version, &port); err != nil { return "", err } @@ -79,7 +78,7 @@ func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, // Azure MySQL set users port to a different value by design, replace it by gh-ost param if migrationContext.AliyunRDS || migrationContext.GoogleCloudPlatform || migrationContext.AzureMySQL { port.Int64 = connectionConfig.Key.Port - port.Valid = true + port.Valid = connectionConfig.Key.Port > 0 } if !port.Valid || extraPort == 0 { @@ -89,5 +88,5 @@ func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, } migrationContext.Log.Infof("%s connection validated on %+v", name, connectionConfig.Key) - return version, nil + return version, err } From 4328aa0fef759ecabc08e680eeb22b9bb7a683b6 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 11 Dec 2022 19:29:22 +0100 Subject: [PATCH 3/5] Fix conditional --- go/base/utils.go | 2 +- go/mysql/instance_key.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go/base/utils.go b/go/base/utils.go index e5367300b..7634ee6ef 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -83,7 +83,7 @@ func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, if !port.Valid || extraPort == 0 { return "", fmt.Errorf("Unexpected database port reported: %+v", port.Int64) - } else if connectionConfig.Key.Port != port.Int64 || extraPort == 0 || connectionConfig.Key.Port != extraPort { + } else if connectionConfig.Key.Port != port.Int64 && (extraPort == 0 || connectionConfig.Key.Port != extraPort) { return "", fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port.Int64, extraPort) } diff --git a/go/mysql/instance_key.go b/go/mysql/instance_key.go index 80ae29ec5..dec1ba34d 100644 --- a/go/mysql/instance_key.go +++ b/go/mysql/instance_key.go @@ -13,7 +13,7 @@ import ( "strings" ) -const DefaultInstancePort = 3306 +const DefaultInstancePort int64 = 3306 var ( ipv4HostPortRegexp = regexp.MustCompile("^([^:]+):([0-9]+)$") From ae9e2c09646e57af6eb840c4ddc1038d0b27b1a3 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 11 Dec 2022 21:15:35 +0100 Subject: [PATCH 4/5] More cleanup --- go/base/utils.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/go/base/utils.go b/go/base/utils.go index 7634ee6ef..a6f3a9011 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -62,14 +62,13 @@ func StringContainsAll(s string, substrings ...string) bool { } func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, migrationContext *MigrationContext, name string) (version string, err error) { + var port, extraPort gosql.NullInt64 query := `select @@global.version, @@global.port` - var port gosql.NullInt64 if err = db.QueryRow(query).Scan(&version, &port); err != nil { return "", err } extraPortQuery := `select @@global.extra_port` - var extraPort int64 // swallow possible error. not all servers support extra_port _ = db.QueryRow(extraPortQuery).Scan(&extraPort) @@ -81,10 +80,10 @@ func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, port.Valid = connectionConfig.Key.Port > 0 } - if !port.Valid || extraPort == 0 { + if !port.Valid && (!extraPort.Valid || extraPort.Int64 == 0) { return "", fmt.Errorf("Unexpected database port reported: %+v", port.Int64) - } else if connectionConfig.Key.Port != port.Int64 && (extraPort == 0 || connectionConfig.Key.Port != extraPort) { - return "", fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port.Int64, extraPort) + } else if connectionConfig.Key.Port != port.Int64 && connectionConfig.Key.Port != extraPort.Int64 { + return "", fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port.Int64, extraPort.Int64) } migrationContext.Log.Infof("%s connection validated on %+v", name, connectionConfig.Key) From 2c5a1f718eccd101d460bb4aac00dd1c808f30ba Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 11 Dec 2022 22:41:44 +0100 Subject: [PATCH 5/5] Comments --- go/base/utils.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/go/base/utils.go b/go/base/utils.go index a6f3a9011..8518788ca 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -61,11 +61,14 @@ func StringContainsAll(s string, substrings ...string) bool { return nonEmptyStringsFound } +// ValidateConnection confirms the database connection matches the provided connection config. On success the +// function returns the server version as a string, otherwise an empty string and an error is returned. func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, migrationContext *MigrationContext, name string) (version string, err error) { var port, extraPort gosql.NullInt64 + // port may be NULL on AliyunRDS and GCP query := `select @@global.version, @@global.port` if err = db.QueryRow(query).Scan(&version, &port); err != nil { - return "", err + return version, err } extraPortQuery := `select @@global.extra_port` @@ -80,12 +83,11 @@ func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig, port.Valid = connectionConfig.Key.Port > 0 } - if !port.Valid && (!extraPort.Valid || extraPort.Int64 == 0) { - return "", fmt.Errorf("Unexpected database port reported: %+v", port.Int64) + if !port.Valid && !extraPort.Valid { + return version, fmt.Errorf("Unexpected database port reported: %+v", port.Int64) } else if connectionConfig.Key.Port != port.Int64 && connectionConfig.Key.Port != extraPort.Int64 { - return "", fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port.Int64, extraPort.Int64) + return version, fmt.Errorf("Unexpected database port reported: %+v / extra_port: %+v", port.Int64, extraPort.Int64) } - migrationContext.Log.Infof("%s connection validated on %+v", name, connectionConfig.Key) return version, err }