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 <sjust@redhat.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 3ebfd70c41d661c6a4b3de5512a3856b6d3cc5d2..9c6ff428a5c4e2ff4477b2ddbf8aaf75053a5578 100644 (file)
--- 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<collection_manager::CollectionNode>(t, length);
   case extent_types_t::OBJECT_DATA_BLOCK:
     return alloc_new_extent<ObjectDataBlock>(t, length);
+  case extent_types_t::RETIRED_PLACEHOLDER:
+    ceph_assert(0 == "impossible");
+    return CachedExtentRef();
   case extent_types_t::TEST_BLOCK:
     return alloc_new_extent<TestBlock>(t, length);
   case extent_types_t::TEST_BLOCK_PHYSICAL:
@@ -273,6 +276,22 @@ std::optional<record_t> 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<CachedExtentRef> 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<CachedExtentRef>();
     case extent_types_t::TEST_BLOCK:
       return get_extent<TestBlock>(offset, length
       ).safe_then([](auto extent) {

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 70158632f37c5ba9403465303a7cddcabce4e71b..bdfbb2e223bbd8508014d701f837c453ae6607f5 100644 (file)
--- 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<Cache>;
 

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 8ccd2451c157b244cd1ba35012478ca81c893509..88300f1d432320bc96b8c2541fe7c4a99831c480 100644 (file)
--- 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 <typename T>
-  static TCachedExtentRef<T> make_cached_extent_ref(bufferptr &&ptr) {
-    return new T(std::move(ptr));
+  template <typename T, typename... Args>
+  static TCachedExtentRef<T> make_cached_extent_ref(
+    Args&&... args) {
+    return new T(std::forward<Args>(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 <typename... T>
+  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 a651d23cd909bc9b2331c058cbcaa3c6473baff3..a761a21bb968159ce014dbab030551a8d9e5f2cb 100644 (file)
--- 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 39966865db5366e7f3326476e657aa0ee21997a0..59b329c0fcce89989d8647adab192c898de72c34 100644 (file)
--- 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 9a8cd3cc20c86ba5a9754157492612fce7287293..1c5e195bd13055c9720099daafa991d722e4cb38 100644 (file)
--- 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(