Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: disable the foyer disk cache #5154

Merged
merged 1 commit into from
Dec 18, 2024
Merged

chore: disable the foyer disk cache #5154

merged 1 commit into from
Dec 18, 2024

Conversation

sprutton1
Copy link
Contributor

This disables the disk cache portion of Foyer as we investigate issues with memory growing unbounded, leading to OOMs in production environments. Local testing indicates that the disk portion of the cache and the rate at which we flush to it may be causing us to greatly over allocate memory. More investigation needs to happen in this area, so this is mainly to stabilize prod.

@sprutton1
Copy link
Contributor Author

/try

Copy link

github-actions bot commented Dec 18, 2024

Okay, starting a try! I'll update this comment once it's running...\n
🚀 Try running here! 🚀

Copy link
Contributor

@fnichol fnichol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I like it, pretty straight forward and a good experiment


Ok(Self { cache })
}

pub async fn get(&self, key: &str) -> Option<V> {
match self.cache.obtain(key.into()).await {
Ok(Some(entry)) => match entry.value() {
match self.cache.get(key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If memory serves this doesn't really change the sematics, but rather a change in API because we're dealing with a MemCache rather than a HybridCache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, obtain de-dups calls to the disk, so no disk, no obtain.

@sprutton1 sprutton1 added this pull request to the merge queue Dec 18, 2024
Merged via the queue into main with commit 687b89c Dec 18, 2024
26 checks passed
@sprutton1 sprutton1 deleted the disable_disk_cache branch December 18, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants