diff --git a/doc/requirements-and-limitations.md b/doc/requirements-and-limitations.md index 0521028..88642dc 100644 --- a/doc/requirements-and-limitations.md +++ b/doc/requirements-and-limitations.md @@ -20,6 +20,8 @@ The `SUPER` privilege is required for `STOP SLAVE`, `START SLAVE` operations. Th - Switching your `binlog_format` to `ROW`, in the case where it is _not_ `ROW` and you explicitly specified `--switch-to-rbr` - If your replication is already in RBR (`binlog_format=ROW`) you can specify `--assume-rbr` to avoid the `STOP SLAVE/START SLAVE` operations, hence no need for `SUPER`. +- `gh-ost` uses the `REPEATABLE_READ` transaction isolation level for all MySQL connections, regardless of the server default. + - Running `--test-on-replica`: before the cut-over phase, `gh-ost` stops replication so that you can compare the two tables and satisfy that the migration is sound. ### Limitations diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 3a2052c..b99e70b 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -181,7 +181,7 @@ func main() { } if migrationContext.AlterStatement == "" { - log.Fatalf("--alter must be provided and statement must not be empty") + log.Fatal("--alter must be provided and statement must not be empty") } parser := sql.NewParserFromAlterStatement(migrationContext.AlterStatement) migrationContext.AlterStatementOptions = parser.GetAlterStatementOptions() @@ -190,7 +190,7 @@ func main() { if parser.HasExplicitSchema() { migrationContext.DatabaseName = parser.GetExplicitSchema() } else { - log.Fatalf("--database must be provided and database name must not be empty, or --alter must specify database name") + log.Fatal("--database must be provided and database name must not be empty, or --alter must specify database name") } } @@ -202,48 +202,48 @@ func main() { if parser.HasExplicitTable() { migrationContext.OriginalTableName = parser.GetExplicitTable() } else { - log.Fatalf("--table must be provided and table name must not be empty, or --alter must specify table name") + log.Fatal("--table must be provided and table name must not be empty, or --alter must specify table name") } } migrationContext.Noop = !(*executeFlag) if migrationContext.AllowedRunningOnMaster && migrationContext.TestOnReplica { - migrationContext.Log.Fatalf("--allow-on-master and --test-on-replica are mutually exclusive") + migrationContext.Log.Fatal("--allow-on-master and --test-on-replica are mutually exclusive") } if migrationContext.AllowedRunningOnMaster && migrationContext.MigrateOnReplica { - migrationContext.Log.Fatalf("--allow-on-master and --migrate-on-replica are mutually exclusive") + migrationContext.Log.Fatal("--allow-on-master and --migrate-on-replica are mutually exclusive") } if migrationContext.MigrateOnReplica && migrationContext.TestOnReplica { - migrationContext.Log.Fatalf("--migrate-on-replica and --test-on-replica are mutually exclusive") + migrationContext.Log.Fatal("--migrate-on-replica and --test-on-replica are mutually exclusive") } if migrationContext.SwitchToRowBinlogFormat && migrationContext.AssumeRBR { - migrationContext.Log.Fatalf("--switch-to-rbr and --assume-rbr are mutually exclusive") + migrationContext.Log.Fatal("--switch-to-rbr and --assume-rbr are mutually exclusive") } if migrationContext.TestOnReplicaSkipReplicaStop { if !migrationContext.TestOnReplica { - migrationContext.Log.Fatalf("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled") + migrationContext.Log.Fatal("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled") } migrationContext.Log.Warning("--test-on-replica-skip-replica-stop enabled. We will not stop replication before cut-over. Ensure you have a plugin that does this.") } if migrationContext.CliMasterUser != "" && migrationContext.AssumeMasterHostname == "" { - migrationContext.Log.Fatalf("--master-user requires --assume-master-host") + migrationContext.Log.Fatal("--master-user requires --assume-master-host") } if migrationContext.CliMasterPassword != "" && migrationContext.AssumeMasterHostname == "" { - migrationContext.Log.Fatalf("--master-password requires --assume-master-host") + migrationContext.Log.Fatal("--master-password requires --assume-master-host") } if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS { - migrationContext.Log.Fatalf("--ssl-ca requires --ssl") + migrationContext.Log.Fatal("--ssl-ca requires --ssl") } if migrationContext.TLSCertificate != "" && !migrationContext.UseTLS { - migrationContext.Log.Fatalf("--ssl-cert requires --ssl") + migrationContext.Log.Fatal("--ssl-cert requires --ssl") } if migrationContext.TLSKey != "" && !migrationContext.UseTLS { - migrationContext.Log.Fatalf("--ssl-key requires --ssl") + migrationContext.Log.Fatal("--ssl-key requires --ssl") } if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS { - migrationContext.Log.Fatalf("--ssl-allow-insecure requires --ssl") + migrationContext.Log.Fatal("--ssl-allow-insecure requires --ssl") } if *replicationLagQuery != "" { - migrationContext.Log.Warningf("--replication-lag-query is deprecated") + migrationContext.Log.Warning("--replication-lag-query is deprecated") } switch *cutOver { diff --git a/go/mysql/connection.go b/go/mysql/connection.go index 1c24a34..6a5c890 100644 --- a/go/mysql/connection.go +++ b/go/mysql/connection.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ @@ -12,12 +12,14 @@ import ( "fmt" "io/ioutil" "net" + "strings" "github.com/go-sql-driver/mysql" ) const ( - TLS_CONFIG_KEY = "ghost" + transactionIsolation = "REPEATABLE-READ" + TLS_CONFIG_KEY = "ghost" ) // ConnectionConfig is the minimal configuration required to connect to a MySQL server @@ -112,12 +114,23 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string { // Wrap IPv6 literals in square brackets hostname = fmt.Sprintf("[%s]", hostname) } - interpolateParams := true + // go-mysql-driver defaults to false if tls param is not provided; explicitly setting here to // simplify construction of the DSN below. tlsOption := "false" if this.tlsConfig != nil { tlsOption = TLS_CONFIG_KEY } - return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?timeout=%fs&readTimeout=%fs&writeTimeout=%fs&interpolateParams=%t&autocommit=true&charset=utf8mb4,utf8,latin1&tls=%s", this.User, this.Password, hostname, this.Key.Port, databaseName, this.Timeout, this.Timeout, this.Timeout, interpolateParams, tlsOption) + connectionParams := []string{ + "autocommit=true", + "charset=utf8mb4,utf8,latin1", + "interpolateParams=true", + fmt.Sprintf("tls=%s", tlsOption), + fmt.Sprintf("transaction_isolation=%q", transactionIsolation), + fmt.Sprintf("timeout=%fs", this.Timeout), + fmt.Sprintf("readTimeout=%fs", this.Timeout), + fmt.Sprintf("writeTimeout=%fs", this.Timeout), + } + + return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?%s", this.User, this.Password, hostname, this.Key.Port, databaseName, strings.Join(connectionParams, "&")) } diff --git a/go/mysql/connection_test.go b/go/mysql/connection_test.go index f9c45de..390774c 100644 --- a/go/mysql/connection_test.go +++ b/go/mysql/connection_test.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ @@ -67,9 +67,10 @@ func TestGetDBUri(t *testing.T) { c.Key = InstanceKey{Hostname: "myhost", Port: 3306} c.User = "gromit" c.Password = "penguin" + c.Timeout = 1.2345 uri := c.GetDBUri("test") - test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?timeout=0.000000s&readTimeout=0.000000s&writeTimeout=0.000000s&interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=false") + test.S(t).ExpectEquals(uri, `gromit:penguin@tcp(myhost:3306)/test?autocommit=true&charset=utf8mb4,utf8,latin1&interpolateParams=true&tls=false&transaction_isolation="REPEATABLE-READ"&timeout=1.234500s&readTimeout=1.234500s&writeTimeout=1.234500s`) } func TestGetDBUriWithTLSSetup(t *testing.T) { @@ -77,8 +78,9 @@ func TestGetDBUriWithTLSSetup(t *testing.T) { c.Key = InstanceKey{Hostname: "myhost", Port: 3306} c.User = "gromit" c.Password = "penguin" + c.Timeout = 1.2345 c.tlsConfig = &tls.Config{} uri := c.GetDBUri("test") - test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?timeout=0.000000s&readTimeout=0.000000s&writeTimeout=0.000000s&interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=ghost") + test.S(t).ExpectEquals(uri, `gromit:penguin@tcp(myhost:3306)/test?autocommit=true&charset=utf8mb4,utf8,latin1&interpolateParams=true&tls=ghost&transaction_isolation="REPEATABLE-READ"&timeout=1.234500s&readTimeout=1.234500s&writeTimeout=1.234500s`) }