crimson/os/seastore/transaction: refine version_stat_t as rewrite_stats_t

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

Author: cyx1231st

src/crimson/os/seastore/cache.cc

@@ -705,22 +705,22 @@ void Cache::register_metrics()
     {
       sm::make_counter(
         "version_count_dirty",
-        stats.committed_dirty_version.num,
+        stats.trim_rewrites.num,
         sm::description("total number of rewrite-dirty extents")
       ),
       sm::make_counter(
         "version_sum_dirty",
-        stats.committed_dirty_version.version,
+        stats.trim_rewrites.version,
         sm::description("sum of the version from rewrite-dirty extents")
       ),
       sm::make_counter(
         "version_count_reclaim",
-        stats.committed_reclaim_version.num,
+        stats.reclaim_rewrites.num,
         sm::description("total number of rewrite-reclaim extents")
       ),
       sm::make_counter(
         "version_sum_reclaim",
-        stats.committed_reclaim_version.version,
+        stats.reclaim_rewrites.version,
         sm::description("sum of the version from rewrite-reclaim extents")
       ),
     }
@@ -1586,14 +1586,14 @@ record_t Cache::prepare_record(
   efforts.inline_record_metadata_bytes +=
     (record.size.get_raw_mdlength() - record.get_delta_size());
 
-  auto &rewrite_version_stats = t.get_rewrite_version_stats();
+  auto &rewrite_stats = t.get_rewrite_stats();
   if (trans_src == Transaction::src_t::TRIM_DIRTY) {
-    stats.committed_dirty_version.increment_stat(rewrite_version_stats);
+    stats.trim_rewrites.add(rewrite_stats);
   } else if (trans_src == Transaction::src_t::CLEANER_MAIN ||
              trans_src == Transaction::src_t::CLEANER_COLD) {
-    stats.committed_reclaim_version.increment_stat(rewrite_version_stats);
+    stats.reclaim_rewrites.add(rewrite_stats);
   } else {
-    assert(rewrite_version_stats.is_clear());
+    assert(rewrite_stats.is_clear());
   }
 
   return record;

src/crimson/os/seastore/cache.h

@@ -1631,8 +1631,8 @@ class Cache {
     std::array<uint64_t, NUM_SRC_COMB> trans_conflicts_by_srcs;
     counter_by_src_t<uint64_t> trans_conflicts_by_unknown;
 
-    version_stat_t committed_dirty_version;
-    version_stat_t committed_reclaim_version;
+    rewrite_stats_t trim_rewrites;
+    rewrite_stats_t reclaim_rewrites;
   } stats;
 
   mutable dirty_io_stats_t last_dirty_io;

src/crimson/os/seastore/transaction.h

@@ -41,22 +41,22 @@ inline std::ostream& operator<<(std::ostream& out, const io_stat_t& stat) {
   return out << stat.num << "(" << stat.bytes << "B)";
 }
 
-struct version_stat_t {
+struct rewrite_stats_t {
   uint64_t num = 0;
   uint64_t version = 0;
 
   bool is_clear() const {
-    return (num == 0 && version == 0);
+    return num == 0;
   }
 
-  void increment(extent_version_t v) {
+  void account(extent_version_t v) {
     ++num;
     version += v;
   }
 
-  void increment_stat(const version_stat_t& stat) {
-    num += stat.num;
-    version += stat.version;
+  void add(const rewrite_stats_t& o) {
+    num += o.num;
+    version += o.version;
   }
 };
 
@@ -433,7 +433,7 @@ class Transaction {
     lba_tree_stats = {};
     backref_tree_stats = {};
     ool_write_stats = {};
-    rewrite_version_stats = {};
+    rewrite_stats = {};
     conflicted = false;
     if (!has_reset) {
       has_reset = true;
@@ -492,8 +492,8 @@ class Transaction {
   ool_write_stats_t& get_ool_write_stats() {
     return ool_write_stats;
   }
-  version_stat_t& get_rewrite_version_stats() {
-    return rewrite_version_stats;
+  rewrite_stats_t& get_rewrite_stats() {
+    return rewrite_stats;
   }
 
   struct existing_block_stats_t {
@@ -617,7 +617,7 @@ class Transaction {
   tree_stats_t lba_tree_stats;
   tree_stats_t backref_tree_stats;
   ool_write_stats_t ool_write_stats;
-  version_stat_t rewrite_version_stats;
+  rewrite_stats_t rewrite_stats;
 
   bool conflicted = false;
 

src/crimson/os/seastore/transaction_manager.cc

@@ -638,6 +638,7 @@ TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent(
 
   assert(extent->is_valid() && !extent->is_initial_pending());
   if (extent->is_dirty()) {
+    assert(extent->get_version() > 0);
     if (epm->can_inplace_rewrite(t, extent)) {
       DEBUGT("delta overwriting extent -- {}", t, *extent);
       t.add_inplace_rewrite_extent(extent);
@@ -649,9 +650,10 @@ TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent(
     extent->set_target_rewrite_generation(target_generation);
     ceph_assert(modify_time != NULL_TIME);
     extent->set_modify_time(modify_time);
+    assert(extent->get_version() == 0);
   }
 
-  t.get_rewrite_version_stats().increment(extent->get_version());
+  t.get_rewrite_stats().account(extent->get_version());
 
   if (is_backref_node(extent->get_type())) {
     DEBUGT("rewriting backref extent -- {}", t, *extent);