block / mysql / 34064861637

85%

Build Type

push

github

Committed by web-flow

Commit Message

Always reject read-only connections (#5)

* Always reject read-only connections

Upstream makes this an option (rejectReadOnly) defaulting to off. It is
unconditional here, and Config.RejectReadOnly is gone.

The failure it prevents is silent, and the mistake that causes it is invisible.
RDS and Aurora fail over by moving DNS: a pooled connection to the demoted
writer stays open and stays usable, and every write on it fails for as long as
the pool keeps it — potentially until the process restarts. Nothing in the DSN
or in the error says the connection is the problem, so a deployment that left
the option off does not find out until a failover, which is the worst possible
moment to learn it. An option whose only correct setting in the environment we
deploy in is "on" is not really an option; it is a step you can forget.

What happens to the parameter: rejectReadOnly=true still parses and does
nothing, so a DSN written for upstream keeps working. rejectReadOnly=false is
an error rather than a silent no-op — it states an expectation the driver will
not meet, and quietly ignoring it is the same class of problem this change
exists to remove.

One carve-out, which upstream's own test suite found: a transaction opened with
driver.TxOptions.ReadOnly is exempt. There the read-only error is the answer
the caller asked for, and database/sql does not retry inside a transaction, so
rejecting would replace a usable *MySQLError with a dead transaction —
TestContextBeginReadOnly failed exactly that way before the exemption, and
passes unmodified with it. A session the application makes read-only with its
own SET is deliberately not exempt: nothing distinguishes it from a demoted
writer.

Upstream's TestRejectReadOnly loses the case where the option is off, since
that is no longer a state the driver can be in; the first case now covers a DSN
that says nothing, which is where the old default did the wrong thing.

Full suite passes against MySQL 8.0.44, race enabled.

* ... (continued)

Coverage Stats

45 of 45 new or added lines in 4 files covered. (100.0%)

1 existing line in 1 file now uncovered.

3617 of 4267 relevant lines covered (84.77%)

328350.56 hits per line