crimson/os/seastore: decouple segment release from transaction manager

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

diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc
index 35b8b0ed7e5a317e8a392d8953b293b709677be4..70e590ec24087fd4e4ab7ceeed8039c5bd690446 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.cc
+++ b/src/crimson/os/seastore/async_cleaner.cc
@@ -1013,9 +1013,6 @@ AsyncCleaner::gc_reclaim_space_ret AsyncCleaner::gc_reclaim_space()
                });
              });
            }).si_then([this, &t] {
-             if (reclaim_state->is_complete()) {
-               t.mark_segment_to_release(reclaim_state->get_segment_id());
-             }
              return ecb->submit_transaction_direct(t);
            });
          });
@@ -1028,14 +1025,32 @@ AsyncCleaner::gc_reclaim_space_ret AsyncCleaner::gc_reclaim_space()
       auto d = seastar::lowres_system_clock::now() - start;
       DEBUG("duration: {}, pavail_ratio before: {}, repeats: {}", d, pavail_ratio, runs);
       if (reclaim_state->is_complete()) {
-       INFO("reclaim {} finish, reclaimed alive/total={}, usage={}",
-             reclaim_state->get_segment_id(),
-             stats.reclaiming_bytes/(double)segments.get_segment_size(),
-             space_tracker->calc_utilization(reclaim_state->get_segment_id()));
+        auto segment_to_release = reclaim_state->get_segment_id();
+        INFO("reclaim {} finish, reclaimed alive/total={}",
+             segment_to_release,
+             stats.reclaiming_bytes/(double)segments.get_segment_size());
        stats.reclaimed_bytes += stats.reclaiming_bytes;
        stats.reclaimed_segment_bytes += segments.get_segment_size();
        stats.reclaiming_bytes = 0;
        reclaim_state.reset();
+        return sm_group->release_segment(segment_to_release
+        ).safe_then([this, FNAME, segment_to_release] {
+          auto old_usage = calc_utilization(segment_to_release);
+          if(unlikely(old_usage != 0)) {
+            space_tracker->dump_usage(segment_to_release);
+            ERRORT("segment {} old_usage {} != 0",
+                   segment_to_release, old_usage);
+            ceph_abort();
+          }
+          segments.mark_empty(segment_to_release);
+          auto new_usage = calc_utilization(segment_to_release);
+          adjust_segment_util(old_usage, new_usage);
+          INFO("released {}, {}",
+               segment_to_release, gc_stat_printer_t{this, false});
+          maybe_wake_gc_blocked_io();
+        });
+      } else {
+        return SegmentManager::release_ertr::now();
       }
     });
   });
@@ -1189,36 +1204,6 @@ AsyncCleaner::scan_extents_ret AsyncCleaner::scan_no_tail_segment(
   });
 }
 
-AsyncCleaner::release_ertr::future<>
-AsyncCleaner::maybe_release_segment(Transaction &t)
-{
-  auto to_release = t.get_segment_to_release();
-  if (to_release != NULL_SEG_ID) {
-    LOG_PREFIX(AsyncCleaner::maybe_release_segment);
-    INFOT("releasing segment {}", t, to_release);
-    return sm_group->release_segment(to_release
-    ).safe_then([this, FNAME, &t, to_release] {
-      auto old_usage = calc_utilization(to_release);
-      if(unlikely(old_usage != 0)) {
-       space_tracker->dump_usage(to_release);
-       ERRORT("segment {} old_usage {} != 0", t, to_release, old_usage);
-       ceph_abort();
-      }
-      segments.mark_empty(to_release);
-      auto new_usage = calc_utilization(to_release);
-      adjust_segment_util(old_usage, new_usage);
-      INFOT("released, {}", t, gc_stat_printer_t{this, false});
-      if (space_tracker->get_usage(to_release) != 0) {
-        space_tracker->dump_usage(to_release);
-        ceph_abort();
-      }
-      maybe_wake_gc_blocked_io();
-    });
-  } else {
-    return SegmentManager::release_ertr::now();
-  }
-}
-
 void AsyncCleaner::complete_init()
 {
   LOG_PREFIX(AsyncCleaner::complete_init);

diff --git a/src/crimson/os/seastore/async_cleaner.h b/src/crimson/os/seastore/async_cleaner.h
index a107694a55c59e45a3f6802356369d98d75fceb5..963efe3a805501956d2d267b400bd9f5b37cd8a4 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.h
+++ b/src/crimson/os/seastore/async_cleaner.h
@@ -861,9 +861,6 @@ public:
   void update_journal_tails(
       journal_seq_t dirty_tail, journal_seq_t alloc_tail) final;
 
-  using release_ertr = SegmentManagerGroup::release_ertr;
-  release_ertr::future<> maybe_release_segment(Transaction &t);
-
   void adjust_segment_util(double old_usage, double new_usage) {
     auto old_index = get_bucket_index(old_usage);
     auto new_index = get_bucket_index(new_usage);

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 2917065b178fee2821c46c8d1c48d808e6e4458b..e38d3c56f179c890c4863dd4ca9de0891b747b36 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -226,15 +226,6 @@ public:
     }
   }
 
-  void mark_segment_to_release(segment_id_t segment) {
-    assert(to_release == NULL_SEG_ID);
-    to_release = segment;
-  }
-
-  segment_id_t get_segment_to_release() const {
-    return to_release;
-  }
-
   auto get_delayed_alloc_list() {
     std::list<LogicalCachedExtentRef> ret;
     for (auto& extent : delayed_alloc_list) {
@@ -363,7 +354,6 @@ public:
     backref_tree_stats = {};
     ool_write_stats = {};
     rewrite_version_stats = {};
-    to_release = NULL_SEG_ID;
     conflicted = false;
     if (!has_reset) {
       has_reset = true;
@@ -519,9 +509,6 @@ private:
   ool_write_stats_t ool_write_stats;
   version_stat_t rewrite_version_stats;
 
-  ///< if != NULL_SEG_ID, release this segment after completion
-  segment_id_t to_release = NULL_SEG_ID;
-
   bool conflicted = false;
 
   bool has_reset = false;

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index f5dee399be7b6f1d086eab176e4ebddb883bdf67..806e646daff8afc0e299ced8f91b5052de3da2fc 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -389,9 +389,6 @@ TransactionManager::submit_transaction_direct(
       async_cleaner->update_journal_tails(
        cache->get_oldest_dirty_from().value_or(start_seq),
        cache->get_oldest_backref_dirty_from().value_or(start_seq));
-      return async_cleaner->maybe_release_segment(tref);
-    }).safe_then([FNAME, &tref] {
-      SUBTRACET(seastore_t, "completed", tref);
       return tref.get_handle().complete();
     }).handle_error(
       submit_transaction_iertr::pass_further{},