* Run transaction context unit tests for both sync and async rm

* Unit test case for BF abort before before_statement() and
  after after_statement()
* Cleaned up empty client_context_test.cpp

src/CMakeLists.txt

@@ -16,7 +16,6 @@ target_link_libraries(wsrep-lib wsrep_api_v26 pthread dl)
 add_executable(wsrep-lib_test
   mock_client_context.cpp
   test_utils.cpp
-  client_context_test.cpp
   server_context_test.cpp
   transaction_context_test.cpp
   wsrep-lib_test.cpp

src/client_context.cpp

@@ -44,18 +44,37 @@ int wsrep::client_context::before_command()
     state(lock, s_exec);
     assert(transaction_.active() == false ||
            (transaction_.state() == wsrep::transaction_context::s_executing ||
-            transaction_.state() == wsrep::transaction_context::s_aborted));
+            transaction_.state() == wsrep::transaction_context::s_aborted ||
+            (transaction_.state() == wsrep::transaction_context::s_must_abort &&
+             server_context_.rollback_mode() == wsrep::server_context::rm_async)));
 
+    if (transaction_.active())
+    {
+        if (transaction_.state() == wsrep::transaction_context::s_must_abort)
+        {
+            assert(server_context_.rollback_mode() ==
+                   wsrep::server_context::rm_async);
+            override_error(wsrep::e_deadlock_error);
+            lock.unlock();
+            rollback();
+            (void)transaction_.after_statement();
+            lock.lock();
+            assert(transaction_.state() ==
+                   wsrep::transaction_context::s_aborted);
+            assert(transaction_.active() == false);
+            assert(current_error() != wsrep::e_success);
+        }
+        else if (transaction_.state() == wsrep::transaction_context::s_aborted)
+        {
             // Transaction was rolled back either just before sending result
             // to the client, or after client_context become idle.
             // Clean up the transaction and return error.
-    if (transaction_.active() &&
-        transaction_.state() == wsrep::transaction_context::s_aborted)
-    {
             override_error(wsrep::e_deadlock_error);
             lock.unlock();
             (void)transaction_.after_statement();
             lock.lock();
+            assert(transaction_.active() == false);
+        }
         return 1;
     }
     return 0;
@@ -118,10 +137,7 @@ int wsrep::client_context::before_statement()
     if (transaction_.active() &&
         transaction_.state() == wsrep::transaction_context::s_must_abort)
     {
-        override_error(wsrep::e_deadlock_error);
+        // Rollback and cleanup will happen in after_command_before_result()
-        lock.unlock();
-        rollback();
-        lock.lock();
         return 1;
     }
     return 0;
@@ -176,7 +192,7 @@ void wsrep::client_context::state(
     {
         std::ostringstream os;
         os << "client_context: Unallowed state transition: "
-           << state_ << " -> " << state << "\n";
+           << state_ << " -> " << state;
         throw wsrep::runtime_error(os.str());
     }
 }

src/client_context_test.cpp

@@ -1,30 +0,0 @@
-
-//
-// Copyright (C) 2018 Codership Oy <info@codership.com>
-//
-
-#include "mock_client_context.hpp"
-#include "mock_server_context.hpp"
-#include "test_utils.hpp"
-
-#include <boost/test/unit_test.hpp>
-
-BOOST_AUTO_TEST_CASE(client_context_test_error_codes)
-{
-    wsrep::mock_server_context sc("s1", "s1", wsrep::server_context::rm_async);
-    wsrep::mock_client_context cc(sc,
-                                  wsrep::client_id(1),
-                                  wsrep::client_context::m_applier,
-                                  false);
-    const wsrep::transaction_context& txc(cc.transaction());
-    cc.before_command();
-    cc.before_statement();
-
-    BOOST_REQUIRE(txc.active() == false);
-    cc.start_transaction(1);
-    wsrep_test::bf_abort_unordered(cc);
-
-    cc.after_statement();
-    cc.after_command_before_result();
-    cc.after_command_after_result();
-}

src/dbms_simulator.cpp

@@ -394,6 +394,7 @@ private:
     int client_command(Func f)
     {
         int err(before_command());
+        // If err != 0, transaction was BF aborted while client idle
         if (err == 0)
         {
             err = before_statement();
@@ -404,6 +405,12 @@ private:
             after_statement();
         }
         after_command_before_result();
+        if (current_error())
+        {
+            assert(transaction_.state() ==
+                   wsrep::transaction_context::s_aborted);
+            err = 1;
+        }
         after_command_after_result();
         return err;
     }
@@ -419,7 +426,6 @@ private:
                 se_trx_.start(this);
                 return err;
             });
-        err = err || current_error();
         err = err || client_command(
             [&]()
             {
@@ -438,7 +444,6 @@ private:
                                                      os.str().size()));
                 return err;
             });
-        err = err || current_error();
         err = err || client_command(
             [&]()
             {
@@ -455,8 +460,8 @@ private:
                 return err;
             });
 
-        assert((current_error() &&
+        assert(err ||
-                transaction().state() == wsrep::transaction_context::s_aborted) ||
+            transaction().state() == wsrep::transaction_context::s_aborted ||
             transaction().state() == wsrep::transaction_context::s_committed);
         assert(se_trx_.active() == false);
         assert(transaction().active() == false);

src/transaction_context_test.cpp

@@ -11,12 +11,13 @@
 #include "test_utils.hpp"
 
 #include <boost/test/unit_test.hpp>
+#include <boost/mpl/vector.hpp>
 
 namespace
 {
-    struct replicating_client_fixture
+    struct replicating_client_fixture_sync_rm
     {
-        replicating_client_fixture()
+        replicating_client_fixture_sync_rm()
             : sc("s1", "s1", wsrep::server_context::rm_sync)
             , cc(sc, wsrep::client_id(1),
                  wsrep::client_context::m_replicating)
@@ -33,6 +34,25 @@ namespace
         const wsrep::transaction_context& tc;
     };
 
+    struct replicating_client_fixture_async_rm
+    {
+        replicating_client_fixture_async_rm()
+            : sc("s1", "s1", wsrep::server_context::rm_async)
+            , cc(sc, wsrep::client_id(1),
+                 wsrep::client_context::m_replicating)
+            , tc(cc.transaction())
+        {
+            BOOST_REQUIRE(cc.before_command() == 0);
+            BOOST_REQUIRE(cc.before_statement() == 0);
+            // Verify initial state
+            BOOST_REQUIRE(tc.active() == false);
+            BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_executing);
+        }
+        wsrep::mock_server_context sc;
+        wsrep::mock_client_context cc;
+        const wsrep::transaction_context& tc;
+    };
+
     struct applying_client_fixture
     {
         applying_client_fixture()
@@ -63,15 +83,22 @@ namespace
         const wsrep::transaction_context& tc;
     };
 
+    typedef
+    boost::mpl::vector<replicating_client_fixture_sync_rm,
+                       replicating_client_fixture_async_rm>
+    replicating_fixtures;
 }
 
+
+
 //
 // Test a succesful 1PC transaction lifecycle
 //
-BOOST_FIXTURE_TEST_CASE(transaction_context_1pc, replicating_client_fixture)
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(transaction_context_1pc, T,
+                                 replicating_fixtures, T)
 {
-
+    wsrep::client_context& cc(T::cc);
-
+    const wsrep::transaction_context& tc(T::tc);
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -101,8 +128,11 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_1pc, replicating_client_fixture)
 //
 // Test a succesful 2PC transaction lifecycle
 //
-BOOST_FIXTURE_TEST_CASE(transaction_context_2pc, replicating_client_fixture)
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(transaction_context_2pc, T,
+                                 replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -140,8 +170,12 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_2pc, replicating_client_fixture)
 //
 // Test a voluntary rollback
 //
-BOOST_FIXTURE_TEST_CASE(transaction_context_rollback, replicating_client_fixture)
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(transaction_context_rollback, T,
+                                 replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -167,9 +201,13 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_rollback, replicating_client_fixture
 //
 // Test a 1PC transaction which gets BF aborted before before_commit
 //
-BOOST_FIXTURE_TEST_CASE(transaction_context_1pc_bf_before_before_commit,
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-                        replicating_client_fixture)
+    transaction_context_1pc_bf_before_before_commit, T,
+    replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -201,9 +239,13 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_1pc_bf_before_before_commit,
 //
 // Test a 2PC transaction which gets BF aborted before before_prepare
 //
-BOOST_FIXTURE_TEST_CASE(transaction_context_2pc_bf_before_before_prepare,
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-                        replicating_client_fixture)
+    transaction_context_2pc_bf_before_before_prepare, T,
+    replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -235,9 +277,13 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_2pc_bf_before_before_prepare,
 //
 // Test a 2PC transaction which gets BF aborted before before_prepare
 //
-BOOST_FIXTURE_TEST_CASE(transaction_context_2pc_bf_before_after_prepare,
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-                        replicating_client_fixture)
+    transaction_context_2pc_bf_before_after_prepare, T,
+    replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -274,10 +320,14 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_2pc_bf_before_after_prepare,
 // Test a 1PC transaction which gets BF aborted during before_commit via
 // provider before the write set was ordered and certified.
 //
-BOOST_FIXTURE_TEST_CASE(
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-    transaction_context_1pc_bf_during_before_commit_uncertified,
+    transaction_context_1pc_bf_during_before_commit_uncertified, T,
-    replicating_client_fixture)
+    replicating_fixtures, T)
 {
+    wsrep::mock_server_context& sc(T::sc);
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -306,13 +356,48 @@ BOOST_FIXTURE_TEST_CASE(
     BOOST_REQUIRE(cc.current_error());
 }
 
+//
+// Test a transaction which gets BF aborted before before_statement.
+//
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
+    transaction_context_1pc_bf_before_before_statement, T,
+    replicating_fixtures, T)
+{
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
+    // Start a new transaction with ID 1
+    cc.start_transaction(1);
+    BOOST_REQUIRE(tc.active());
+    BOOST_REQUIRE(tc.id() == wsrep::transaction_id(1));
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_executing);
+    cc.after_statement();
+    cc.after_command_before_result();
+    cc.after_command_after_result();
+    cc.before_command();
+    BOOST_REQUIRE(tc.active());
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_executing);
+    wsrep_test::bf_abort_unordered(cc);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_must_abort);
+    BOOST_REQUIRE(cc.before_statement() == 1);
+    BOOST_REQUIRE(tc.active());
+    cc.after_command_before_result();
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_deadlock_error);
+    cc.after_command_after_result();
+    BOOST_REQUIRE(tc.active() == false);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_aborted);
+}
+
 //
 // Test a transaction which gets BF aborted before after_statement.
 //
-BOOST_FIXTURE_TEST_CASE(
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-    transaction_context_1pc_bf_during_before_after_statement,
+    transaction_context_1pc_bf_before_after_statement, T,
-    replicating_client_fixture)
+    replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -332,10 +417,14 @@ BOOST_FIXTURE_TEST_CASE(
 // Test a 1PC transaction which gets BF aborted during before_commit via
 // provider after the write set was ordered and certified.
 //
-BOOST_FIXTURE_TEST_CASE(
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-    transaction_context_1pc_bf_during_before_commit_certified,
+    transaction_context_1pc_bf_during_before_commit_certified, T,
-    replicating_client_fixture)
+    replicating_fixtures, T)
 {
+    wsrep::mock_server_context& sc(T::sc);
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     // Start a new transaction with ID 1
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -364,10 +453,38 @@ BOOST_FIXTURE_TEST_CASE(
     BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
 }
 
-BOOST_FIXTURE_TEST_CASE(
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
-    transaction_context_1pc_bf_abort_after_after_command_before_result,
+    transaction_context_1pc_bf_abort_after_after_statement, T,
-    replicating_client_fixture)
+    replicating_fixtures, T)
 {
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
+    cc.start_transaction(1);
+    BOOST_REQUIRE(tc.active());
+    cc.after_statement();
+    BOOST_REQUIRE(cc.state() == wsrep::client_context::s_exec);
+    wsrep_test::bf_abort_unordered(cc);
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
+    BOOST_REQUIRE(tc.active());
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_must_abort);
+    cc.after_command_before_result();
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_deadlock_error);
+    BOOST_REQUIRE(tc.active() == false);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_aborted);
+    cc.after_command_after_result();
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
+    BOOST_REQUIRE(tc.active() == false);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_aborted);
+}
+
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
+    transaction_context_1pc_bf_abort_after_after_command_before_result, T,
+    replicating_fixtures, T)
+{
+    wsrep::client_context& cc(T::cc);
+    const wsrep::transaction_context& tc(T::tc);
+
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
     cc.after_statement();
@@ -398,8 +515,8 @@ BOOST_FIXTURE_TEST_CASE(
 }
 
 BOOST_FIXTURE_TEST_CASE(
-    transaction_context_1pc_bf_abort_after_after_command_after_result,
+    transaction_context_1pc_bf_abort_after_after_command_after_result_sync_rm,
-    replicating_client_fixture)
+    replicating_client_fixture_sync_rm)
 {
     cc.start_transaction(1);
     BOOST_REQUIRE(tc.active());
@@ -422,6 +539,31 @@ BOOST_FIXTURE_TEST_CASE(
     BOOST_REQUIRE(tc.active() == false);
 }
 
+BOOST_FIXTURE_TEST_CASE(
+    transaction_context_1pc_bf_abort_after_after_command_after_result_async_rm,
+    replicating_client_fixture_async_rm)
+{
+    cc.start_transaction(1);
+    BOOST_REQUIRE(tc.active());
+    cc.after_statement();
+    BOOST_REQUIRE(cc.state() == wsrep::client_context::s_exec);
+    cc.after_command_before_result();
+    BOOST_REQUIRE(cc.state() == wsrep::client_context::s_result);
+    cc.after_command_after_result();
+    BOOST_REQUIRE(cc.state() == wsrep::client_context::s_idle);
+    wsrep_test::bf_abort_unordered(cc);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_must_abort);
+    BOOST_REQUIRE(tc.active());
+    BOOST_REQUIRE(cc.before_command() == 1);
+    BOOST_REQUIRE(tc.active() == false);
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_deadlock_error);
+    cc.after_command_before_result();
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_deadlock_error);
+    cc.after_command_after_result();
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
+    BOOST_REQUIRE(tc.active() == false);
+}
+
 BOOST_FIXTURE_TEST_CASE(transaction_context_1pc_applying,
                         applying_client_fixture)
 {