From b3f0e05da4e626256bcb4a77383852bebfcbe8d0 Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Sun, 1 May 2022 16:37:24 +0300 Subject: [PATCH] Pass victim context for provider on BF abort This change is needed for custom provider implementations to have a way to access the victim in the application context. Helper interface operation_context to pass caller context for service/provider callbacks in more type safe way. --- include/wsrep/client_state.hpp | 32 +++++++++++++++++++--- include/wsrep/operation_context.hpp | 41 +++++++++++++++++++++++++++++ include/wsrep/provider.hpp | 2 ++ include/wsrep/transaction.hpp | 7 +++-- src/transaction.cpp | 10 ++++--- src/wsrep_provider_v26.cpp | 1 + src/wsrep_provider_v26.hpp | 1 + test/mock_provider.hpp | 1 + test/test_utils.cpp | 3 ++- 9 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 include/wsrep/operation_context.hpp diff --git a/include/wsrep/client_state.hpp b/include/wsrep/client_state.hpp index b502cbd..8d02f60 100644 --- a/include/wsrep/client_state.hpp +++ b/include/wsrep/client_state.hpp @@ -43,6 +43,7 @@ #include "thread.hpp" #include "xid.hpp" #include "chrono.hpp" +#include "operation_context.hpp" namespace wsrep { @@ -662,22 +663,45 @@ namespace wsrep * called by a transaction which needs to BF abort a conflicting * locally processing transaction. */ - int bf_abort(wsrep::seqno bf_seqno) + int bf_abort(wsrep::seqno bf_seqno, + wsrep::operation_context& victim_ctx) { wsrep::unique_lock lock(mutex_); assert(mode_ == m_local || transaction_.is_streaming()); - return transaction_.bf_abort(lock, bf_seqno); + return transaction_.bf_abort(lock, bf_seqno, victim_ctx); + } + + /** + * For backwards compatibility when the caller does not pass + * victim_ctx. + */ + int bf_abort(wsrep::seqno bf_seqno) + { + wsrep::null_operation_context victim_ctx; + return bf_abort(bf_seqno, victim_ctx); } /** * Brute force abort a transaction in total order. This method * should be called by the TOI operation which needs to * BF abort a transaction. */ - int total_order_bf_abort(wsrep::seqno bf_seqno) + int total_order_bf_abort(wsrep::seqno bf_seqno, + wsrep::operation_context& victim_ctx) { wsrep::unique_lock lock(mutex_); assert(mode_ == m_local || transaction_.is_streaming()); - return transaction_.total_order_bf_abort(lock, bf_seqno); + return transaction_.total_order_bf_abort(lock, bf_seqno, + victim_ctx); + } + + /** + * For backwards compatibility when the caller does not pass + * victim_ctx. + */ + int total_order_bf_abort(wsrep::seqno bf_seqno) + { + wsrep::null_operation_context victim_ctx; + return total_order_bf_abort(bf_seqno, victim_ctx); } /** diff --git a/include/wsrep/operation_context.hpp b/include/wsrep/operation_context.hpp new file mode 100644 index 0000000..3bd5c1c --- /dev/null +++ b/include/wsrep/operation_context.hpp @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2022 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 . + */ + +/** @file operation_context.hpp + * + * A slightly more type safe way compared to void pointers to + * pass operation contexts through wsrep-lib to callbacks. + */ + +#ifndef WSREP_OPERATION_CONTEXT_HPP +#define WSREP_OPERATION_CONTEXT_HPP + +namespace wsrep +{ + struct operation_context + { + virtual ~operation_context() = default; + }; + + struct null_operation_context : operation_context + { + }; +} // namespace wsrep + +#endif /* WSREP_OPERATION_CONTEXT_HPP */ diff --git a/include/wsrep/provider.hpp b/include/wsrep/provider.hpp index 4da01f8..b5b4672 100644 --- a/include/wsrep/provider.hpp +++ b/include/wsrep/provider.hpp @@ -26,6 +26,7 @@ #include "client_id.hpp" #include "transaction_id.hpp" #include "compiler.hpp" +#include "operation_context.hpp" #include #include @@ -342,6 +343,7 @@ namespace wsrep */ virtual enum status bf_abort(wsrep::seqno bf_seqno, wsrep::transaction_id victim_trx, + wsrep::operation_context& victim_ctx, wsrep::seqno& victim_seqno) = 0; virtual enum status rollback(wsrep::transaction_id) = 0; virtual enum status commit_order_enter(const wsrep::ws_handle&, diff --git a/include/wsrep/transaction.hpp b/include/wsrep/transaction.hpp index 76835fd..9e70ccd 100644 --- a/include/wsrep/transaction.hpp +++ b/include/wsrep/transaction.hpp @@ -30,6 +30,7 @@ #include "buffer.hpp" #include "client_service.hpp" #include "xid.hpp" +#include "operation_context.hpp" #include #include @@ -199,9 +200,11 @@ namespace wsrep void after_applying(); bool bf_abort(wsrep::unique_lock& lock, - wsrep::seqno bf_seqno); + wsrep::seqno bf_seqno, + wsrep::operation_context&); bool total_order_bf_abort(wsrep::unique_lock&, - wsrep::seqno bf_seqno); + wsrep::seqno bf_seqno, + wsrep::operation_context&); void clone_for_replay(const wsrep::transaction& other); diff --git a/src/transaction.cpp b/src/transaction.cpp index 69380a4..c961f8d 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -964,7 +964,8 @@ void wsrep::transaction::after_applying() bool wsrep::transaction::bf_abort( wsrep::unique_lock& lock, - wsrep::seqno bf_seqno) + wsrep::seqno bf_seqno, + wsrep::operation_context& victim_ctx) { bool ret(false); const enum wsrep::transaction::state state_at_enter(state()); @@ -989,7 +990,7 @@ bool wsrep::transaction::bf_abort( wsrep::seqno victim_seqno; enum wsrep::provider::status status(client_state_.provider().bf_abort( - bf_seqno, id_, victim_seqno)); + bf_seqno, id_, victim_ctx, victim_seqno)); switch (status) { case wsrep::provider::success: @@ -1077,9 +1078,10 @@ bool wsrep::transaction::bf_abort( bool wsrep::transaction::total_order_bf_abort( wsrep::unique_lock& lock WSREP_UNUSED, - wsrep::seqno bf_seqno) + wsrep::seqno bf_seqno, + wsrep::operation_context& victim_ctx) { - bool ret(bf_abort(lock, bf_seqno)); + bool ret(bf_abort(lock, bf_seqno, victim_ctx)); if (ret) { bf_aborted_in_total_order_ = true; diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp index 68e3f58..b8803ae 100644 --- a/src/wsrep_provider_v26.cpp +++ b/src/wsrep_provider_v26.cpp @@ -895,6 +895,7 @@ enum wsrep::provider::status wsrep::wsrep_provider_v26::bf_abort( wsrep::seqno bf_seqno, wsrep::transaction_id victim_id, + wsrep::operation_context& /* Ignored here */, wsrep::seqno& victim_seqno) { wsrep_seqno_t wsrep_victim_seqno; diff --git a/src/wsrep_provider_v26.hpp b/src/wsrep_provider_v26.hpp index a8c9366..c945a24 100644 --- a/src/wsrep_provider_v26.hpp +++ b/src/wsrep_provider_v26.hpp @@ -63,6 +63,7 @@ namespace wsrep enum wsrep::provider::status bf_abort(wsrep::seqno, wsrep::transaction_id, + wsrep::operation_context&, wsrep::seqno&) WSREP_OVERRIDE; enum wsrep::provider::status rollback(const wsrep::transaction_id) WSREP_OVERRIDE; diff --git a/test/mock_provider.hpp b/test/mock_provider.hpp index dc7cbfd..e19fc6d 100644 --- a/test/mock_provider.hpp +++ b/test/mock_provider.hpp @@ -310,6 +310,7 @@ namespace wsrep enum wsrep::provider::status bf_abort(wsrep::seqno bf_seqno, wsrep::transaction_id trx_id, + wsrep::operation_context&, wsrep::seqno& victim_seqno) WSREP_OVERRIDE { diff --git a/test/test_utils.cpp b/test/test_utils.cpp index da8a342..ec31078 100644 --- a/test/test_utils.cpp +++ b/test/test_utils.cpp @@ -40,7 +40,8 @@ void wsrep_test::bf_abort_provider(wsrep::mock_server_state& sc, wsrep::seqno bf_seqno) { wsrep::seqno victim_seqno; - sc.provider().bf_abort(bf_seqno, tc.id(), victim_seqno); + wsrep::null_operation_context victim_ctx; + sc.provider().bf_abort(bf_seqno, tc.id(), victim_ctx, victim_seqno); (void)victim_seqno; }