From 1cf5ca7848a48fd0ad1d098937c8eb7b48febc60 Mon Sep 17 00:00:00 2001 From: sjaakola Date: Thu, 16 Mar 2023 22:34:58 +0200 Subject: [PATCH 1/2] Changed background_rollback to return true/false depending on if background rollback did happen. Background rollback should be skipped if the aborting happens due to KILL command issued by user. Some KILL signals, like KILL CONECTION, wake up the victim too early so that background rollback could happen in parallel with the victim waking up and continuing execution. --- dbsim/db_server_service.cpp | 3 ++- dbsim/db_server_service.hpp | 2 +- include/wsrep/server_service.hpp | 2 +- src/transaction.cpp | 4 +++- test/mock_server_state.hpp | 3 ++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/dbsim/db_server_service.cpp b/dbsim/db_server_service.cpp index 8fc58dde..6d49c6f2 100644 --- a/dbsim/db_server_service.cpp +++ b/dbsim/db_server_service.cpp @@ -86,8 +86,9 @@ int db::server_service::start_sst( return 0; } -void db::server_service::background_rollback(wsrep::client_state&) +bool db::server_service::background_rollback(wsrep::client_state&) { + return false; } void db::server_service::bootstrap() diff --git a/dbsim/db_server_service.hpp b/dbsim/db_server_service.hpp index e5cd70d2..fd47c5f8 100644 --- a/dbsim/db_server_service.hpp +++ b/dbsim/db_server_service.hpp @@ -41,7 +41,7 @@ namespace db bool sst_before_init() const override; int start_sst(const std::string&, const wsrep::gtid&, bool) override; std::string sst_request() override; - void background_rollback(wsrep::client_state&) override; + bool background_rollback(wsrep::client_state&) override; void bootstrap() override; void log_message(enum wsrep::log::level, const char* message) override; void log_dummy_write_set(wsrep::client_state&, const wsrep::ws_meta&) diff --git a/include/wsrep/server_service.hpp b/include/wsrep/server_service.hpp index 2021e66d..25b265e5 100644 --- a/include/wsrep/server_service.hpp +++ b/include/wsrep/server_service.hpp @@ -86,7 +86,7 @@ namespace wsrep /** * Perform a background rollback for a transaction. */ - virtual void background_rollback(wsrep::client_state&) = 0; + virtual bool background_rollback(wsrep::client_state&) = 0; /** * Bootstrap a DBMS state for a new cluster. diff --git a/src/transaction.cpp b/src/transaction.cpp index c864a0fa..d581c2f7 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -1074,7 +1074,9 @@ bool wsrep::transaction::bf_abort( } lock.unlock(); - server_service_.background_rollback(client_state_); + /* if background rollback is skipped, reset rollbacker activity */ + if (server_service_.background_rollback(client_state_)) + client_state_.set_rollbacker_active(false); lock.lock(); } } diff --git a/test/mock_server_state.hpp b/test/mock_server_state.hpp index fabdf6b7..49fb4900 100644 --- a/test/mock_server_state.hpp +++ b/test/mock_server_state.hpp @@ -183,11 +183,12 @@ namespace wsrep return start_sst_action(); } - void + bool background_rollback(wsrep::client_state& client_state) WSREP_OVERRIDE { client_state.before_rollback(); client_state.after_rollback(); + return false; } int wait_committing_transactions(int) WSREP_OVERRIDE { return 0; } From e20e53fb1af03209588012038d0deff8bbd3a49d Mon Sep 17 00:00:00 2001 From: sjaakola Date: Sun, 9 Apr 2023 17:15:04 +0300 Subject: [PATCH 2/2] Fixes according to review comments --- include/wsrep/server_service.hpp | 1 + src/transaction.cpp | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/wsrep/server_service.hpp b/include/wsrep/server_service.hpp index 25b265e5..75572ae3 100644 --- a/include/wsrep/server_service.hpp +++ b/include/wsrep/server_service.hpp @@ -85,6 +85,7 @@ namespace wsrep /** * Perform a background rollback for a transaction. + * @return true if rollbacker was not started, false otherwise */ virtual bool background_rollback(wsrep::client_state&) = 0; diff --git a/src/transaction.cpp b/src/transaction.cpp index d581c2f7..bcc430f9 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -1076,8 +1076,17 @@ bool wsrep::transaction::bf_abort( lock.unlock(); /* if background rollback is skipped, reset rollbacker activity */ if (server_service_.background_rollback(client_state_)) - client_state_.set_rollbacker_active(false); - lock.lock(); + { + lock.lock(); + client_state_.set_rollbacker_active(false); + + /* release the victim from waiting, if it has advanced to + wait_rollback_complete_and_acquire_ownership stage */ + if (client_state_.state() == wsrep::client_state::s_idle) + client_state_.cond_.notify_all(); + } + else + lock.lock(); } } return ret;