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 <sjust@redhat.com>

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index 0f36cb44b8424db5d1e08e3952bc7f8681f8c1b7..069b5ec156e70c3cfa417393580d911fa5cd6b29 100644 (file)
--- 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<OSDOp>& 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 920ef16fc2c8b438c10b84490e93dae95249fb16..cb139c1a26dc48ab575ca0f49156679ba1dcac49 100644 (file)
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -399,12 +399,10 @@ public:
     std::tuple<interruptible_future<>, interruptible_future<>>;
   using rep_op_fut_t =
     interruptible_future<rep_op_fut_tuple>;
-  template <typename MutFunc>
   rep_op_fut_t flush_changes_and_submit(
     const std::vector<OSDOp>& ops,
     SnapMapper& snap_mapper,
-    OSDriver& osdriver,
-    MutFunc mut_func) &&;
+    OSDriver& osdriver);
   std::vector<pg_log_entry_t> prepare_transaction(
     const std::vector<OSDOp>& ops);
   void fill_op_params(modified_by m);
@@ -484,76 +482,6 @@ auto OpsExecuter::with_effect_on_obc(
   return std::forward<MainFunc>(main_func)(ctx_ref);
 }
 
-template <typename MutFunc>
-OpsExecuter::rep_op_fut_t
-OpsExecuter::flush_changes_and_submit(
-  const std::vector<OSDOp>& 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 <class Func>
 struct OpsExecuter::RollbackHelper {
   void rollback_obc_if_modified();

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 3ec8f0e3ef42f07cc5a8d164e92c7996599f65b2..00527da0f1ce1806ea6320bf3d0bc92e34ebabd3 100644 (file)
--- 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 2adaf69f26b3016da468db96824be31782d48c94..78d37c8cd1244f7fe4d3eedf36ee76b6d7b4d5ca 100644 (file)
--- 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<MURef<MOSDOpReply>> do_pg_ops(Ref<MOSDOp> m);
+
+public:
   interruptible_future<
     std::tuple<interruptible_future<>, interruptible_future<>>>
   submit_transaction(
@@ -684,6 +686,8 @@ private:
     ceph::os::Transaction&& txn,
     osd_op_params_t&& oop,
     std::vector<pg_log_entry_t>&& 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<pg_shard_t> &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();