From fc029270ef410f8520931bc7cf2fd867102fe979 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 14 Nov 2024 17:32:03 -0800 Subject: [PATCH] crimson/.../ops_executer: just call submit_transaction in flush_changes_n_do_ops_effects Templating MutFunc was pretty confusing, and flush_changes_n_do_ops_effects is already closely coupled to PG::submit_transaction. Signed-off-by: Samuel Just --- src/crimson/osd/ops_executer.cc | 70 +++++++++++++++++++++++++++++++ src/crimson/osd/ops_executer.h | 74 +-------------------------------- src/crimson/osd/pg.cc | 18 +------- src/crimson/osd/pg.h | 6 +++ 4 files changed, 79 insertions(+), 89 deletions(-) diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc index 0f36cb44b8424..069b5ec156e70 100644 --- a/src/crimson/osd/ops_executer.cc +++ b/src/crimson/osd/ops_executer.cc @@ -821,6 +821,76 @@ OpsExecuter::do_execute_op(OSDOp& osd_op) } } +OpsExecuter::rep_op_fut_t +OpsExecuter::flush_changes_and_submit( + const std::vector& ops, + SnapMapper& snap_mapper, + OSDriver& osdriver) +{ + const bool want_mutate = !txn.empty(); + // osd_op_params are instantiated by every wr-like operation. + assert(osd_op_params || !want_mutate); + assert(obc); + + auto submitted = interruptor::now(); + auto all_completed = interruptor::now(); + + if (cloning_ctx) { + ceph_assert(want_mutate); + } + + apply_stats(); + if (want_mutate) { + auto log_entries = flush_clone_metadata( + prepare_transaction(ops), + snap_mapper, + osdriver, + txn); + + if (auto log_rit = log_entries.rbegin(); log_rit != log_entries.rend()) { + ceph_assert(log_rit->version == osd_op_params->at_version); + } + + pg->mutate_object(obc, txn, *osd_op_params); + /* + * This works around the gcc bug causing the generated code to incorrectly + * execute unconditionally before the predicate. + * + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101244 + */ + auto clone_obc = cloning_ctx + ? std::move(cloning_ctx->clone_obc) + : nullptr; + auto [_submitted, _all_completed] = co_await pg->submit_transaction( + std::move(obc), + std::move(clone_obc), + std::move(txn), + std::move(*osd_op_params), + std::move(log_entries) + ); + + submitted = std::move(_submitted); + all_completed = std::move(_all_completed); + } + + if (op_effects.size()) [[unlikely]] { + // need extra ref pg due to apply_stats() which can be executed after + // informing snap mapper + all_completed = + std::move(all_completed).then_interruptible([this, pg=this->pg] { + // let's do the cleaning of `op_effects` in destructor + return interruptor::do_for_each(op_effects, + [pg=std::move(pg)](auto& op_effect) { + return op_effect->execute(pg); + }); + }); + } + + co_return std::make_tuple( + std::move(submitted), + std::move(all_completed)); +} + void OpsExecuter::fill_op_params(OpsExecuter::modified_by m) { osd_op_params.emplace(); diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h index 920ef16fc2c8b..cb139c1a26dc4 100644 --- a/src/crimson/osd/ops_executer.h +++ b/src/crimson/osd/ops_executer.h @@ -399,12 +399,10 @@ public: std::tuple, interruptible_future<>>; using rep_op_fut_t = interruptible_future; - template rep_op_fut_t flush_changes_and_submit( const std::vector& ops, SnapMapper& snap_mapper, - OSDriver& osdriver, - MutFunc mut_func) &&; + OSDriver& osdriver); std::vector prepare_transaction( const std::vector& ops); void fill_op_params(modified_by m); @@ -484,76 +482,6 @@ auto OpsExecuter::with_effect_on_obc( return std::forward(main_func)(ctx_ref); } -template -OpsExecuter::rep_op_fut_t -OpsExecuter::flush_changes_and_submit( - const std::vector& ops, - SnapMapper& snap_mapper, - OSDriver& osdriver, - MutFunc mut_func) && -{ - const bool want_mutate = !txn.empty(); - // osd_op_params are instantiated by every wr-like operation. - assert(osd_op_params || !want_mutate); - assert(obc); - - auto submitted = interruptor::now(); - auto all_completed = interruptor::now(); - - if (cloning_ctx) { - ceph_assert(want_mutate); - } - - apply_stats(); - if (want_mutate) { - auto log_entries = flush_clone_metadata( - prepare_transaction(ops), - snap_mapper, - osdriver, - txn); - - if (auto log_rit = log_entries.rbegin(); log_rit != log_entries.rend()) { - ceph_assert(log_rit->version == osd_op_params->at_version); - } - - /* - * This works around the gcc bug causing the generated code to incorrectly - * execute unconditionally before the predicate. - * - * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101244 - */ - auto clone_obc = cloning_ctx - ? std::move(cloning_ctx->clone_obc) - : nullptr; - auto [_submitted, _all_completed] = co_await mut_func( - std::move(txn), - std::move(obc), - std::move(*osd_op_params), - std::move(log_entries), - std::move(clone_obc)); - - submitted = std::move(_submitted); - all_completed = std::move(_all_completed); - } - - if (op_effects.size()) [[unlikely]] { - // need extra ref pg due to apply_stats() which can be executed after - // informing snap mapper - all_completed = - std::move(all_completed).then_interruptible([this, pg=this->pg] { - // let's do the cleaning of `op_effects` in destructor - return interruptor::do_for_each(op_effects, - [pg=std::move(pg)](auto& op_effect) { - return op_effect->execute(pg); - }); - }); - } - - co_return std::make_tuple( - std::move(submitted), - std::move(all_completed)); -} - template struct OpsExecuter::RollbackHelper { void rollback_obc_if_modified(); diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index 3ec8f0e3ef42f..00527da0f1ce1 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -1175,22 +1175,8 @@ PG::submit_executer_fut PG::submit_executer( ).flush_changes_and_submit( ops, snap_mapper, - osdriver, - [FNAME, this](auto&& txn, - auto&& obc, - auto&& osd_op_p, - auto&& log_entries, - auto&& new_clone) { - DEBUGDPP("object {} submitting txn", *this, obc->get_oid()); - mutate_object(obc, txn, osd_op_p); - return submit_transaction( - std::move(obc), - std::move(new_clone), - std::move(txn), - std::move(osd_op_p), - std::move(log_entries)); - }); - + osdriver + ); co_return std::make_tuple( std::move(submitted).then_interruptible([unlocker=std::move(unlocker)] {}), std::move(completed)); diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index 2adaf69f26b30..78d37c8cd1244 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -676,6 +676,8 @@ private: struct do_osd_ops_params_t; interruptible_future> do_pg_ops(Ref m); + +public: interruptible_future< std::tuple, interruptible_future<>>> submit_transaction( @@ -684,6 +686,8 @@ private: ceph::os::Transaction&& txn, osd_op_params_t&& oop, std::vector&& log_entries); + +private: interruptible_future<> repair_object( const hobject_t& oid, eversion_t& v); @@ -892,10 +896,12 @@ private: const hobject_t &obj, const eversion_t &v, const std::vector &peers); +public: void mutate_object( ObjectContextRef& obc, ceph::os::Transaction& txn, osd_op_params_t& osd_op_p); +private: bool can_discard_replica_op(const Message& m, epoch_t m_map_epoch) const; bool can_discard_op(const MOSDOp& m) const; void context_registry_on_change(); -- 2.39.5