refactor(storage): one eviction entry point — evict_to_budget + EvictionRun replace the 13-name try_evict_if_needed* family (W4)#399
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TinDang97
force-pushed
the
refactor/w3-ttl-ms
branch
from
July 20, 2026 05:33
6fc7600 to
edbc9e6
Compare
TinDang97
force-pushed
the
refactor/w4-eviction-api
branch
from
July 20, 2026 05:33
e1df75f to
888e46b
Compare
…ionRun replace the 13-name try_evict_if_needed* family (W4) The eviction API had grown 13 public variants that encoded every optional parameter combination in the function name: try_evict_if_needed try_evict_if_needed_budget[_reporting] try_evict_if_needed_with_spill[_and_total[_budget[_reporting]]] try_evict_if_needed_async_spill[_with_total][_budget][_reporting] try_evict_if_needed_async_spill_with_total_budget[_reporting] Five of them already had zero external callers (chain-internal only), and the two "cores" at the bottom of each forwarding chain duplicated the same reclaim-loop skeleton five times (sync plain, sync batch, async plain-fallback, async durable-batch, async AOF-fast-path). This PR replaces the family with one entry point: evict_to_budget(db, config, EvictionRun) -> Result<(), Frame> where EvictionRun is an options struct built from three constructors (plain / sync_spill / async_spill = the EvictionSink choice) plus chainable knobs .total(n) (aggregate accounting), .budget(n) (elastic per-shard override, capped at instance maxmemory), and .report(f) (plain-drop DEL emission, task #34). The reclaim loop now exists once with a per-iteration sink dispatch; the --appendonly durability- ordering branches (AOF-backstopped fast path vs durable-batch vs plain-drop fallback) are byte-for-byte the same decisions, now documented in one place on EvictionSink::AsyncSpill. Pure refactor — no behavior change: - Red-first: three new entry-point behavior pins (plain sink honors noeviction-OOM + reports every plain-dropped victim; sync-spill sink batches 40 victims into shared .mpf files, all cold-readable; elastic override respected and clamped to maxmemory) written RED against the missing API, then GREEN. - All ~27 call sites across 9 files migrated (connection handlers, SPSC cross-shard gate, Lua bridge, blocking path, memory-pressure cascade, eviction tick, db_quota docs); every historical eviction pin test migrated and green. - Stale doc references to the old names scrubbed repo-wide. Gates: lib suite 4425 green (4422 + 3 new pins) on macOS + Linux VM release-fast, monoio + tokio,jemalloc feature sets; crash matrix + kill-9 offload suites on Linux VM; clippy -D warnings on both feature sets; fmt. Stacked on refactor/w3-ttl-ms (PR #398). author: Tin Dang
TinDang97
force-pushed
the
refactor/w4-eviction-api
branch
from
July 20, 2026 05:36
888e46b to
19a13ba
Compare
TinDang97
added a commit
that referenced
this pull request
Jul 20, 2026
…generics replace Database's per-type accessor matrix (W5)
Database had grown a ~20-method typed-accessor matrix: get_X,
get_or_create_X, and get_X_ref_if_alive for each container type (hash,
list, set, sorted set, stream), every one a copy-paste of the same
skeleton — expiry check -> cold promote/read-through -> compact-
encoding upgrade -> variant projection — with only the enum arms
differing. Skeleton fixes (the P0 cold-collection-visibility fix, the
promote-before-fabricate fix) had to be applied N times and could
silently miss a copy.
The skeleton now exists ONCE per access shape as a generic on
Database:
get_ref_if_alive::<K> &self; hot probe -> expiry -> classify ->
non-promoting cold read-through (Owned view)
get_or_create::<K> &mut; expiry-drop -> cold promote ->
insert-on-miss -> upgrade -> project mut
get_promoted::<K> &mut; expiry-drop -> cold promote ->
upgrade -> project shared
Everything genuinely per-type — which enum variants map to which view,
how a compact encoding upgrades (incl. the deliberate asymmetries:
sorted sets upgrade the legacy BTreeMap form but NOT SortedSetListpack,
whose upgrade lives in the zset command layer), what a fresh entry
looks like — lives in the new storage::db_kind module as
ValueKind/OwnedKind impls on zero-sized markers. Static dispatch only:
every K is a concrete marker resolved at compile time, so the
generated code is identical to the hand-written originals.
The public per-type methods (get_hash, get_or_create_set,
get_list_ref_if_alive, get_stream_if_alive, ...) remain as thin
delegators — command-layer call sites are untouched. The generic
get_or_create also replaces the per-type unwrap-after-insert with the
defensive log-and-error path only the hash accessor previously had.
Deleted: the four caller-less get_X_if_alive variants (hash/list/set/
sorted_set; compact encodings returned None — long superseded by the
_ref_if_alive family). get_or_create_intset and the *_listpack
threshold accessors keep their bespoke shapes (their Option-returning
contracts don't fit the trait and have single call sites).
Pure refactor — no behavior change:
- Behavior pins added GREEN BEFORE the refactor and re-verified after:
hot-WRONGTYPE through all four ref accessors; a hot HashWithTtl entry
maps to HashRef::WithTtl carrying the CALLER's now_ms (expired field
filters, live field reads through). Cold-Owned legs, promote-instead-
of-fabricate, and wrongtype pins already existed and still pass.
- Net -318 lines; db.rs 3431 -> 3113 lines.
Gates: lib suite 4427 green (4425 + 2 new pins) on macOS + Linux VM
release-fast, monoio + tokio,jemalloc feature sets; crash matrix +
kill-9 offload + cold-visibility suites on Linux VM; clippy -D
warnings on both feature sets; fmt.
Stacked on refactor/w4-eviction-api (PR #399).
author: Tin Dang
pilotspacex-byte
added a commit
that referenced
this pull request
Jul 20, 2026
…generics replace Database's per-type accessor matrix (W5) (#400) Database had grown a ~20-method typed-accessor matrix: get_X, get_or_create_X, and get_X_ref_if_alive for each container type (hash, list, set, sorted set, stream), every one a copy-paste of the same skeleton — expiry check -> cold promote/read-through -> compact- encoding upgrade -> variant projection — with only the enum arms differing. Skeleton fixes (the P0 cold-collection-visibility fix, the promote-before-fabricate fix) had to be applied N times and could silently miss a copy. The skeleton now exists ONCE per access shape as a generic on Database: get_ref_if_alive::<K> &self; hot probe -> expiry -> classify -> non-promoting cold read-through (Owned view) get_or_create::<K> &mut; expiry-drop -> cold promote -> insert-on-miss -> upgrade -> project mut get_promoted::<K> &mut; expiry-drop -> cold promote -> upgrade -> project shared Everything genuinely per-type — which enum variants map to which view, how a compact encoding upgrades (incl. the deliberate asymmetries: sorted sets upgrade the legacy BTreeMap form but NOT SortedSetListpack, whose upgrade lives in the zset command layer), what a fresh entry looks like — lives in the new storage::db_kind module as ValueKind/OwnedKind impls on zero-sized markers. Static dispatch only: every K is a concrete marker resolved at compile time, so the generated code is identical to the hand-written originals. The public per-type methods (get_hash, get_or_create_set, get_list_ref_if_alive, get_stream_if_alive, ...) remain as thin delegators — command-layer call sites are untouched. The generic get_or_create also replaces the per-type unwrap-after-insert with the defensive log-and-error path only the hash accessor previously had. Deleted: the four caller-less get_X_if_alive variants (hash/list/set/ sorted_set; compact encodings returned None — long superseded by the _ref_if_alive family). get_or_create_intset and the *_listpack threshold accessors keep their bespoke shapes (their Option-returning contracts don't fit the trait and have single call sites). Pure refactor — no behavior change: - Behavior pins added GREEN BEFORE the refactor and re-verified after: hot-WRONGTYPE through all four ref accessors; a hot HashWithTtl entry maps to HashRef::WithTtl carrying the CALLER's now_ms (expired field filters, live field reads through). Cold-Owned legs, promote-instead- of-fabricate, and wrongtype pins already existed and still pass. - Net -318 lines; db.rs 3431 -> 3113 lines. Gates: lib suite 4427 green (4425 + 2 new pins) on macOS + Linux VM release-fast, monoio + tokio,jemalloc feature sets; crash matrix + kill-9 offload + cold-visibility suites on Linux VM; clippy -D warnings on both feature sets; fmt. Stacked on refactor/w4-eviction-api (PR #399). author: Tin Dang Co-authored-by: Tin Dang <tindang.ht97@gmail.com>
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
2 participants
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.
Summary
Fourth PR of the storage-unification campaign (stacked on #398 ← #397 ← #396). Fixes finding F4 (eviction API explosion).
Before
13 public
try_evict_if_needed*variants encoded every optional-parameter combination as a function-name suffix (_budget,_with_spill,_and_total,_async_spill,_reporting, …). Five had zero external callers (forwarding-chain-internal only), and the two chain-bottom "cores" duplicated the same reclaim-loop skeleton five times (sync plain, sync batch, async plain-fallback, async durable-batch, async AOF-fast-path).After
One entry point:
EvictionSink(Plain/SyncSpill/AsyncSpill) makes the victim destination data instead of a name suffix.EvictionRunconstructors (plain()/sync_spill()/async_spill()) + chainable knobs.total(n)(aggregate accounting),.budget(n)(elastic per-shard override, clamped to instance maxmemory),.report(f)(plain-drop dual-plane DEL emission, task GPU: CI/CD infrastructure for CUDA builds and testing #34).--appendonlydurability-ordering branches (AOF-backstopped fast path / durable-batch / plain-drop fallback) are decision-for-decision identical, now documented in one place onEvictionSink::AsyncSpill.Pure refactor — no behavior change. Net −59 lines despite three new tests and consolidated docs (453 lines of wrappers deleted).
Migration
All ~27 call sites across 9 files (monoio/sharded/single connection handlers, SPSC cross-shard gate, Lua bridge, blocking path, memory-pressure cascade, eviction tick) plus every eviction pin test; stale doc references scrubbed repo-wide.
Gates
crash_matrix_cross_plane46/46 ·spill_batch_kill92/2 ·disk_offload_no_aof1/1 ·oom_bypass_closure8/8 on Linux VM with pinned fresh-ELFMOON_BIN-D warnings×2 feature sets · fmt · CHANGELOG entry