Skip to content

perf(storage): volatile-ttl evicts the exact nearest-expiry victim via the expiry index (#551) - #555

Merged
TinDang97 merged 1 commit into
mainfrom
feat/551-volatile-ttl-exact
Aug 19, 2026
Merged

perf(storage): volatile-ttl evicts the exact nearest-expiry victim via the expiry index (#551)#555
TinDang97 merged 1 commit into
mainfrom
feat/551-volatile-ttl-exact

Conversation

@TinDang97

@TinDang97 TinDang97 commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

volatile-ttl eviction picked its victim by sampling maxmemory-samples random volatile keys and evicting the sample's minimum expiry — an approximation that can miss the globally soonest key entirely. With 5 samples over 5,001 volatile keys the true nearest-deadline key is chosen ~0.1% of the time: the policy evicts a key hours from expiring while one expiring in seconds survives, expires naturally moments later, and the eviction is wasted.

The #541 deadline-ordered expiry index already holds every hot volatile key sorted by (expires_at_ms, key), so the exact answer is the index head:

  • Database::peek_nearest_expiry() — first index pair regardless of due-ness, O(log n); None when no hot key is volatile. Cold-spilled keys are not indexed, matching the old sampler which also only saw hot entries.
  • find_victim_volatile_ttl(db) drops the samples parameter and returns the index head's key. maxmemory-samples still governs LRU/LFU/random, which remain sampling-based.

Red/green

New test volatile_ttl_evicts_globally_nearest_expiry buries one now+60s key among 5,000 keys expiring in an hour — red under the sampling picker (probabilistically guaranteed miss), green on the index head.

Validation

Closes #551

Summary by CodeRabbit

  • Bug Fixes

    • Improved volatile-ttl eviction to consistently remove the volatile key with the nearest expiration time.
    • Prevented distant-expiration keys from being selected due to random sampling.
    • Preserved maxmemory-samples behavior for LRU, LFU, and random eviction policies.
    • Added coverage confirming accurate selection across large sets of volatile keys.
  • Documentation

    • Updated the changelog to describe the improved eviction behavior.

…a the expiry index (#551)

The volatile-ttl eviction policy sampled maxmemory-samples random volatile
keys and evicted the sample's minimum expiry — an approximation that could
miss the globally soonest key entirely (with 5 samples over 5,001 volatile
keys, the true nearest-deadline key is picked ~0.1% of the time), evicting
a key hours from expiring while one expiring in seconds survived to expire
naturally moments later, wasting the eviction.

The #541 deadline-ordered expiry index already holds every hot volatile
key sorted by (expires_at_ms, key), so the exact answer is the index head:

- Database::peek_nearest_expiry() — first index pair regardless of
  due-ness, O(log n); None when no hot key is volatile. Cold-spilled keys
  are not indexed, matching the old sampler which also only saw hot
  entries.
- find_victim_volatile_ttl(db) drops the samples parameter and returns
  the index head's key. maxmemory-samples still governs the LRU/LFU/
  random sampling policies, which are unchanged.

Red/green: new test volatile_ttl_evicts_globally_nearest_expiry buries
one now+60s key among 5,000 keys expiring in an hour — red under the
sampling picker (probabilistically guaranteed miss), green on the index
head. Eviction module 42/42, full host lib suite 4688/4688, clippy -D
warnings clean.

Closes #551
author: Tin Dang
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a0501e07-abd9-477f-9db9-2e843f5ae26f

📥 Commits

Reviewing files that changed from the base of the PR and between 7b9940a and b45c86f.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/storage/db/mod.rs
  • src/storage/eviction.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

volatile-ttl eviction now reads the expiry-index head to select the globally nearest-expiring volatile key. Database exposes this lookup, sampling is removed from this policy, and a regression test covers 5,000 far-future keys.

Changes

Volatile-TTL eviction

Layer / File(s) Summary
Nearest expiry lookup
src/storage/db/mod.rs
Database::peek_nearest_expiry returns the earliest expiry-index entry, including deadlines that have not elapsed.
Exact eviction integration and validation
src/storage/eviction.rs, CHANGELOG.md
volatile-ttl uses the exact expiry lookup instead of sampling. The regression test verifies selection among 5,000 far-future keys. The changelog records the new behavior and sampling rules for other policies.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to b45c8

This PR changes volatile-TTL eviction to select the nearest-expiry key directly and reports passing targeted and full-suite validation. No actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • pilotspace/moon#549: Introduced the expiry index used by exact volatile-ttl victim selection.

Suggested reviewers: pilotspacex-byte

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the exact nearest-expiry selection change for volatile-ttl eviction.
Description check ✅ Passed The description clearly explains the change, design, testing, validation, and policy scope, although it does not mirror every template heading.
Linked Issues check ✅ Passed The changes satisfy issue #551 by using the expiry index for volatile-ttl while preserving sampling for volatile-random.
Out of Scope Changes check ✅ Passed The changelog, database accessor, eviction update, and regression test directly support the linked issue objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/551-volatile-ttl-exact

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@TinDang97
TinDang97 merged commit 54e3f79 into main Aug 19, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eviction: volatile-ttl picks its victim by sampling — the expiry index head is the exact answer

1 participant