crimson/os/seastore: convert transaction related paths with interruptor

Signed-off-by: Yingxin Cheng <[email protected]>

src/crimson/os/seastore/backref/btree_backref_manager.cc

@@ -28,28 +28,22 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node<
     ceph_assert(backref_root->is_initial_pending()
       == root_block->is_pending());
     return {true,
-	    trans_intr::make_interruptible(
-	      c.cache.get_extent_viewable_by_trans(c.trans, backref_root))};
+            c.cache.get_extent_viewable_by_trans(c.trans, backref_root)};
   } else if (root_block->is_pending()) {
     auto &prior = static_cast<RootBlock&>(*root_block->get_prior_instance());
     backref_root = prior.backref_root_node;
     if (backref_root) {
       return {true,
-	      trans_intr::make_interruptible(
-		c.cache.get_extent_viewable_by_trans(c.trans, backref_root))};
+              c.cache.get_extent_viewable_by_trans(c.trans, backref_root)};
     } else {
       c.cache.account_absent_access(c.trans.get_src());
       return {false,
-	      trans_intr::make_interruptible(
-		Cache::get_extent_ertr::make_ready_future<
-		  CachedExtentRef>())};
+              Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
     }
   } else {
     c.cache.account_absent_access(c.trans.get_src());
     return {false,
-	    trans_intr::make_interruptible(
-	      Cache::get_extent_ertr::make_ready_future<
-		CachedExtentRef>())};
+            Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
   }
 }
 

src/crimson/os/seastore/btree/fixed_kv_btree.h

@@ -32,10 +32,6 @@ inline ChildableCachedExtent* get_reserved_ptr() {
 template <typename T>
 phy_tree_root_t& get_phy_tree_root(root_t& r);
 
-using get_child_iertr =
-  ::crimson::interruptible::interruptible_errorator<
-    typename trans_intr::condition,
-    get_child_ertr>;
 using get_phy_tree_root_node_ret =
   std::pair<bool, get_child_iertr::future<CachedExtentRef>>;
 
@@ -1501,7 +1497,7 @@ class FixedKVBtree {
     // checking the lba child must be atomic with creating
     // and linking the absent child
     if (v.has_child()) {
-      return trans_intr::make_interruptible(std::move(v.get_child_fut())
+      return std::move(v.get_child_fut()
       ).si_then([on_found=std::move(on_found), node_iter, c,
                 parent_entry](auto child) {
         LOG_PREFIX(FixedKVBtree::lookup_internal_level);
@@ -1571,7 +1567,7 @@ class FixedKVBtree {
     // checking the lba child must be atomic with creating
     // and linking the absent child
     if (v.has_child()) {
-      return trans_intr::make_interruptible(std::move(v.get_child_fut())
+      return std::move(v.get_child_fut()
       ).si_then([on_found=std::move(on_found), node_iter, c,
                 parent_entry](auto child) {
         LOG_PREFIX(FixedKVBtree::lookup_leaf);
@@ -2126,7 +2122,7 @@ class FixedKVBtree {
     // checking the lba child must be atomic with creating
     // and linking the absent child
     if (v.has_child()) {
-      return trans_intr::make_interruptible(std::move(v.get_child_fut())
+      return std::move(v.get_child_fut()
       ).si_then([do_merge=std::move(do_merge), &pos,
                 donor_iter, donor_is_left, c, parent_pos](auto child) {
         LOG_PREFIX(FixedKVBtree::merge_level);

src/crimson/os/seastore/cache.h

@@ -527,8 +527,7 @@ class Cache {
     return view->is_data_stable();
   }
 
-  using get_extent_ertr = base_ertr;
-  get_extent_ertr::future<CachedExtentRef>
+  get_extent_iertr::future<CachedExtentRef>
   get_extent_viewable_by_trans(
     Transaction &t,
     CachedExtentRef extent)
@@ -553,7 +552,7 @@ class Cache {
         if (p_extent->is_mutable()) {
           assert(p_extent->is_fully_loaded());
           assert(!p_extent->is_pending_io());
-          return get_extent_ertr::make_ready_future<CachedExtentRef>(
+          return get_extent_iertr::make_ready_future<CachedExtentRef>(
             CachedExtentRef(p_extent));
         } else {
           assert(p_extent->is_exist_clean());
@@ -588,7 +587,7 @@ class Cache {
       if (extent->is_mutable()) {
         assert(extent->is_fully_loaded());
         assert(!extent->is_pending_io());
-        return get_extent_ertr::make_ready_future<CachedExtentRef>(extent);
+        return get_extent_iertr::make_ready_future<CachedExtentRef>(extent);
       } else {
         assert(extent->is_exist_clean());
         p_extent = extent.get();
@@ -601,30 +600,31 @@ class Cache {
     // also see read_extent_maybe_partial() and get_absent_extent()
     assert(is_logical_type(p_extent->get_type()) ||
            p_extent->is_fully_loaded());
-    return p_extent->wait_io(
-    ).then([p_extent] {
-      return get_extent_ertr::make_ready_future<CachedExtentRef>(
+
+    return trans_intr::make_interruptible(
+      p_extent->wait_io()
+    ).then_interruptible([p_extent] {
+      return get_extent_iertr::make_ready_future<CachedExtentRef>(
         CachedExtentRef(p_extent));
     });
   }
 
   template <typename T>
-  using read_extent_ret = get_extent_ertr::future<TCachedExtentRef<T>>;
-
-  template <typename T>
-  read_extent_ret<T> get_extent_viewable_by_trans(
+  get_extent_iertr::future<TCachedExtentRef<T>>
+  get_extent_viewable_by_trans(
     Transaction &t,
     TCachedExtentRef<T> extent)
   {
     return get_extent_viewable_by_trans(t, CachedExtentRef(extent.get())
-    ).safe_then([](auto p_extent) {
+    ).si_then([](auto p_extent) {
       return p_extent->template cast<T>();
     });
   }
 
   // wait extent io or do partial reads
   template <typename T>
-  read_extent_ret<T> read_extent_maybe_partial(
+  get_extent_iertr::future<TCachedExtentRef<T>>
+  read_extent_maybe_partial(
     Transaction &t,
     TCachedExtentRef<T> extent,
     extent_len_t partial_off,
@@ -642,13 +642,16 @@ class Cache {
         extent->get_type());
       ++access_stats.load_present;
       ++stats.access.s.load_present;
-      return do_read_extent_maybe_partial(
-          std::move(extent), partial_off, partial_len, &t_src);
+      return trans_intr::make_interruptible(
+        do_read_extent_maybe_partial(
+          std::move(extent), partial_off, partial_len, &t_src));
     } else {
       // TODO(implement fine-grained-wait):
       // the range might be already loaded, but we don't know
-      return extent->wait_io().then([extent]() -> read_extent_ret<T> {
-        return seastar::make_ready_future<TCachedExtentRef<T>>(extent);
+      return trans_intr::make_interruptible(
+        extent->wait_io()
+      ).then_interruptible([extent] {
+        return get_extent_iertr::make_ready_future<TCachedExtentRef<T>>(extent);
       });
     }
   }
@@ -664,6 +667,9 @@ class Cache {
   }
 
 private:
+  using get_extent_ertr = base_ertr;
+  template <typename T>
+  using read_extent_ret = get_extent_ertr::future<TCachedExtentRef<T>>;
   /// Implements exclusive call to read_extent() for the extent
   template <typename T>
   read_extent_ret<T> do_read_extent_maybe_partial(

src/crimson/os/seastore/cached_extent.h

@@ -12,9 +12,8 @@
 #include "seastar/core/shared_future.hh"
 
 #include "include/buffer.h"
-#include "crimson/common/errorator.h"
-#include "crimson/common/interruptible_future.h"
 #include "crimson/os/seastore/seastore_types.h"
+#include "crimson/os/seastore/transaction_interruptor.h"
 
 struct btree_lba_manager_test;
 struct lba_btree_test;
@@ -23,7 +22,6 @@ struct cache_test_t;
 
 namespace crimson::os::seastore {
 
-class Transaction;
 class CachedExtent;
 using CachedExtentRef = boost::intrusive_ptr<CachedExtent>;
 class SegmentedAllocator;
@@ -1303,14 +1301,17 @@ class child_pos_t {
   uint16_t pos = std::numeric_limits<uint16_t>::max();
 };
 
-using get_child_ertr = crimson::errorator<
-  crimson::ct_error::input_output_error>;
+using get_child_iertr = trans_iertr<crimson::errorator<
+  crimson::ct_error::input_output_error>>;
+template <typename T>
+using get_child_ifut = get_child_iertr::future<TCachedExtentRef<T>>;
+
 template <typename T>
 struct get_child_ret_t {
-  std::variant<child_pos_t, get_child_ertr::future<TCachedExtentRef<T>>> ret;
+  std::variant<child_pos_t, get_child_ifut<T>> ret;
   get_child_ret_t(child_pos_t pos)
     : ret(std::move(pos)) {}
-  get_child_ret_t(get_child_ertr::future<TCachedExtentRef<T>> child)
+  get_child_ret_t(get_child_ifut<T> child)
     : ret(std::move(child)) {}
 
   bool has_child() const {
@@ -1322,7 +1323,7 @@ struct get_child_ret_t {
     return std::get<0>(ret);
   }
 
-  get_child_ertr::future<TCachedExtentRef<T>> &get_child_fut() {
+  get_child_ifut<T> &get_child_fut() {
     ceph_assert(ret.index() == 1);
     return std::get<1>(ret);
   }

src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc

@@ -52,28 +52,22 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node<
     ceph_assert(lba_root->is_initial_pending()
       == root_block->is_pending());
     return {true,
-	    trans_intr::make_interruptible(
-	      c.cache.get_extent_viewable_by_trans(c.trans, lba_root))};
+            c.cache.get_extent_viewable_by_trans(c.trans, lba_root)};
   } else if (root_block->is_pending()) {
     auto &prior = static_cast<RootBlock&>(*root_block->get_prior_instance());
     lba_root = prior.lba_root_node;
     if (lba_root) {
       return {true,
-	      trans_intr::make_interruptible(
-		c.cache.get_extent_viewable_by_trans(c.trans, lba_root))};
+              c.cache.get_extent_viewable_by_trans(c.trans, lba_root)};
     } else {
       c.cache.account_absent_access(c.trans.get_src());
       return {false,
-	      trans_intr::make_interruptible(
-		Cache::get_extent_ertr::make_ready_future<
-		  CachedExtentRef>())};
+              Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
     }
   } else {
     c.cache.account_absent_access(c.trans.get_src());
     return {false,
-	    trans_intr::make_interruptible(
-	      Cache::get_extent_ertr::make_ready_future<
-		CachedExtentRef>())};
+            Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
   }
 }
 

src/crimson/os/seastore/transaction_manager.h

@@ -954,7 +954,7 @@ class TransactionManager : public ExtentCallbackInterface {
   shard_stats_t& shard_stats;
 
   template <typename T>
-  std::variant<LBAMappingRef, base_iertr::future<TCachedExtentRef<T>>>
+  std::variant<LBAMappingRef, get_child_ifut<T>>
   get_extent_if_linked(
     Transaction &t,
     LBAMappingRef pin)
@@ -964,7 +964,8 @@ class TransactionManager : public ExtentCallbackInterface {
     // and linking the absent child
     auto v = pin->get_logical_extent(t);
     if (v.has_child()) {
-      return v.get_child_fut().safe_then([pin=std::move(pin)](auto extent) {
+      return v.get_child_fut(
+      ).si_then([pin=std::move(pin)](auto extent) {
 #ifndef NDEBUG
         auto lextent = extent->template cast<LogicalCachedExtent>();
         auto pin_laddr = pin->get_key();

src/test/crimson/seastore/test_transaction_manager.cc

@@ -2209,9 +2209,11 @@ TEST_P(tm_single_device_test_t, invalid_lba_mapping_detect)
       assert(pin->is_parent_viewable());
       assert(pin->parent_modified());
       pin->maybe_fix_pos();
-      auto v = pin->get_logical_extent(*t.t);
-      assert(v.has_child());
-      auto extent2 = v.get_child_fut().unsafe_get();
+      auto extent2 = with_trans_intr(*(t.t), [&pin](auto& trans) {
+        auto v = pin->get_logical_extent(trans);
+        assert(v.has_child());
+        return std::move(v.get_child_fut());
+      }).unsafe_get();
       assert(extent.get() == extent2.get());
       submit_transaction(std::move(t));
     }