From 2b1402b3d5b44be8217a87cd640ff5d584f5bb87 Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Tue, 28 Sep 2021 15:10:23 +0800
Subject: [PATCH] crimson/os/seastore/transaction: count fresh blocks
 separately

Correct get_num_fresh_blocks() to be accurate under the effect of
delayed allocations.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---
 src/crimson/os/seastore/cache.cc               | 16 ++++++++++------
 .../os/seastore/extent_placement_manager.h     |  1 +
 .../lba_manager/btree/btree_lba_manager.cc     |  2 +-
 src/crimson/os/seastore/transaction.h          | 18 ++++++++++++++++--
 4 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 2b854cee03b53..1c6783fd41be3 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -614,10 +614,9 @@ void Cache::mark_transaction_conflicted(
       efforts.retire.bytes += i->get_length();
     }
 
-    efforts.fresh.extents += t.get_num_fresh_blocks();
-    t.for_each_fresh_block([&](auto &i) {
-      efforts.fresh.bytes += i->get_length();
-    });
+    auto& fresh_stats = t.get_fresh_block_stats();
+    efforts.fresh.extents += fresh_stats.num;
+    efforts.fresh.bytes += fresh_stats.bytes;
 
     for (auto &i: t.mutated_block_list) {
       if (!i->is_valid()) {
@@ -641,7 +640,7 @@ void Cache::mark_transaction_conflicted(
   } else {
     // read transaction won't have non-read efforts
     assert(t.retired_set.empty());
-    assert(t.get_num_fresh_blocks() == 0);
+    assert(t.get_fresh_block_stats().num == 0);
     assert(t.mutated_block_list.empty());
     assert(t.onode_tree_stats.is_clear());
     assert(t.lba_tree_stats.is_clear());
@@ -664,7 +663,7 @@ void Cache::on_transaction_destruct(Transaction& t)
     }
     // read transaction won't have non-read efforts
     assert(t.retired_set.empty());
-    assert(t.get_num_fresh_blocks() == 0);
+    assert(t.get_fresh_block_stats().num == 0);
     assert(t.mutated_block_list.empty());
     assert(t.onode_tree_stats.is_clear());
     assert(t.lba_tree_stats.is_clear());
@@ -868,6 +867,11 @@ record_t Cache::prepare_record(Transaction &t)
       });
   }
 
+  ceph_assert(t.get_fresh_block_stats().num ==
+              t.inline_block_list.size() +
+              t.ool_block_list.size() +
+              t.num_delayed_invalid_extents);
+
   return record;
 }
 
diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h
index 277a8c4556298..8a1fa5b2309b7 100644
--- a/src/crimson/os/seastore/extent_placement_manager.h
+++ b/src/crimson/os/seastore/extent_placement_manager.h
@@ -365,6 +365,7 @@ public:
       for (auto& extent : alloc_list) {
         // extents may be invalidated
         if (!extent->is_valid()) {
+          t.increment_delayed_invalid_extents();
           continue;
         }
         if (should_be_inline(extent)) {
diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
index e9861ae306fe9..c68b6d4b3ecd6 100644
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
@@ -232,7 +232,7 @@ void BtreeLBAManager::complete_transaction(
 
   // ...but add_pin from parent->leaf
   std::vector<CachedExtentRef> to_link;
-  to_link.reserve(t.get_num_fresh_blocks());
+  to_link.reserve(t.get_fresh_block_stats().num);
   t.for_each_fresh_block([&](auto &e) {
     if (e->is_valid() && (is_lba_node(*e) || e->is_logical()))
       to_link.push_back(e);
diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index fe4adb9d6ad79..91ac1cd286925 100644
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -100,6 +100,8 @@ public:
       offset += ref->get_length();
       inline_block_list.push_back(ref);
     }
+    ++fresh_block_stats.num;
+    fresh_block_stats.bytes += ref->get_length();
     TRACET("adding {} to write_set", *this, *ref);
     write_set.insert(*ref);
   }
@@ -185,8 +187,12 @@ public:
     std::for_each(inline_block_list.begin(), inline_block_list.end(), f);
   }
 
-  auto get_num_fresh_blocks() const {
-    return inline_block_list.size() + ool_block_list.size();
+  struct io_stat_t {
+    uint64_t num = 0;
+    uint64_t bytes = 0;
+  };
+  const io_stat_t& get_fresh_block_stats() const {
+    return fresh_block_stats;
   }
 
   enum class src_t : uint8_t {
@@ -250,6 +256,8 @@ public:
     read_set.clear();
     invalidate_clear_write_set();
     mutated_block_list.clear();
+    fresh_block_stats = {};
+    num_delayed_invalid_extents = 0;
     delayed_alloc_list.clear();
     inline_block_list.clear();
     ool_block_list.clear();
@@ -285,6 +293,10 @@ public:
     return lba_tree_stats;
   }
 
+  void increment_delayed_invalid_extents() {
+    ++num_delayed_invalid_extents;
+  }
+
 private:
   friend class Cache;
   friend Ref make_test_transaction();
@@ -322,6 +334,8 @@ private:
   /**
    * lists of fresh blocks, holds refcounts, subset of write_set
    */
+  io_stat_t fresh_block_stats;
+  uint64_t num_delayed_invalid_extents = 0;
   /// blocks that will be committed with journal record inline
   std::list<CachedExtentRef> inline_block_list;
   /// blocks that will be committed with out-of-line record
-- 
2.39.5