From 7843345a19fddf9c12cb48bba4e135cd9263f43f Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Tue, 28 Feb 2023 16:20:18 +0200 Subject: [PATCH] Extracted duplicate logic into return_from_donor_state The condition to skip changing to `s_joined` for all codepaths which return from donor state. Extracted the logic into separate method. Commented start_sst_action in mock_server_service. --- include/wsrep/server_state.hpp | 3 +++ src/server_state.cpp | 30 ++++++++++++++++-------------- test/mock_server_state.hpp | 11 +++++------ 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/include/wsrep/server_state.hpp b/include/wsrep/server_state.hpp index daa90dc..b280b55 100644 --- a/include/wsrep/server_state.hpp +++ b/include/wsrep/server_state.hpp @@ -674,6 +674,9 @@ namespace wsrep enum wsrep::provider::status send_pending_rollback_events( wsrep::unique_lock& lock); + // Handle returning from donor state. + void return_from_donor_state(wsrep::unique_lock& lock); + wsrep::mutex& mutex_; wsrep::condition_variable& cond_; wsrep::server_service& server_service_; diff --git a/src/server_state.cpp b/src/server_state.cpp index b608442..2fc9b19 100644 --- a/src/server_state.cpp +++ b/src/server_state.cpp @@ -674,13 +674,7 @@ int wsrep::server_state::start_sst(const std::string& sst_request, { lock.lock(); wsrep::log_warning() << "SST preparation failed"; - // v26 API does not have JOINED event, so in anticipation of SYNCED - // we must do it here. Do not modify the state if donor lost the - // donor state e.g. due to cluster partitioning. - if (state(lock) == s_donor) - { - state(lock, s_joined); - } + return_from_donor_state(lock); ret = 1; } return ret; @@ -694,13 +688,8 @@ void wsrep::server_state::sst_sent(const wsrep::gtid& gtid, int error) wsrep::log_info() << "SST sending failed: " << error; wsrep::unique_lock lock(mutex_); - // v26 API does not have JOINED event, so in anticipation of SYNCED - // we must do it here. Do not modify the state if donor lost the - // donor state e.g. due to cluster partitioning. - if (state(lock) == s_donor) - { - state(lock, s_joined); - } + + return_from_donor_state(lock); lock.unlock(); enum provider::status const retval(provider().sst_sent(gtid, error)); @@ -1650,3 +1639,16 @@ wsrep::server_state::send_pending_rollback_events() wsrep::unique_lock lock(mutex_); return send_pending_rollback_events(lock); } + +void wsrep::server_state::return_from_donor_state( + wsrep::unique_lock& lock) +{ + assert(lock.owns_lock()); + // v26 API does not have JOINED event, so in anticipation of SYNCED + // we must do it here. Do not modify the state if donor lost the + // donor state e.g. due to cluster partitioning. + if (state(lock) == s_donor) + { + state(lock, s_joined); + } +} diff --git a/test/mock_server_state.hpp b/test/mock_server_state.hpp index 73f93ec..fabdf6b 100644 --- a/test/mock_server_state.hpp +++ b/test/mock_server_state.hpp @@ -173,15 +173,14 @@ namespace wsrep { return sst_before_init_; } std::string sst_request() WSREP_OVERRIDE { return ""; } - std::function start_sst_action{}; + // Action to take when start_sst() method is called. + // This can be overriden by test case to inject custom + // behavior. + std::function start_sst_action{[](){ return 0; }}; int start_sst(const std::string&, const wsrep::gtid&, bool) WSREP_OVERRIDE { - if (start_sst_action) - { - return start_sst_action(); - } - return 0; + return start_sst_action(); } void