crimson/os/seastore/cache: add logs to debug alloc_tail trimming

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 938267066c8c3ecc735a9929f37faa7c24e8d76c..4a9765f61ae32075d5443f3a91b66ed4ddcd1fd3 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.cc
+++ b/src/crimson/os/seastore/async_cleaner.cc
@@ -819,6 +819,8 @@ AsyncCleaner::gc_trim_alloc() {
       "trim_alloc",
       [this](auto &t)
     {
+      LOG_PREFIX(AsyncCleaner::gc_trim_alloc);
+      DEBUGT("target {}", t, get_alloc_tail_target());
       return trim_alloc(t, get_alloc_tail_target()
       ).si_then([this, &t](auto trim_alloc_to)
         -> ExtentCallbackInterface::submit_transaction_direct_iertr::future<>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index eb37bad958263b8e1cb2d19da1798a450f6f165b..3326f0a131c06b8f5c3b6d0d64fa8eb0580edd99 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1259,10 +1259,18 @@ record_t Cache::prepare_record(
     } else {
       dirty_tail = *maybe_dirty_tail;
     }
-    auto tails = journal_tail_delta_t{
-      get_oldest_backref_dirty_from().value_or(JOURNAL_SEQ_NULL),
-      dirty_tail
-    };
+    journal_seq_t alloc_tail;
+    auto maybe_alloc_tail = get_oldest_backref_dirty_from();
+    if (!maybe_alloc_tail.has_value()) {
+      alloc_tail = JOURNAL_SEQ_NULL;
+      // see notes in Cache::complete_commit().
+      SUBWARNT(seastore_t, "backref_buffer all trimmed", t);
+    } else if (*maybe_alloc_tail == JOURNAL_SEQ_NULL) {
+      ceph_abort("impossible");
+    } else {
+      alloc_tail = *maybe_alloc_tail;
+    }
+    auto tails = journal_tail_delta_t{alloc_tail, dirty_tail};
     SUBDEBUGT(seastore_t, "update tails as delta {}", t, tails);
     bufferlist bl;
     encode(tails, bl);
@@ -1374,6 +1382,7 @@ void Cache::backref_batch_update(
 {
   LOG_PREFIX(Cache::backref_batch_update);
   DEBUG("inserting {} entries at {}", list.size(), seq);
+  ceph_assert(seq != JOURNAL_SEQ_NULL);
   if (!backref_buffer) {
     backref_buffer = std::make_unique<backref_cache_t>();
   }
@@ -1433,6 +1442,19 @@ void Cache::complete_commit(
               t,
               i->get_paddr(),
               i->get_length());
+        // FIXME: In theroy, adding backref_list in the finalize phase
+        //        can result in wrong alloc_tail for replay:
+        // * trans-1 enters prepare record with allocations.
+        // * trans-2 (cleaner) enters prepare record, see no alloc-tail,
+        //   so it assume the alloc-tail is the start-seq of trans-2.
+        // * trans-1 enters finalize and update the backref_list,
+        //   implying that alloc-tail is the start-seq of trans-1.
+        // * trans-2 (cleaner) enters finalize.
+        // During replay, the alloc-tail will be set to the start-seq of
+        //   trans-2 according to journal_tail_delta_t, but it should be
+        //   actually the start-seq of trans-1.
+        // This can only happen if alloc_tail trimming is able to trim to
+        //   the journal head.
        backref_list.emplace_back(
          std::make_unique<backref_buf_entry_t>(
            i->get_paddr(),
@@ -1547,8 +1569,9 @@ void Cache::complete_commit(
       add_extent(i, &t_src);
     }
   }
-  if (!backref_list.empty())
+  if (!backref_list.empty()) {
     backref_batch_update(std::move(backref_list), seq);
+  }
 }
 
 void Cache::init()

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 6c691801af24a1ab02d118cb8424bcb09e7e398e..a5b0f9bb68f2534fd2b436fd69499742176b9482 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -636,11 +636,19 @@ public:
     LOG_PREFIX(Cache::trim_backref_bufs);
     SUBDEBUG(seastore_cache, "trimming to {}", trim_to);
     if (backref_buffer && !backref_buffer->backrefs_by_seq.empty()) {
+      SUBDEBUG(seastore_cache, "backrefs {} ~ {}, size={}",
+               backref_buffer->backrefs_by_seq.rbegin()->first,
+               backref_buffer->backrefs_by_seq.begin()->first,
+               backref_buffer->backrefs_by_seq.size());
       assert(backref_buffer->backrefs_by_seq.rbegin()->first >= trim_to);
       auto iter = backref_buffer->backrefs_by_seq.upper_bound(trim_to);
       backref_buffer->backrefs_by_seq.erase(
        backref_buffer->backrefs_by_seq.begin(), iter);
     }
+    if (!backref_buffer || backref_buffer->backrefs_by_seq.empty()) {
+      // see notes in Cache::complete_commit().
+      SUBWARN(seastore_cache, "backref_buffer all trimmed");
+    }
   }
 
   /**
@@ -889,20 +897,17 @@ public:
     journal_seq_t seq,
     size_t max_bytes);
 
+  /// returns std::nullopt if no pending alloc-infos
   std::optional<journal_seq_t> get_oldest_backref_dirty_from() const {
     LOG_PREFIX(Cache::get_oldest_backref_dirty_from);
-    journal_seq_t backref_oldest = JOURNAL_SEQ_NULL;
-    if (backref_buffer && !backref_buffer->backrefs_by_seq.empty()) {
-      backref_oldest = backref_buffer->backrefs_by_seq.begin()->first;
-    }
-    if (backref_oldest == JOURNAL_SEQ_NULL) {
+    if (!backref_buffer || backref_buffer->backrefs_by_seq.empty()) {
       SUBDEBUG(seastore_cache, "backref_oldest: null");
       return std::nullopt;
-    } else {
-      SUBDEBUG(seastore_cache, "backref_oldest: {}",
-       backref_oldest);
-      return backref_oldest;
     }
+    auto oldest = backref_buffer->backrefs_by_seq.begin()->first;
+    SUBDEBUG(seastore_cache, "backref_oldest: {}", oldest);
+    ceph_assert(oldest != JOURNAL_SEQ_NULL);
+    return oldest;
   }
 
   /// returns std::nullopt if no dirty extents

diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc
index d89b7168675059e13c0ddbf50e49738331d8f8a3..06c1ae86192fa2d53702ba9bf070b067bdd78bdf 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_cache.cc
+++ b/src/test/crimson/seastore/test_seastore_cache.cc
@@ -24,7 +24,7 @@ struct cache_test_t : public seastar_test_suite_t {
   ExtentPlacementManagerRef epm;
   CacheRef cache;
   paddr_t current;
-  journal_seq_t seq;
+  journal_seq_t seq = JOURNAL_SEQ_MIN;
 
   cache_test_t() = default;