From 2c1e37d25bfd7e8a5acd926c792a0231dbf1ea06 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Mon, 25 Jul 2022 16:26:09 +0800 Subject: [PATCH] crimson/os/seastore/cache: add logs to debug alloc_tail trimming Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/async_cleaner.cc | 2 ++ src/crimson/os/seastore/cache.cc | 33 ++++++++++++++++--- src/crimson/os/seastore/cache.h | 23 ++++++++----- .../crimson/seastore/test_seastore_cache.cc | 2 +- 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc index 938267066c8..4a9765f61ae 100644 --- 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 eb37bad9582..3326f0a131c 100644 --- 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(); } @@ -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( 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 6c691801af2..a5b0f9bb68f 100644 --- 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 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 d89b7168675..06c1ae86192 100644 --- 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; -- 2.39.5