From ff55c4f8737ed8a804e248433e412121437c9c8e Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 14 Nov 2024 18:08:10 -0800 Subject: [PATCH] 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 --- src/crimson/osd/ops_executer.cc | 106 +++++++++++++++++--------------- src/crimson/osd/ops_executer.h | 16 ++--- src/crimson/osd/pg.cc | 39 ------------ src/crimson/osd/pg.h | 6 -- 4 files changed, 61 insertions(+), 106 deletions(-) diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc index 069b5ec156e70..59339d989be58 100644 --- a/src/crimson/osd/ops_executer.cc +++ b/src/crimson/osd/ops_executer.cc @@ -15,12 +15,15 @@ #include +#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 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 OpsExecuter::prepare_transaction( - const std::vector& ops) +pg_log_entry_t OpsExecuter::prepare_head_update( + const std::vector& 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 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 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& 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 -OpsExecuter::flush_clone_metadata( - std::vector&& 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 cb139c1a26dc4..a6745ffaa4bcb 100644 --- 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& log_entries, - ObjectContext& processed_obc); }; std::unique_ptr cloning_ctx; @@ -263,12 +259,6 @@ private: */ void update_clone_overlap(); - std::vector flush_clone_metadata( - std::vector&& 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& ops, SnapMapper& snap_mapper, OSDriver& osdriver); - std::vector prepare_transaction( - const std::vector& ops); void fill_op_params(modified_by m); + pg_log_entry_t prepare_head_update( + const std::vector& 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 00527da0f1ce1..ec273f1d8609d 100644 --- 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 78d37c8cd1244..d2afeb59388e9 100644 --- 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 &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