From 275a0af8c5b92f0ee33cfe9e23f3db5f59b56e9d Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Mon, 16 Jan 2023 17:34:47 +0200 Subject: [PATCH] Return error codes instead of throwing exception Changed server_state public methods sst_received() and wait_until_state() to report errors as return value instead of throwing exceptions. This was done to gradually get rid of public methods which report errors via exceptions. This change was part of MDEV-30419. --- dbsim/db_simulator.cpp | 25 +++++++++++++++++++------ include/wsrep/server_state.hpp | 16 +++++++++------- src/server_state.cpp | 31 +++++++++++++++++++++++++++---- test/server_context_test.cpp | 11 +++++------ 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/dbsim/db_simulator.cpp b/dbsim/db_simulator.cpp index 0971b11..97a1818 100644 --- a/dbsim/db_simulator.cpp +++ b/dbsim/db_simulator.cpp @@ -77,7 +77,10 @@ void db::simulator::sst(db::server& server, db::client dummy(*(i->second), wsrep::client_id(-1), wsrep::client_state::m_local, params()); - i->second->server_state().sst_received(dummy.client_service(), 0); + if (i->second->server_state().sst_received(dummy.client_service(), 0)) + { + throw wsrep::runtime_error("Call to SST received failed"); + } server.server_state().sst_sent(gtid, 0); } @@ -169,12 +172,19 @@ void db::simulator::start() wsrep::log_debug() << "main: Starting applier"; server.start_applier(); wsrep::log_debug() << "main: Waiting initializing state"; - server.server_state().wait_until_state(wsrep::server_state::s_initializing); + if (server.server_state().wait_until_state( + wsrep::server_state::s_initializing)) + { + throw wsrep::runtime_error("Failed to reach initializing state"); + } wsrep::log_debug() << "main: Calling initialized"; server.server_state().initialized(); wsrep::log_debug() << "main: Waiting for synced state"; - server.server_state().wait_until_state( - wsrep::server_state::s_synced); + if (server.server_state().wait_until_state( + wsrep::server_state::s_synced)) + { + throw wsrep::runtime_error("Failed to reach synced state"); + } wsrep::log_debug() << "main: Server synced"; } @@ -220,8 +230,11 @@ void db::simulator::stop() wsrep::log_info() << sv.name() << " = " << sv.value(); }); server.server_state().disconnect(); - server.server_state().wait_until_state( - wsrep::server_state::s_disconnected); + if (server.server_state().wait_until_state( + wsrep::server_state::s_disconnected)) + { + throw wsrep::runtime_error("Failed to reach disconnected state"); + } server.stop_applier(); server.server_state().unload_provider(); } diff --git a/include/wsrep/server_state.hpp b/include/wsrep/server_state.hpp index 0fa6420..26ca073 100644 --- a/include/wsrep/server_state.hpp +++ b/include/wsrep/server_state.hpp @@ -369,12 +369,11 @@ namespace wsrep /** * Wait until server reaches given state. + * + * @return Zero in case of success, non-zero if the + * wait was interrupted. */ - void wait_until_state(enum state state) const - { - wsrep::unique_lock lock(mutex_); - wait_until_state(lock, state); - } + int wait_until_state(enum state state) const; /** * Return GTID at the position when server connected to @@ -509,8 +508,10 @@ namespace wsrep * * @param client_service * @param error code of the SST operation + * + * @return Zero in case of success, non-zero on error. */ - void sst_received(wsrep::client_service& cs, int error); + int sst_received(wsrep::client_service& cs, int error); /** * This method must be called after the server initialization @@ -644,7 +645,8 @@ namespace wsrep int desync(wsrep::unique_lock&); void resync(wsrep::unique_lock&); void state(wsrep::unique_lock&, enum state); - void wait_until_state(wsrep::unique_lock&, enum state) const; + void wait_until_state(wsrep::unique_lock&, + enum state) const; // Interrupt all threads which are waiting for state void interrupt_state_waiters(wsrep::unique_lock&); diff --git a/src/server_state.cpp b/src/server_state.cpp index d75ffd8..795127c 100644 --- a/src/server_state.cpp +++ b/src/server_state.cpp @@ -703,8 +703,9 @@ void wsrep::server_state::sst_sent(const wsrep::gtid& gtid, int error) } } -void wsrep::server_state::sst_received(wsrep::client_service& cs, +int wsrep::server_state::sst_received(wsrep::client_service& cs, int const error) +try { wsrep::log_info() << "SST received"; wsrep::gtid gtid(wsrep::gtid::undefined()); @@ -800,10 +801,20 @@ void wsrep::server_state::sst_received(wsrep::client_service& cs, enum provider::status const retval(provider().sst_received(gtid, error)); if (retval != provider::success) { - std::string msg("wsrep::sst_received() failed: "); - msg += wsrep::provider::to_string(retval); - throw wsrep::runtime_error(msg); + wsrep::log_error() << "provider.sst_received() failed: " + << wsrep::provider::to_string(retval); + return 1; } + return 0; +} +catch (const wsrep::runtime_error& e) +{ + wsrep::log_error() << "sst_received failed: " << e.what(); + if (provider_) + { + provider_->sst_received(wsrep::gtid::undefined(), -EINTR); + } + return 1; } void wsrep::server_state::initialized() @@ -1388,6 +1399,18 @@ void wsrep::server_state::wait_until_state( cond_.notify_all(); } +int wsrep::server_state::wait_until_state(enum state state) const +try +{ + wsrep::unique_lock lock(mutex_); + wait_until_state(lock, state); + return 0; +} +catch (...) +{ + return 1; +} + void wsrep::server_state::interrupt_state_waiters( wsrep::unique_lock& lock WSREP_UNUSED) { diff --git a/test/server_context_test.cpp b/test/server_context_test.cpp index 1c0b8bd..6934b1c 100644 --- a/test/server_context_test.cpp +++ b/test/server_context_test.cpp @@ -343,7 +343,7 @@ BOOST_FIXTURE_TEST_CASE(server_state_sst_first_join_with_sst, // case where SST contains the view in which SST happens. server_service.logged_view(second_view); server_service.position(wsrep::gtid(cluster_id, wsrep::seqno(2))); - ss.sst_received(cc, 0); + BOOST_REQUIRE(ss.sst_received(cc, 0) == 0); clear_sync_point_action(); BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joined); ss.on_sync(); @@ -429,7 +429,7 @@ BOOST_FIXTURE_TEST_CASE( ss.prepare_for_sst(); BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joiner); server_service.position(wsrep::gtid::undefined()); - ss.sst_received(cc, 1); + BOOST_REQUIRE(ss.sst_received(cc, 1) == 0); disconnect(); } @@ -442,8 +442,7 @@ BOOST_FIXTURE_TEST_CASE( BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joiner); initialization_failure_action(); server_service.position(wsrep::gtid(second_view.state_id())); - BOOST_REQUIRE_EXCEPTION(ss.sst_received(cc, 0), - wsrep::runtime_error, exception_check); + BOOST_REQUIRE(ss.sst_received(cc, 0) != 0); BOOST_REQUIRE(ss.state() == wsrep::server_state::s_initializing); BOOST_REQUIRE_EXCEPTION(ss.on_view(second_view, &hps), wsrep::runtime_error, exception_check); @@ -466,7 +465,7 @@ BOOST_FIXTURE_TEST_CASE( // case where SST contains the view in which SST happens. server_service.logged_view(second_view); server_service.position(wsrep::gtid(cluster_id, wsrep::seqno(2))); - ss.sst_received(cc, 0); + BOOST_REQUIRE(ss.sst_received(cc, 0) == 0); clear_sync_point_action(); BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joined); disconnect(); @@ -507,7 +506,7 @@ BOOST_FIXTURE_TEST_CASE(server_state_init_first_join_with_sst, BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joiner); server_service.logged_view(second_view); server_service.position(wsrep::gtid(cluster_id, wsrep::seqno(2))); - ss.sst_received(cc, 0); + BOOST_REQUIRE(ss.sst_received(cc, 0) == 0); BOOST_REQUIRE(ss.state() == wsrep::server_state::s_joined); ss.on_sync(); BOOST_REQUIRE(ss.state() == wsrep::server_state::s_synced);