From dd72c0d8cb104d04173aec44af48ae743e0ff14f Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Wed, 8 May 2024 20:06:58 -0700 Subject: [PATCH] crimson/osd: remove osd_op_params_t::user_at_version osd_op_params_t::user_at_version was populated from osd_op_params_t::at_version before the call to prepare_transaction, which incremented osd_op_params_t::at_version.version. As a result, the value stored in object_info_t::user_version ended up one version behind object_info_t::version. The log entry, on the other hand, ended up with the correct version as OpsExecutor::prepare_transaction populates it directly from at_version. As a result, the primary could return different versions to the client depending on whether the IO was already in the log. This commit eliminates osd_op_params_t::user_at_version and updates PGBackend::mutate_object to behave like prepare_transaction. Because the prior commit removes the prepare_transaction increment, this isn't strictly necessary, but it is simpler. Fixes: https://tracker.ceph.com/issues/65857 Signed-off-by: Samuel Just (cherry picked from commit 85fdadab2adcfbec83a82c6f1bae1f4f31a96f54) --- src/crimson/osd/ops_executer.h | 3 --- src/crimson/osd/osd_operations/osdop_params.h | 1 - src/crimson/osd/pg_backend.cc | 4 ++-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h index 126b23213f50d..92d7b89c4a4fa 100644 --- a/src/crimson/osd/ops_executer.h +++ b/src/crimson/osd/ops_executer.h @@ -520,9 +520,6 @@ OpsExecuter::flush_changes_n_do_ops_effects( ceph_assert(want_mutate); } if (want_mutate) { - if (osd_op_params->user_modify) { - osd_op_params->user_at_version = osd_op_params->at_version.version; - } maybe_mutated = flush_clone_metadata( prepare_transaction(ops), snap_mapper, diff --git a/src/crimson/osd/osd_operations/osdop_params.h b/src/crimson/osd/osd_operations/osdop_params.h index 0f842f185f414..102cb7fff6b61 100644 --- a/src/crimson/osd/osd_operations/osdop_params.h +++ b/src/crimson/osd/osd_operations/osdop_params.h @@ -14,7 +14,6 @@ struct osd_op_params_t { eversion_t pg_trim_to; eversion_t min_last_complete_ondisk; eversion_t last_complete; - version_t user_at_version = 0; bool user_modify = false; ObjectCleanRegions clean_regions; interval_set modified_ranges; diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc index 664ebf6fe6509..e065a004d24b2 100644 --- a/src/crimson/osd/pg_backend.cc +++ b/src/crimson/osd/pg_backend.cc @@ -165,8 +165,8 @@ PGBackend::mutate_object( 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_at_version > obc->obs.oi.user_version) - obc->obs.oi.user_version = osd_op_p.user_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(); -- 2.39.5