Skip to content

Commit 15cb6c7

Browse files
committed
crimson/os/seastore/transaction_manager: invalidate
RetiredExtentPlaceholder when reading extents Signed-off-by: Xuehan Xu <[email protected]>
1 parent 25be5fd commit 15cb6c7

File tree

4 files changed

+75
-8
lines changed

4 files changed

+75
-8
lines changed

src/crimson/os/seastore/cache.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ class SegmentProvider;
102102
* - TRACE: DEBUG details
103103
* - seastore_t logs
104104
*/
105-
class Cache : public ExtentTransViewRetriever {
105+
class Cache : public ExtentTransViewRetriever,
106+
public RetiredExtentPlaceholderInvalidater {
106107
public:
107108
using base_ertr = crimson::errorator<
108109
crimson::ct_error::input_output_error>;
@@ -369,6 +370,18 @@ class Cache : public ExtentTransViewRetriever {
369370
}
370371
}
371372

373+
void invalidate_retired_placeholder(
374+
Transaction &t,
375+
CachedExtent &retired_placeholder,
376+
CachedExtent &extent) {
377+
// replace placeholder in transactions
378+
while (!retired_placeholder.read_transactions.empty()) {
379+
auto t = retired_placeholder.read_transactions.begin()->t;
380+
t->replace_placeholder(retired_placeholder, extent);
381+
}
382+
retired_placeholder.set_invalid(t);
383+
}
384+
372385
/*
373386
* get_absent_extent
374387
*

src/crimson/os/seastore/linked_tree_node.h

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,27 @@
99

1010
namespace crimson::os::seastore {
1111

12+
struct RetiredExtentPlaceholderInvalidater {
13+
virtual void invalidate_retired_placeholder(
14+
Transaction &t,
15+
CachedExtent &retired_placeholder,
16+
CachedExtent &extent) = 0;
17+
};
18+
1219
template <typename ParentT>
1320
class child_pos_t {
1421
public:
1522
child_pos_t(TCachedExtentRef<ParentT> stable_parent, btreenode_pos_t pos)
1623
: stable_parent(stable_parent), pos(pos) {}
24+
child_pos_t(
25+
TCachedExtentRef<ParentT> stable_parent,
26+
btreenode_pos_t pos,
27+
CachedExtent* placeholder)
28+
: stable_parent(stable_parent),
29+
pos(pos),
30+
retired_placeholder(placeholder) {
31+
assert(retired_placeholder->is_placeholder());
32+
}
1733

1834
TCachedExtentRef<ParentT> get_parent() {
1935
ceph_assert(stable_parent);
@@ -26,9 +42,18 @@ class child_pos_t {
2642
void link_child(ChildT *c) {
2743
get_parent()->link_child(c, pos);
2844
}
45+
void invalidate_retired_placeholder(
46+
Transaction &t,
47+
RetiredExtentPlaceholderInvalidater &repi,
48+
CachedExtent &extent) {
49+
if (retired_placeholder) {
50+
repi.invalidate_retired_placeholder(t, *retired_placeholder, extent);
51+
}
52+
}
2953
private:
3054
TCachedExtentRef<ParentT> stable_parent;
3155
btreenode_pos_t pos = std::numeric_limits<btreenode_pos_t>::max();
56+
CachedExtentRef retired_placeholder;
3257
};
3358

3459
using get_child_iertr = trans_iertr<crimson::errorator<
@@ -204,6 +229,7 @@ class BaseChildNode {
204229
return parent_tracker->get_parent();
205230
}
206231
virtual key_t node_begin() const = 0;
232+
virtual bool is_retired_placeholder() const = 0;
207233
protected:
208234
parent_tracker_ref<ParentT> parent_tracker;
209235
virtual bool _is_valid() const = 0;
@@ -328,15 +354,26 @@ class ParentNode {
328354
auto child = children[pos];
329355
ceph_assert(!is_reserved_ptr(child));
330356
if (is_valid_child_ptr(child)) {
331-
return etvr.get_extent_viewable_by_trans<ChildT>(
332-
t, static_cast<ChildT*>(child));
357+
if (child->is_retired_placeholder()) {
358+
assert(me.is_stable());
359+
return child_pos_t<T>(
360+
&me, pos, dynamic_cast<CachedExtent*>(child));
361+
} else {
362+
return etvr.get_extent_viewable_by_trans<ChildT>(
363+
t, static_cast<ChildT*>(child));
364+
}
333365
} else if (me.is_pending()) {
334366
auto &sparent = me.get_stable_for_key(key);
335367
auto spos = sparent.lower_bound(key).get_offset();
336368
auto child = sparent.children[spos];
337369
if (is_valid_child_ptr(child)) {
338-
return etvr.get_extent_viewable_by_trans<ChildT>(
339-
t, static_cast<ChildT*>(child));
370+
if (child->is_retired_placeholder()) {
371+
return child_pos_t<T>(
372+
&sparent, spos, dynamic_cast<CachedExtent*>(child));
373+
} else {
374+
return etvr.get_extent_viewable_by_trans<ChildT>(
375+
t, static_cast<ChildT*>(child));
376+
}
340377
} else {
341378
return child_pos_t<T>(&sparent, spos);
342379
}
@@ -352,7 +389,10 @@ class ParentNode {
352389
assert(child);
353390
ceph_assert(me.is_stable());
354391
assert(child->_is_stable());
355-
assert(!children[pos]);
392+
if (unlikely(children[pos] != nullptr)) {
393+
assert(is_valid_child_ptr(children[pos]));
394+
assert(children[pos]->is_retired_placeholder());
395+
}
356396
ceph_assert(is_valid_child_ptr(child));
357397
update_child_ptr(pos, child);
358398
}
@@ -1034,6 +1074,10 @@ class ChildNode : public BaseChildNode<ParentT, key_t> {
10341074
}
10351075
}
10361076

1077+
bool is_retired_placeholder() const final {
1078+
auto &me = down_cast();
1079+
return me.is_placeholder();
1080+
}
10371081
protected:
10381082
void on_invalidated() {
10391083
this->reset_parent_tracker();

src/crimson/os/seastore/logical_child_node.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class RetiredExtentPlaceholder
2525
: public CachedExtent,
2626
public ChildNode<lba::LBALeafNode, RetiredExtentPlaceholder, laddr_t> {
2727

28+
using lba_child_node_t =
29+
ChildNode<lba::LBALeafNode, RetiredExtentPlaceholder, laddr_t>;
2830
public:
2931
RetiredExtentPlaceholder(extent_len_t length)
3032
: CachedExtent(CachedExtent::retired_placeholder_construct_t{}, length) {}
@@ -35,6 +37,10 @@ class RetiredExtentPlaceholder
3537
}
3638
}
3739

40+
void on_invalidated(Transaction&) final {
41+
this->lba_child_node_t::on_invalidated();
42+
}
43+
3844
CachedExtentRef duplicate_for_write(Transaction&) final {
3945
ceph_abort_msg("Should never happen for a placeholder");
4046
return CachedExtentRef();

src/crimson/os/seastore/transaction_manager.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,12 +1445,14 @@ class TransactionManager : public ExtentCallbackInterface {
14451445
partial_len,
14461446
[laddr=pin.get_intermediate_base(),
14471447
maybe_init=std::move(maybe_init),
1448-
child_pos=std::move(child_pos)]
1448+
child_pos=std::move(child_pos),
1449+
&t, this]
14491450
(T &extent) mutable {
14501451
assert(extent.is_logical());
14511452
assert(!extent.has_laddr());
14521453
assert(!extent.has_been_invalidated());
14531454
child_pos.link_child(&extent);
1455+
child_pos.invalidate_retired_placeholder(t, *cache, extent);
14541456
extent.set_laddr(laddr);
14551457
maybe_init(extent);
14561458
extent.set_seen_by_users();
@@ -1517,12 +1519,14 @@ class TransactionManager : public ExtentCallbackInterface {
15171519
pin.get_val(),
15181520
direct_key,
15191521
direct_length,
1520-
[direct_key, child_pos=std::move(child_pos)](CachedExtent &extent) mutable {
1522+
[direct_key, child_pos=std::move(child_pos),
1523+
&t, this](CachedExtent &extent) mutable {
15211524
assert(extent.is_logical());
15221525
auto &lextent = static_cast<LogicalChildNode&>(extent);
15231526
assert(!lextent.has_laddr());
15241527
assert(!lextent.has_been_invalidated());
15251528
child_pos.link_child(&lextent);
1529+
child_pos.invalidate_retired_placeholder(t, *cache, lextent);
15261530
lextent.set_laddr(direct_key);
15271531
// No change to extent::seen_by_user because this path is only
15281532
// for background cleaning.

0 commit comments

Comments
 (0)