Browse Source

Workaround for changed deadlock handling in MySQL 5.7+

According to the MySQL docs, when a deadlock error occurs, the
transaction is rolled back but the BEGIN statement is not
cancelled. This is the case up to MySQL 5.6 but in 5.7+, the
BEGIN statement needs to be executed again when restarting a
transaction. The extra BEGIN has no effect on versions where it's
not needed.
Viktor Söderqvist 5 years ago
parent
commit
a0cd93362c
3 changed files with 19 additions and 21 deletions
  1. 13 0
      src/mysql.erl
  2. 3 2
      src/mysql_conn.erl
  3. 3 19
      test/transaction_tests.erl

+ 13 - 0
src/mysql.erl

@@ -622,6 +622,11 @@ transaction(Conn, Fun, Args, Retries) when is_list(Args),
 %% rollback does not cancel that statement."
 %% (https://dev.mysql.com/doc/refman/5.6/en/innodb-error-handling.html)
 %%
+%% This seems to have changed in MySQL 5.7.x though (although the MySQL
+%% documentation hasn't been updated). Now, also the BEGIN is cancelled, so a
+%% new BEGIN has to be issued when restarting the transaction. This has no
+%% effect on older versions, not even a warning.
+%%
 %% Lock Wait Timeout:
 %% "InnoDB rolls back only the last statement on a transaction timeout by
 %% default. If --innodb_rollback_on_timeout is specified, a transaction timeout
@@ -638,15 +643,23 @@ execute_transaction(Conn, Fun, Args, Retries) ->
         %% retries left
         ?EXCEPTION(throw, {implicit_rollback, 1, _}, _Stacktrace)
           when Retries == infinity ->
+            %% In MySQL < 5.7 we're not in a transaction here, but in earlier
+            %% versions we are, so we can't use `gen_server:call(Conn,
+            %% start_transaction, infinity)' here.
+            ok = query(Conn, <<"BEGIN">>),
             execute_transaction(Conn, Fun, Args, infinity);
         ?EXCEPTION(throw, {implicit_rollback, 1, _}, _Stacktrace)
           when Retries > 0 ->
+            ok = query(Conn, <<"BEGIN">>),
             execute_transaction(Conn, Fun, Args, Retries - 1);
         ?EXCEPTION(throw, {implicit_rollback, 1, Reason}, Stacktrace)
           when Retries == 0 ->
             %% No more retries. Return 'aborted' along with the deadlock error
             %% and a the trace to the line where the deadlock occured.
             Trace = ?GET_STACK(Stacktrace),
+            %% In MySQL < 5.7, we are still in a transaction here, but in 5.7+
+            %% we're not.  The ROLLBACK executed here has no effect if no
+            %% transaction is ongoing.
             ok = gen_server:call(Conn, rollback, infinity),
             {aborted, {Reason, Trace}};
         ?EXCEPTION(throw, {implicit_rollback, N, Reason}, Stacktrace)

+ 3 - 2
src/mysql_conn.erl

@@ -677,8 +677,9 @@ handle_query_call_result([Rec|Recs], RecNum, Query,
         #error{code = ?ERROR_DEADLOCK} when L /= [] ->
             %% These errors result in an implicit rollback.
             Reply = {implicit_rollback, length(L), error_to_reason(Rec)},
-            %% Everything in the transaction is rolled back, except the BEGIN
-            %% statement itself. Thus, we are in transaction level 1.
+            %% The transaction is rollbacked, except the BEGIN, so we're still
+            %% in a transaction.  (In 5.7+, also the BEGIN has been rolled back,
+            %% but here we assume the old behaviour.)
             NewMonitors = demonitor_processes(L, length(L) - 1),
             {Reply, State#state{transaction_levels = NewMonitors}};
         #error{} ->

+ 3 - 19
test/transaction_tests.erl

@@ -203,30 +203,14 @@ deadlock_test_() ->
          ok = mysql:query(Conn1, "INSERT INTO foo (k,v) VALUES (1,0), (2,0)"),
          {ok, Conn2} = mysql:start_link([{user, ?user}, {password, ?password}]),
          ok = mysql:query(Conn2, <<"USE otptest">>),
-         {ok, [_], [[VersionBin]]} = mysql:query(Conn1, "SELECT @@version"),
-         {Conn1, Conn2, VersionBin}
+         {Conn1, Conn2}
      end,
-     fun ({Conn1, Conn2, _VersionBin}) ->
+     fun ({Conn1, Conn2}) ->
          ok = mysql:query(Conn1, <<"DROP DATABASE otptest">>, 1000),
          mysql:stop(Conn1),
          mysql:stop(Conn2)
      end,
-     fun ({_Conn1, _Conn2, <<"5.7.", _/binary>>}) ->
-             fun () ->
-                error_logger:info_msg(
-                    "The deadlock test fails in some MySQL versions so it is "
-                    "currently disabled for MySQL 5.7.x. TODO: Confirm if "
-                    "there is a bug or a changed behavior for this scenario.")
-             end;
-         ({_Conn1, _Conn2, <<"8.", _/binary>>}) ->
-             fun () ->
-                error_logger:info_msg(
-                    "The deadlock test fails in some MySQL versions so it is "
-                    "currently disabled for MySQL 8.x.y. TODO: Confirm if "
-                    "there is a bug or a changed behavior for this scenario.")
-             end;
-         ({Conn1, Conn2, _VersionBin}) ->
-             Conns = {Conn1, Conn2},
+     fun (Conns) ->
              [{"Plain queries", fun () -> deadlock_plain_queries(Conns) end},
               {"Prep stmts", fun () -> deadlock_prepared_statements(Conns) end},
               {"No retry", fun () -> deadlock_no_retry(Conns) end},