diff --git a/include/wsrep/client_service.hpp b/include/wsrep/client_service.hpp index 6664880..d47396d 100644 --- a/include/wsrep/client_service.hpp +++ b/include/wsrep/client_service.hpp @@ -184,13 +184,13 @@ namespace wsrep */ virtual enum wsrep::provider::status commit_by_xid() = 0; - /* - Returns true if the client has an ongoing XA transaction. - This method is used to determine when to cleanup the - corresponding wsrep-lib transaction object. - This method should return false when the XA transaction - is over, and the wsrep-lib transaction object can be - cleaned up. + /** + * Returns true if the client has an ongoing XA transaction. + * This method is used to determine when to cleanup the + * corresponding wsrep-lib transaction object. + * This method should return false when the XA transaction + * is over, and the wsrep-lib transaction object can be + * cleaned up. */ virtual bool is_explicit_xa() = 0; diff --git a/include/wsrep/transaction.hpp b/include/wsrep/transaction.hpp index ff7c3f0..f9d1659 100644 --- a/include/wsrep/transaction.hpp +++ b/include/wsrep/transaction.hpp @@ -128,12 +128,7 @@ namespace wsrep return !xid_.is_null(); } - void assign_xid(const wsrep::xid& xid) - { - assert(active()); - assert(!is_xa()); - xid_ = xid; - } + void assign_xid(const wsrep::xid& xid); const wsrep::xid& xid() const { diff --git a/include/wsrep/xid.hpp b/include/wsrep/xid.hpp index 005202d..c500d63 100644 --- a/include/wsrep/xid.hpp +++ b/include/wsrep/xid.hpp @@ -22,6 +22,7 @@ #include #include "buffer.hpp" +#include "exception.hpp" namespace wsrep { @@ -42,6 +43,10 @@ namespace wsrep , bqual_len_(bqual_len) , data_() { + if (gtrid_len_ > 64 || bqual_len_ > 64) + { + throw wsrep::runtime_error("maximum wsrep::xid size exceeded"); + } const long len = gtrid_len_ + bqual_len_; if (len > 0) { @@ -64,6 +69,8 @@ namespace wsrep void clear() { format_id_ = -1; + gtrid_len_ = 0; + bqual_len_ = 0; data_.clear(); } diff --git a/src/server_state.cpp b/src/server_state.cpp index 2d6b39c..399bcce 100644 --- a/src/server_state.cpp +++ b/src/server_state.cpp @@ -1429,9 +1429,10 @@ void wsrep::server_state::recover_streaming_appliers_if_not_recovered( class transaction_state_cmp { public: - transaction_state_cmp(const enum wsrep::transaction::state s) : state_(s) - { } - bool operator()(std::pair& vt) const + transaction_state_cmp(const enum wsrep::transaction::state s) + : state_(s) { } + bool operator()(const std::pair& vt) const { return vt.second->transaction().state() == state_; } diff --git a/src/transaction.cpp b/src/transaction.cpp index fbe8f89..7f9b2ca 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -1071,6 +1071,13 @@ void wsrep::transaction::clone_for_replay(const wsrep::transaction& other) state_ = s_replaying; } +void wsrep::transaction::assign_xid(const wsrep::xid& xid) +{ + assert(active()); + assert(!is_xa()); + xid_ = xid; +} + int wsrep::transaction::restore_to_prepared_state(const wsrep::xid& xid) { wsrep::unique_lock lock(client_state_.mutex_); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index aad18f7..b6e179c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -17,6 +17,7 @@ add_executable(wsrep-lib_test transaction_test_2pc.cpp transaction_test_xa.cpp view_test.cpp + xid_test.cpp wsrep-lib_test.cpp ) diff --git a/test/client_state_fixture.hpp b/test/client_state_fixture.hpp index 63b946f..a26adee 100644 --- a/test/client_state_fixture.hpp +++ b/test/client_state_fixture.hpp @@ -51,6 +51,35 @@ namespace const wsrep::transaction& tc; }; + struct replicating_two_clients_fixture_sync_rm + { + replicating_two_clients_fixture_sync_rm() + : server_service(sc) + , sc("s1", wsrep::server_state::rm_sync, server_service) + , cc1(sc, wsrep::client_id(1), + wsrep::client_state::m_local) + , cc2(sc, wsrep::client_id(2), + wsrep::client_state::m_local) + , tc(cc1.transaction()) + { + sc.mock_connect(); + cc1.open(cc1.id()); + BOOST_REQUIRE(cc1.before_command() == 0); + BOOST_REQUIRE(cc1.before_statement() == 0); + cc2.open(cc2.id()); + BOOST_REQUIRE(cc2.before_command() == 0); + BOOST_REQUIRE(cc2.before_statement() == 0); + // Verify initial state + BOOST_REQUIRE(tc.active() == false); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_executing); + } + wsrep::mock_server_service server_service; + wsrep::mock_server_state sc; + wsrep::mock_client cc1; + wsrep::mock_client cc2; + const wsrep::transaction& tc; + }; + struct replicating_client_fixture_async_rm { replicating_client_fixture_async_rm() diff --git a/test/mock_client_state.hpp b/test/mock_client_state.hpp index 7e718cf..56434f7 100644 --- a/test/mock_client_state.hpp +++ b/test/mock_client_state.hpp @@ -74,6 +74,7 @@ namespace wsrep , client_state_(client_state) , will_replay_called_() , replays_() + , unordered_replays_() , aborts_() { } @@ -108,6 +109,7 @@ namespace wsrep enum wsrep::provider::status replay_unordered() WSREP_OVERRIDE { + unordered_replays_++; return wsrep::provider::success; } @@ -221,11 +223,13 @@ namespace wsrep // bool will_replay_called() const { return will_replay_called_; } size_t replays() const { return replays_; } + size_t unordered_replays() const { return unordered_replays_; } size_t aborts() const { return aborts_; } private: wsrep::mock_client_state& client_state_; bool will_replay_called_; size_t replays_; + size_t unordered_replays_; size_t aborts_; }; diff --git a/test/mock_high_priority_service.cpp b/test/mock_high_priority_service.cpp index c4cfd0b..6c93601 100644 --- a/test/mock_high_priority_service.cpp +++ b/test/mock_high_priority_service.cpp @@ -37,6 +37,10 @@ int wsrep::mock_high_priority_service::adopt_transaction( const wsrep::transaction& transaction) { client_state_->adopt_transaction(transaction); + if (transaction.state() == wsrep::transaction::s_prepared) + { + client_state_->restore_xid(transaction.xid()); + } return 0; } diff --git a/test/mock_provider.hpp b/test/mock_provider.hpp index da4f765..c2a89f0 100644 --- a/test/mock_provider.hpp +++ b/test/mock_provider.hpp @@ -104,7 +104,6 @@ namespace wsrep } if (rolls_back_transaction(flags)) { - assert(0); ++rollback_fragments_; } diff --git a/test/transaction_test_xa.cpp b/test/transaction_test_xa.cpp index a64c875..2fcead5 100644 --- a/test/transaction_test_xa.cpp +++ b/test/transaction_test_xa.cpp @@ -2,7 +2,7 @@ #include // -// Test a succesful XA transaction lifecycle +// Test a successful XA transaction lifecycle // BOOST_FIXTURE_TEST_CASE(transaction_xa, replicating_client_fixture_sync_rm) @@ -49,7 +49,89 @@ BOOST_FIXTURE_TEST_CASE(transaction_xa, // -// Test a succesful XA transaction lifecycle (applying side) +// Test detaching of XA transactions +// +BOOST_FIXTURE_TEST_CASE(transaction_xa_detach_commit_by_xid, + replicating_two_clients_fixture_sync_rm) +{ + wsrep::xid xid(1, 1, 1, "id"); + + cc1.start_transaction(wsrep::transaction_id(1)); + cc1.assign_xid(xid); + cc1.before_prepare(); + cc1.after_prepare(); + BOOST_REQUIRE(sc.provider().fragments() == 1); + BOOST_REQUIRE(tc.streaming_context().fragments_certified() == 1); + + cc1.xa_detach(); + + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_aborted); + BOOST_REQUIRE(sc.find_streaming_applier(xid)); + BOOST_REQUIRE(cc1.after_statement() == 0); + + cc2.start_transaction(wsrep::transaction_id(2)); + cc2.assign_xid(xid); + BOOST_REQUIRE(cc2.client_state::commit_by_xid(xid) == 0); + BOOST_REQUIRE(cc2.after_statement() == 0); + BOOST_REQUIRE(sc.provider().commit_fragments() == 1); +} + +BOOST_FIXTURE_TEST_CASE(transaction_xa_detach_rollback_by_xid, + replicating_two_clients_fixture_sync_rm) +{ + wsrep::xid xid(1, 1, 1, "id"); + + cc1.start_transaction(wsrep::transaction_id(1)); + cc1.assign_xid(xid); + cc1.before_prepare(); + cc1.after_prepare(); + BOOST_REQUIRE(sc.provider().fragments() == 1); + BOOST_REQUIRE(tc.streaming_context().fragments_certified() == 1); + + cc1.xa_detach(); + + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_aborted); + BOOST_REQUIRE(sc.find_streaming_applier(xid)); + BOOST_REQUIRE(cc1.after_statement() == 0); + + cc2.start_transaction(wsrep::transaction_id(2)); + cc2.assign_xid(xid); + BOOST_REQUIRE(cc2.rollback_by_xid(xid) == 0); + BOOST_REQUIRE(cc2.after_statement() == 0); + BOOST_REQUIRE(sc.provider().rollback_fragments() == 1); +} + + +// +// Test XA replay +// +BOOST_FIXTURE_TEST_CASE(transaction_xa_replay, + replicating_client_fixture_sync_rm) +{ + wsrep::xid xid(1, 1, 1, "id"); + + cc.start_transaction(wsrep::transaction_id(1)); + cc.assign_xid(xid); + cc.before_prepare(); + cc.after_prepare(); + cc.after_command_before_result(); + cc.after_command_after_result(); + BOOST_REQUIRE(cc.state() == wsrep::client_state::s_idle); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_prepared); + wsrep_test::bf_abort_unordered(cc); + BOOST_REQUIRE(tc.state() == wsrep::transaction::s_must_replay); + + // this is normally done by rollbacker + cc.xa_replay(); + cc.sync_rollback_complete(); + + BOOST_REQUIRE(cc.unordered_replays() == 1); + BOOST_REQUIRE(sc.find_streaming_applier(sc.id(), + wsrep::transaction_id(1))); +} + +// +// Test a successful XA transaction lifecycle (applying side) // BOOST_FIXTURE_TEST_CASE(transaction_xa_applying, applying_client_fixture) @@ -84,7 +166,7 @@ BOOST_FIXTURE_TEST_CASE(transaction_xa_applying, /////////////////////////////////////////////////////////////////////////////// // -// Test a succesful XA transaction lifecycle +// Test a successful XA transaction lifecycle // BOOST_FIXTURE_TEST_CASE(transaction_xa_sr, streaming_client_fixture_byte) diff --git a/test/xid_test.cpp b/test/xid_test.cpp new file mode 100644 index 0000000..ce2ffab --- /dev/null +++ b/test/xid_test.cpp @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2020 Codership Oy + * + * This file is part of wsrep-lib. + * + * Wsrep-lib is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * Wsrep-lib is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wsrep-lib. If not, see . + */ + +#include "wsrep/xid.hpp" +#include + +BOOST_AUTO_TEST_CASE(xid_test_is_null) +{ + wsrep::xid null_xid; + BOOST_REQUIRE(null_xid.is_null()); + wsrep::xid test_xid(1,0,0,nullptr); + BOOST_REQUIRE(!test_xid.is_null()); +} + +BOOST_AUTO_TEST_CASE(xid_test_equal) +{ + wsrep::xid a(1,1,1,"ab"); + wsrep::xid b(1,1,1,"ab"); + BOOST_REQUIRE(a == b); +} + +BOOST_AUTO_TEST_CASE(xid_test_null_equal) +{ + wsrep::xid a; + wsrep::xid b; + BOOST_REQUIRE(a == b); + BOOST_REQUIRE(a.is_null()); +} + +BOOST_AUTO_TEST_CASE(xid_test_not_equal) +{ + wsrep::xid a(1,1,0,"a"); + wsrep::xid b(1,0,1,"a"); + wsrep::xid c(-1,1,0,"a"); + wsrep::xid d(1,1,0,"b"); + BOOST_REQUIRE(!(a == b)); + BOOST_REQUIRE(!(a == c)); + BOOST_REQUIRE(!(a == d)); +} + +BOOST_AUTO_TEST_CASE(xid_clear) +{ + wsrep::xid null_xid; + wsrep::xid to_clear(1, 1, 0, "a"); + to_clear.clear(); + BOOST_REQUIRE(to_clear.is_null()); + BOOST_REQUIRE(null_xid == to_clear); +} + +BOOST_AUTO_TEST_CASE(xid_to_string) +{ + wsrep::xid null_xid; + std::stringstream null_xid_str; + null_xid_str << null_xid; + BOOST_REQUIRE(null_xid_str.str() == ""); + + wsrep::xid test_xid(1,4,0,"test"); + std::string xid_str(to_string(test_xid)); + BOOST_REQUIRE(xid_str == "test"); +} + +static bool exception_check(const wsrep::runtime_error&) +{ + return true; +} + +BOOST_AUTO_TEST_CASE(xid_too_big) +{ + std::string s(65,'a'); + BOOST_REQUIRE_EXCEPTION(wsrep::xid a(1, 65, 0, s.c_str()), + wsrep::runtime_error, exception_check); + BOOST_REQUIRE_EXCEPTION(wsrep::xid b(1, 0, 65, s.c_str()), + wsrep::runtime_error, exception_check); +}