Merge pull request ceph#59652 from xxhdx1985126/wip-67945

crimson/osd/osd_operations/snaptrim_event: only increase osd_osd_param_t::at_version after clone adjustments

Reviewed-by: Matan Breizman <[email protected]>

Author: Matan-B

src/crimson/osd/ops_executer.cc

@@ -844,58 +844,6 @@ std::vector<pg_log_entry_t> OpsExecuter::prepare_transaction(
   return log_entries;
 }
 
-OpsExecuter::interruptible_future<> OpsExecuter::snap_map_remove(
-  const hobject_t& soid,
-  SnapMapper& snap_mapper,
-  OSDriver& osdriver,
-  ceph::os::Transaction& txn)
-{
-  logger().debug("{}: soid {}", __func__, soid);
-  return interruptor::async([soid, &snap_mapper,
-                             _t=osdriver.get_transaction(&txn)]() mutable {
-    const auto r = snap_mapper.remove_oid(soid, &_t);
-    if (r) {
-      logger().error("{}: remove_oid {} failed with {}",
-                     __func__, soid, r);
-    }
-    // On removal tolerate missing key corruption
-    assert(r == 0 || r == -ENOENT);
-  });
-}
-
-OpsExecuter::interruptible_future<> OpsExecuter::snap_map_modify(
-  const hobject_t& soid,
-  const std::set<snapid_t>& snaps,
-  SnapMapper& snap_mapper,
-  OSDriver& osdriver,
-  ceph::os::Transaction& txn)
-{
-  logger().debug("{}: soid {}, snaps {}", __func__, soid, snaps);
-  // TODO: avoid seastar::async https://tracker.ceph.com/issues/67704
-  return interruptor::async([soid, snaps, &snap_mapper,
-                             _t=osdriver.get_transaction(&txn)]() mutable {
-    assert(std::size(snaps) > 0);
-    [[maybe_unused]] const auto r = snap_mapper.update_snaps(
-      soid, snaps, 0, &_t);
-    assert(r == 0);
-  });
-}
-
-OpsExecuter::interruptible_future<> OpsExecuter::snap_map_clone(
-  const hobject_t& soid,
-  const std::set<snapid_t>& snaps,
-  SnapMapper& snap_mapper,
-  OSDriver& osdriver,
-  ceph::os::Transaction& txn)
-{
-  logger().debug("{}: soid {}, snaps {}", __func__, soid, snaps);
-  return interruptor::async([soid, snaps, &snap_mapper,
-                             _t=osdriver.get_transaction(&txn)]() mutable {
-    assert(std::size(snaps) > 0);
-    snap_mapper.add_oid(soid, snaps, &_t);
-  });
-}
-
 // Defined here because there is a circular dependency between OpsExecuter and PG
 uint32_t OpsExecuter::get_pool_stripe_width() const {
   return pg->get_pgpool().info.get_stripe_width();

src/crimson/osd/ops_executer.h

@@ -273,25 +273,6 @@ class OpsExecuter : public seastar::enable_lw_shared_from_this<OpsExecuter> {
     OSDriver& osdriver,
     ceph::os::Transaction& txn);
 
-public:
-  static interruptible_future<> snap_map_remove(
-    const hobject_t& soid,
-    SnapMapper& snap_mapper,
-    OSDriver& osdriver,
-    ceph::os::Transaction& txn);
-  static interruptible_future<> snap_map_modify(
-    const hobject_t& soid,
-    const std::set<snapid_t>& snaps,
-    SnapMapper& snap_mapper,
-    OSDriver& osdriver,
-    ceph::os::Transaction& txn);
-  static interruptible_future<> snap_map_clone(
-    const hobject_t& soid,
-    const std::set<snapid_t>& snaps,
-    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

src/crimson/osd/osd_operations/snaptrim_event.cc

@@ -197,7 +197,7 @@ SnapTrimObjSubEvent::remove_clone(
     pg->get_collection_ref()->get_cid(),
     ghobject_t{coid, ghobject_t::NO_GEN, shard_id_t::NO_SHARD});
   obc->obs.oi = object_info_t(coid);
-  return OpsExecuter::snap_map_remove(coid, pg->snap_mapper, pg->osdriver, txn);
+  return interruptor::now();
 }
 
 void SnapTrimObjSubEvent::remove_head_whiteout(
@@ -363,6 +363,7 @@ SnapTrimObjSubEvent::remove_or_update(
       // save head snapset
       logger().debug("{}: {} new snapset {} on {}",
 		     *this, coid, head_obc->ssc->snapset, head_obc->obs.oi);
+      osd_op_p.at_version.version++;
       if (head_obc->ssc->snapset.clones.empty() && head_obc->obs.oi.is_whiteout()) {
 	remove_head_whiteout(obc, head_obc, txn);
       } else {

src/crimson/osd/osd_operations/snaptrim_event.h

@@ -155,7 +155,6 @@ class SnapTrimObjSubEvent : public PhasedOperationT<SnapTrimObjSubEvent> {
       rid,
       mt,
       return_code);
-    osd_op_p.at_version.version++;
     return log_entries.back();
   }
 

src/crimson/osd/pg.cc

@@ -1498,6 +1498,7 @@ PG::interruptible_future<> PG::update_snap_map(
     log_entries,
     [this, &t](const auto& entry) mutable {
     if (entry.soid.snap < CEPH_MAXSNAP) {
+      // TODO: avoid seastar::async https://tracker.ceph.com/issues/67704
       return interruptor::async(
         [this, entry, _t=osdriver.get_transaction(&t)]() mutable {
         snap_mapper.update_snap_map(entry, &_t);