Unit test for append key/data, BF abort during commit order enter.

src/mock_provider.hpp

@@ -23,6 +23,7 @@ namespace wsrep
             , server_id_("1")
             , group_seqno_(0)
             , bf_abort_map_()
+            , next_error_(wsrep::provider::success)
         { }
 
         int connect(const std::string&, const std::string&, const std::string&,
@@ -80,17 +81,21 @@ namespace wsrep
             }
         }
 
-        int append_key(wsrep::ws_handle&, const wsrep::key&) { return 0; }
+        int append_key(wsrep::ws_handle&, const wsrep::key&)
-        int append_data(wsrep::ws_handle&, const wsrep::data&) { return 0; }
+        { return next_error_; }
-        int rollback(const wsrep::transaction_id) { return 0; }
+        int append_data(wsrep::ws_handle&, const wsrep::data&)
+        { return next_error_; }
+        int rollback(const wsrep::transaction_id)
+        { return next_error_; }
         enum wsrep::provider::status
         commit_order_enter(const wsrep::ws_handle&,
                                           const wsrep::ws_meta&)
-        { return wsrep::provider::success; }
+        { return next_error_; }
         int commit_order_leave(const wsrep::ws_handle&,
                                const wsrep::ws_meta&)
-        { return 0;}
+        { return next_error_;}
-        int release(wsrep::ws_handle&) { return 0; }
+        int release(wsrep::ws_handle&)
+        { return next_error_; }
 
         int replay(wsrep::ws_handle&, void*) { ::abort(); /* not impl */}
 
@@ -102,7 +107,6 @@ namespace wsrep
             return std::vector<status_variable>();
         }
 
-
         // Methods to modify mock state
         /*! Inject BF abort event into the provider.
          *
@@ -129,12 +133,16 @@ namespace wsrep
          * \todo Inject an error so that the next call to any
          *       provider call will return the given error.
          */
-        void inject_error(enum wsrep::provider::status);
+        void inject_error(enum wsrep::provider::status error)
+        {
+            next_error_ = error;
+        }
     private:
         wsrep::id group_id_;
         wsrep::id server_id_;
         long long group_seqno_;
         bf_abort_map bf_abort_map_;
+        enum wsrep::provider::status next_error_;
     };
 }
 

src/test_utils.cpp

@@ -15,7 +15,7 @@ void wsrep_test::bf_abort_unordered(wsrep::client_context& cc)
     cc.bf_abort(lock, 1);
 }
 
-    // BF abort method to abort transactions via provider
+// BF abort method to abort transactions via provider
 void wsrep_test::bf_abort_provider(wsrep::mock_server_context& sc,
                                    const wsrep::transaction_context& tc,
                                    wsrep::seqno bf_seqno)

src/transaction_context.cpp

@@ -156,19 +156,11 @@ int wsrep::transaction_context::after_prepare()
     switch (client_context_.mode())
     {
     case wsrep::client_context::m_replicating:
-        if (state() == s_preparing)
-        {
         ret = certify_commit(lock);
         assert((ret == 0 || state() == s_committing) ||
                (state() == s_must_abort ||
                 state() == s_must_replay ||
                 state() == s_cert_failed));
-        }
-        else
-        {
-            assert(state() == s_must_abort);
-            client_context_.override_error(wsrep::e_deadlock_error);
-        }
         break;
     case wsrep::client_context::m_local:
     case wsrep::client_context::m_applier:
@@ -229,10 +221,10 @@ int wsrep::transaction_context::before_commit()
         }
         if (ret == 0)
         {
+            assert(ordered());
             lock.unlock();
             enum wsrep::provider::status
-                status(provider_.commit_order_enter(
+                status(provider_.commit_order_enter(ws_handle_, ws_meta_));
-                           ws_handle_, ws_meta_));
             lock.lock();
             switch (status)
             {
@@ -243,6 +235,7 @@ int wsrep::transaction_context::before_commit()
                 {
                     state(lock, s_must_abort);
                 }
+                state(lock, s_must_replay);
                 ret = 1;
                 break;
             default:

src/transaction_context_test.cpp

@@ -89,7 +89,25 @@ namespace
     replicating_fixtures;
 }
 
-
+BOOST_FIXTURE_TEST_CASE(transaction_context_append_key_data,
+                        replicating_client_fixture_sync_rm)
+{
+    cc.start_transaction(1);
+    BOOST_REQUIRE(tc.active());
+    int vals[3] = {1, 2, 3};
+    wsrep::key key(wsrep::key::exclusive);
+    for (int i(0); i < 3; ++i)
+    {
+        key.append_key_part(&vals[i], sizeof(vals[i]));
+    }
+    BOOST_REQUIRE(cc.append_key(key) == 0);
+    wsrep::data data(&vals[2], sizeof(vals[2]));
+    BOOST_REQUIRE(cc.append_data(data) == 0);
+    BOOST_REQUIRE(cc.before_commit() == 0);
+    BOOST_REQUIRE(cc.ordered_commit() == 0);
+    BOOST_REQUIRE(cc.after_commit() == 0);
+    cc.after_statement();
+}
 //
 // Test a succesful 1PC transaction lifecycle
 //
@@ -513,6 +531,50 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE(
     BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
 }
 
+//
+// Test a 2PC transaction which gets BF aborted when trying to grab
+// commit order.
+//
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(
+    transaction_context_2pc_bf_during_commit_order_enter, T,
+    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());
+    BOOST_REQUIRE(tc.id() == wsrep::transaction_id(1));
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_executing);
+
+    BOOST_REQUIRE(cc.before_prepare() == 0);
+    BOOST_REQUIRE(cc.after_prepare() == 0);
+
+    sc.provider().inject_error(wsrep::provider::error_bf_abort);
+
+    // Run before commit
+    BOOST_REQUIRE(cc.before_commit());
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_must_replay);
+    BOOST_REQUIRE(tc.certified() == true);
+    BOOST_REQUIRE(tc.ordered() == true);
+
+    sc.provider().inject_error(wsrep::provider::success);
+    // Rollback sequence
+    BOOST_REQUIRE(cc.before_rollback() == 0);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_must_replay);
+    BOOST_REQUIRE(cc.after_rollback() == 0);
+    BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_must_replay);
+
+    // Cleanup after statement
+    cc.after_statement();
+    BOOST_REQUIRE(tc.active() == false);
+    BOOST_REQUIRE(tc.ordered() == false);
+    BOOST_REQUIRE(tc.certified() == false);
+    BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
+}
+
 //
 // Test a transaction which gets BF aborted before before_statement.
 //