Return an error on deadlock if all retries fail in a transaction

include/exception.hrl

@@ -1,9 +1,7 @@
 -ifdef(OTP_RELEASE). %% this implies 21 or higher
 -define(EXCEPTION(Class, Reason, Stacktrace), Class:Reason:Stacktrace).
--define(EXCEPTION(Class, Reason, Stacktrace, N), Class:Reason:Stacktrace when N >= 1).
 -define(GET_STACK(Stacktrace), Stacktrace).
 -else.
 -define(EXCEPTION(Class, Reason, _), Class:Reason).
--define(EXCEPTION(Class, Reason, _, N), Class:Reason when N >= 1).
 -define(GET_STACK(_), erlang:get_stacktrace()).
 -endif.

src/mysql.erl

@@ -1,5 +1,5 @@
 %% MySQL/OTP – MySQL client library for Erlang/OTP
-%% Copyright (C) 2014-2015 Viktor Söderqvist,
+%% Copyright (C) 2014-2015, 2018 Viktor Söderqvist,
 %%               2016 Johan Lövdahl
 %%               2017 Piotr Nosek, Michal Slaski
 %%
@@ -25,8 +25,6 @@
 %% gen_server is locally registered.
 -module(mysql).
 
--include("exception.hrl").
-
 -export([start_link/1, query/2, query/3, query/4, execute/3, execute/4,
          prepare/2, prepare/3, unprepare/2,
          warning_count/1, affected_rows/1, autocommit/1, insert_id/1,
@@ -72,6 +70,8 @@
                       | {ok, [{column_names(), rows()}, ...]}
                       | {error, server_reason()}.
 
+-include("exception.hrl").
+
 %% @doc Starts a connection gen_server process and connects to a database. To
 %% disconnect just do `exit(Pid, normal)'.
 %%
@@ -314,8 +314,7 @@ insert_id(Conn) ->
 
 %% @doc Returns true if the connection is in a transaction and false otherwise.
 %% This works regardless of whether the transaction has been started using
-%% transaction/2,3 or using a plain `mysql:query(Connection, "START
-%% TRANSACTION")'.
+%% transaction/2,3 or using a plain `mysql:query(Connection, "BEGIN")'.
 %% @see transaction/2
 %% @see transaction/4
 -spec in_transaction(connection()) -> boolean().
@@ -352,10 +351,14 @@ transaction(Conn, Fun, Retries) ->
 %%
 %% Note that an error response from a query does not cause a transaction to be
 %% rollbacked. To force a rollback on a MySQL error you can trigger a `badmatch'
-%% using e.g. `ok = mysql:query(Pid, "SELECT some_non_existent_value")'.
-%% Exceptions to this are error 1213 "Deadlock" (after the specified number
-%% retries all have failed) and error 1205 "Lock wait timeout" which causes an
-%% *implicit rollback*.
+%% using e.g. `ok = mysql:query(Pid, "SELECT some_non_existent_value")'. An
+%% exception to this is the error 1213 "Deadlock", after the specified number
+%% of retries, all failed. In this case, the transaction is aborted and the
+%% error is retured as the reason for the aborted transaction, along with a
+%% stacktrace pointing to where the last deadlock was detected. (In earlier
+%% versions, up to and including 1.3.2, transactions where automatically
+%% restarted also for the error 1205 "Lock wait timeout". This is no longer the
+%% case.)
 %%
 %% Some queries such as ALTER TABLE cause an *implicit commit* on the server.
 %% If such a query is executed within a transaction, an error on the form
@@ -386,6 +389,23 @@ transaction(Conn, Fun, Args, Retries) when is_list(Args),
     ok = gen_server:call(Conn, start_transaction, infinity),
     execute_transaction(Conn, Fun, Args, Retries).
 
+%% @private
+%% @doc This is a helper for transaction/2,3,4. It performs everything except
+%% executing the BEGIN statement. It is called recursively when a transaction
+%% is retried.
+%%
+%% "When a transaction rollback occurs due to a deadlock or lock wait timeout,
+%% it cancels the effect of the statements within the transaction. But if the
+%% start-transaction statement was START TRANSACTION or BEGIN statement,
+%% rollback does not cancel that statement."
+%% (https://dev.mysql.com/doc/refman/5.6/en/innodb-error-handling.html)
+%%
+%% 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
+%% causes InnoDB to abort and roll back the entire transaction (the same
+%% behavior as in MySQL 4.1)."
+%% (https://dev.mysql.com/doc/refman/5.6/en/innodb-parameters.html)
 execute_transaction(Conn, Fun, Args, Retries) ->
     try apply(Fun, Args) of
         ResultOfFun ->
@@ -394,11 +414,22 @@ execute_transaction(Conn, Fun, Args, Retries) ->
     catch
         %% We are at the top level, try to restart the transaction if there are
         %% retries left
-        ?EXCEPTION(throw, {implicit_rollback, 1, _}, _Stacktrace) when Retries =:= infinity ->
+        ?EXCEPTION(throw, {implicit_rollback, 1, _}, _Stacktrace)
+          when Retries == infinity ->
             execute_transaction(Conn, Fun, Args, infinity);
-        ?EXCEPTION(throw, {implicit_rollback, 1, _}, _Stacktrace) when Retries > 0 ->
+        ?EXCEPTION(throw, {implicit_rollback, 1, _}, _Stacktrace)
+          when Retries > 0 ->
             execute_transaction(Conn, Fun, Args, Retries - 1);
-        ?EXCEPTION(throw, {implicit_rollback, N, Reason}, Stacktrace, N) ->
+        ?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),
+            ok = gen_server:call(Conn, rollback, infinity),
+            {aborted, {Reason, Trace}};
+        ?EXCEPTION(throw, {implicit_rollback, N, Reason}, Stacktrace)
+          when N > 1 ->
+            %% Nested transaction. Bubble out to the outermost level.
             erlang:raise(throw, {implicit_rollback, N - 1, Reason},
                          ?GET_STACK(Stacktrace));
         ?EXCEPTION(error, {implicit_commit, _Query} = E, Stacktrace) ->
@@ -542,13 +573,9 @@ init(Opts) ->
 %%       able to handle this in the caller's process, we also return the
 %%       nesting level.</dd>
 %%   <dt>`{implicit_rollback, NestingLevel, ServerReason}'</dt>
-%%   <dd>These errors result in an implicit rollback:
-%%       <ul>
-%%         <li>`{1205, <<"HY000">>, <<"Lock wait timeout exceeded;
-%%                                     try restarting transaction">>}'</li>
-%%         <li>`{1213, <<"40001">>, <<"Deadlock found when trying to get lock;
-%%                                     try restarting transaction">>}'</li>
-%%       </ul>
+%%   <dd>This errors results in an implicit rollback: `{1213, <<"40001">>,
+%%       <<"Deadlock found when trying to get lock; try restarting "
+%%         "transaction">>}'.
 %%
 %%       If the caller is in a (nested) transaction, it must be aborted. To be
 %%       able to handle this in the caller's process, we also return the
@@ -591,10 +618,9 @@ handle_call({param_query, Query, Params, Timeout}, _From, State) ->
         not_found ->
             %% Prepare
             SockMod:setopts(Socket, [{active, false}]),
-	    SockMod = State#state.sockmod,
+            SockMod = State#state.sockmod,
             Rec = mysql_protocol:prepare(Query, SockMod, Socket),
             SockMod:setopts(Socket, [{active, once}]),
-            %State1 = update_state(Rec, State),
             case Rec of
                 #error{} = E ->
                     {{error, error_to_reason(E)}, Cache};
@@ -773,9 +799,7 @@ terminate(Reason, #state{socket = Socket, sockmod = SockMod})
   when Reason == normal; Reason == shutdown ->
       %% Send the goodbye message for politeness.
       SockMod:setopts(Socket, [{active, false}]),
-      R = mysql_protocol:quit(SockMod, Socket),
-      SockMod:setopts(Socket, [{active, once}]),
-      R;
+      mysql_protocol:quit(SockMod, Socket);
 terminate(_Reason, _State) ->
     ok.
 
@@ -869,13 +893,13 @@ handle_query_call_reply([Rec|Recs], Query, State, ResultSetsAcc) ->
             Names = [Def#col.name || Def <- ColDefs],
             ResultSetsAcc1 = [{Names, Rows} | ResultSetsAcc],
             handle_query_call_reply(Recs, Query, State, ResultSetsAcc1);
-        #error{code = Code} when State#state.transaction_level > 0,
-                                 Code == ?ERROR_DEADLOCK ->
+        #error{code = ?ERROR_DEADLOCK} when State#state.transaction_level > 0 ->
             %% These errors result in an implicit rollback.
             Reply = {implicit_rollback, State#state.transaction_level,
                      error_to_reason(Rec)},
-            State2 = State#state{transaction_level = 1},
-            {reply, Reply, State2};
+            %% Everything in the transaction is rolled back, except the BEGIN
+            %% statement itself. Thus, we are in transaction level 1.
+            {reply, Reply, State#state{transaction_level = 1}};
         #error{} ->
             {reply, {error, error_to_reason(Rec)}, State}
     end.

src/mysql_protocol.erl

@@ -275,6 +275,8 @@ maybe_do_ssl_upgrade(SockModule0, Socket0, SeqNum1, Handshake, SSLOpts,
             exit({failed_to_upgrade_socket, Reason})
     end.
 
+%% @doc This function is used when upgrading to encrypted socket. In other,
+%% cases, build_handshake_response/5 is used.
 -spec build_handshake_response(#handshake{}, iodata() | undefined, boolean()) ->
     binary().
 build_handshake_response(Handshake, Database, SetFoundRows) ->

test/transaction_tests.erl

@@ -141,6 +141,7 @@ deadlock_test_() ->
      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},
           {"Lock wait timeout", fun () -> lock_wait_timeout(Conns) end}]
      end}.
 
@@ -156,18 +157,14 @@ deadlock_plain_queries({Conn1, Conn2}) ->
     Worker2 = spawn_link(fun () ->
         {atomic, ok} = mysql:transaction(Conn2, fun () ->
             MainPid ! start,
-            %?debugMsg("Worker 2: Starting. First get a lock on row 2."),
             ok = mysql:query(Conn2, "UPDATE foo SET v = 2 WHERE k = 2"),
-            %?debugMsg("Worker 2: Got lock on foo. Now wait for signal from 1."),
             %% Sync. Send 'go' to worker 1 multiple times in case it restarts.
             MainPid ! go, MainPid ! go, MainPid ! go,
             receive go -> ok after 10000 -> throw(too_long) end,
-            %?debugMsg("Worker 2: Got signal from 1. Now get a lock on row 1."),
             {atomic, ok} = mysql:transaction(Conn2, fun () ->
                 %% Nested transaction, just to make sure we can handle nested.
                 ok = mysql:query(Conn2, "UPDATE foo SET v = 2 WHERE k = 1")
             end),
-            %?debugMsg("Worker 2: Got both locks and is done."),
             ok
         end),
         MainPid ! done
@@ -176,18 +173,14 @@ deadlock_plain_queries({Conn1, Conn2}) ->
     %% Do worker 1's job and lock the rows in the opposite order.
     {atomic, ok} = mysql:transaction(Conn1, fun () ->
         MainPid ! start,
-        %?debugMsg("Worker 1: Starting. First get a lock on row 1."),
         ok = mysql:query(Conn1, "UPDATE foo SET v = 1 WHERE k = 1"),
-        %?debugMsg("Worker 1: Got lock on bar. Now wait for signal from 2."),
         %% Sync. Send 'go' to worker 2 multiple times in case it restarts.
         Worker2 ! go, Worker2 ! go, Worker2 ! go,
         receive go -> ok after 10000 -> throw(too_long) end,
-        %?debugMsg("Worker 1: Got signal from 2. Now get lock on row 2."),
         {atomic, ok} = mysql:transaction(Conn1, fun () ->
             %% Nested transaction, just to make sure we can handle nested.
             ok = mysql:query(Conn1, "UPDATE foo SET v = 1 WHERE k = 2")
         end),
-        %?debugMsg("Worker 1: Got both locks and is done."),
         ok
     end),
 
@@ -256,6 +249,62 @@ deadlock_prepared_statements({Conn1, Conn2}) ->
     ?assertEqual(ok, receive start -> ok after 0 -> there_was_no_deadlock end),
     flush_inbox().
 
+deadlock_no_retry({Conn1, Conn2}) ->
+    {ok, _, [[2]]} = mysql:query(Conn1, "SELECT COUNT(*) FROM foo"),
+    MainPid = self(),
+    %?debugMsg("\nExtra output from the deadlock test:"),
+
+    %% Spawn worker 2 to lock rows; first in table foo, then in bar.
+    Worker2 = spawn_link(fun () ->
+        Result = mysql:transaction(Conn2, fun () ->
+            MainPid ! start,
+            ok = mysql:query(Conn2, "UPDATE foo SET v = 2 WHERE k = 2"),
+            %% Sync. Send 'go' to worker 1 multiple times in case it restarts.
+            MainPid ! go, MainPid ! go, MainPid ! go,
+            receive go -> ok after 10000 -> throw(too_long) end,
+            {atomic, ok} = mysql:transaction(Conn2, fun () ->
+                %% Nested transaction, just to make sure we can handle nested.
+                ok = mysql:query(Conn2, "UPDATE foo SET v = 2 WHERE k = 1")
+            end),
+            ok
+        end, 0),
+        MainPid ! {done, Result}
+    end),
+
+    %% Do worker 1's job and lock the rows in the opposite order.
+    Result1 = mysql:transaction(Conn1, fun () ->
+        MainPid ! start,
+        ok = mysql:query(Conn1, "UPDATE foo SET v = 1 WHERE k = 1"),
+        %% Sync. Send 'go' to worker 2 multiple times in case it restarts.
+        Worker2 ! go, Worker2 ! go, Worker2 ! go,
+        receive go -> ok after 10000 -> throw(too_long) end,
+        {atomic, ok} = mysql:transaction(Conn1, fun () ->
+            %% Nested transaction, just to make sure we can handle nested.
+            ok = mysql:query(Conn1, "UPDATE foo SET v = 1 WHERE k = 2")
+        end),
+        ok
+    end, 0),
+
+    %% Wait for a reply from worker 2 to make sure it is done.
+    Result2 = receive {done, Result} -> Result end,
+
+    %% Check that one of them was ok, the other one was aborted.
+    [ResultAborted, ResultAtomic] = lists:sort([Result1, Result2]),
+    ?assertEqual({atomic, ok}, ResultAtomic),
+    ?assertMatch({aborted,
+                  {{1213, <<"40001">>, <<"Deadlock", _/binary>>}, _Trace}},
+                 ResultAborted),
+
+    %% None of the connections should be in a transaction at this point
+    ?assertNot(mysql:in_transaction(Conn1)),
+    ?assertNot(mysql:in_transaction(Conn2)),
+
+    %% Make sure we got exactly 2 start messages, i.e. there was no restart.
+    ?assertEqual(ok, receive start -> ok after 0 -> no_worker_ever_started end),
+    ?assertEqual(ok, receive start -> ok after 0 -> only_one_worker_started end),
+    ?assertEqual(ok, receive start -> there_was_a_restart after 0 -> ok end),
+    flush_inbox().
+
 lock_wait_timeout({_Conn1, Conn2} = Conns) ->
     %% Set the lowest timeout possible to speed up the test.
     case mysql:query(Conn2, "SET innodb_lock_wait_timeout = 1") of