From 82ebaa182d269690af0b21596c61e625708f8317 Mon Sep 17 00:00:00 2001 From: Remi Dettai Date: Wed, 2 Sep 2026 09:36:33 +0200 Subject: [PATCH 1/4] Fix cache eviction metric --- .../quickwit-storage/src/cache/base_cache.rs | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/quickwit/quickwit-storage/src/cache/base_cache.rs b/quickwit/quickwit-storage/src/cache/base_cache.rs index caa6447d9ac..bc82088f356 100644 --- a/quickwit/quickwit-storage/src/cache/base_cache.rs +++ b/quickwit/quickwit-storage/src/cache/base_cache.rs @@ -205,8 +205,8 @@ impl Lru { } return; } - if let Some(previous_data) = self.lru_cache.pop(&key) { - self.drop_item(previous_data.len() as u64); + if self.lru_cache.contains(&key) { + return; } let now = Instant::now(); @@ -334,6 +334,9 @@ impl S3Fifo { } return; } + if self.cache.contains_key(&key) { + return; + } self.cache_metrics.in_cache_count.inc(); self.cache_metrics @@ -459,6 +462,9 @@ impl = + AnyCache::from_policy_and_capacity(policy, ByteSize::kb(10), cache_metrics.clone()); + cache.put("key".to_string(), OwnedBytes::new(&b"hello"[..])); + // cached values are assumed immutable: re-putting the same key must be a no-op, + // not an eviction of the existing entry. + cache.put("key".to_string(), OwnedBytes::new(&b"hello"[..])); + assert_eq!(cache_metrics.evict_num_items.get(), 0, "policy {policy}"); + assert_eq!(cache_metrics.in_cache_count.get(), 1, "policy {policy}"); + } + } } From 97d5a904eb52db42db0cd0c4f021ddc8e0e09b2e Mon Sep 17 00:00:00 2001 From: Remi Dettai Date: Wed, 2 Sep 2026 09:36:33 +0200 Subject: [PATCH 2/4] Still increase recency and frequency on concurrent metric put --- quickwit/quickwit-storage/src/cache/base_cache.rs | 7 ++++--- quickwit/quickwit-storage/src/cache/stored_item.rs | 6 +++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/quickwit/quickwit-storage/src/cache/base_cache.rs b/quickwit/quickwit-storage/src/cache/base_cache.rs index bc82088f356..12f4e7c09e0 100644 --- a/quickwit/quickwit-storage/src/cache/base_cache.rs +++ b/quickwit/quickwit-storage/src/cache/base_cache.rs @@ -205,7 +205,8 @@ impl Lru { } return; } - if self.lru_cache.contains(&key) { + if let Some(item) = self.lru_cache.get_mut(&key) { + item.touch(); return; } @@ -334,7 +335,7 @@ impl S3Fifo { } return; } - if self.cache.contains_key(&key) { + if self.cache.get(&key).is_some() { return; } @@ -462,7 +463,7 @@ impl StoredItem { impl StoredItem { pub fn payload(&mut self) -> V { - self.last_access_time = Instant::now(); + self.touch(); self.payload.clone() } + pub fn touch(&mut self) { + self.last_access_time = Instant::now(); + } + pub fn len(&self) -> usize { self.payload.len() } From 9b28a64f13ffd3e953e3ee6fb7008ee0d9ffdeef Mon Sep 17 00:00:00 2001 From: Remi Dettai Date: Wed, 2 Sep 2026 09:36:33 +0200 Subject: [PATCH 3/4] Fix file description eviction metric --- quickwit/quickwit-storage/src/file_descriptor_cache.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/quickwit/quickwit-storage/src/file_descriptor_cache.rs b/quickwit/quickwit-storage/src/file_descriptor_cache.rs index 4b117153ffb..f4f2473931e 100644 --- a/quickwit/quickwit-storage/src/file_descriptor_cache.rs +++ b/quickwit/quickwit-storage/src/file_descriptor_cache.rs @@ -101,10 +101,13 @@ impl FileDescriptorCache { fn put_split_file(&self, split_id: Ulid, split_file: SplitFile) { let mut fd_cache_lock = self.fd_cache.lock().unwrap(); - fd_cache_lock.push(split_id, split_file); + let evicted = fd_cache_lock.push(split_id, split_file); self.fd_cache_metrics .in_cache_count .set(fd_cache_lock.len() as i64); + if evicted.is_some() { + self.fd_cache_metrics.evict_num_items.inc(); + } } /// Evicts the given list of split ids from the file descriptor cache. @@ -224,6 +227,7 @@ mod tests { assert_eq!(cache_metrics.in_cache_count.get(), 10); assert_eq!(cache_metrics.hits_num_items.get(), 20); assert_eq!(cache_metrics.misses_num_items.get(), 10); + assert_eq!(cache_metrics.evict_num_items.get(), 0); } // This mimics Quickwit's workload where the fd cache is much smaller than the number of @@ -257,6 +261,9 @@ mod tests { assert_eq!(cache_metrics.in_cache_count.get(), 10); assert_eq!(cache_metrics.hits_num_items.get(), 100 * 9); assert_eq!(cache_metrics.misses_num_items.get(), 100); + // 100 distinct splits went through a 10-entry cache: the 90 oldest ones were pushed + // out by capacity, one at a time, as later splits were first opened. + assert_eq!(cache_metrics.evict_num_items.get(), 90); } #[tokio::test] From dcfc7750b4d9528fa99e6568e1b50182fd18a04b Mon Sep 17 00:00:00 2001 From: Remi Dettai Date: Wed, 2 Sep 2026 09:36:34 +0200 Subject: [PATCH 4/4] Fix fd eviction metric in case of duplicate eviction --- .../quickwit-storage/src/file_descriptor_cache.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/quickwit/quickwit-storage/src/file_descriptor_cache.rs b/quickwit/quickwit-storage/src/file_descriptor_cache.rs index f4f2473931e..740334e0011 100644 --- a/quickwit/quickwit-storage/src/file_descriptor_cache.rs +++ b/quickwit/quickwit-storage/src/file_descriptor_cache.rs @@ -105,7 +105,9 @@ impl FileDescriptorCache { self.fd_cache_metrics .in_cache_count .set(fd_cache_lock.len() as i64); - if evicted.is_some() { + if let Some((evicted_split_id, _)) = evicted + && split_id != evicted_split_id + { self.fd_cache_metrics.evict_num_items.inc(); } } @@ -114,15 +116,17 @@ impl FileDescriptorCache { /// This method does NOT remove the actual files. pub fn evict_split_files(&self, split_ids: &[Ulid]) { let mut fd_cache_lock = self.fd_cache.lock().unwrap(); + let mut evicted_count = 0; for split_id in split_ids { - fd_cache_lock.pop(split_id); + let evicted = fd_cache_lock.pop(split_id); + if evicted.is_some() { + evicted_count += 1; + } } self.fd_cache_metrics .in_cache_count .set(fd_cache_lock.len() as i64); - self.fd_cache_metrics - .evict_num_items - .inc_by(split_ids.len() as u64); + self.fd_cache_metrics.evict_num_items.inc_by(evicted_count); } pub async fn get_or_open_split_file(