Skip to content

Commit 5e787fd

Browse files
rzarzynskiMatan-B
andcommitted
crimson/osd: make osd_op_params::at_version coherent with last log entry
Before this commit we were doing something like: 1. initialize `at_version` with PG::projected_last_update` **incremented by one**. 2. produce a log entry at such version. 3. increment `at_version` for the sake of a further production that may never come. The problem is `osd_op_params::at_version` is higher by one than the last log entry which hurts at later stages of `osd_op_params` processing (I was hit in the shared EC code by the assertion in `PG::op_applied`). This patch changes the algorithm to: A. initialize `at_version` with PG::projected_last_update` **incremented by one**. B. increment `at_version` for the sake of the very next production. C. produce a log entry at this version. Co-authored-by: Matan Breizman <[email protected]> Signed-off-by: Radoslaw Zarzynski <[email protected]>
1 parent eca3d26 commit 5e787fd

File tree

2 files changed

+5
-3
lines changed

2 files changed

+5
-3
lines changed

src/crimson/osd/ops_executer.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,7 @@ std::vector<pg_log_entry_t> OpsExecuter::prepare_transaction(
819819
// entry.
820820
assert(obc->obs.oi.soid.snap >= CEPH_MAXSNAP);
821821
std::vector<pg_log_entry_t> log_entries;
822+
osd_op_params->at_version.version++;
822823
log_entries.emplace_back(
823824
obc->obs.exists ?
824825
pg_log_entry_t::MODIFY : pg_log_entry_t::DELETE,
@@ -829,7 +830,6 @@ std::vector<pg_log_entry_t> OpsExecuter::prepare_transaction(
829830
osd_op_params->req_id,
830831
osd_op_params->mtime,
831832
op_info.allows_returnvec() && !ops.empty() ? ops.back().rval.code : 0);
832-
osd_op_params->at_version.version++;
833833
if (op_info.allows_returnvec()) {
834834
// also the per-op values are recorded in the pg log
835835
log_entries.back().set_op_returns(ops);
@@ -1042,15 +1042,14 @@ ObjectContextRef OpsExecuter::prepare_clone(
10421042
const hobject_t& coid)
10431043
{
10441044
ceph_assert(pg->is_primary());
1045+
osd_op_params->at_version.version++;
10451046
ObjectState clone_obs{coid};
10461047
clone_obs.exists = true;
10471048
clone_obs.oi.version = osd_op_params->at_version;
10481049
clone_obs.oi.prior_version = obc->obs.oi.version;
10491050
clone_obs.oi.copy_user_bits(obc->obs.oi);
10501051
clone_obs.oi.clear_flag(object_info_t::FLAG_WHITEOUT);
10511052

1052-
osd_op_params->at_version.version++;
1053-
10541053
auto [clone_obc, existed] = pg->obc_registry.get_cached_obc(std::move(coid));
10551054
ceph_assert(!existed);
10561055

src/crimson/osd/ops_executer.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,9 @@ OpsExecuter::flush_changes_n_do_ops_effects(
529529
txn
530530
).then_interruptible([mut_func=std::move(mut_func),
531531
this](auto&& log_entries) mutable {
532+
if (auto log_rit = log_entries.rbegin(); log_rit != log_entries.rend()) {
533+
ceph_assert(log_rit->version == osd_op_params->at_version);
534+
}
532535
auto [submitted, all_completed] =
533536
std::forward<MutFunc>(mut_func)(std::move(txn),
534537
std::move(obc),

0 commit comments

Comments
 (0)