From: Matan Breizman Date: Mon, 8 Apr 2024 07:52:20 +0000 (+0000) Subject: crimson/osd/object_context_loader: SnapTrim to not resolve_oid X-Git-Tag: v19.1.1~341^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=6e93f37567ed41e75de95ddc02fea8fd2cf6f9a0;p=ceph.git 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 (cherry picked from commit 263b2ae77ac1df8a939b82101eca09f1d8d4089a) --- diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc index 158c61ee5c43e..b53cbabd04c91 100644 --- 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 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( 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(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( [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 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(oid, std::move(func)); } else { - return with_clone_obc(oid, std::move(func)); + return with_clone_obc(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(hobject_t, - with_obc_func_t&&); + with_obc_func_t&&, + bool resolve_clone); template ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::with_obc(hobject_t, - with_obc_func_t&&); + with_obc_func_t&&, + bool resolve_clone); template ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::with_obc(hobject_t, - with_obc_func_t&&); + with_obc_func_t&&, + bool resolve_clone); template ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::with_obc(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 1d9c3bb90a276..77805e11bc167 100644 --- 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 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 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 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 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 fea83cbaf4623..60f98c6f5365f 100644 --- 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(