Skip to content

Commit

Permalink
Avoid using the shared cache for internally stored files.
Browse files Browse the repository at this point in the history
  • Loading branch information
Swatinem committed Oct 6, 2023
1 parent 4cb8161 commit 4f57938
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 0 deletions.
7 changes: 7 additions & 0 deletions crates/symbolicator-js/src/bundle_index_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,11 @@ impl CacheItemRequest for BundleIndexRequest {
fn weight(item: &Self::Item) -> u32 {
item.0.max(std::mem::size_of::<Self::Item>() as u32)
}

fn use_shared_cache(&self) -> bool {
// These change frequently, and are being downloaded from Sentry directly.
// Here again, the question is whether we want to waste a bit of storage on GCS vs doing
// more requests to Python.
true
}
}
4 changes: 4 additions & 0 deletions crates/symbolicator-service/src/services/bitcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ impl CacheItemRequest for FetchFileRequest {
data,
}))
}

fn use_shared_cache(&self) -> bool {
self.file_source.worth_using_shared_cache()
}
}

#[derive(Debug, Clone)]
Expand Down
4 changes: 4 additions & 0 deletions crates/symbolicator-service/src/services/il2cpp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ impl CacheItemRequest for FetchFileRequest {
data,
})
}

fn use_shared_cache(&self) -> bool {
self.file_source.worth_using_shared_cache()
}
}

#[derive(Debug, Clone)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ impl CacheItemRequest for FetchFileDataRequest {

Ok(Arc::new(object_handle))
}

fn use_shared_cache(&self) -> bool {
self.0.file_source.worth_using_shared_cache()
}
}

#[cfg(test)]
Expand Down
5 changes: 5 additions & 0 deletions crates/symbolicator-service/src/services/objects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ pub struct FindResult {

#[derive(Clone, Debug)]
pub struct ObjectsActor {
// FIXME(swatinem): Having a fully fledged filesystem and shared cache for these tiny file meta
// items is heavy handed and wasteful. However, we *do* want to have this in shared cache, as
// it is the primary thing that makes cold starts fast, as we do not need to fetch the whole
// objects, but just the derived caches. Some lighter weight solution, like Redis might be more
// appropriate at some point.
meta_cache: Arc<Cacher<FetchFileMetaRequest>>,
data_cache: Arc<Cacher<FetchFileDataRequest>>,
download_svc: Arc<DownloadService>,
Expand Down
17 changes: 17 additions & 0 deletions crates/symbolicator-sources/src/remotefile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,23 @@ impl RemoteFile {
RemoteFile::Sentry(source) => source.host(),
}
}

/// Determines if this [`RemoteFile`] is worth caching in shared cache or not.
///
/// This is `true` for all external symbol servers, but not for our internal source, as we would
/// just save the same bytes twice, with an extra roundtrip on fetches.
pub fn worth_using_shared_cache(&self) -> bool {
match self {
// This is our internal sentry source. It is debatable if we want to rather fetch
// the file through the Python->FileStore->GCS indirection,
// or rather pay to save it twice so we can access GCS (shared cache) directly.
RemoteFile::Sentry(_) => true,
// This is most likely our scraped apple symbols, which are hosted on GCS directly, no
// need to save them twice in shared cache.
RemoteFile::Gcs(source) if source.source.id.as_str().starts_with("sentry:") => false,
_ => true,
}
}
}

/// A URI representing an [`RemoteFile`].
Expand Down

0 comments on commit 4f57938

Please sign in to comment.