Skip to content

Commit d56779e

Browse files
committed
crimson/os/seastore/transaction_manager: consider inconsistency between
backrefs and lbas acceptable when cleaning segments Consider the following scene: 1. Trans.A writes the final record of Segment S, in which it overwrite another extent E in the same segment S; 2. Before Trans.A "complete_commit", Trans.B tries to rewrite new records and roll the segments, which closes Segment S; 3. Before Trans.A "complete_commit", a new cleaner Transaction C tries to clean the segment; In this scenario, C might see a part of extent E's laddr space mapped to another location within the same segment S. This is actually a valid case. Fixes: https://tracker.ceph.com/issues/66924 Signed-off-by: Xuehan Xu <[email protected]>
1 parent 67afcf6 commit d56779e

File tree

1 file changed

+17
-4
lines changed

1 file changed

+17
-4
lines changed

src/crimson/os/seastore/transaction_manager.cc

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -681,13 +681,26 @@ TransactionManager::get_extents_if_live(
681681
auto pin_paddr = pin->get_val();
682682
auto &pin_seg_paddr = pin_paddr.as_seg_paddr();
683683
auto pin_paddr_seg_id = pin_seg_paddr.get_segment_id();
684-
auto pin_len = pin->get_length();
684+
// auto pin_len = pin->get_length();
685685
if (pin_paddr_seg_id != paddr_seg_id) {
686686
return seastar::now();
687687
}
688-
// Only extent split can happen during the lookup
689-
ceph_assert(pin_seg_paddr >= paddr &&
690-
pin_seg_paddr.add_offset(pin_len) <= paddr.add_offset(len));
688+
689+
// pin may be out of the range paddr~len, consider the following scene:
690+
// 1. Trans.A writes the final record of Segment S, in which it overwrite
691+
// another extent E in the same segment S;
692+
// 2. Before Trans.A "complete_commit", Trans.B tries to rewrite new
693+
// records and roll the segments, which closes Segment S;
694+
// 3. Before Trans.A "complete_commit", a new cleaner Transaction C tries
695+
// to clean the segment;
696+
//
697+
// In this scenario, C might see a part of extent E's laddr space mapped
698+
// to another location within the same segment S.
699+
//
700+
// FIXME: this assert should be re-enabled once we have space reclaiming
701+
// recognize committed segments: https://tracker.ceph.com/issues/66941
702+
// ceph_assert(pin_seg_paddr >= paddr &&
703+
// pin_seg_paddr.add_offset(pin_len) <= paddr.add_offset(len));
691704
return read_pin_by_type(t, std::move(pin), type
692705
).si_then([&list](auto ret) {
693706
list.emplace_back(std::move(ret));

0 commit comments

Comments
 (0)