Skip to content

Commit 93b0f0a

Browse files
Matan-BMatan-B
committed
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>
1 parent 55c2990 commit 93b0f0a

3 files changed

Lines changed: 39 additions & 23 deletions

File tree

‎src/crimson/osd/object_context_loader.cc‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,33 +37,38 @@ using crimson::common::local_conf;
3737
template<RWState::State State>
3838
ObjectContextLoader::load_obc_iertr::future<>
3939
ObjectContextLoader::with_clone_obc(const hobject_t& oid,
40-
with_obc_func_t&& func)
40+
with_obc_func_t&& func,
41+
bool resolve_clone)
4142
{
4243
LOG_PREFIX(ObjectContextLoader::with_clone_obc);
4344
assert(!oid.is_head());
4445
return with_head_obc<RWState::RWREAD>(
4546
oid.get_head(),
46-
[FNAME, oid, func=std::move(func), this](auto head, auto) mutable
47-
-> load_obc_iertr::future<> {
47+
[FNAME, oid, func=std::move(func), resolve_clone, this]
48+
(auto head, auto) mutable -> load_obc_iertr::future<> {
4849
if (!head->obs.exists) {
4950
ERRORDPP("head doesn't exist for object {}", dpp, head->obs.oi.soid);
5051
return load_obc_iertr::future<>{
5152
crimson::ct_error::enoent::make()
5253
};
5354
}
54-
auto resolved_oid = resolve_oid(head->get_head_ss(), oid);
55-
if (!resolved_oid) {
56-
ERRORDPP("clone {} not found", dpp, oid);
57-
return load_obc_iertr::future<>{
58-
crimson::ct_error::enoent::make()
59-
};
60-
}
61-
if (resolved_oid->is_head()) {
62-
// See resolve_oid
63-
return std::move(func)(head, head);
55+
hobject_t clone_oid = oid;
56+
if (resolve_clone) {
57+
auto resolved_oid = resolve_oid(head->get_head_ss(), oid);
58+
if (!resolved_oid) {
59+
ERRORDPP("clone {} not found", dpp, oid);
60+
return load_obc_iertr::future<>{
61+
crimson::ct_error::enoent::make()
62+
};
63+
}
64+
if (resolved_oid->is_head()) {
65+
// See resolve_oid
66+
return std::move(func)(head, head);
67+
}
68+
clone_oid = *resolved_oid;
6469
}
6570
return this->with_clone_obc_only<State>(std::move(head),
66-
*resolved_oid,
71+
clone_oid,
6772
std::move(func));
6873
});
6974
}
@@ -93,12 +98,13 @@ using crimson::common::local_conf;
9398
template<RWState::State State>
9499
ObjectContextLoader::load_obc_iertr::future<>
95100
ObjectContextLoader::with_obc(hobject_t oid,
96-
with_obc_func_t&& func)
101+
with_obc_func_t&& func,
102+
bool resolve_clone)
97103
{
98104
if (oid.is_head()) {
99105
return with_head_obc<State>(oid, std::move(func));
100106
} else {
101-
return with_clone_obc<State>(oid, std::move(func));
107+
return with_clone_obc<State>(oid, std::move(func), resolve_clone);
102108
}
103109
}
104110

@@ -191,17 +197,21 @@ using crimson::common::local_conf;
191197
// explicitly instantiate the used instantiations
192198
template ObjectContextLoader::load_obc_iertr::future<>
193199
ObjectContextLoader::with_obc<RWState::RWNONE>(hobject_t,
194-
with_obc_func_t&&);
200+
with_obc_func_t&&,
201+
bool resolve_clone);
195202

196203
template ObjectContextLoader::load_obc_iertr::future<>
197204
ObjectContextLoader::with_obc<RWState::RWREAD>(hobject_t,
198-
with_obc_func_t&&);
205+
with_obc_func_t&&,
206+
bool resolve_clone);
199207

200208
template ObjectContextLoader::load_obc_iertr::future<>
201209
ObjectContextLoader::with_obc<RWState::RWWRITE>(hobject_t,
202-
with_obc_func_t&&);
210+
with_obc_func_t&&,
211+
bool resolve_clone);
203212

204213
template ObjectContextLoader::load_obc_iertr::future<>
205214
ObjectContextLoader::with_obc<RWState::RWEXCL>(hobject_t,
206-
with_obc_func_t&&);
215+
with_obc_func_t&&,
216+
bool resolve_clone);
207217
}

‎src/crimson/osd/object_context_loader.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,13 @@ class ObjectContextLoader {
3535
// Use this variant by default
3636
// If oid is a clone object, the clone obc *and* it's
3737
// matching head obc will be locked and can be used in func.
38+
// resolve_clone: For SnapTrim, it may be possible that it
39+
// won't be possible to resolve the clone.
40+
// See SnapTrimObjSubEvent::remove_or_update - in_removed_snaps_queue usage.
3841
template<RWState::State State>
3942
load_obc_iertr::future<> with_obc(hobject_t oid,
40-
with_obc_func_t&& func);
43+
with_obc_func_t&& func,
44+
bool resolve_clone = true);
4145

4246
// Use this variant in the case where the head object
4347
// obc is already locked and only the clone obc is needed.
@@ -60,7 +64,8 @@ class ObjectContextLoader {
6064

6165
template<RWState::State State>
6266
load_obc_iertr::future<> with_clone_obc(const hobject_t& oid,
63-
with_obc_func_t&& func);
67+
with_obc_func_t&& func,
68+
bool resolve_clone);
6469

6570
template<RWState::State State>
6671
load_obc_iertr::future<> with_head_obc(const hobject_t& oid,

‎src/crimson/osd/osd_operations/snaptrim_event.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,8 @@ SnapTrimObjSubEvent::start()
469469
});
470470
});
471471
});
472-
}).si_then([this] {
472+
},
473+
false).si_then([this] {
473474
logger().debug("{}: completed", *this);
474475
return handle.complete();
475476
}).handle_error_interruptible(

0 commit comments

Comments
 (0)