Repository navigation
fix: rebalance LFU queues on resize to prevent post-shrink cache freeze - #19
Open
detail-app[bot] wants to merge 2 commits into
Open
detail-app[bot] wants to merge 2 commits into
detail-app[bot] wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Detail bug report: View on Detail
Bug
Lfu::update(foyer-memory/src/eviction/lfu.rs), invoked on everyCache::resizeviaRawCacheShard::resizeto refresh per-queue weight budgets, recomputed the budgets but never rebalanced records across queues. After a capacity shrink, thepop-driven eviction loop drainedwindowandprobationcompletely before touchingprotected(poponly evicts fromprotectedwhen both are empty), leavingprotectedover its new budget andwindow/probationempty. From that state every new insert was unconditionally evicted (popsaw an emptyprobationand removed fromwindowwith no frequency comparison), so the cache froze and rejected all new entries until oldprotectedentries were removed by TTL/invalidation. Theupdatepath had no test coverage, so this went undetected.Fix
Rebalance the queues inside
Lfu::updateafter recomputing budgets: overflowprotected→probationwhileprotected_weight > protected_weight_capacity, thenwindow→probationwhilewindow_weight > window_weight_capacity. This mirrorsLru::update'smay_overflow_high_priority_pool()and reuses the exact primitives already inpush(window→probation) andacquire(protected→probation), includingpush_backso demoted entries become MRU ofprobation— preserving LRU/MRU ordering and w-TinyLFU admission (recently-hot demoted entries shouldn't be evicted ahead of fresh window entries).Testing
eviction::lfu::tests::test_lfu_update_rebalance_on_shrink— reproduces a 100→50 resize, assertsprotected/windoware within budget afterupdate, total weight is unchanged acrossupdate(move-only), invariants survive the subsequentevict,probationis non-empty (no freeze), and a re-inserted high-frequency key is admitted.eviction::lfu::tests::test_lfu_update_grow_is_noop— grow moves no records (guards against spurious demotion on grow).eviction::lfu::tests::test_lfu_update_to_zero— resize to 0 demotes all entries to probation, then clears (loop termination at zero budget).eviction::lfu::tests::test_lfu_update_with_new_config— new ratios are applied and rebalanced against; an invalid config is rejected with budgets left untouched.cache::tests::test_lfu_cache_resize_admits_after_shrink— end-to-end through the publicCache::resizeAPI cited in the bug; re-inserts a high-frequency key after the shrink and asserts it's admitted.Ok(())body, the shrink/zero/config/public-API tests fail with the expected invariant panics (e.g.protected_weight 60 must be <= protected_weight_capacity 30; public test:high-frequency re-inserted key must be admitted after shrink); all pass with the fix.fmt --check, and the fullfoyer-memory --features "strict_assertions"suite (36 tests, including the pre-existingtest_lfu_cache_resize, all 5 concurrent fuzzy stress tests, andtest_lru_pin_resize_no_panic) pass. The publicCache::resizepath also builds acrossfoyer,foyer-memory, andfoyer-storage.Automatic Fixes PRs can be configured here.