crimson/osd/object_context_loader: SnapTrim to not resolve_oid

SnapTrimObjSubEvent::remove_or_update partially resolves the to be
trimmed clone taking into account in_removed_snaps_queue.
The general resolve_oid is not suitable for this scenario.
Specifically the following check:
```
    if (std::find(
      citer->second.begin(),
      citer->second.end(),
      oid.snap) == citer->second.end()) {
       logger().debug("{} {} does not contain {} -- DNE",
                      __func__, ss.clone_snaps, oid.snap);
       return std::nullopt;
    }
```
because of earlier snap_map_modify call.

Example:
```
INFO  2024-04-07 13:44:01,118 [shard 0:main] osd - SnapTrimObjSubEvent(coid=2:e8855410:::folio011816418-576:8 snapid=8): 2:e8855410:::folio011816418-576:8 snaps [8, 7] -> {7}
DEBUG 2024-04-07 13:44:01,118 [shard 0:main] osd - snap_map_modify: soid 2:e8855410:::folio011816418-576:8, snaps {7}
...
This case will fail:
INFO  2024-04-07 13:44:04,139 [shard 0:main] osd - SnapTrimObjSubEvent(coid=2:e8855410:::folio011816418-576:8 snapid=7): 2:e8855410:::folio011816418-576:8 snaps [7] -> {} ... deleting
DEBUG 2024-04-07 13:44:04,139 [shard 0:main] osd - snap_map_remove: soid 2:e8855410:::folio011816418-576:8
```

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
(cherry picked from commit 263b2ae77ac1df8a939b82101eca09f1d8d4089a)

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index 158c61ee5c43e2428dd25ef246570178bb451a14..b53cbabd04c91c7d3b2dca655a343b650b861ba4 100644 (file)
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -37,14 +37,15 @@ using crimson::common::local_conf;
   template<RWState::State State>
   ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_clone_obc(const hobject_t& oid,
-                                      with_obc_func_t&& func)
+                                      with_obc_func_t&& func,
+                                      bool resolve_clone)
   {
     LOG_PREFIX(ObjectContextLoader::with_clone_obc);
     assert(!oid.is_head());
     return with_head_obc<RWState::RWREAD>(
       oid.get_head(),
-      [FNAME, oid, func=std::move(func), this](auto head, auto) mutable
-      -> load_obc_iertr::future<> {
+      [FNAME, oid, func=std::move(func), resolve_clone, this]
+      (auto head, auto) mutable -> load_obc_iertr::future<> {
       if (!head->obs.exists) {
        ERRORDPP("head doesn't exist for object {}", dpp, head->obs.oi.soid);
         return load_obc_iertr::future<>{
@@ -53,7 +54,8 @@ using crimson::common::local_conf;
       }
       return this->with_clone_obc_only<State>(std::move(head),
                                               oid,
-                                              std::move(func));
+                                              std::move(func),
+                                              resolve_clone);
     });
   }
 
@@ -61,23 +63,27 @@ using crimson::common::local_conf;
   ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_clone_obc_only(ObjectContextRef head,
                                            hobject_t clone_oid,
-                                           with_obc_func_t&& func)
+                                           with_obc_func_t&& func,
+                                           bool resolve_clone)
   {
     LOG_PREFIX(ObjectContextLoader::with_clone_obc_only);
     DEBUGDPP("{}", clone_oid);
     assert(!clone_oid.is_head());
-    auto resolved_oid = resolve_oid(head->get_head_ss(), clone_oid);
-    if (!resolved_oid) {
-      ERRORDPP("clone {} not found", dpp, clone_oid);
-      return load_obc_iertr::future<>{
-        crimson::ct_error::enoent::make()
-      };
-    }
-    if (resolved_oid->is_head()) {
-      // See resolve_oid
-      return std::move(func)(head, head);
+    if (resolve_clone) {
+      auto resolved_oid = resolve_oid(head->get_head_ss(), clone_oid);
+      if (!resolved_oid) {
+        ERRORDPP("clone {} not found", dpp, clone_oid);
+        return load_obc_iertr::future<>{
+          crimson::ct_error::enoent::make()
+        };
+      }
+      if (resolved_oid->is_head()) {
+        // See resolve_oid
+        return std::move(func)(head, head);
+      }
+      clone_oid = *resolved_oid;
     }
-    auto [clone, existed] = obc_registry.get_cached_obc(*resolved_oid);
+    auto [clone, existed] = obc_registry.get_cached_obc(clone_oid);
     return clone->template with_lock<State, IOInterruptCondition>(
       [existed=existed, clone=std::move(clone),
        func=std::move(func), head=std::move(head), this]() mutable
@@ -94,12 +100,13 @@ using crimson::common::local_conf;
   template<RWState::State State>
   ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc(hobject_t oid,
-                                with_obc_func_t&& func)
+                                with_obc_func_t&& func,
+                                bool resolve_clone)
   {
     if (oid.is_head()) {
       return with_head_obc<State>(oid, std::move(func));
     } else {
-      return with_clone_obc<State>(oid, std::move(func));
+      return with_clone_obc<State>(oid, std::move(func), resolve_clone);
     }
   }
 
@@ -192,17 +199,21 @@ using crimson::common::local_conf;
   // explicitly instantiate the used instantiations
   template ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc<RWState::RWNONE>(hobject_t,
-                                                 with_obc_func_t&&);
+                                                 with_obc_func_t&&,
+                                                 bool resolve_clone);
 
   template ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc<RWState::RWREAD>(hobject_t,
-                                                 with_obc_func_t&&);
+                                                 with_obc_func_t&&,
+                                                 bool resolve_clone);
 
   template ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc<RWState::RWWRITE>(hobject_t,
-                                                  with_obc_func_t&&);
+                                                  with_obc_func_t&&,
+                                                 bool resolve_clone);
 
   template ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc<RWState::RWEXCL>(hobject_t,
-                                                 with_obc_func_t&&);
+                                                 with_obc_func_t&&,
+                                                 bool resolve_clone);
 }

diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h
index 1d9c3bb90a276bdcfcb400e38a5fd88913311f9f..77805e11bc167ec3c504c985c1d672a4fa298b9b 100644 (file)
--- a/src/crimson/osd/object_context_loader.h
+++ b/src/crimson/osd/object_context_loader.h
@@ -35,9 +35,13 @@ public:
   // Use this variant by default
   // If oid is a clone object, the clone obc *and* it's
   // matching head obc will be locked and can be used in func.
+  // resolve_clone: For SnapTrim, it may be possible that it
+  //                won't be possible to resolve the clone.
+  // See SnapTrimObjSubEvent::remove_or_update - in_removed_snaps_queue usage.
   template<RWState::State State>
   load_obc_iertr::future<> with_obc(hobject_t oid,
-                                    with_obc_func_t&& func);
+                                    with_obc_func_t&& func,
+                                    bool resolve_clone = true);
 
   // Use this variant in the case where the head object
   // obc is already locked and only the clone obc is needed.
@@ -46,7 +50,8 @@ public:
   template<RWState::State State>
   load_obc_iertr::future<> with_clone_obc_only(ObjectContextRef head,
                                                hobject_t clone_oid,
-                                               with_obc_func_t&& func);
+                                               with_obc_func_t&& func,
+                                               bool resolve_clone = true);
 
   load_obc_iertr::future<> reload_obc(ObjectContext& obc) const;
 
@@ -60,7 +65,8 @@ private:
 
   template<RWState::State State>
   load_obc_iertr::future<> with_clone_obc(const hobject_t& oid,
-                                          with_obc_func_t&& func);
+                                          with_obc_func_t&& func,
+                                          bool resolve_clone);
 
   template<RWState::State State>
   load_obc_iertr::future<> with_head_obc(const hobject_t& oid,

diff --git a/src/crimson/osd/osd_operations/snaptrim_event.cc b/src/crimson/osd/osd_operations/snaptrim_event.cc
index fea83cbaf46232b2bc04734e7d9f64080cf29f5a..60f98c6f5365f0978f14d7a3ed0e46b04b198bb1 100644 (file)
--- a/src/crimson/osd/osd_operations/snaptrim_event.cc
+++ b/src/crimson/osd/osd_operations/snaptrim_event.cc
@@ -479,7 +479,8 @@ SnapTrimObjSubEvent::start()
           });
         });
       });
-    }).si_then([this] {
+    },
+    false).si_then([this] {
       logger().debug("{}: completed", *this);
       return handle.complete();
     }).handle_error_interruptible(