From: Samuel Just Date: Mon, 19 Apr 2021 23:48:21 +0000 (-0700) Subject: crimson/os/seastore: retain placeholders for retired, uncached extents X-Git-Tag: v17.1.0~2174^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=8e88bcac8cc7339be559491c6d552a2fa7850fc1;p=ceph.git crimson/os/seastore: retain placeholders for retired, uncached extents We need to track extents retired without first being in cache. Create RetiredExtentPlaceholder extent type for conflict detection on those cases. Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 3ebfd70c41d66..9c6ff428a5c4e 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -32,7 +32,7 @@ Cache::~Cache() ceph_assert(extents.empty()); } -Cache::retire_extent_ret Cache::retire_extent_if_cached( +Cache::retire_extent_ret Cache::retire_extent( Transaction &t, paddr_t addr, extent_len_t length) { if (auto ext = t.write_set.find_offset(addr); ext != t.write_set.end()) { @@ -167,6 +167,9 @@ CachedExtentRef Cache::alloc_new_extent_by_type( return alloc_new_extent(t, length); case extent_types_t::OBJECT_DATA_BLOCK: return alloc_new_extent(t, length); + case extent_types_t::RETIRED_PLACEHOLDER: + ceph_assert(0 == "impossible"); + return CachedExtentRef(); case extent_types_t::TEST_BLOCK: return alloc_new_extent(t, length); case extent_types_t::TEST_BLOCK_PHYSICAL: @@ -273,6 +276,22 @@ std::optional Cache::try_construct_record(Transaction &t) retire_extent(i); } + for (auto &&i : t.retired_uncached) { + CachedExtentRef to_retire; + if (query_cache_for_extent(i.first, &to_retire) == + Transaction::get_extent_ret::ABSENT) { + to_retire = CachedExtent::make_cached_extent_ref< + RetiredExtentPlaceholder + >(i.second); + to_retire->set_paddr(i.first); + } + + t.retired_set.insert(to_retire); + extents.insert(*to_retire); + to_retire->dirty_from_or_retired_at = JOURNAL_SEQ_MAX; + retired_extent_gate.add_extent(*to_retire); + } + record.extents.reserve(t.fresh_block_list.size()); for (auto &i: t.fresh_block_list) { logger().debug("try_construct_record: fresh block {}", *i); @@ -344,11 +363,6 @@ void Cache::complete_commit( i->get_paddr(), i->get_length()); } - for (auto &i: t.retired_uncached) { - cleaner->mark_space_free( - i.first, - i.second); - } } for (auto &i: t.mutated_block_list) { @@ -360,6 +374,7 @@ void Cache::complete_commit( logger().debug("try_construct_record: retiring {}", *i); i->dirty_from_or_retired_at = last_commit; } + retired_extent_gate.prune(); } @@ -573,6 +588,9 @@ Cache::get_extent_ertr::future Cache::get_extent_by_type( ).safe_then([](auto extent) { return CachedExtentRef(extent.detach(), false /* add_ref */); }); + case extent_types_t::RETIRED_PLACEHOLDER: + ceph_assert(0 == "impossible"); + return get_extent_ertr::make_ready_future(); case extent_types_t::TEST_BLOCK: return get_extent(offset, length ).safe_then([](auto extent) { diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 70158632f37c5..bdfbb2e223bbd 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -130,10 +130,10 @@ public: t.add_to_retired_set(ref); } - /// Declare paddr retired in t, noop if not cached + /// Declare paddr retired in t using retire_extent_ertr = base_ertr; using retire_extent_ret = retire_extent_ertr::future<>; - retire_extent_ret retire_extent_if_cached( + retire_extent_ret retire_extent( Transaction &t, paddr_t addr, extent_len_t length); /** @@ -579,14 +579,10 @@ private: void replace_extent(CachedExtentRef next, CachedExtentRef prev); Transaction::get_extent_ret query_cache_for_extent( - Transaction &t, paddr_t offset, CachedExtentRef *out) { - auto result = t.get_extent(offset, out); - if (result != Transaction::get_extent_ret::ABSENT) { - return result; - } else if (auto iter = extents.find_offset(offset); - iter != extents.end()) { + if (auto iter = extents.find_offset(offset); + iter != extents.end()) { if (out) *out = &*iter; return Transaction::get_extent_ret::PRESENT; @@ -595,6 +591,18 @@ private: } } + Transaction::get_extent_ret query_cache_for_extent( + Transaction &t, + paddr_t offset, + CachedExtentRef *out) { + auto result = t.get_extent(offset, out); + if (result != Transaction::get_extent_ret::ABSENT) { + return result; + } else { + return query_cache_for_extent(offset, out); + } + } + }; using CacheRef = std::unique_ptr; diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 8ccd2451c157b..88300f1d43232 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -259,7 +259,7 @@ public: paddr_t get_paddr() const { return poffset; } /// Returns length of extent - extent_len_t get_length() const { return ptr.length(); } + virtual extent_len_t get_length() const { return ptr.length(); } /// Returns version, get_version() == 0 iff is_clean() extent_version_t get_version() const { @@ -375,11 +375,14 @@ protected: version(other.version), poffset(other.poffset) {} + struct retired_placeholder_t{}; + CachedExtent(retired_placeholder_t) : state(extent_state_t::RETIRED) {} friend class Cache; - template - static TCachedExtentRef make_cached_extent_ref(bufferptr &&ptr) { - return new T(std::move(ptr)); + template + static TCachedExtentRef make_cached_extent_ref( + Args&&... args) { + return new T(std::forward(args)...); } CachedExtentRef get_prior_instance() { @@ -647,6 +650,63 @@ inline retired_extent_gate_t::token_t::~token_t() { } } +/** + * RetiredExtentPlaceholder + * + * Cache::retire_extent(Transaction&, paddr_t, extent_len_t) can retire + * an extent not currently in cache. In that case, we need to add a + * placeholder to the cache until transactions that might reference + * the extent complete as in the case where the extent is already cached. + * Cache::complete_commit thus creates a RetiredExtentPlaceholder to + * serve that purpose. ptr is not populated, and state is set to + * RETIRED. Cache interfaces check for RETIRED and return EAGAIN if + * encountered, so references to these placeholder extents should not + * escape the Cache interface boundary. + */ +class RetiredExtentPlaceholder : public CachedExtent { + extent_len_t length; + +public: + template + RetiredExtentPlaceholder(extent_len_t length) + : CachedExtent(CachedExtent::retired_placeholder_t{}), + length(length) {} + + extent_len_t get_length() const final { return length; } + + CachedExtentRef duplicate_for_write() final { + ceph_assert(0 == "Should never happen for a placeholder"); + return CachedExtentRef(); + } + + ceph::bufferlist get_delta() final { + ceph_assert(0 == "Should never happen for a placeholder"); + return ceph::bufferlist(); + } + + static constexpr extent_types_t TYPE = extent_types_t::RETIRED_PLACEHOLDER; + extent_types_t get_type() const final { + return TYPE; + } + + void apply_delta_and_adjust_crc( + paddr_t base, const ceph::bufferlist &bl) final { + ceph_assert(0 == "Should never happen for a placeholder"); + } + + bool is_logical() const final { + return false; + } + + std::ostream &print_detail(std::ostream &out) const final { + return out << "RetiredExtentPlaceholder"; + } + + void on_delta_write(paddr_t record_block_offset) final { + ceph_assert(0 == "Should never happen for a placeholder"); + } +}; + /** * LogicalCachedExtent * diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc index a651d23cd909b..a761a21bb9681 100644 --- a/src/crimson/os/seastore/seastore_types.cc +++ b/src/crimson/os/seastore/seastore_types.cc @@ -65,6 +65,8 @@ std::ostream &operator<<(std::ostream &out, extent_types_t t) return out << "COLL_BLOCK"; case extent_types_t::OBJECT_DATA_BLOCK: return out << "OBJECT_DATA_BLOCK"; + case extent_types_t::RETIRED_PLACEHOLDER: + return out << "RETIRED_PLACEHOLDER"; case extent_types_t::TEST_BLOCK: return out << "TEST_BLOCK"; case extent_types_t::TEST_BLOCK_PHYSICAL: diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index 39966865db536..59b329c0fcce8 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -335,6 +335,7 @@ enum class extent_types_t : uint8_t { ONODE_BLOCK_STAGED = 6, COLL_BLOCK = 7, OBJECT_DATA_BLOCK = 8, + RETIRED_PLACEHOLDER = 9, // Test Block Types TEST_BLOCK = 0xF0, diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 9a8cd3cc20c86..1c5e195bd1305 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -160,7 +160,7 @@ TransactionManager::ref_ret TransactionManager::dec_ref( logger().debug( "TransactionManager::dec_ref: offset {} refcount 0", offset); - return cache->retire_extent_if_cached( + return cache->retire_extent( t, result.addr, result.length ).safe_then([] { return ref_ret(