Skip to content

Add max age to instance cache #3105

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add max age to instance cache #3105

wants to merge 1 commit into from

Conversation

SleeplessOne1917
Copy link
Member

Fixes #2792

It turns out that the problem was just that a max age wasn't specified for the instance-cache. This PR makes it so everything in that cache expires in 10 minutes.

@Nothing4You
Copy link
Collaborator

what is the point of this cache?

considering it's network first, i assume this is intended to be usable as a fallback when offline or the server is unreachable. having this limited to 10 minutes doesn't sound like it'd be of much use.

https://developer.chrome.com/docs/workbox/caching-strategies-overview/#network_first_falling_back_to_cache

@SleeplessOne1917
Copy link
Member Author

@Nothing4You I could make the max age larger (maybe a few hours, or a day like all the other caches) and use maxEntries instead.

We definitely need some way of invalidating the cache though. When I looked at a session for lemmy.ml I had logged in in one browser for a long time, I saw requests from as early as October of last year still cached. As one of the commenters on the linked issue show, this can add up to a lot if you use a particular logged in session a lot (in that person's case, over 8GB).

@Nothing4You
Copy link
Collaborator

Nothing4You commented Apr 23, 2025

do we need cache for this at all?
is it intended to have some level of offline functionality in the web app?

maybe just remove this cache entirely, as it's not used anyway as long as the server is responsive?

it might explain some oddities i saw in the past where outdated info was shown.

edit:

if it ends up getting removed, do we need some kind of cleanup config to get it purged or is it enough to remove it from the config?

@tepozoa
Copy link

tepozoa commented May 10, 2025

Hiya, issue reporter here, wasn't aware of this PR (my fault). I'd like to suggest that the given change - perhaps relaxed to 24 hours to just match other code and nobody will argue - is so simple let's roll it out for the next minor release and fix an actual problem people are having. The change seems very resistant to regressions of other code.

If the devs feel the cache should be removed etc. great! Thanks. But for now, I'd like to advocate for the existing users being affected and adding this triage now, while planning the future in an issue of it's own (thanks!). Perfection is the enemy of progress with this PR after 3 weeks in the queue.

Much appreciated for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lemmy UI silently fills Firefox local storage (posts) without automatic deletion/cleaning
4 participants