Use the last commit result even when there are intermediate retries

Previously, if a transaction got a `commit_unknown_result`, during the next
successful attempt that result would be returned to the user. However, if the
next attempt was another retryable error, then the committed result was ignored
and the whole transaction would be applied again. This could result in document
update transactions conflicting with themselves as described in issue
https://github.com/apache/couchdb/issues/3560

To prevent that from happening we remember that there was an
`commit_unknown_result` error during the course of any previous retries and try
to return that result.

3 files changed, 43 insertions, 6 deletions

diff --git a/src/fabric/include/fabric2.hrl b/src/fabric/include/fabric2.hrl
index f0d789940..c8b0d6e53 100644
--- a/src/fabric/include/fabric2.hrl
+++ b/src/fabric/include/fabric2.hrl
@@ -78,6 +78,7 @@
 -define(PDICT_CHECKED_MD_IS_CURRENT, '$fabric_checked_md_is_current').
 -define(PDICT_TX_ID_KEY, '$fabric_tx_id').
 -define(PDICT_TX_RES_KEY, '$fabric_tx_result').
+-define(PDICT_TX_RES_WAS_UNKNOWN, '$fabric_tx_res_was_unknown').
 -define(PDICT_FOLD_ACC_STATE, '$fabric_fold_acc_state').
 
 -define(DEFAULT_BINARY_CHUNK_SIZE, 100000).
diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl
index f3585e334..8ccfeab91 100644
--- a/src/fabric/src/fabric2_fdb.erl
+++ b/src/fabric/src/fabric2_fdb.erl
@@ -1969,7 +1969,7 @@ check_db_instance(#{} = Db) ->
 
 
 is_transaction_applied(Tx) ->
-    is_commit_unknown_result()
+    was_commit_unknown_result()
         andalso has_transaction_id()
         andalso transaction_id_exists(Tx).
 
@@ -1997,11 +1997,23 @@ clear_transaction() ->
     erase(?PDICT_CHECKED_DB_IS_CURRENT),
     erase(?PDICT_CHECKED_MD_IS_CURRENT),
     erase(?PDICT_TX_ID_KEY),
-    erase(?PDICT_TX_RES_KEY).
+    erase(?PDICT_TX_RES_KEY),
+    erase(?PDICT_TX_RES_WAS_UNKNOWN).
 
 
-is_commit_unknown_result() ->
-    erlfdb:get_last_error() == ?ERLFDB_COMMIT_UNKNOWN_RESULT.
+was_commit_unknown_result() ->
+    case get(?PDICT_TX_RES_WAS_UNKNOWN) of
+        true ->
+            true;
+        undefined ->
+            case erlfdb:get_last_error() == ?ERLFDB_COMMIT_UNKNOWN_RESULT of
+                true ->
+                    put(?PDICT_TX_RES_WAS_UNKNOWN, true),
+                    true;
+                false ->
+                    false
+            end
+    end.
 
 
 has_transaction_id() ->
diff --git a/src/fabric/test/fabric2_fdb_tx_retry_tests.erl b/src/fabric/test/fabric2_fdb_tx_retry_tests.erl
index 7fb0f21d0..72277246e 100644
--- a/src/fabric/test/fabric2_fdb_tx_retry_tests.erl
+++ b/src/fabric/test/fabric2_fdb_tx_retry_tests.erl
@@ -28,7 +28,8 @@ retry_test_() ->
             ?TDEF(run_on_first_try),
             ?TDEF(retry_when_commit_conflict),
             ?TDEF(retry_when_txid_not_found),
-            ?TDEF(no_retry_when_txid_found)
+            ?TDEF(no_retry_when_txid_found),
+            ?TDEF(use_last_unknown_result)
         ])
     }.
 
@@ -173,4 +174,27 @@ no_retry_when_txid_found(_) ->
     end),
 
     ?assertEqual(did_not_run, Result),
-    ?assert(meck:validate([erlfdb, fabric2_txids])).
\ No newline at end of file
+    ?assert(meck:validate([erlfdb, fabric2_txids])).
+
+
+use_last_unknown_result(_) ->
+    meck:expect(erlfdb, transactional, fun(_Db, UserFun) ->
+        UserFun(not_a_real_transaction)
+    end),
+    % Set another retryable error as last error
+    meck:expect(erlfdb, get_last_error, fun() -> 1009 end),
+    meck:expect(erlfdb, get, fun(_, <<"a txid">>) -> future end),
+    meck:expect(erlfdb, wait, fun(future) -> <<>> end),
+    meck:expect(fabric2_txids, remove, fun(<<"a txid">>) -> ok end),
+
+    put('$fabric_tx_id', <<"a txid">>),
+    put('$fabric_tx_result', did_not_run),
+    put('$fabric_tx_res_was_unknown', true),
+
+    Result = fabric2_fdb:transactional(fun(_Tx) ->
+        ?assert(false),
+        did_run
+    end),
+
+    ?assertEqual(did_not_run, Result),
+    ?assert(meck:validate([erlfdb, fabric2_txids])).