During our teuthology test, resolve_oid here will lead to the failure of recovering clone objects to replica osds, when the SnapSet::clone_snaps is like f:[c, b]. In this case, snapshot f has been removed, if we need to recover clone object oid:f to replica osds, the resolve_oid will fail.

Maybe we should allow resolved_oid to be nullopt

Perhaps with_clone_obc_direct can be used here. It's similar to with_clone_obc but without actually resolving the clone. See WIP 93b0f0a from #56752.
What do you think?

During our teuthology test, resolve_oid here will lead to the failure of recovering clone objects to replica osds, when the SnapSet::clone_snaps is like f:[c, b]. In this case, snapshot f has been removed, if we need to recover clone object oid:f to replica osds, the resolve_oid will fail.

Why are we trying to recover oid:f if the snapshot was removed?

Can you please share the osd logs?

we modified the code as follows, and it seemed that the issue was fixed:

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index 32a448cfce0..521f81e391a 100644
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -48,18 +48,14 @@ using crimson::common::local_conf;
         };
       }
       auto resolved_oid = resolve_oid(head->get_head_ss(), oid);
-      if (!resolved_oid) {
-        ERRORDPP("clone {} not found", dpp, oid);
-        return load_obc_iertr::future<>{
-          crimson::ct_error::enoent::make()
-        };
-      }
-      if (resolved_oid->is_head()) {
+      if (resolved_oid && resolved_oid->is_head()) {
         // See resolve_oid
         return std::move(func)(head, head);
       }
       return this->with_clone_obc_only<State>(std::move(head),
-                                              *resolved_oid,
+                                              ((bool)resolved_oid)
+                                               ? *resolved_oid
+                                               : oid,
                                               std::move(func));
     });
   }

we modified the code as follows, and it seemed that the issue was fixed:

There are correct cases where we should expect resolve to fail and return an error, we can't remove this.
However, there are users (snaptrim) which are expected to not resolve_oid. Which is essentially the code you have added. I have implemented this in 1dffd26.

we modified the code as follows, and it seemed that the issue was fixed:

There are correct cases where we should expect resolve to fail and return an error, we can't remove this. However, there are users (snaptrim) which are expected to not resolve_oid. Which is essentially the code you have added. I have implemented this in 1dffd26.

I think the issue mentioned above (#56610 (comment)) was caused by the primary osd trying to recover a clone object (the latest snap of which had been removed, which further led to the failure of "resolve_oid"). This doesn't seem to be related to snap trimming. On the other hand, could you elaborate a little bit about the correct cases in which the failure of "resolve_oid" is expected, please? Thanks:-)

I think the issue mentioned above (#56610 (comment)) was caused by the primary osd trying to recover a clone object (the latest snap of which had been removed, which further led to the failure of "resolve_oid"). This doesn't seem to be related to snap trimming. On the other hand,

I'll update recovery users to also skip resolve_oid same as snap trim does.

could you elaborate a little bit about the correct cases in which the failure of "resolve_oid" is expected, please? Thanks:-)

There are cases we resolve_oid returning null and causing and error is correct. When clone doesn't exist or when the snapset's clone_snap does not contain the snap.

Please take a look at the last commit