Skip to content

Commit 4820212

Browse files
timvaillancourttimvaillancourt
committed
Set transaction isolation in connections
1 parent 7d8e4e8 commit 4820212

File tree

5 files changed

+42
-27
lines changed

5 files changed

+42
-27
lines changed

‎doc/requirements-and-limitations.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ The `SUPER` privilege is required for `STOP SLAVE`, `START SLAVE` operations. Th
2020
- Switching your `binlog_format` to `ROW`, in the case where it is _not_ `ROW` and you explicitly specified `--switch-to-rbr`
2121
- 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`.
2222

23+
- `gh-ost` uses the `REPEATABLE_READ` transaction isolation level for all MySQL connections, regardless of the server default.
24+
2325
- 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.
2426

2527
### Limitations

‎go/base/load_map.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func NewLoadMap() LoadMap {
2323

2424
// NewLoadMap parses a `--*-load` flag (e.g. `--max-load`), which is in multiple
2525
// key-value format, such as:
26-
// 'Threads_running=100,Threads_connected=500'
26+
// 'Threads_running=100,Threads_connected=500'
2727
func ParseLoadMap(loadList string) (LoadMap, error) {
2828
result := NewLoadMap()
2929
if loadList == "" {

‎go/cmd/gh-ost/main.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func main() {
180180
}
181181

182182
if migrationContext.AlterStatement == "" {
183-
log.Fatalf("--alter must be provided and statement must not be empty")
183+
log.Fatal("--alter must be provided and statement must not be empty")
184184
}
185185
parser := sql.NewParserFromAlterStatement(migrationContext.AlterStatement)
186186
migrationContext.AlterStatementOptions = parser.GetAlterStatementOptions()
@@ -189,7 +189,7 @@ func main() {
189189
if parser.HasExplicitSchema() {
190190
migrationContext.DatabaseName = parser.GetExplicitSchema()
191191
} else {
192-
log.Fatalf("--database must be provided and database name must not be empty, or --alter must specify database name")
192+
log.Fatal("--database must be provided and database name must not be empty, or --alter must specify database name")
193193
}
194194
}
195195

@@ -201,48 +201,48 @@ func main() {
201201
if parser.HasExplicitTable() {
202202
migrationContext.OriginalTableName = parser.GetExplicitTable()
203203
} else {
204-
log.Fatalf("--table must be provided and table name must not be empty, or --alter must specify table name")
204+
log.Fatal("--table must be provided and table name must not be empty, or --alter must specify table name")
205205
}
206206
}
207207
migrationContext.Noop = !(*executeFlag)
208208
if migrationContext.AllowedRunningOnMaster && migrationContext.TestOnReplica {
209-
migrationContext.Log.Fatalf("--allow-on-master and --test-on-replica are mutually exclusive")
209+
migrationContext.Log.Fatal("--allow-on-master and --test-on-replica are mutually exclusive")
210210
}
211211
if migrationContext.AllowedRunningOnMaster && migrationContext.MigrateOnReplica {
212-
migrationContext.Log.Fatalf("--allow-on-master and --migrate-on-replica are mutually exclusive")
212+
migrationContext.Log.Fatal("--allow-on-master and --migrate-on-replica are mutually exclusive")
213213
}
214214
if migrationContext.MigrateOnReplica && migrationContext.TestOnReplica {
215-
migrationContext.Log.Fatalf("--migrate-on-replica and --test-on-replica are mutually exclusive")
215+
migrationContext.Log.Fatal("--migrate-on-replica and --test-on-replica are mutually exclusive")
216216
}
217217
if migrationContext.SwitchToRowBinlogFormat && migrationContext.AssumeRBR {
218-
migrationContext.Log.Fatalf("--switch-to-rbr and --assume-rbr are mutually exclusive")
218+
migrationContext.Log.Fatal("--switch-to-rbr and --assume-rbr are mutually exclusive")
219219
}
220220
if migrationContext.TestOnReplicaSkipReplicaStop {
221221
if !migrationContext.TestOnReplica {
222-
migrationContext.Log.Fatalf("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled")
222+
migrationContext.Log.Fatal("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled")
223223
}
224224
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.")
225225
}
226226
if migrationContext.CliMasterUser != "" && migrationContext.AssumeMasterHostname == "" {
227-
migrationContext.Log.Fatalf("--master-user requires --assume-master-host")
227+
migrationContext.Log.Fatal("--master-user requires --assume-master-host")
228228
}
229229
if migrationContext.CliMasterPassword != "" && migrationContext.AssumeMasterHostname == "" {
230-
migrationContext.Log.Fatalf("--master-password requires --assume-master-host")
230+
migrationContext.Log.Fatal("--master-password requires --assume-master-host")
231231
}
232232
if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS {
233-
migrationContext.Log.Fatalf("--ssl-ca requires --ssl")
233+
migrationContext.Log.Fatal("--ssl-ca requires --ssl")
234234
}
235235
if migrationContext.TLSCertificate != "" && !migrationContext.UseTLS {
236-
migrationContext.Log.Fatalf("--ssl-cert requires --ssl")
236+
migrationContext.Log.Fatal("--ssl-cert requires --ssl")
237237
}
238238
if migrationContext.TLSKey != "" && !migrationContext.UseTLS {
239-
migrationContext.Log.Fatalf("--ssl-key requires --ssl")
239+
migrationContext.Log.Fatal("--ssl-key requires --ssl")
240240
}
241241
if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS {
242-
migrationContext.Log.Fatalf("--ssl-allow-insecure requires --ssl")
242+
migrationContext.Log.Fatal("--ssl-allow-insecure requires --ssl")
243243
}
244244
if *replicationLagQuery != "" {
245-
migrationContext.Log.Warningf("--replication-lag-query is deprecated")
245+
migrationContext.Log.Warning("--replication-lag-query is deprecated")
246246
}
247247

248248
switch *cutOver {

‎go/mysql/connection.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2016 GitHub Inc.
2+
Copyright 2022 GitHub Inc.
33
See https://github.com/github/gh-ost/blob/master/LICENSE
44
*/
55

@@ -12,12 +12,14 @@ import (
1212
"fmt"
1313
"io/ioutil"
1414
"net"
15+
"strings"
1516

1617
"github.com/go-sql-driver/mysql"
1718
)
1819

1920
const (
20-
TLS_CONFIG_KEY = "ghost"
21+
transactionIsolation = "REPEATABLE-READ"
22+
TLS_CONFIG_KEY = "ghost"
2123
)
2224

2325
// ConnectionConfig is the minimal configuration required to connect to a MySQL server
@@ -112,12 +114,21 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string {
112114
// Wrap IPv6 literals in square brackets
113115
hostname = fmt.Sprintf("[%s]", hostname)
114116
}
115-
interpolateParams := true
116-
// go-mysql-driver defaults to false if tls param is not provided; explicitly setting here to
117-
// simplify construction of the DSN below.
118-
tlsOption := "false"
117+
118+
tls := "false"
119119
if this.tlsConfig != nil {
120-
tlsOption = TLS_CONFIG_KEY
120+
tls = TLS_CONFIG_KEY
121+
}
122+
connectionParams := []string{
123+
"autocommit=true",
124+
"charset=utf8mb4,utf8,latin1",
125+
"interpolateParams=true",
126+
fmt.Sprintf("tls=%s", tls),
127+
fmt.Sprintf("transaction_isolation=%q", transactionIsolation),
128+
fmt.Sprintf("timeout=%fs", this.Timeout),
129+
fmt.Sprintf("readTimeout=%fs", this.Timeout),
130+
fmt.Sprintf("writeTimeout=%fs", this.Timeout),
121131
}
122-
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)
132+
133+
return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?%s", this.User, this.Password, hostname, this.Key.Port, databaseName, strings.Join(connectionParams, "&"))
123134
}

‎go/mysql/connection_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2016 GitHub Inc.
2+
Copyright 2022 GitHub Inc.
33
See https://github.com/github/gh-ost/blob/master/LICENSE
44
*/
55

@@ -67,18 +67,20 @@ func TestGetDBUri(t *testing.T) {
6767
c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
6868
c.User = "gromit"
6969
c.Password = "penguin"
70+
c.Timeout = 1.2345
7071

7172
uri := c.GetDBUri("test")
72-
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")
73+
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`)
7374
}
7475

7576
func TestGetDBUriWithTLSSetup(t *testing.T) {
7677
c := NewConnectionConfig()
7778
c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
7879
c.User = "gromit"
7980
c.Password = "penguin"
81+
c.Timeout = 1.2345
8082
c.tlsConfig = &tls.Config{}
8183

8284
uri := c.GetDBUri("test")
83-
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")
85+
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`)
8486
}

0 commit comments

Comments
 (0)