From b45c86fbb7412aa40104cfc61c823cbf1d0f9cd9 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Wed, 19 Aug 2026 10:28:37 +0700 Subject: [PATCH] perf(storage): volatile-ttl evicts the exact nearest-expiry victim via the expiry index (#551) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 6 ++++ src/storage/db/mod.rs | 10 ++++++ src/storage/eviction.rs | 67 +++++++++++++++++++++++------------------ 3 files changed, 53 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8ac4c99..ee926542 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 (entry gone or TTL retargeted) so a backwards wall-clock step can't discard live pairs; the per-key budget clock read is batched to every 64 pops; and sweep 2 lowers the hash latch from its own reap outcomes instead of a second O(N) rescan. +- **`volatile-ttl` eviction picks the exact nearest-expiry victim** (#551). The policy now reads + the head of the #541 deadline-ordered expiry index — O(log n), always the globally soonest + deadline — instead of sampling `maxmemory-samples` random volatile keys and taking the sample's + minimum, which could evict a key hours from expiring while the one expiring in seconds + survived. `maxmemory-samples` still governs the LRU/LFU/random policies, which remain + sampling-based. ### Added - **An acceptance suite driven by unmodified redis-py** (`scripts/client-compat/redis_py/`), wired diff --git a/src/storage/db/mod.rs b/src/storage/db/mod.rs index 34a104fd..141f4a47 100644 --- a/src/storage/db/mod.rs +++ b/src/storage/db/mod.rs @@ -523,6 +523,16 @@ impl Database { .cloned() } + /// Earliest `(expires_at_ms, key)` pair in the index regardless of + /// whether it is due yet — the exact nearest-deadline victim for + /// `volatile-ttl` eviction. O(log n). `None` when no hot key is + /// volatile. Cold-spilled keys are not indexed, matching the old + /// sampling picker which also only saw hot entries. + #[inline] + pub fn peek_nearest_expiry(&self) -> Option<(u64, CompactKey)> { + self.expiry_index.first().cloned() + } + /// Drop one specific index pair. The sweep calls this when a popped /// pair is PROVABLY stale (the entry is gone or carries a different /// TTL than `ts`): without dropping it, the sweep would peek the same diff --git a/src/storage/eviction.rs b/src/storage/eviction.rs index c4d4b586..4551645d 100644 --- a/src/storage/eviction.rs +++ b/src/storage/eviction.rs @@ -763,7 +763,7 @@ fn select_victim( find_victim_lfu(db, config.maxmemory_samples, config.lfu_decay_time, true) } EvictionPolicy::VolatileRandom => find_victim_random(db, true), - EvictionPolicy::VolatileTtl => find_victim_volatile_ttl(db, config.maxmemory_samples), + EvictionPolicy::VolatileTtl => find_victim_volatile_ttl(db), } } @@ -1202,33 +1202,12 @@ fn find_victim_random(db: &Database, volatile_only: bool) -> Option sample_random_keys(db, 1, volatile_only).into_iter().next() } -/// Find the victim key with the soonest TTL expiration from a random sample. -fn find_victim_volatile_ttl(db: &Database, samples: usize) -> Option { - let sampled = sample_random_keys(db, samples, true); - if sampled.is_empty() { - return None; - } - - let mut evict_key: Option = None; - let mut soonest_expiry: Option = None; - - for key in sampled.iter() { - if let Some(entry) = db.data().get(key.as_bytes()) { - if entry.has_expiry() { - let exp = entry.expires_at_ms(); - let should_evict = match soonest_expiry { - None => true, - Some(soonest) => exp < soonest, - }; - if should_evict { - evict_key = Some(key.clone()); - soonest_expiry = Some(exp); - } - } - } - } - - evict_key +/// Find the victim with the globally nearest expiry — exact, via the +/// deadline-ordered expiry index (#541), O(log n). Replaces the old +/// random-sampling approximation (#551), which could miss the soonest +/// key entirely when it was buried among many far-future volatiles. +fn find_victim_volatile_ttl(db: &Database) -> Option { + db.peek_nearest_expiry().map(|(_, key)| key) } #[cfg(test)] @@ -1243,8 +1222,8 @@ mod tests { } } - fn evict_one_volatile_ttl(db: &mut super::Database, samples: usize) -> bool { - if let Some(key) = super::find_victim_volatile_ttl(db, samples) { + fn evict_one_volatile_ttl(db: &mut super::Database, _samples: usize) -> bool { + if let Some(key) = super::find_victim_volatile_ttl(db) { db.remove(key.as_bytes()); true } else { @@ -1257,6 +1236,34 @@ mod tests { use crate::persistence::manifest::ShardManifest; use crate::storage::entry::{Entry, current_secs, current_time_ms}; + /// moon#551: volatile-ttl must evict the GLOBALLY nearest-expiry key, + /// not the nearest within a random sample. With one soon-expiring key + /// buried among 5_000 far-future volatile keys, a 5-key sample picks + /// it with probability ~0.1%; the deadline-index head always does. + #[test] + fn volatile_ttl_evicts_globally_nearest_expiry() { + let mut db = super::Database::new(); + let far_ms = current_time_ms() + 3_600_000; + for i in 0..5_000u32 { + db.set( + Bytes::from(format!("far_{i}")), + Entry::new_string_with_expiry(Bytes::from_static(b"v"), far_ms + u64::from(i)), + ); + } + // Volatile and clearly the soonest, but NOT expired. + db.set( + Bytes::from_static(b"soonest"), + Entry::new_string_with_expiry(Bytes::from_static(b"v"), current_time_ms() + 60_000), + ); + + assert!(evict_one_volatile_ttl(&mut db, 5)); + assert!( + db.data().get(&b"soonest"[..]).is_none(), + "the globally nearest-expiry key must be the victim" + ); + assert_eq!(db.len(), 5_000, "exactly one key evicted"); + } + #[test] fn spill_payload_round_trips_through_rehydrate() { // F1 (deep review): a failed background spill must be able to