crimson/.../ops_executer: rework prepare_transaction/mutate_object

That the log entry's verison matches the object_info on the actual
object is a pretty core invariant.  This commit moves creating the
log entry for head and populating the metadata into
OpsExecuter::prepare_head_update.

As a side effect, flush_clone_metadata and CloningCtx::apply_to
were removed and split between prepare_head_update (portions
related to the head's ssc) and flush_changes_and_submit.

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 069b5ec156e70c3cfa417393580d911fa5cd6b29..59339d989be58681a3e8a9bdb21e6c5a8f4e0311 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -15,12 +15,15 @@
 
 #include <seastar/core/thread.hh>
 
+#include "crimson/common/log.h"
 #include "crimson/osd/exceptions.h"
 #include "crimson/osd/pg.h"
 #include "crimson/osd/watch.h"
 #include "osd/ClassHandler.h"
 #include "osd/SnapMapper.h"
 
+SET_SUBSYS(osd);
+
 namespace {
   seastar::logger& logger() {
     return crimson::get_logger(ceph_subsys_osd);
@@ -841,17 +844,20 @@ OpsExecuter::flush_changes_and_submit(
 
   apply_stats();
   if (want_mutate) {
-    auto log_entries = flush_clone_metadata(
-      prepare_transaction(ops),
-      snap_mapper,
-      osdriver,
-      txn);
+    std::vector<pg_log_entry_t> log_entries;
+
+    if (cloning_ctx) {
+      cloning_ctx->log_entry.mtime =
+       cloning_ctx->clone_obc->obs.oi.mtime;
+      log_entries.emplace_back(std::move(cloning_ctx->log_entry));
+    }
+
+    log_entries.emplace_back(prepare_head_update(ops, 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.
@@ -903,14 +909,24 @@ void OpsExecuter::fill_op_params(OpsExecuter::modified_by m)
   osd_op_params->user_modify = (m == modified_by::user);
 }
 
-std::vector<pg_log_entry_t> OpsExecuter::prepare_transaction(
-  const std::vector<OSDOp>& ops)
+pg_log_entry_t OpsExecuter::prepare_head_update(
+  const std::vector<OSDOp>& ops,
+  ceph::os::Transaction &txn)
 {
-  // let's ensure we don't need to inform SnapMapper about this particular
-  // entry.
+  LOG_PREFIX(OpsExecuter::prepare_head_update);
   assert(obc->obs.oi.soid.snap >= CEPH_MAXSNAP);
-  std::vector<pg_log_entry_t> log_entries;
-  log_entries.emplace_back(
+
+  update_clone_overlap();
+  if (cloning_ctx) {
+    obc->ssc->snapset = std::move(cloning_ctx->new_snapset);
+  }
+  if (snapc.seq > obc->ssc->snapset.seq) {
+     // update snapset with latest snap context
+     obc->ssc->snapset.seq = snapc.seq;
+     obc->ssc->snapset.snaps.clear();
+  }
+
+  pg_log_entry_t ret{
     obc->obs.exists ?
       pg_log_entry_t::MODIFY : pg_log_entry_t::DELETE,
     obc->obs.oi.soid,
@@ -919,15 +935,38 @@ std::vector<pg_log_entry_t> OpsExecuter::prepare_transaction(
     osd_op_params->user_modify ? osd_op_params->at_version.version : 0,
     osd_op_params->req_id,
     osd_op_params->mtime,
-    op_info.allows_returnvec() && !ops.empty() ? ops.back().rval.code : 0);
+    op_info.allows_returnvec() && !ops.empty() ? ops.back().rval.code : 0};
+
   if (op_info.allows_returnvec()) {
     // also the per-op values are recorded in the pg log
-    log_entries.back().set_op_returns(ops);
-    logger().debug("{} op_returns: {}",
-                   __func__, log_entries.back().op_returns);
+    ret.set_op_returns(ops);
+    DEBUGDPP("op returns: {}", *pg, ret.op_returns);
+  }
+  ret.clean_regions = std::move(osd_op_params->clean_regions);
+
+
+  if (obc->obs.exists) {
+    obc->obs.oi.prior_version = obc->obs.oi.version;
+    obc->obs.oi.version = osd_op_params->at_version;
+    if (osd_op_params->user_modify)
+      obc->obs.oi.user_version = osd_op_params->at_version.version;
+    obc->obs.oi.last_reqid = osd_op_params->req_id;
+    obc->obs.oi.mtime = osd_op_params->mtime;
+    obc->obs.oi.local_mtime = ceph_clock_now();
+    
+    obc->ssc->exists = true;
+    pg->get_backend().set_metadata(
+      obc->obs.oi.soid,
+      obc->obs.oi,
+      obc->obs.oi.soid.is_head() ? &(obc->ssc->snapset) : nullptr,
+      txn);
+  } else {
+    // reset cached ObjectState without enforcing eviction
+    obc->obs.oi = object_info_t(obc->obs.oi.soid);
   }
-  log_entries.back().clean_regions = std::move(osd_op_params->clean_regions);
-  return log_entries;
+  
+  DEBUGDPP("entry: {}", *pg, ret);
+  return ret;
 }
 
 // Defined here because there is a circular dependency between OpsExecuter and PG
@@ -1038,37 +1077,6 @@ void OpsExecuter::update_clone_overlap() {
   delta_stats.num_bytes += osd_op_params->modified_ranges.size();
 }
 
-void OpsExecuter::CloningContext::apply_to(
-  std::vector<pg_log_entry_t>& log_entries,
-  ObjectContext& processed_obc)
-{
-  log_entry.mtime = processed_obc.obs.oi.mtime;
-  log_entries.insert(log_entries.begin(), std::move(log_entry));
-  processed_obc.ssc->snapset = std::move(new_snapset);
-}
-
-std::vector<pg_log_entry_t>
-OpsExecuter::flush_clone_metadata(
-  std::vector<pg_log_entry_t>&& log_entries,
-  SnapMapper& snap_mapper,
-  OSDriver& osdriver,
-  ceph::os::Transaction& txn)
-{
-  assert(!txn.empty());
-  update_clone_overlap();
-  if (cloning_ctx) {
-    cloning_ctx->apply_to(log_entries, *obc);
-  }
-  if (snapc.seq > obc->ssc->snapset.seq) {
-     // update snapset with latest snap context
-     obc->ssc->snapset.seq = snapc.seq;
-     obc->ssc->snapset.snaps.clear();
-  }
-  logger().debug("{} done, initial snapset={}, new snapset={}",
-    __func__, obc->obs.oi.soid, obc->ssc->snapset);
-  return std::move(log_entries);
-}
-
 ObjectContextRef OpsExecuter::prepare_clone(
   const hobject_t& coid,
   eversion_t version)

diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h
index cb139c1a26dc48ab575ca0f49156679ba1dcac49..a6745ffaa4bcbf96cd7ec024804b0cfabc5125a2 100644 (file)
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -198,10 +198,6 @@ private:
     SnapSet new_snapset;
     pg_log_entry_t log_entry;
     ObjectContextRef clone_obc;
-
-    void apply_to(
-      std::vector<pg_log_entry_t>& log_entries,
-      ObjectContext& processed_obc);
   };
   std::unique_ptr<CloningContext> cloning_ctx;
 
@@ -263,12 +259,6 @@ private:
   */
   void update_clone_overlap();
 
-  std::vector<pg_log_entry_t> flush_clone_metadata(
-    std::vector<pg_log_entry_t>&& log_entries,
-    SnapMapper& snap_mapper,
-    OSDriver& osdriver,
-    ceph::os::Transaction& txn);
-
 private:
   // this gizmo could be wrapped in std::optional for the sake of lazy
   // initialization. we don't need it for ops that doesn't have effect
@@ -403,9 +393,11 @@ public:
     const std::vector<OSDOp>& ops,
     SnapMapper& snap_mapper,
     OSDriver& osdriver);
-  std::vector<pg_log_entry_t> prepare_transaction(
-    const std::vector<OSDOp>& ops);
   void fill_op_params(modified_by m);
+  pg_log_entry_t prepare_head_update(
+    const std::vector<OSDOp>& ops,
+    ceph::os::Transaction &txn);
+
 
   ObjectContextRef get_obc() const {
     return obc;

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 00527da0f1ce1806ea6320bf3d0bc92e34ebabd3..ec273f1d8609d239a6e4157a9b3d8a50879479f9 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -868,45 +868,6 @@ std::ostream& operator<<(std::ostream& os, const PG& pg)
   return os;
 }
 
-void PG::mutate_object(
-  ObjectContextRef& obc,
-  ceph::os::Transaction& txn,
-  osd_op_params_t& osd_op_p)
-{
-  if (obc->obs.exists) {
-    obc->obs.oi.prior_version = obc->obs.oi.version;
-    obc->obs.oi.version = osd_op_p.at_version;
-    if (osd_op_p.user_modify)
-      obc->obs.oi.user_version = osd_op_p.at_version.version;
-    obc->obs.oi.last_reqid = osd_op_p.req_id;
-    obc->obs.oi.mtime = osd_op_p.mtime;
-    obc->obs.oi.local_mtime = ceph_clock_now();
-
-    // object_info_t
-    {
-      ceph::bufferlist osv;
-      obc->obs.oi.encode_no_oid(osv, CEPH_FEATURES_ALL);
-      // TODO: get_osdmap()->get_features(CEPH_ENTITY_TYPE_OSD, nullptr));
-      txn.setattr(coll_ref->get_cid(), ghobject_t{obc->obs.oi.soid}, OI_ATTR, osv);
-    }
-
-    // snapset
-    if (obc->obs.oi.soid.snap == CEPH_NOSNAP) {
-      logger().debug("final snapset {} in {}",
-        obc->ssc->snapset, obc->obs.oi.soid);
-      ceph::bufferlist bss;
-      encode(obc->ssc->snapset, bss);
-      txn.setattr(coll_ref->get_cid(), ghobject_t{obc->obs.oi.soid}, SS_ATTR, bss);
-      obc->ssc->exists = true;
-    } else {
-      logger().debug("no snapset (this is a clone)");
-    }
-  } else {
-    // reset cached ObjectState without enforcing eviction
-    obc->obs.oi = object_info_t(obc->obs.oi.soid);
-  }
-}
-
 void PG::enqueue_push_for_backfill(
   const hobject_t &obj,
   const eversion_t &v,

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index 78d37c8cd1244f7fe4d3eedf36ee76b6d7b4d5ca..d2afeb59388e9b6155f1d8d494ff584fedca2afc 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -896,12 +896,6 @@ 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();