crimson/os/seastore: reuse with_transaction_intr for weak trans

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/async_cleaner.h b/src/crimson/os/seastore/async_cleaner.h
index 294a8e0c3c97d69837bdb777db016424ef04fa0d..8e9c7b33fad3915d0cbd27ae0e51243860e6aa3a 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.h
+++ b/src/crimson/os/seastore/async_cleaner.h
@@ -592,8 +592,10 @@ public:
   public:
     virtual ~ExtentCallbackInterface() = default;
 
+    /// Creates empty transaction
+    /// weak transaction should be type READ
     virtual TransactionRef create_transaction(
-        Transaction::src_t, const char*) = 0;
+        Transaction::src_t, const char *name, bool is_weak=false) = 0;
 
     /// Creates empty transaction with interruptible context
     template <typename Func>
@@ -601,16 +603,21 @@ public:
         Transaction::src_t src,
         const char* name,
         Func &&f) {
-      return seastar::do_with(
-        create_transaction(src, name),
-        [f=std::forward<Func>(f)](auto &ref_t) mutable {
-          return with_trans_intr(
-            *ref_t,
-            [f=std::forward<Func>(f)](auto& t) mutable {
-              return f(t);
-            }
-          );
-        }
+      return do_with_transaction_intr<Func, false>(
+          src, name, std::forward<Func>(f));
+    }
+
+    template <typename Func>
+    auto with_transaction_weak(
+        const char* name,
+        Func &&f) {
+      return do_with_transaction_intr<Func, true>(
+          Transaction::src_t::READ, name, std::forward<Func>(f)
+      ).handle_error(
+        crimson::ct_error::eagain::handle([] {
+          ceph_assert(0 == "eagain impossible");
+        }),
+        crimson::ct_error::pass_further_all{}
       );
     }
 
@@ -684,6 +691,25 @@ public:
     virtual submit_transaction_direct_ret submit_transaction_direct(
       Transaction &t,
       std::optional<journal_seq_t> seq_to_trim = std::nullopt) = 0;
+
+  private:
+    template <typename Func, bool IsWeak>
+    auto do_with_transaction_intr(
+        Transaction::src_t src,
+        const char* name,
+        Func &&f) {
+      return seastar::do_with(
+        create_transaction(src, name, IsWeak),
+        [f=std::forward<Func>(f)](auto &ref_t) mutable {
+          return with_trans_intr(
+            *ref_t,
+            [f=std::forward<Func>(f)](auto& t) mutable {
+              return f(t);
+            }
+          );
+        }
+      );
+    }
   };
 
 private:

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index d4e583465376ee6327f37b3892fbc2879f2c88fa..01e163afe35339e8f66eca47d214095ca211da4a 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -237,6 +237,7 @@ public:
     );
     SUBDEBUGT(seastore_t, "created name={}, source={}, is_weak={}",
              *ret, name, src, is_weak);
+    assert(!is_weak || src == Transaction::src_t::READ);
     return ret;
   }
 

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 81c79524a3b5774915f75e8aed88945fdb33a5cd..7f1d8daf131691fa4ccba85e5e7b50ccde55b71a 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -110,72 +110,67 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount()
     return journal->open_for_mount();
   }).safe_then([this, FNAME](auto start_seq) {
     async_cleaner->set_journal_head(start_seq);
-    return seastar::do_with(
-      create_weak_transaction(
-        Transaction::src_t::READ, "mount"),
-      [this, FNAME](auto &tref) {
-       return with_trans_intr(
-         *tref,
-         [this, FNAME](auto &t) {
-           return cache->init_cached_extents(t, [this](auto &t, auto &e) {
-             if (is_backref_node(e->get_type()))
-               return backref_manager->init_cached_extent(t, e);
-             else
-               return lba_manager->init_cached_extent(t, e);
-           }).si_then([this, FNAME, &t] {
-             assert(async_cleaner->debug_check_space(
-                      *async_cleaner->get_empty_space_tracker()));
-             return backref_manager->scan_mapped_space(
-               t,
-               [this, FNAME, &t](
-                 paddr_t addr,
-                 extent_len_t len,
-                 depth_t depth,
-                 extent_types_t type)
-               {
-                 TRACET(
-                   "marking {}~{} used",
-                   t,
-                   addr,
-                   len);
-                 async_cleaner->mark_space_used(
-                   addr,
-                   len ,
-                   /* init_scan = */ true);
-                 if (is_backref_node(type)) {
-                   ceph_assert(depth);
-                   backref_manager->cache_new_backref_extent(addr, type);
-                   cache->update_tree_extents_num(type, 1);
-                   return seastar::now();
-                 } else {
-                   ceph_assert(!depth);
-                   cache->update_tree_extents_num(type, 1);
-                   return seastar::now();
-                 }
-               }).si_then([this, &t] {
-                 LOG_PREFIX(TransactionManager::mount);
-                 auto &backrefs = backref_manager->get_cached_backrefs();
-                 DEBUGT("scan backref cache", t);
-                 for (auto &backref : backrefs) {
-                   if (backref.laddr == L_ADDR_NULL) {
-                     async_cleaner->mark_space_free(
-                       backref.paddr,
-                       backref.len,
-                       true);
-                     cache->update_tree_extents_num(backref.type, -1);
-                   } else {
-                     async_cleaner->mark_space_used(
-                       backref.paddr,
-                       backref.len,
-                       true);
-                     cache->update_tree_extents_num(backref.type, 1);
-                   }
-                 }
-                 return seastar::now();
-               });
-           });
-         });
+    return with_transaction_weak(
+      "mount",
+      [this, FNAME](auto &t)
+    {
+      return cache->init_cached_extents(t, [this](auto &t, auto &e) {
+        if (is_backref_node(e->get_type())) {
+          return backref_manager->init_cached_extent(t, e);
+        } else {
+          return lba_manager->init_cached_extent(t, e);
+        }
+      }).si_then([this, FNAME, &t] {
+        assert(async_cleaner->debug_check_space(
+                 *async_cleaner->get_empty_space_tracker()));
+        return backref_manager->scan_mapped_space(
+          t,
+          [this, FNAME, &t](
+            paddr_t addr,
+            extent_len_t len,
+            depth_t depth,
+            extent_types_t type) {
+          TRACET(
+            "marking {}~{} used",
+            t,
+            addr,
+            len);
+          async_cleaner->mark_space_used(
+            addr,
+            len ,
+            /* init_scan = */ true);
+          if (is_backref_node(type)) {
+            ceph_assert(depth);
+            backref_manager->cache_new_backref_extent(addr, type);
+            cache->update_tree_extents_num(type, 1);
+            return seastar::now();
+          } else {
+            ceph_assert(!depth);
+            cache->update_tree_extents_num(type, 1);
+            return seastar::now();
+          }
+        });
+      }).si_then([this, FNAME, &t] {
+        auto &backrefs = backref_manager->get_cached_backrefs();
+        DEBUGT("scan backref cache", t);
+        for (auto &backref : backrefs) {
+          if (backref.laddr == L_ADDR_NULL) {
+            async_cleaner->mark_space_free(
+              backref.paddr,
+              backref.len,
+              true);
+            cache->update_tree_extents_num(backref.type, -1);
+          } else {
+            async_cleaner->mark_space_used(
+              backref.paddr,
+              backref.len,
+              true);
+            cache->update_tree_extents_num(backref.type, 1);
+          }
+        }
+        return seastar::now();
       });
+    });
   }).safe_then([this] {
     return epm->open();
   }).safe_then([FNAME, this] {
@@ -186,7 +181,8 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount()
     crimson::ct_error::all_same_way([] {
       ceph_assert(0 == "unhandled error");
       return mount_ertr::now();
-    }));
+    })
+  );
 }
 
 TransactionManager::close_ertr::future<> TransactionManager::close() {

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index e123e14cf891f76f42956e70a5d7fd85ed8ee48d..28cf66880c6a6e4312c6a2d06ec5f5705d81291f 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -130,17 +130,12 @@ public:
   close_ertr::future<> close();
 
   /// Creates empty transaction
+  /// weak transaction should be type READ
   TransactionRef create_transaction(
       Transaction::src_t src,
-      const char* name) final {
-    return cache->create_transaction(src, name, false);
-  }
-
-  /// Creates empty weak transaction
-  TransactionRef create_weak_transaction(
-      Transaction::src_t src,
-      const char* name) {
-    return cache->create_transaction(src, name, true);
+      const char* name,
+      bool is_weak=false) final {
+    return cache->create_transaction(src, name, is_weak);
   }
 
   /// Resets transaction

diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h
index f288a074a0d0fc95c793e7cec845e8e38379055c..5176540147711eb3b789e7a096899c9deb6f99c7 100644 (file)
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -246,8 +246,8 @@ protected:
   }
 
   auto create_weak_transaction() {
-    return tm->create_weak_transaction(
-        Transaction::src_t::READ, "test_read_weak");
+    return tm->create_transaction(
+        Transaction::src_t::READ, "test_read_weak", true);
   }
 
   auto submit_transaction_fut2(Transaction& t) {