feat: Read and write metadata files (#1618)

This adds functionality to write and read the metadata structures introduced in #1617 to and from the filesystem. A cache file's metadata file has the same path, but with the extension `.metadata`.

The read happens in check_expiry, which gives the cleanup job access to the metadata. All of the old logic is still there—if no metadata file exists, we use the mtime from the FS as before.

The write happens in compute. It should be noted that currently metadata are not propagated through the shared cache; this is left as future work.

Author: loewenheim

crates/symbolicator-native/tests/integration/e2e.rs

@@ -1,7 +1,9 @@
+use std::fs::File;
 use std::path::Path;
 use std::sync::Arc;
 
 use symbolicator_native::interface::{FrameStatus, SymbolicateStacktraces};
+use symbolicator_service::caching::Metadata;
 use symbolicator_service::objects::ObjectDownloadInfo;
 use symbolicator_service::types::{ObjectFileStatus, Scope};
 use symbolicator_sources::{
@@ -480,11 +482,12 @@ async fn test_basic_windows() {
                     // NOTE: the cache key depends on the exact location of the file, which is
                     // random because it includes the [`Server`]s random port.
                     cached_objects.sort_by_key(|(_, size)| *size);
-                    assert_eq!(cached_objects.len(), 4); // 2 filename patterns, 2 metadata files
+                    assert_eq!(cached_objects.len(), 6); // 2 filename patterns, 2 metadata files, 2 debug txt files
                     assert_eq!(cached_objects[0].1, 0);
-                    assert_eq!(cached_objects[3].1, 846_848);
+                    assert_eq!(cached_objects[5].1, 846_848);
 
-                    let metadata_file = &cached_objects[1].0;
+                    // Checks the .txt file that we only write in debug mode
+                    let metadata_text_file = &cached_objects[3].0;
                     let cached_scope = if is_public { "global" } else { "myscope" };
                     let mut expected_metadata = format!(
                         "scope: {cached_scope}\n\nsource: microsoft\nlocation: {}",
@@ -493,26 +496,40 @@ async fn test_basic_windows() {
                         )
                     );
                     let metadata =
-                        std::fs::read_to_string(objects_dir.join(metadata_file)).unwrap();
+                        std::fs::read_to_string(objects_dir.join(metadata_text_file)).unwrap();
                     // NOTE: due to random sort order, we have either `.pdb` or `.pd_`,
                     // thus we only check for the substring
                     assert!(
                         metadata.starts_with(&expected_metadata),
                         "{metadata:?} == {expected_metadata:?}"
                     );
 
+                    // Checks the metadata file
+                    let metadata_file = &cached_objects[1].0;
+                    let cached_scope = if is_public { "global" } else { "myscope" };
+                    let file = File::open(objects_dir.join(metadata_file)).unwrap();
+                    let metadata: Metadata = serde_json::from_reader(&file).unwrap();
+                    assert_eq!(metadata.scope.as_ref(), cached_scope);
+
                     let symcaches_dir = cache_dir.path().join("symcaches");
                     let mut cached_symcaches = get_cache_files(&symcaches_dir);
 
                     cached_symcaches.sort_by_key(|(_, size)| *size);
-                    assert_eq!(cached_symcaches.len(), 2); // 1 symcache, 1 metadata file
-                    assert_eq!(cached_symcaches[1].1, 142_365);
+                    assert_eq!(cached_symcaches.len(), 3); // 1 symcache, 1 metadata file, 1 debug text file
+                    assert_eq!(cached_symcaches[2].1, 142_365);
 
-                    let metadata_file = &cached_symcaches[0].0;
+                    // Checks the .txt file that we only write in debug mode
+                    let metadata_text_file = &cached_symcaches[1].0;
                     let metadata =
-                        std::fs::read_to_string(symcaches_dir.join(metadata_file)).unwrap();
+                        std::fs::read_to_string(symcaches_dir.join(metadata_text_file)).unwrap();
                     expected_metadata.push_str("b\n"); // this truely ends in `.pdb` now
                     assert_eq!(metadata, expected_metadata);
+
+                    // Checks the metadata file
+                    let metadata_file = &cached_symcaches[0].0;
+                    let file = File::open(symcaches_dir.join(metadata_file)).unwrap();
+                    let metadata: Metadata = serde_json::from_reader(&file).unwrap();
+                    assert_eq!(metadata.scope.as_ref(), cached_scope);
                 }
 
                 // our use of in-memory caching should make sure we only ever request each file once

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

@@ -6,12 +6,26 @@ use symbolicator_sources::RemoteFile;
 
 use crate::types::Scope;
 
+/// The key of an item in an in-memory or on-disk
+/// cache.
+///
+/// Each key belongs to a [Scope], determined by
+/// the scope of the symbolication request and the symbol
+/// source in question.
 #[derive(Debug, Clone, Eq)]
 pub struct CacheKey {
+    scope: Scope,
     metadata: Arc<str>,
     hash: [u8; 32],
 }
 
+impl CacheKey {
+    /// Returns the scope of this cache key.
+    pub fn scope(&self) -> &Scope {
+        &self.scope
+    }
+}
+
 impl fmt::Display for CacheKey {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self.cache_path(1234))
@@ -63,24 +77,28 @@ impl CacheKey {
     /// contributing sources.
     pub fn scoped_builder(scope: &Scope) -> CacheKeyBuilder {
         let metadata = format!("scope: {scope}\n\n");
-        CacheKeyBuilder { metadata }
+        CacheKeyBuilder {
+            scope: scope.clone(),
+            metadata,
+        }
     }
 
     #[cfg(test)]
-    pub fn for_testing(key: impl Into<String>) -> Self {
+    pub fn for_testing(scope: Scope, key: impl Into<String>) -> Self {
         let metadata = key.into();
 
-        CacheKeyBuilder { metadata }.build()
+        CacheKeyBuilder { scope, metadata }.build()
     }
 }
 
 /// A builder for [`CacheKey`]s.
 ///
 /// This builder implements the [`Write`] trait, and the intention of it is to
 /// accept human readable, but most importantly **stable**, input.
-/// This input in then being hashed to form the [`CacheKey`], and can also be serialized alongside
+/// This input is then hashed to form the [`CacheKey`], and can also be serialized alongside
 /// the cache files to help debugging.
 pub struct CacheKeyBuilder {
+    scope: Scope,
     metadata: String,
 }
 
@@ -99,6 +117,7 @@ impl CacheKeyBuilder {
         let hash = Sha256::digest(&self.metadata);
 
         CacheKey {
+            scope: self.scope,
             metadata: self.metadata.into(),
             hash: hash.into(),
         }

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

@@ -1,3 +1,4 @@
+use std::ffi::OsStr;
 use std::fs::{read_dir, remove_dir, remove_file};
 use std::io;
 use std::path::Path;
@@ -6,6 +7,7 @@ use anyhow::{anyhow, Result};
 use rand::seq::SliceRandom;
 use rand::thread_rng;
 
+use crate::caching::fs::{metadata_path, METADATA_EXTENSION};
 use crate::config::Config;
 use crate::metric;
 
@@ -166,6 +168,10 @@ impl Cache {
         let mut is_empty = true;
         for entry in entries {
             let path = entry?.path();
+            // Skip metadata files—they will be handled together with their cache files.
+            if path.extension().and_then(OsStr::to_str) == Some(METADATA_EXTENSION) {
+                continue;
+            }
             if path.is_dir() {
                 let mut dir_is_empty = self.cleanup_directory_recursive(&path, stats, dry_run)?;
                 if dir_is_empty {
@@ -204,6 +210,7 @@ impl Cache {
 
     /// Tries to clean up the file at `path`, returning `true` if it was removed.
     ///
+    /// This also removes the file's corresponding metadata file, if it exists.
     /// If `dry_run` is `true`, the file will not actually be deleted.
     fn try_cleanup_path(
         &self,
@@ -222,16 +229,18 @@ impl Cache {
             tracing::debug!("Removing file `{}`", path.display());
             if !dry_run {
                 catch_not_found(|| remove_file(path))?;
+                catch_not_found(|| remove_file(metadata_path(path)))?;
             }
 
             stats.removed_bytes += size;
             stats.removed_files += 1;
 
-            return Ok(true);
-        }
-        stats.retained_bytes += size;
-        stats.retained_files += 1;
+            Ok(true)
+        } else {
+            stats.retained_bytes += size;
+            stats.retained_files += 1;
 
-        Ok(false)
+            Ok(false)
+        }
     }
 }