Skip to content

Commit d2fefe8

Browse files
committed
add test and improve doc
1 parent 83d56b7 commit d2fefe8

File tree

3 files changed

+63
-19
lines changed

3 files changed

+63
-19
lines changed

Diff for: README.md

+14-12
Original file line numberDiff line numberDiff line change
@@ -276,18 +276,20 @@ Default: false
276276
```
277277

278278

279-
RejectreadOnly causes mysql driver to reject read-only connections. This is
280-
specifically for AWS Aurora: During a failover, there seems to be a race
281-
condition on Aurora, where we get connected to the [old master before
282-
failover], i.e. the [new read-only slave after failover].
283-
284-
Note that this should be a fairly rare case, as automatic failover normally
285-
happens when master is down, and the race condition shouldn't happen unless it
286-
comes back up online as soon as the failover is kicked off. But it's pretty
287-
easy to reproduce using a manual failover. In case this happens, we should
288-
reconnect to the Aurora cluster by returning a driver.ErrBadConnection.
289-
290-
tl;dr: Set this if you are using Aurora.
279+
RejectreadOnly causes the driver to reject read-only connections. This is for a
280+
possible race condition during an automatic failover, where the mysql client
281+
gets connected to a read-only replica after the failover.
282+
283+
Note that this should be a fairly rare case, as an automatic failover normally
284+
happens when the primary is down, and the race condition shouldn't happen
285+
unless it comes back up online as soon as the failover is kicked off. On the
286+
other hand, when this happens, an mysql application can get stuck on a
287+
read-only connection until restarted. It is however fairly easy to reproduce,
288+
for example, using a manual failover on AWS Aurora's MySQL-compatible cluster.
289+
290+
If you are not relying on read-only transactions to reject writes that aren't
291+
supposed to happen, setting this on some MySQL providers (such as AWS Aurora)
292+
is safer for failovers.
291293

292294

293295
##### `strict`

Diff for: driver_test.go

+47-5
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,17 @@ func (dbt *DBTest) mustQuery(query string, args ...interface{}) (rows *sql.Rows)
171171
return rows
172172
}
173173

174+
func maybeSkip(t *testing.T, err error, skipErrno uint16) {
175+
mySQLErr, ok := err.(*MySQLError)
176+
if !ok {
177+
return
178+
}
179+
180+
if mySQLErr.Number == skipErrno {
181+
t.Skipf("skipping test for error: %v", err)
182+
}
183+
}
184+
174185
func TestEmptyQuery(t *testing.T) {
175186
runTests(t, dsn, func(dbt *DBTest) {
176187
// just a comment, no query
@@ -1168,11 +1179,9 @@ func TestStrict(t *testing.T) {
11681179
if conn != nil {
11691180
conn.Close()
11701181
}
1171-
if me, ok := err.(*MySQLError); ok && me.Number == 1231 {
1172-
// Error 1231: Variable 'sql_mode' can't be set to the value of 'ALLOW_INVALID_DATES'
1173-
// => skip test, MySQL server version is too old
1174-
return
1175-
}
1182+
// Error 1231: Variable 'sql_mode' can't be set to the value of
1183+
// 'ALLOW_INVALID_DATES' => skip test, MySQL server version is too old
1184+
maybeSkip(t, err, 1231)
11761185
runTests(t, relaxedDsn, func(dbt *DBTest) {
11771186
dbt.mustExec("CREATE TABLE test (a TINYINT NOT NULL, b CHAR(4))")
11781187

@@ -1949,3 +1958,36 @@ func TestColumnsReusesSlice(t *testing.T) {
19491958
t.Fatalf("expected columnNames to be set, got nil")
19501959
}
19511960
}
1961+
1962+
func TestRejectReadOnly(t *testing.T) {
1963+
runTests(t, dsn, func(dbt *DBTest) {
1964+
// Create Table
1965+
dbt.mustExec("CREATE TABLE test (value BOOL)")
1966+
// Set the session to read-only. We didn't set the `rejectReadOnly`
1967+
// option, so any writes after this should fail.
1968+
_, err := dbt.db.Exec("SET SESSION TRANSACTION READ ONLY")
1969+
// Error 1193: Unknown system variable 'TRANSACTION' => skip test,
1970+
// MySQL server version is too old
1971+
maybeSkip(t, err, 1193)
1972+
if _, err := dbt.db.Exec("DROP TABLE test"); err == nil {
1973+
t.Fatalf("writing to DB in read-only session without " +
1974+
"rejectReadOnly did not error")
1975+
}
1976+
// Set the session back to read-write so runTests() can properly clean
1977+
// up the table `test`.
1978+
dbt.mustExec("SET SESSION TRANSACTION READ WRITE")
1979+
})
1980+
1981+
// Enable the `rejectReadOnly` option.
1982+
runTests(t, dsn+"&rejectReadOnly=true", func(dbt *DBTest) {
1983+
// Create Table
1984+
dbt.mustExec("CREATE TABLE test (value BOOL)")
1985+
// Set the session to read only. Any writes after this should error on
1986+
// a driver.ErrBadConn, and cause `database/sql` to initiate a new
1987+
// connection.
1988+
dbt.mustExec("SET SESSION TRANSACTION READ ONLY")
1989+
// This would error, but `database/sql` should automatically retry on a
1990+
// new connection which is not read-only, and eventually succeed.
1991+
dbt.mustExec("DROP TABLE test")
1992+
})
1993+
}

Diff for: packets.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -556,8 +556,8 @@ func (mc *mysqlConn) handleErrorPacket(data []byte) error {
556556
// Oops; we are connected to a read-only connection, and won't be able
557557
// to issue any write statements. Since RejectReadOnly is configured,
558558
// we throw away this connection hoping this one would have write
559-
// permission. This is specifically for an AWS Aurora behavior during
560-
// failover. See README.md for more.
559+
// permission. This is specifically for a possible race condition
560+
// during failover (e.g. on AWS Aurora). See README.md for more.
561561
//
562562
// We explicitly close the connection before returning
563563
// driver.ErrBadConn to ensure that `database/sql` purges this

0 commit comments

Comments
 (0)