From d1482feb32c70b36f1c3a364abb9e50107d1b96b Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Mon, 19 Oct 2020 06:12:17 +0300 Subject: [PATCH] Ensure that client_service::will_replay() is called. Modified tests to verify that client_service::will_replay() is called whenever it is determined that the transaction must replay. Added a test to verify behavior when provider::commit_order_enter() returns BF abort error. Moved call to client_service::will_replay() into transaction::state() to ensure that it is always called when shift to s_must_replay happens. --- src/transaction.cpp | 7 ++++-- test/mock_client_state.hpp | 10 +++++--- test/transaction_test.cpp | 45 +++++++++++++++++++++++++++++++++++ test/transaction_test_2pc.cpp | 4 ++++ 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/transaction.cpp b/src/transaction.cpp index e461907..17085ba 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -1142,6 +1142,11 @@ void wsrep::transaction::state( state_hist_.erase(state_hist_.begin()); } state_ = next_state; + + if (state_ == s_must_replay) + { + client_service_.will_replay(); + } } bool wsrep::transaction::abort_or_interrupt( @@ -1531,7 +1536,6 @@ int wsrep::transaction::certify_commit( // We got BF aborted after succesful certification // and before acquiring client state lock. The trasaction // must be replayed. - client_service_.will_replay(); state(lock, s_must_replay); break; default: @@ -1557,7 +1561,6 @@ int wsrep::transaction::certify_commit( // yet known. Therefore the transaction must roll back // and go through replay either to replay and commit the whole // transaction or to determine failed certification status. - client_service_.will_replay(); if (state() != s_must_abort) { state(lock, s_must_abort); diff --git a/test/mock_client_state.hpp b/test/mock_client_state.hpp index 095b2b9..c7d5dc7 100644 --- a/test/mock_client_state.hpp +++ b/test/mock_client_state.hpp @@ -72,6 +72,7 @@ namespace wsrep , sync_point_action_() , bytes_generated_() , client_state_(client_state) + , will_replay_called_() , replays_() , aborts_() { } @@ -98,10 +99,11 @@ namespace wsrep return 0; } } - void will_replay() WSREP_OVERRIDE { } - enum wsrep::provider::status - replay() WSREP_OVERRIDE; + void will_replay() WSREP_OVERRIDE { will_replay_called_ = true; } + + enum wsrep::provider::status replay() WSREP_OVERRIDE; + void wait_for_replayers( wsrep::unique_lock& lock) WSREP_OVERRIDE @@ -195,10 +197,12 @@ namespace wsrep // // Verifying the state // + bool will_replay_called() const { return will_replay_called_; } size_t replays() const { return replays_; } size_t aborts() const { return aborts_; } private: wsrep::mock_client_state& client_state_; + bool will_replay_called_; size_t replays_; size_t aborts_; }; diff --git a/test/transaction_test.cpp b/test/transaction_test.cpp index 2993e75..7386c99 100644 --- a/test/transaction_test.cpp +++ b/test/transaction_test.cpp @@ -249,6 +249,46 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE( BOOST_REQUIRE(cc.current_error()); } +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + transaction_1pc_bf_during_commit_order_enter, T, + replicating_fixtures, T) +{ + wsrep::mock_client& cc(T::cc); + const wsrep::transaction& tc(T::tc); + auto& sc(T::sc); + + // Start a new transaction with ID 1 + cc.start_transaction(wsrep::transaction_id(1)); + BOOST_REQUIRE(tc.active()); + BOOST_REQUIRE(tc.id() == wsrep::transaction_id(1)); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_executing); + + sc.provider().commit_order_enter_result_ = wsrep::provider::error_bf_abort; + + // Run before commit + BOOST_REQUIRE(cc.before_commit()); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); + BOOST_REQUIRE(tc.certified() == true); + BOOST_REQUIRE(tc.ordered() == true); + + sc.provider().commit_order_enter_result_ = wsrep::provider::success; + + // Rollback sequence + BOOST_REQUIRE(cc.before_rollback() == 0); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.after_rollback() == 0); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + + // Replay from after_statement() + cc.after_statement(); + BOOST_REQUIRE(cc.replays() > 0); + BOOST_REQUIRE(tc.active() == false); + BOOST_REQUIRE(tc.ordered() == false); + BOOST_REQUIRE(tc.certified() == false); + BOOST_REQUIRE(not cc.current_error()); +} + // // Test a 1PC transaction for which prepare data fails // @@ -341,6 +381,7 @@ BOOST_FIXTURE_TEST_CASE( BOOST_REQUIRE(cc.before_commit()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(cc.after_statement() == 0); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_committed); BOOST_REQUIRE(cc.current_error() == wsrep::e_success); @@ -361,6 +402,7 @@ BOOST_FIXTURE_TEST_CASE( BOOST_REQUIRE(cc.before_commit()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(cc.after_statement() ); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_aborted); BOOST_REQUIRE(cc.current_error() == wsrep::e_deadlock_error); @@ -391,6 +433,7 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE( // Run before commit BOOST_REQUIRE(cc.before_commit()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(tc.certified() == false); BOOST_REQUIRE(tc.ordered() == true); @@ -755,6 +798,7 @@ BOOST_FIXTURE_TEST_CASE_TEMPLATE( // Run before commit BOOST_REQUIRE(cc.before_commit()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(tc.certified() == true); BOOST_REQUIRE(tc.ordered() == true); @@ -1268,6 +1312,7 @@ BOOST_FIXTURE_TEST_CASE(transaction_row_streaming_bf_abort_committing, BOOST_REQUIRE(cc.before_rollback() == 0); BOOST_REQUIRE(cc.after_rollback() == 0); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(cc.after_statement() == 0); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_committed); BOOST_REQUIRE(sc.provider().fragments() == 2); diff --git a/test/transaction_test_2pc.cpp b/test/transaction_test_2pc.cpp index a8f3a0d..8f95828 100644 --- a/test/transaction_test_2pc.cpp +++ b/test/transaction_test_2pc.cpp @@ -94,6 +94,7 @@ BOOST_FIXTURE_TEST_CASE( wsrep_test::bf_abort_ordered(cc); BOOST_REQUIRE(cc.after_prepare()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(cc.before_rollback() == 0); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); BOOST_REQUIRE(cc.after_rollback() == 0); @@ -124,6 +125,7 @@ BOOST_FIXTURE_TEST_CASE( BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_abort); BOOST_REQUIRE(cc.before_rollback() == 0); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(cc.after_rollback() == 0); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); BOOST_REQUIRE(cc.after_statement() == 0); @@ -152,6 +154,7 @@ BOOST_FIXTURE_TEST_CASE( BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_abort); BOOST_REQUIRE(cc.before_commit()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(tc.certified() == true); BOOST_REQUIRE(tc.ordered() == true); BOOST_REQUIRE(cc.before_rollback() == 0); @@ -183,6 +186,7 @@ BOOST_FIXTURE_TEST_CASE( sc.provider().commit_order_enter_result_ = wsrep::provider::error_bf_abort; BOOST_REQUIRE(cc.before_commit()); BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + BOOST_REQUIRE(cc.will_replay_called() == true); BOOST_REQUIRE(tc.certified() == true); BOOST_REQUIRE(tc.ordered() == true); sc.provider().commit_order_enter_result_ = wsrep::provider::success;