crimson/os/seastore/extent_pinboard: don't add extents under promotion

to cache

This is to avoid an under-promotion extent from being added back to and
evicted from the cache again before the promotion ends.

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc
index a863d0a03410b6cece51dd364ed95e1e45686630..f2efd08cb9af14ccf274564845f8265d26702d87 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.cc
+++ b/src/crimson/os/seastore/cached_extent.cc
@@ -72,6 +72,8 @@ std::ostream &operator<<(std::ostream &out, const extent_pin_state_t &s) {
     return out << "Fresh";
   case extent_pin_state_t::PendingPromote:
     return out << "PendingPromote";
+  case extent_pin_state_t::Promoting:
+    return out << "Promoting";
   case extent_pin_state_t::WarmIn:
     return out << "WarmIn";
   case extent_pin_state_t::Hot:

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index bb66e20428a8033f163a183b4ad162c11f4cd26e..2a528a2a259861fd8bada5ffd4020233bc944779 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -260,6 +260,7 @@ enum class extent_pin_state_t : uint8_t {
   // shared state between LRU and 2Q impl
   Fresh = 0,
   PendingPromote,
+  Promoting,
   // 2Q impl only
   WarmIn,
   Hot,
@@ -836,7 +837,7 @@ public:
     using crimson::common::get_conf;
     auto type = get_conf<std::string>("seastore_cachepin_type");
     if (type == "LRU") {
-      assert(pin_state <= extent_pin_state_t::PendingPromote);
+      assert(pin_state <= extent_pin_state_t::Promoting);
     } else if (type == "2Q") {
       assert(pin_state < extent_pin_state_t::Max);
     } else {
@@ -852,7 +853,7 @@ public:
     using crimson::common::get_conf;
     auto type = get_conf<std::string>("seastore_cachepin_type");
     if (type == "LRU") {
-      assert(pin_state <= extent_pin_state_t::PendingPromote);
+      assert(pin_state <= extent_pin_state_t::Promoting);
     } else if (type == "2Q") {
       assert(pin_state < extent_pin_state_t::Max);
     } else {

diff --git a/src/crimson/os/seastore/extent_pinboard.cc b/src/crimson/os/seastore/extent_pinboard.cc
index e149d3b0bad0724d53b336affc438908d381fb66..f9f856d33ec0f160e7ffa35d7635d1323c4fa40b 100644 (file)
--- a/src/crimson/os/seastore/extent_pinboard.cc
+++ b/src/crimson/os/seastore/extent_pinboard.cc
@@ -386,7 +386,7 @@ public:
         extents.emplace_back(&extent);
       }
       for (auto &extent : extents) {
-        remove_extent(*extent, extent_pin_state_t::Fresh);
+        remove_extent(*extent, extent_pin_state_t::Promoting);
       }
       promoting_extents.insert(
         promoting_extents.end(),
@@ -541,7 +541,8 @@ public:
        promoter.remove_extent(extent, extent_pin_state_t::Fresh);
       }
     } else {
-      ceph_assert(s == extent_pin_state_t::Fresh);
+      ceph_assert(s == extent_pin_state_t::Fresh ||
+                  s == extent_pin_state_t::Promoting);
     }
   }
 
@@ -563,8 +564,10 @@ public:
     const Transaction::src_t* p_src,
     extent_len_t /*load_start*/,
     extent_len_t /*load_length*/) final {
-    if (extent.is_linked_to_list()) {
-      auto s = extent.get_pin_state();
+    if (auto s = extent.get_pin_state();
+        s == extent_pin_state_t::Promoting) {
+      assert(!extent.is_linked_to_list());
+    } else if (extent.is_linked_to_list()) {
       assert(s <= extent_pin_state_t::PendingPromote);
       if (s == extent_pin_state_t::Fresh) {
        lru.move_to_top(extent, p_src);
@@ -588,7 +591,7 @@ public:
       lru.increase_cached_size(extent, increased_length, p_src);
     } else {
       // promoter take the complete extent size for content size calculation
-      assert(extent.get_pin_state() <= extent_pin_state_t::PendingPromote);
+      assert(extent.get_pin_state() <= extent_pin_state_t::Promoting);
     }
   }
 
@@ -782,7 +785,8 @@ public:
       }
       extent.set_pin_state(extent_pin_state_t::Fresh);
     } else {
-      ceph_assert(s == extent_pin_state_t::Fresh);
+      ceph_assert(s == extent_pin_state_t::Fresh ||
+                  s == extent_pin_state_t::Promoting);
     }
   }
 
@@ -793,6 +797,10 @@ public:
     extent_len_t load_length) final {
     auto state = extent.get_pin_state();
     auto type = extent.get_type();
+    if (state == extent_pin_state_t::Promoting) {
+      assert(!extent.is_linked_to_list());
+      return;
+    }
     if (extent.is_linked_to_list()) {
       if (state == extent_pin_state_t::Hot) {
        hot.move_to_top(extent, p_src);

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index c454cd1d9cc9e5af1edd1dd2d530d881760e6cc3..09655e513577f1cc2a2ad4bde3abfc49504912c1 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -1353,8 +1353,9 @@ TransactionManager::promote_extent(
       auto slice_laddr = (orig_laddr + offset).checked_to_laddr();
       auto slice_length = extent->get_length();
       extent->rewrite(t, *orig_ext, offset);
-      assert(!extent->get_paddr().is_absolute() ||
-             !cache->is_on_cold_tier(extent->get_paddr()));
+      if (extent->get_paddr().is_absolute()) {
+        assert(!cache->is_on_cold_tier(extent->get_paddr()));
+      }
 
       auto lext = extent->cast<LogicalChildNode>();
       lext->set_laddr(slice_laddr);
@@ -1401,8 +1402,9 @@ TransactionManager::promote_extent(
     auto lext = promoted_extent->cast<LogicalChildNode>();
     lext->set_laddr(orig_ext->get_laddr());
     lext->rewrite(t, *orig_ext, 0);
-    assert(!extent->get_paddr().is_absolute() ||
-           !cache->is_on_cold_tier(lext->get_paddr()));
+    if (lext->get_paddr().is_absolute()) {
+      assert(!cache->is_on_cold_tier(lext->get_paddr()));
+    }
     t.touch_laddr_prefix(orig_ext->get_laddr().get_object_prefix());
     //TODO: this memory copy should be saved
     orig_ext->get_bptr().copy_out(