diff --git a/src/mock_provider.hpp b/src/mock_provider.hpp index 1868365..8cb216b 100644 --- a/src/mock_provider.hpp +++ b/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_data(wsrep::ws_handle&, const wsrep::data&) { return 0; } - int rollback(const wsrep::transaction_id) { return 0; } + int append_key(wsrep::ws_handle&, const wsrep::key&) + { return next_error_; } + 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;} - int release(wsrep::ws_handle&) { return 0; } + { return next_error_;} + 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(); } - // 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_; }; } diff --git a/src/test_utils.cpp b/src/test_utils.cpp index 47e7089..126ba7f 100644 --- a/src/test_utils.cpp +++ b/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) diff --git a/src/transaction_context.cpp b/src/transaction_context.cpp index ca062b4..5007305 100644 --- a/src/transaction_context.cpp +++ b/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); - } + ret = certify_commit(lock); + assert((ret == 0 || state() == s_committing) || + (state() == s_must_abort || + state() == s_must_replay || + state() == s_cert_failed)); 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( - ws_handle_, ws_meta_)); + status(provider_.commit_order_enter(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: diff --git a/src/transaction_context_test.cpp b/src/transaction_context_test.cpp index 0cc4a97..f0a38ce 100644 --- a/src/transaction_context_test.cpp +++ b/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. //