crimson: make OpsExecuter unaware about PG.

`PG` is heavy-weight class with many responsibilities.
Exposing it to lower-layer may suggest there is far more
coupling between them than in reality.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index 027fac9a7039ea8d20ff033c494e1abf8dae8ee4..6b6614e93d8841cb28c88012107db9850620e740 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -16,6 +16,7 @@
 #include <seastar/core/thread.hh>
 
 #include "crimson/osd/exceptions.h"
+#include "crimson/osd/pg.h"
 #include "crimson/osd/watch.h"
 #include "osd/ClassHandler.h"
 

diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h
index 0efe6b43c7a757e717be52d6384fa7d6cdcea6fd..b05a99911594da69743aefe031fbf80af0564500 100644 (file)
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -25,12 +25,12 @@
 #include "crimson/osd/shard_services.h"
 #include "crimson/osd/osdmap_gate.h"
 
-#include "crimson/osd/pg.h"
 #include "crimson/osd/pg_backend.h"
 #include "crimson/osd/exceptions.h"
 
 #include "messages/MOSDOp.h"
 
+class PG;
 class PGLSFilter;
 class OSDOp;
 
@@ -85,7 +85,7 @@ private:
 
   ObjectContextRef obc;
   const OpInfo& op_info;
-  PG& pg;
+  const pg_pool_t& pool_info;  // for the sake of the ObjClass API
   PGBackend& backend;
   Ref<MOSDOp> msg;
   std::optional<osd_op_params_t> osd_op_params;
@@ -166,11 +166,15 @@ private:
   }
 
 public:
-  OpsExecuter(ObjectContextRef obc, const OpInfo& op_info, PG& pg, Ref<MOSDOp> msg)
+  OpsExecuter(ObjectContextRef obc,
+              const OpInfo& op_info,
+              const pg_pool_t& pool_info,
+              PGBackend& backend,
+              Ref<MOSDOp> msg)
     : obc(std::move(obc)),
       op_info(op_info),
-      pg(pg),
-      backend(pg.get_backend()),
+      pool_info(pool_info),
+      backend(backend),
       msg(std::move(msg)) {
   }
 
@@ -188,7 +192,7 @@ public:
   }
 
   uint32_t get_pool_stripe_width() const {
-    return pg.get_pool().info.get_stripe_width();
+    return pool_info.get_stripe_width();
   }
 };
 
@@ -233,29 +237,21 @@ OpsExecuter::osd_op_errorator::future<> OpsExecuter::flush_changes(
   Func&& func,
   MutFunc&& mut_func) &&
 {
-  assert(obc);
   const bool want_mutate = !txn.empty();
-  if (want_mutate) {
-    // osd_op_params are instantiated by every wr-like operation.
-    assert(osd_op_params);
-    osd_op_params->req = std::move(msg);
-    osd_op_params->at_version = pg.next_version();
-    osd_op_params->pg_trim_to = pg.get_pg_trim_to();
-    osd_op_params->min_last_complete_ondisk = pg.get_min_last_complete_ondisk();
-    osd_op_params->last_complete = pg.get_info().last_complete;
-    if (user_modify) {
-      osd_op_params->user_at_version = osd_op_params->at_version.version;
-    }
-  }
+  // osd_op_params are instantiated by every wr-like operation.
+  assert(osd_op_params || !want_mutate);
+  assert(obc);
   if (__builtin_expect(op_effects.empty(), true)) {
     return want_mutate ? std::forward<MutFunc>(mut_func)(std::move(txn),
                                                          std::move(obc),
-                                                         std::move(*osd_op_params))
+                                                         std::move(*osd_op_params),
+                                                         user_modify)
                        : std::forward<Func>(func)(std::move(obc));
   } else {
     return (want_mutate ? std::forward<MutFunc>(mut_func)(std::move(txn),
                                                           std::move(obc),
-                                                          std::move(*osd_op_params))
+                                                          std::move(*osd_op_params),
+                                                          user_modify)
                         : std::forward<Func>(func)(std::move(obc))
     ).safe_then([this] {
       // let's do the cleaning of `op_effects` in destructor

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 38c913c4d2396c315015df2b84d520f2dbb4d793..2b17b37d3c423a917bf1a42fb8d9aa64fade080a 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -596,6 +596,22 @@ seastar::future<> PG::submit_transaction(const OpInfo& op_info,
   });
 }
 
+osd_op_params_t&& PG::fill_op_params_bump_pg_version(
+  osd_op_params_t&& osd_op_p,
+  Ref<MOSDOp> m,
+  const bool user_modify)
+{
+  osd_op_p.req = std::move(m);
+  osd_op_p.at_version = next_version();
+  osd_op_p.pg_trim_to = get_pg_trim_to();
+  osd_op_p.min_last_complete_ondisk = get_min_last_complete_ondisk();
+  osd_op_p.last_complete = get_info().last_complete;
+  if (user_modify) {
+    osd_op_p.user_at_version = osd_op_p.at_version.version;
+  }
+  return std::move(osd_op_p);
+}
+
 seastar::future<Ref<MOSDOpReply>> PG::do_osd_ops(
   Ref<MOSDOp> m,
   ObjectContextRef obc,
@@ -608,11 +624,8 @@ seastar::future<Ref<MOSDOpReply>> PG::do_osd_ops(
   using osd_op_errorator = OpsExecuter::osd_op_errorator;
   const auto oid = m->get_snapid() == CEPH_SNAPDIR ? m->get_hobj().get_head()
                                                    : m->get_hobj();
-  // OpsExecuter gets PG as non-const as it's responsible for modying
-  // e.g. `projected_last_update`.
-  auto ox =
-    std::make_unique<OpsExecuter>(obc, op_info, *this, m);
-
+  auto ox = std::make_unique<OpsExecuter>(
+    obc, op_info, get_pool().info, get_backend(), m);
   return crimson::do_for_each(
     m->ops, [obc, m, ox = ox.get()](OSDOp& osd_op) {
     logger().debug(
@@ -636,13 +649,22 @@ seastar::future<Ref<MOSDOpReply>> PG::do_osd_ops(
       },
       [this, m, &op_info] (auto&& txn,
                           auto&& obc,
-                          auto&& osd_op_p) -> osd_op_errorator::future<> {
+                          auto&& osd_op_p,
+                           bool user_modify) -> osd_op_errorator::future<> {
        logger().debug(
          "do_osd_ops: {} - object {} submitting txn",
          *m,
          obc->obs.oi.soid);
+        auto filled_osd_op_p = fill_op_params_bump_pg_version(
+          std::move(osd_op_p),
+          std::move(m),
+          user_modify);
        return submit_transaction(
-          op_info, m->ops, std::move(obc), std::move(txn), std::move(osd_op_p));
+          op_info,
+          filled_osd_op_p.req->ops,
+          std::move(obc),
+          std::move(txn),
+          std::move(filled_osd_op_p));
       });
   }).safe_then([this,
                 m,

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index 33782da01d3efe70e54e7c0edc23a31d49acb0bd..4a2715fbc9a17bb92444bf781d26cc3aa4a54003 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -531,6 +531,10 @@ private:
   void do_peering_event(
     const boost::statechart::event_base &evt,
     PeeringCtx &rctx);
+  osd_op_params_t&& fill_op_params_bump_pg_version(
+    osd_op_params_t&& osd_op_p,
+    Ref<MOSDOp> m,
+    const bool user_modify);
   seastar::future<Ref<MOSDOpReply>> do_osd_ops(
     Ref<MOSDOp> m,
     ObjectContextRef obc,