crimson/os/seastore: invalidate transaction referencing invalid extents

Modify read_set to retain a reverse mapping from extents back to
transactions and use it to update Transaction::conflicted upon
invalidation.

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 2dc005628150a94ff64a8178cc7f9acaf74b4434..19a874017c230494ba6cf04fd2cea8a9b2fd70b1 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -152,7 +152,14 @@ void Cache::replace_extent(CachedExtentRef next, CachedExtentRef prev)
     add_to_dirty(next);
   }
 
-  prev->state = CachedExtent::extent_state_t::INVALID;
+  invalidate(*prev);
+}
+
+void Cache::invalidate(CachedExtent &extent) {
+  for (auto &&i: extent.transactions) {
+    i.t->conflicted = true;
+  }
+  extent.state = CachedExtent::extent_state_t::INVALID;
 }
 
 CachedExtentRef Cache::alloc_new_extent_by_type(
@@ -223,14 +230,12 @@ std::optional<record_t> Cache::try_construct_record(Transaction &t)
   LOG_PREFIX(Cache::try_construct_record);
   DEBUGT("enter", t);
 
-  // First, validate read set
+  // Should be valid due to interruptible future
   for (auto &i: t.read_set) {
-    if (i->state == CachedExtent::extent_state_t::INVALID) {
-      return std::nullopt;
-    }
+    assert(i.ref->is_valid());
   }
-
   DEBUGT("read_set validated", t);
+  t.read_set.clear();
 
   record_t record;
 

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 2400f875c190d76ccbac866c08eb827d39c814ff..15fc98b729ad6766b943b78d0646c3383a02bc77 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -587,6 +587,9 @@ private:
   /// Replace prev with next
   void replace_extent(CachedExtentRef next, CachedExtentRef prev);
 
+  /// Invalidate extent and mark affected transactions
+  void invalidate(CachedExtent &extent);
+
   Transaction::get_extent_ret query_cache_for_extent(
     paddr_t offset,
     CachedExtentRef *out) {

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index a14e835fb6f1674f79a926e1170722edcf11ea34..c32d01ade073adfed0c9c051970e7e744c40b35a 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -17,6 +17,7 @@
 
 namespace crimson::os::seastore {
 
+class Transaction;
 class CachedExtent;
 using CachedExtentRef = boost::intrusive_ptr<CachedExtent>;
 
@@ -38,6 +39,40 @@ namespace onode {
   class DummyNodeExtent;
   class TestReplayExtent;
 }
+
+template <typename T>
+class read_set_item_t {
+  boost::intrusive::list_member_hook<> list_hook;
+  using list_hook_options = boost::intrusive::member_hook<
+    read_set_item_t,
+    boost::intrusive::list_member_hook<>,
+    &read_set_item_t::list_hook>;
+
+public:
+  struct cmp_t {
+    using is_transparent = paddr_t;
+    bool operator()(const read_set_item_t<T> &lhs, const read_set_item_t &rhs) const;
+    bool operator()(const paddr_t &lhs, const read_set_item_t<T> &rhs) const;
+    bool operator()(const read_set_item_t<T> &lhs, const paddr_t &rhs) const;
+  };
+
+  using list =  boost::intrusive::list<
+    read_set_item_t,
+    list_hook_options>;
+
+  T *t = nullptr;
+  CachedExtentRef ref;
+
+  read_set_item_t(T *t, CachedExtentRef ref);
+  read_set_item_t(const read_set_item_t &) = delete;
+  read_set_item_t(read_set_item_t &&) = default;
+  ~read_set_item_t();
+};
+template <typename T>
+struct read_set_t : public std::set<
+  read_set_item_t<T>,
+  typename read_set_item_t<T>::cmp_t> {};
+
 class ExtentIndex;
 class CachedExtent : public boost::intrusive_ref_counter<
   CachedExtent, boost::thread_unsafe_counter> {
@@ -286,6 +321,9 @@ public:
   virtual ~CachedExtent();
 
 private:
+  template <typename T>
+  friend class read_set_item_t;
+
   friend struct paddr_cmp;
   friend struct ref_paddr_cmp;
   friend class ExtentIndex;
@@ -351,6 +389,8 @@ private:
     }
   }
 
+  read_set_item_t<Transaction>::list transactions;
+
 protected:
   CachedExtent(CachedExtent &&other) = delete;
   CachedExtent(ceph::bufferptr &&ptr) : ptr(std::move(ptr)) {}
@@ -775,6 +815,35 @@ struct ref_laddr_cmp {
   }
 };
 
+template <typename T>
+read_set_item_t<T>::read_set_item_t(T *t, CachedExtentRef ref)
+  : t(t), ref(ref)
+{
+  ref->transactions.push_back(*this);
+}
+
+template <typename T>
+read_set_item_t<T>::~read_set_item_t()
+{
+  ref->transactions.erase(ref->transactions.s_iterator_to(*this));
+}
+
+template <typename T>
+inline bool read_set_item_t<T>::cmp_t::operator()(
+  const read_set_item_t<T> &lhs, const read_set_item_t<T> &rhs) const {
+  return lhs.ref->poffset < rhs.ref->poffset;
+}
+template <typename T>
+inline bool read_set_item_t<T>::cmp_t::operator()(
+  const paddr_t &lhs, const read_set_item_t<T> &rhs) const {
+  return lhs < rhs.ref->poffset;
+}
+template <typename T>
+inline bool read_set_item_t<T>::cmp_t::operator()(
+  const read_set_item_t<T> &lhs, const paddr_t &rhs) const {
+  return lhs.ref->poffset < rhs;
+}
+
 using lextent_set_t = addr_extent_set_base_t<
   laddr_t,
   LogicalCachedExtentRef,

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 7a46e5c299374efff472e5137dae7e09f8ad25fb..a8a7998988d7eb3a30941235d672422459c01b1f 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -16,6 +16,7 @@ namespace crimson::os::seastore {
 
 struct retired_extent_gate_t;
 class SeaStore;
+class Transaction;
 
 /**
  * Transaction
@@ -44,7 +45,7 @@ public:
       auto iter = read_set.find(addr);
       iter != read_set.end()) {
       if (out)
-       *out = CachedExtentRef(*iter);
+       *out = iter->ref;
       return get_extent_ret::PRESENT;
     } else {
       return get_extent_ret::ABSENT;
@@ -73,8 +74,8 @@ public:
   void add_to_read_set(CachedExtentRef ref) {
     if (is_weak()) return;
 
-    ceph_assert(read_set.count(ref) == 0);
-    read_set.insert(ref);
+    auto [iter, inserted] = read_set.emplace(this, ref);
+    ceph_assert(inserted);
   }
 
   void add_fresh_extent(CachedExtentRef ref) {
@@ -130,8 +131,8 @@ private:
 
   segment_off_t offset = 0; ///< relative offset of next block
 
-  pextent_set_t read_set;   ///< set of extents read by paddr
-  ExtentIndex write_set;    ///< set of extents written by paddr
+  read_set_t<Transaction> read_set; ///< set of extents read by paddr
+  ExtentIndex write_set;            ///< set of extents written by paddr
 
   std::list<CachedExtentRef> fresh_block_list;   ///< list of fresh blocks
   std::list<CachedExtentRef> mutated_block_list; ///< list of mutated blocks
@@ -147,6 +148,8 @@ private:
 
   retired_extent_gate_t::token_t retired_gate_token;
 
+  bool conflicted = false;
+
 public:
   Transaction(
     OrderingHandle &&handle,