From: junxiang Mu <1948535941@qq.com>
Date: Mon, 1 Apr 2024 07:00:14 +0000 (-0400)
Subject: crimson/osd/ops_executer: fix snap overlap range error
X-Git-Tag: v20.0.0~2086^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F56606%2Fhead;p=ceph.git

crimson/osd/ops_executer: fix snap overlap range error

Fixes: https://tracker.ceph.com/issues/65113
Signed-off-by: junxiang Mu <1948535941@qq.com>
---

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index b23c23bfca4d..10020e9adf8a 100644
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -970,20 +970,30 @@ std::unique_ptr<OpsExecuter::CloningContext> OpsExecuter::execute_clone(
   osd_op_params->at_version.version++;
   encode(cloned_snaps, cloning_ctx->log_entry.snaps);
 
-  // update most recent clone_overlap and usage stats
-  assert(cloning_ctx->new_snapset.clones.size() > 0);
-  // In classic, we check for evicted clones before
-  // adjusting the clone_overlap.
-  // This check is redundant here since `clone_obc`
-  // was just created (See prepare_clone()).
-  interval_set<uint64_t> &newest_overlap =
-    cloning_ctx->new_snapset.clone_overlap.rbegin()->second;
-  osd_op_params->modified_ranges.intersection_of(newest_overlap);
-  delta_stats.num_bytes += osd_op_params->modified_ranges.size();
-  newest_overlap.subtract(osd_op_params->modified_ranges);
   return cloning_ctx;
 }
 
+void OpsExecuter::update_clone_overlap() {
+  interval_set<uint64_t> *newest_overlap;
+  if (cloning_ctx) {
+    newest_overlap =
+      &cloning_ctx->new_snapset.clone_overlap.rbegin()->second;
+  } else if (op_info.may_write() 
+    && obc->obs.exists 
+    && !snapc.snaps.empty() 
+    && !obc->ssc->snapset.clones.empty()) {
+    newest_overlap =
+      &obc->ssc->snapset.clone_overlap.rbegin()->second;
+  } else {
+    return;
+  }
+
+  assert(osd_op_params);
+  osd_op_params->modified_ranges.intersection_of(*newest_overlap);
+  newest_overlap->subtract(osd_op_params->modified_ranges);
+  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) &&
@@ -1002,6 +1012,7 @@ OpsExecuter::flush_clone_metadata(
 {
   assert(!txn.empty());
   auto maybe_snap_mapped = interruptor::now();
+  update_clone_overlap();
   if (cloning_ctx) {
     std::move(*cloning_ctx).apply_to(log_entries, *obc);
     const auto& coid = log_entries.front().soid;
diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h
index 6bcbe5580208..60b5c20aecdd 100644
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -255,6 +255,14 @@ private:
       && snapc.snaps[0] > initial_obc.ssc->snapset.seq; // existing obj is old
   }
 
+  /**
+  * update_clone_overlap
+  *
+  * We need to update the most recent snapshot and the overlapping
+  * part of the head object for each write operation.
+  */
+  void update_clone_overlap();
+
   interruptible_future<std::vector<pg_log_entry_t>> flush_clone_metadata(
     std::vector<pg_log_entry_t>&& log_entries,
     SnapMapper& snap_mapper,
diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc
index ab60cfd12730..143f522bb375 100644
--- a/src/crimson/osd/pg_backend.cc
+++ b/src/crimson/osd/pg_backend.cc
@@ -596,11 +596,11 @@ void PGBackend::update_size_and_usage(object_stat_sum_t& delta_stats,
   if (write_full) {
     if (oi.size) {
       ch.insert(0, oi.size);
-    } else if (length) {
-      ch.insert(offset, length);
     }
-    modified.union_of(ch);
+  } else if (length) {
+    ch.insert(offset, length);
   }
+  modified.union_of(ch);
   if (write_full ||
       (offset + length > oi.size && length)) {
     uint64_t new_size = offset + length;
diff --git a/src/test/librados/snapshots_cxx.cc b/src/test/librados/snapshots_cxx.cc
index 95dbe5da0125..3338b62056ea 100644
--- a/src/test/librados/snapshots_cxx.cc
+++ b/src/test/librados/snapshots_cxx.cc
@@ -238,8 +238,6 @@ TEST_F(LibRadosSnapshotsSelfManagedPP, RollbackPP) {
 }
 
 TEST_F(LibRadosSnapshotsSelfManagedPP, SnapOverlapPP) {
-  // WIP https://tracker.ceph.com/issues/58263
-  SKIP_IF_CRIMSON();
   std::vector<uint64_t> my_snaps;
   IoCtx readioctx;
   ASSERT_EQ(0, cluster.ioctx_create(pool_name.c_str(), readioctx));