fix(caching): Eagerly cleanup fallback versions

crates/symbolicator-service/src/caching/memory.rs

@@ -1,4 +1,5 @@
 use std::collections::HashSet;
+use std::fs;
 use std::path::Path;
 use std::sync::atomic::Ordering;
 use std::sync::{Arc, Mutex};
@@ -430,7 +431,28 @@ impl<T: CacheItemRequest> Cacher<T> {
             let value = (expiration.as_instant(), item);
 
             // refresh the memory cache with the newly refreshed result
-            this.cache.insert(cache_key, value).await;
+            this.cache.insert(cache_key.clone(), value).await;
+
+            // Clean up old versions
+            let cache_dir = this
+                .config
+                .cache_dir()
+                .expect("cache dir must exist if we're doing recomputations");
+            for &version in T::VERSIONS.fallbacks {
+                let item_path = cache_dir.join(cache_key.cache_path(version));
+
+                if let Err(e) = fs::remove_file(&item_path) {
+                    // `NotFound` errors are no cause for concern—it's likely that not all fallback versions exist anymore.
+                    if e.kind() != std::io::ErrorKind::NotFound {
+                        let dynerror = &e as &dyn std::error::Error;
+                        tracing::error!(
+                            error = dynerror,
+                            path = cache_key.cache_path(version),
+                            "Failed to remove old cache file"
+                        );
+                    }
+                }
+            }
 
             transaction.finish();
         };

crates/symbolicator-service/src/caching/tests.rs

@@ -784,12 +784,9 @@ async fn test_cache_fallback() {
     let request = TestCacheItem::new();
     let key = CacheKey::for_testing("global/some_cache_key");
 
-    {
-        let cache_dir = cache_dir.path().join("objects");
-        let cache_file = cache_dir.join(key.cache_path(0));
-        fs::create_dir_all(cache_file.parent().unwrap()).unwrap();
-        fs::write(cache_file, "some old cached contents").unwrap();
-    }
+    let old_cache_file = cache_dir.path().join("objects").join(key.cache_path(0));
+    fs::create_dir_all(old_cache_file.parent().unwrap()).unwrap();
+    fs::write(&old_cache_file, "some old cached contents").unwrap();
 
     let config = Config {
         cache_dir: Some(cache_dir.path().to_path_buf()),
@@ -818,6 +815,9 @@ async fn test_cache_fallback() {
 
     // we only want to have the actual computation be done a single time
     assert_eq!(request.computations.load(Ordering::SeqCst), 1);
+
+    // the old cache file should have been removed during the recomputation
+    assert!(!fs::exists(old_cache_file).unwrap());
 }
 
 /// Makes sure that a `NotFound` result does not fall back to older cache versions.