MySqlConnector
MySqlConnector copied to clipboard
Transaction isolation level should not default to `REPEATABLE READ` regardless of server configuration if left unspecified
Software versions
MySqlConnector version: 2.1.6, but still relevant on latest master as far as I can tell
Server type (MySQL, MariaDB, Aurora, etc.) and version: Not relevant
.NET version: Not relevant
Describe the bug
If a transaction is started without specifying an isolation level, it will silently default to IsolationLevel.Unspecified, which is really just REPEATABLE READ:
https://github.com/mysql-net/MySqlConnector/blob/999a3ab913a0112b5819f57f603b109e9c435e6f/src/MySqlConnector/MySqlConnection.cs#L155-L156
The comment is correct in stating that REPEATABLE READ is the default innodb transaction isolation level, however, this is confusing in cases where the global transaction isolation level on the mysql instance that queries are directed to is deliberately changed from the configuration default.
Related: https://github.com/mysql-net/MySqlConnector/issues/772
Exception Not applicable
Code sample
See usage. On a mysql transaction with READ COMMITTED default transaction isolation level this would instead force REPEATABLE READ and as such caused a long investigation into ensuing deadlocks, that was "fixed" by https://github.com/ppy/osu-queue-score-statistics/commit/5c23e0f02b67c6089922ea553ce03894f2690011.
Expected behavior
MySqlConnector does not attempt to explicitly set any transaction isolation level when starting a transaction with IsolationLevel.Unspecified.
Additional context Not applicable
I think this comment explains the current behaviour: https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/1043#issuecomment-599817149
It's "a" default level, but there's absolutely no guarantee that the server hasn't been configured with a different default level. So it should be issued every time so that ADO.NET semantics are followed, regardless of server defaults.
And https://learn.microsoft.com/en-us/dotnet/api/system.data.common.dbconnection.begintransaction?view=net-8.0#remarks:
If you do not specify an isolation level, the default isolation level for the specific type of connection is used.
IMO, the documentation doesn't state or imply that an implicit server-set default will be used if not specified by the client.
I'm a little reticent to change this because
- it's been this way for a very long time (since 783645f52d31c78edd0800cbf7ce7d3917544082) and would be a silent breaking change for almost all users of the library
- it has a very simple workaround: explicitly specify the isolation level you want
I can understand that this would be a pretty big API break and the use case is likely niche, but I hope it is also understandable that the current behaviour results in a rather unexpected and confusing outcome. Maybe at least a remark in documentation could be added for posterity?
Yes, this should certainly be clearly documented.
FWIW, MySql.Data also defaults to REPEATABLE READ: https://github.com/mysql/mysql-connector-net/blob/0bb0e8c4d9f327138e758b6a31c3d8ebb70a2eae/MySQL.Data/src/MySqlConnection.cs#L490-L492