From 459ada6b3b5dc864431e48dce01b7731cbeccdca Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Mon, 11 Jun 2018 14:54:37 +0300 Subject: [PATCH] * 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 | 1 - src/client_context.cpp | 46 +++++--- src/client_context_test.cpp | 30 ----- src/dbms_simulator.cpp | 15 ++- src/transaction_context_test.cpp | 196 ++++++++++++++++++++++++++----- 5 files changed, 210 insertions(+), 78 deletions(-) delete mode 100644 src/client_context_test.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 039a2be..462ef13 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/client_context.cpp b/src/client_context.cpp index 58c1b5e..10366e4 100644 --- a/src/client_context.cpp +++ b/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))); - // 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) + if (transaction_.active()) { - override_error(wsrep::e_deadlock_error); - lock.unlock(); - (void)transaction_.after_statement(); - lock.lock(); + 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. + 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); - lock.unlock(); - rollback(); - lock.lock(); + // Rollback and cleanup will happen in after_command_before_result() 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()); } } diff --git a/src/client_context_test.cpp b/src/client_context_test.cpp deleted file mode 100644 index 2e73547..0000000 --- a/src/client_context_test.cpp +++ /dev/null @@ -1,30 +0,0 @@ - -// -// Copyright (C) 2018 Codership Oy -// - -#include "mock_client_context.hpp" -#include "mock_server_context.hpp" -#include "test_utils.hpp" - -#include - -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(); -} diff --git a/src/dbms_simulator.cpp b/src/dbms_simulator.cpp index cde8a8b..5860c5e 100644 --- a/src/dbms_simulator.cpp +++ b/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,9 +460,9 @@ private: return err; }); - assert((current_error() && - transaction().state() == wsrep::transaction_context::s_aborted) || - transaction().state() == wsrep::transaction_context::s_committed); + assert(err || + transaction().state() == wsrep::transaction_context::s_aborted || + transaction().state() == wsrep::transaction_context::s_committed); assert(se_trx_.active() == false); assert(transaction().active() == false); switch (transaction().state()) diff --git a/src/transaction_context_test.cpp b/src/transaction_context_test.cpp index 8a9ad26..b78ef54 100644 --- a/src/transaction_context_test.cpp +++ b/src/transaction_context_test.cpp @@ -11,12 +11,13 @@ #include "test_utils.hpp" #include +#include 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_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, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + 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, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + 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, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + 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( - transaction_context_1pc_bf_during_before_commit_uncertified, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + transaction_context_1pc_bf_during_before_commit_uncertified, 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()); @@ -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( - transaction_context_1pc_bf_during_before_after_statement, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + transaction_context_1pc_bf_before_after_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()); @@ -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( - transaction_context_1pc_bf_during_before_commit_certified, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + transaction_context_1pc_bf_during_before_commit_certified, 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()); @@ -364,10 +453,38 @@ BOOST_FIXTURE_TEST_CASE( BOOST_REQUIRE(cc.current_error() == wsrep::e_success); } -BOOST_FIXTURE_TEST_CASE( - transaction_context_1pc_bf_abort_after_after_command_before_result, - replicating_client_fixture) +BOOST_FIXTURE_TEST_CASE_TEMPLATE( + transaction_context_1pc_bf_abort_after_after_statement, 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(); + 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, - replicating_client_fixture) + transaction_context_1pc_bf_abort_after_after_command_after_result_sync_rm, + 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) {