From: Zhang Song Date: Wed, 2 Jul 2025 01:59:55 +0000 (+0800) Subject: crimson/os/seastore/ExtentPinboardTwoQ: don't promote extents that were partially... X-Git-Tag: testing/wip-vshankar-testing-20250805.100115-debug~40^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=6a0d1ad98901302e3326d243a6d84e6089bdadc4;p=ceph-ci.git crimson/os/seastore/ExtentPinboardTwoQ: don't promote extents that were partially read from the last access end Signed-off-by: Zhang Song --- diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index ba068e171de..c0ad5ab5e05 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1535,7 +1535,7 @@ record_t Cache::prepare_record( i->state = CachedExtent::extent_state_t::CLEAN; assert(i->is_logical()); i->clear_modified_region(); - touch_extent(*i, &trans_src, t.get_cache_hint()); + touch_extent_fully(*i, &trans_src, t.get_cache_hint()); DEBUGT("inplace rewrite ool block is commmitted -- {}", t, *i); } @@ -1570,7 +1570,7 @@ record_t Cache::prepare_record( if (i->is_stable_dirty()) { add_to_dirty(i, &t_src); } else { - touch_extent(*i, &t_src, t.get_cache_hint()); + touch_extent_fully(*i, &t_src, t.get_cache_hint()); } alloc_delta.alloc_blk_ranges.emplace_back( @@ -1814,7 +1814,7 @@ void Cache::complete_commit( i->invalidate_hints(); add_extent(i); const auto t_src = t.get_src(); - touch_extent(*i, &t_src, t.get_cache_hint()); + touch_extent_fully(*i, &t_src, t.get_cache_hint()); i->complete_io(); epm.commit_space_used(i->get_paddr(), i->get_length()); @@ -2083,7 +2083,7 @@ Cache::replay_delta( ext.cast()->set_laddr(laddr); } // replay is not included by the cache hit metrics - touch_extent(ext, nullptr, CACHE_HINT_TOUCH); + touch_extent_fully(ext, nullptr, CACHE_HINT_TOUCH); }, nullptr) : _get_extent_if_cached( diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index d14f76d6f46..796c394d7e2 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -251,7 +251,7 @@ public: return ret->wait_io().then([ret] { return get_extent_if_cached_iertr::make_ready_future(ret); }); - } + } // result == Transaction::get_extent_ret::ABSENT assert(paddr.is_absolute()); // get_extent_ret::ABSENT from transaction @@ -290,7 +290,7 @@ public: } t.add_to_read_set(ret); - touch_extent(*ret, &t_src, t.get_cache_hint()); + touch_extent_fully(*ret, &t_src, t.get_cache_hint()); if (!ret->is_fully_loaded()) { SUBDEBUGT(seastore_cache, "{} {}~0x{:x} is present without fully loaded in cache -- {}", @@ -360,7 +360,7 @@ public: t, T::TYPE, offset, length); auto f = [&t, this, t_src](CachedExtent &ext) { t.add_to_read_set(CachedExtentRef(&ext)); - touch_extent(ext, &t_src, t.get_cache_hint()); + touch_extent_fully(ext, &t_src, t.get_cache_hint()); }; return trans_intr::make_interruptible( do_get_caching_extent( @@ -397,7 +397,7 @@ public: SUBTRACET(seastore_cache, "{} {}~0x{:x} is absent on t, query cache ...", t, T::TYPE, offset, length); const auto t_src = t.get_src(); - auto f = [&t, this, t_src](CachedExtent &ext) { + auto f = [&t, this, partial_off, partial_len, t_src](CachedExtent &ext) { // XXX: is_stable_dirty() may not be linked in lba tree assert(ext.is_stable()); assert(T::TYPE == ext.get_type()); @@ -408,7 +408,9 @@ public: ++stats.access.load_absent; t.add_to_read_set(CachedExtentRef(&ext)); - touch_extent(ext, &t_src, t.get_cache_hint()); + touch_extent_by_range( + ext, &t_src, t.get_cache_hint(), + partial_off, partial_len); }; return trans_intr::make_interruptible( do_get_caching_extent( @@ -492,7 +494,7 @@ public: ++stats.access.cache_lru; } if (ret.is_paddr_known) { - touch_extent(extent, &t_src, t.get_cache_hint()); + touch_extent_fully(extent, &t_src, t.get_cache_hint()); } else { needs_touch = true; } @@ -519,7 +521,7 @@ public: t.maybe_add_to_read_set_step_2(target_extent.get()); } if (needs_touch) { - touch_extent(*target_extent, &t_src, t.get_cache_hint()); + touch_extent_fully(*target_extent, &t_src, t.get_cache_hint()); } return get_extent_iertr::now(); }); @@ -585,7 +587,7 @@ public: ++stats.access.cache_lru; } if (ret.is_paddr_known) { - touch_extent(*p_extent, &t_src, t.get_cache_hint()); + touch_extent_fully(*p_extent, &t_src, t.get_cache_hint()); } else { needs_touch = true; } @@ -635,7 +637,7 @@ public: t.maybe_add_to_read_set_step_2(p_extent); } if (needs_touch) { - touch_extent(*p_extent, &t_src, t.get_cache_hint()); + touch_extent_fully(*p_extent, &t_src, t.get_cache_hint()); } return get_extent_iertr::make_ready_future( CachedExtentRef(p_extent)); @@ -939,7 +941,7 @@ private: t, type, offset, length, laddr); auto f = [&t, this, t_src](CachedExtent &ext) { t.add_to_read_set(CachedExtentRef(&ext)); - touch_extent(ext, &t_src, t.get_cache_hint()); + touch_extent_fully(ext, &t_src, t.get_cache_hint()); }; return trans_intr::make_interruptible( do_get_caching_extent_by_type( @@ -981,7 +983,7 @@ private: ++stats.access.load_absent; t.add_to_read_set(CachedExtentRef(&ext)); - touch_extent(ext, &t_src, t.get_cache_hint()); + touch_extent_fully(ext, &t_src, t.get_cache_hint()); }; return trans_intr::make_interruptible( do_get_caching_extent_by_type( @@ -1574,18 +1576,31 @@ public: } private: - /// Update extent pinboard for access to ref - void touch_extent( + void touch_extent_fully( CachedExtent &ext, const Transaction::src_t* p_src, cache_hint_t hint) + { + touch_extent_by_range(ext, p_src, hint, 0, ext.get_length()); + } + + /// Update extent pinboard for access to ref + /// + /// The extent must be touched before read_extent(), otherwise it is + /// incorrect to detect sequential read using load_start and load_length. + void touch_extent_by_range( + CachedExtent &ext, + const Transaction::src_t* p_src, + cache_hint_t hint, + extent_len_t load_start, + extent_len_t load_length) { assert(ext.get_paddr().is_absolute()); if (hint == CACHE_HINT_NOCACHE && is_logical_type(ext.get_type())) { return; } if (ext.is_stable_clean() && !ext.is_placeholder()) { - pinboard->move_to_top(ext, p_src); + pinboard->move_to_top(ext, p_src, load_start, load_length); } } diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 7e13e13ab15..163a46608b1 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -838,6 +838,15 @@ public: cache_state = state; } + extent_len_t get_last_touch_end() const { + return last_touch_end; + } + + void set_last_touch_end(extent_len_t touch_end) { + assert(touch_end != 0); + last_touch_end = touch_end; + } + private: template friend class read_set_item_t; @@ -945,6 +954,10 @@ private: // or the rewrite generation for the fresh write rewrite_gen_t rewrite_generation = NULL_GENERATION; + // save the end offset of the most recent of extent touching, + // see Cache::touch_extent_by_range() and ExtentPinboardTwoQ. + extent_len_t last_touch_end = 0; + // This field is unused when the ExtentPinboard use LRU algorithm extent_2q_state_t cache_state = extent_2q_state_t::Fresh; diff --git a/src/crimson/os/seastore/extent_pinboard.cc b/src/crimson/os/seastore/extent_pinboard.cc index c02cc993974..5971ded3287 100644 --- a/src/crimson/os/seastore/extent_pinboard.cc +++ b/src/crimson/os/seastore/extent_pinboard.cc @@ -330,7 +330,9 @@ public: void move_to_top( CachedExtent &extent, - const Transaction::src_t* p_src) final { + const Transaction::src_t* p_src, + extent_len_t /*load_start*/, + extent_len_t /*load_length*/) final { if (extent.is_linked_to_list()) { lru.move_to_top(extent, p_src); } else { @@ -368,20 +370,37 @@ public: clear(); } - bool accessed_recently(laddr_t laddr) { + enum class AccessMode { + Missing, + ContinueFromLastEnd, + Again + }; + + AccessMode accessed_recently(laddr_t laddr, extent_len_t load_start) { auto iter = index.find(laddr); if (iter == index.end()) { - return false; + return AccessMode::Missing; } + // Current read start offset is same as the last access end offset, + // we treat this access as sequential read. + auto &last_end = iter->second->last_access_end; + assert(last_end != 0); + auto ret = last_end == load_start + ? AccessMode::ContinueFromLastEnd + : AccessMode::Again; remove(iter); - return true; + return ret; } - void add(laddr_t laddr, extent_len_t loaded_length) { + void add(laddr_t laddr, + extent_len_t loaded_length, + extent_len_t access_end) { assert(laddr != L_ADDR_NULL); assert(loaded_length != 0); + assert(access_end != 0); assert(!index.contains(laddr)); - index[laddr] = queue.emplace(queue.end(), laddr, loaded_length); + index[laddr] = queue.emplace( + queue.end(), laddr, loaded_length, access_end); current_size += loaded_length; trim_to(capacity); } @@ -400,11 +419,16 @@ public: private: struct entry_t { - entry_t(laddr_t laddr, extent_len_t loaded_length) - : laddr(laddr), loaded_length(loaded_length) {} + entry_t(laddr_t laddr, + extent_len_t loaded_length, + extent_len_t access_end) + : laddr(laddr), + loaded_length(loaded_length), + last_access_end(access_end) {} laddr_t laddr; extent_len_t loaded_length; + extent_len_t last_access_end; }; using entry_queue_t = std::list; @@ -495,7 +519,9 @@ public: void move_to_top( CachedExtent &extent, - const Transaction::src_t* p_src) final { + const Transaction::src_t* p_src, + extent_len_t load_start, + extent_len_t load_length) final { auto state = extent.get_2q_state(); auto type = extent.get_type(); if (extent.is_linked_to_list()) { @@ -520,7 +546,9 @@ public: } else { // the logical extent which is not in warm_in and not in hot ceph_assert(state == extent_2q_state_t::Fresh); auto lext = extent.cast(); - if (warm_out.accessed_recently(lext->get_laddr())) { + auto m = warm_out.accessed_recently(lext->get_laddr(), load_start); + using AccessMode = IndexedFifoQueue::AccessMode; + if (m == AccessMode::Again) { // This extent was accessed recently, consider it's hot enough to // promote to hot queue. extent.set_2q_state(extent_2q_state_t::Hot); @@ -533,9 +561,18 @@ public: extent.set_2q_state(extent_2q_state_t::WarmIn); auto trimmed_extents = warm_in.add_to_top(extent, p_src); on_update_warm_in(trimmed_extents); - hit_queue(overall_hits.absent, p_src, type); + if (m == AccessMode::Missing) { + hit_queue(overall_hits.absent, p_src, type); + } else { // m == AccessMode::ContinueFromLastEnd + hit_queue(overall_hits.sequential_absent, p_src, type); + } } } + auto end = load_start + load_length; + assert(end != 0); + // The last end update should be in the same continuation with touching + // extent, subsequent touch will cover the prior touch end. + extent.set_last_touch_end(end); } void increase_cached_size( @@ -582,10 +619,12 @@ private: ceph_assert(is_logical_type(extent->get_type())); extent->set_2q_state(extent_2q_state_t::Fresh); auto lext = extent->cast(); + auto laddr = lext->get_laddr(); auto len = extent->get_loaded_length(); + auto end = extent->get_last_touch_end(); // the extents evicted from warm_in queue will be recorded // in warm_out FIFO queue as recently accessed extents. - warm_out.add(lext->get_laddr(), len); + warm_out.add(laddr, len, end); } } // 2Q cache algorithm: @@ -596,9 +635,11 @@ private: // Workflow: // 1. New non-logical extents enter warm_in first, physical extents // are placed into hot queue directly - // 2. On warm_in eviction, add extent's metadata(laddr, loaded length) - // to warm_out queue - // 3. If accessed while in warm_out, extent promotes to hot queue + // 2. On warm_in eviction, add extent's metadata(laddr, loaded length and + // last access end) to warm_out queue + // 3. If the extent in warm_out is accessed again and the load start doesn't + // match the last accessed end recorded in the warm_out, promote extent + // to the hot queue, otherwise add add extent to warm_in queue // 4. Hot queue manages extents using LRU algorithm ExtentQueue warm_in; IndexedFifoQueue warm_out; @@ -638,6 +679,7 @@ private: QueueCounter hot_hits; QueueCounter absent; QueueCounter hot_absent; + QueueCounter sequential_absent; }; mutable hit_stats_t overall_hits; mutable hit_stats_t last_hits; diff --git a/src/crimson/os/seastore/extent_pinboard.h b/src/crimson/os/seastore/extent_pinboard.h index 6e6d2f3dd4c..05bd98ac04d 100644 --- a/src/crimson/os/seastore/extent_pinboard.h +++ b/src/crimson/os/seastore/extent_pinboard.h @@ -13,7 +13,9 @@ struct ExtentPinboard { virtual void register_metrics() = 0; virtual void move_to_top( CachedExtent &extent, - const Transaction::src_t *p_src) = 0; + const Transaction::src_t *p_src, + extent_len_t load_start, + extent_len_t load_length) = 0; virtual void remove(CachedExtent &extent) = 0; virtual void get_stats( cache_stats_t &stats,