From a7f8728c12f1f64e958f1fcafbdaaeb57d97653a Mon Sep 17 00:00:00 2001
From: Teemu Ollakka <teemu.ollakka@galeracluster.com>
Date: Wed, 4 Jul 2018 14:28:54 +0300
Subject: [PATCH] After applying call, BF abort fixes

* Added after applying call for high priority threads in order to
  avoid client mode complexity in after_statement() call and
  make high prio transaction cleanup possible
* Mask connection failed error with deadlock error if provider
  returns connection failed and the transaction was BF aborted
---
 include/wsrep/client_state.hpp |  9 +++++++++
 include/wsrep/transaction.hpp  |  7 +++++++
 src/transaction.cpp            | 30 ++++++++++++++++++++++++++----
 3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/include/wsrep/client_state.hpp b/include/wsrep/client_state.hpp
index 393a09a..0406653 100644
--- a/include/wsrep/client_state.hpp
+++ b/include/wsrep/client_state.hpp
@@ -246,6 +246,15 @@ namespace wsrep
         enum after_statement_result after_statement();
         /** @} */
 
+        /**
+         * Perform cleanup after applying a transaction.
+         */
+        void after_applying()
+        {
+            assert(mode_ == m_high_priority);
+            transaction_.after_applying();
+        }
+
         /** @name Replication interface */
         /** @{ */
         /**
diff --git a/include/wsrep/transaction.hpp b/include/wsrep/transaction.hpp
index c8499c8..59bd203 100644
--- a/include/wsrep/transaction.hpp
+++ b/include/wsrep/transaction.hpp
@@ -113,8 +113,14 @@ namespace wsrep
 
         int after_statement();
 
+        void after_applying();
+
         bool bf_abort(wsrep::unique_lock<wsrep::mutex>& lock,
                       wsrep::seqno bf_seqno);
+        bool bf_aborted() const
+        {
+            return (bf_abort_client_state_ != 0);
+        }
 
         int flags() const
         {
@@ -145,6 +151,7 @@ namespace wsrep
         enum state state_;
         std::vector<enum state> state_hist_;
         enum state bf_abort_state_;
+        enum wsrep::provider::status bf_abort_provider_status_;
         int bf_abort_client_state_;
         wsrep::ws_handle ws_handle_;
         wsrep::ws_meta ws_meta_;
diff --git a/src/transaction.cpp b/src/transaction.cpp
index b6e9c1d..b3f082a 100644
--- a/src/transaction.cpp
+++ b/src/transaction.cpp
@@ -31,6 +31,7 @@ wsrep::transaction::transaction(
     , state_(s_executing)
     , state_hist_()
     , bf_abort_state_(s_executing)
+    , bf_abort_provider_status_()
     , bf_abort_client_state_()
     , ws_handle_()
     , ws_meta_()
@@ -588,6 +589,13 @@ int wsrep::transaction::after_statement()
     return ret;
 }
 
+void wsrep::transaction::after_applying()
+{
+    wsrep::unique_lock<wsrep::mutex> lock(client_state_.mutex_);
+    assert(state_ == s_committed || state_ == s_aborted);
+    cleanup();
+}
+
 bool wsrep::transaction::bf_abort(
     wsrep::unique_lock<wsrep::mutex>& lock WSREP_UNUSED,
     wsrep::seqno bf_seqno)
@@ -920,9 +928,20 @@ int wsrep::transaction::certify_commit(
         client_state_.override_error(wsrep::e_error_during_commit);
         break;
     case wsrep::provider::error_connection_failed:
-    case wsrep::provider::error_provider_failed:
         // Galera provider may return CONN_FAIL if the trx is
-        // BF aborted O_o
+        // BF aborted O_o. If we see here that the trx was BF aborted,
+        // return deadlock error instead of error during commit
+        // to reduce number of error state combinations elsewhere.
+        if (state() == s_must_abort)
+        {
+            client_state_.override_error(wsrep::e_deadlock_error);
+        }
+        else
+        {
+            client_state_.override_error(wsrep::e_error_during_commit);
+        }
+        break;
+    case wsrep::provider::error_provider_failed:
         if (state() != s_must_abort)
         {
             state(lock, s_must_abort);
@@ -983,6 +1002,9 @@ void wsrep::transaction::cleanup()
     {
         client_state_.update_last_written_gtid(ws_meta_.gtid());
     }
+    bf_abort_state_ = s_executing;
+    bf_abort_provider_status_ = wsrep::provider::success;
+    bf_abort_client_state_ = 0;
     ws_meta_ = wsrep::ws_meta();
     certified_ = false;
     pa_unsafe_ = false;
@@ -999,7 +1021,7 @@ void wsrep::transaction::debug_log_state(
         << "," << int64_t(id_.get())
         << "," << ws_meta_.seqno().get()
         << "," << wsrep::to_string(state_)
-        << ","
-        << wsrep::to_string(client_state_.current_error())
+        << "," << wsrep::to_string(bf_abort_state_)
+        << "," << wsrep::to_string(client_state_.current_error())
         << ")");
 }