feat: Add retry_missing_after_public config setting (#1623)

This adds a setting retry_missing_after_public to downloaded and derived cache configs. The effect of this setting is to control the time after which negative (missing, failed download, &c.) cache entries from public sources. Whether a file comes from a public source is determined by reading its metadata, which we added in #1617 and started writing in #1618.

The default value for this setting is 24h. This means that each Symbolicator only makes one request per day to public symbol sources for each missing debug file, as opposed to one per hour like before.

Author: loewenheim

CHANGELOG.md

@@ -4,7 +4,10 @@
 
 ### Various fixes & improvements
 
-fix: rectify symsorter `--ignore-errors` handling for ZIP archives. ([#1621](https://github.com/getsentry/symbolicator/pull/1621)
+fix: rectify symsorter `--ignore-errors` handling for ZIP archives. ([#1621](https://github.com/getsentry/symbolicator/pull/1621))
+feat: Added a setting `retry_missing_after_public` to downloaded and derived cache configs.
+      The effect of this setting is to control the time after which negative (missing, failed download, &c.)
+      cache entries from public sources. The default value is 24h. ([#1623](https://github.com/getsentry/symbolicator/pull/1623))
 
 ### Dependencies
 

crates/symbolicator-native/src/caches/symcaches.rs

@@ -382,7 +382,7 @@ mod tests {
             object_type: ObjectType::Macho,
             identifier,
             sources: Arc::new([source]),
-            scope: Scope::Global,
+            scope: Scope::Scoped("12345".into()),
         };
 
         let symcache_actor = symcache_actor(cache_dir.path().to_owned(), TIMEOUT).await;

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

@@ -10,6 +10,7 @@ use symbolic::common::ByteView;
 use tempfile::NamedTempFile;
 
 use crate::config::{CacheConfig, Config};
+use crate::types::Scope;
 
 use super::cache_error::cache_contents_from_bytes;
 use super::{CacheContents, CacheEntry, CacheError, CacheName, Metadata};
@@ -133,14 +134,18 @@ impl Cache {
         let bv = ByteView::open(path)?;
         let contents = cache_contents_from_bytes(bv);
 
+        // A cache entry is considered "public" if it came from a request with global scope.
+        let is_public = external_metadata
+            .as_ref()
+            .is_some_and(|md| md.scope == Scope::Global);
         // States a cache item can be in:
         // * negative/empty: An empty file. Represents a failed download. ctime is used to indicate
         //   when the failed download happened (when the file was created)
         // * malformed: A file with the content `b"malformed"`. Represents a failed symcache
         //   conversion. ctime indicates when we attempted to convert.
         // * ok (don't really have a name): File has any other content, mtime is used to keep track
         //   of last use.
-        let expiration_time = self.compute_expiration_time(&contents, atime, ctime)?;
+        let expiration_time = self.compute_expiration_time(&contents, atime, ctime, is_public)?;
 
         Ok((
             CacheEntry {
@@ -157,12 +162,17 @@ impl Cache {
     /// The returned `ExpirationTime` determines when the cache contents should
     /// be touched to keep them alive or recomputed entirely.
     ///
+    /// Negative cache entries for "public" files have a different (typically longer)
+    /// lifetime than "private" (project-specific) ones. This is because public symbol
+    /// sources are expected to change much more slowly.
+    ///
     /// If the file is expired, this returns [`io::ErrorKind::NotFound`].
     fn compute_expiration_time<T>(
         &self,
         contents: &CacheContents<T>,
         atime: SystemTime,
         ctime: SystemTime,
+        is_public: bool,
     ) -> io::Result<ExpirationTime> {
         let atime_elapsed = atime.elapsed().unwrap_or_default();
         let ctime_elapsed = ctime.elapsed().unwrap_or_default();
@@ -180,10 +190,15 @@ impl Cache {
                 Ok(ExpirationTime::TouchIn(touch_in))
             }
             ExpirationStrategy::Negative => {
-                let retry_misses_after = self
-                    .cache_config
-                    .retry_misses_after()
-                    .unwrap_or(Duration::MAX);
+                let retry_misses_after = if is_public {
+                    self.cache_config
+                        .retry_misses_after_public()
+                        .unwrap_or(Duration::MAX)
+                } else {
+                    self.cache_config
+                        .retry_misses_after()
+                        .unwrap_or(Duration::MAX)
+                };
 
                 let expires_in = retry_misses_after.saturating_sub(ctime_elapsed);