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

revert: cache change #1492

Merged
merged 1 commit into from
Aug 4, 2023
Merged

revert: cache change #1492

merged 1 commit into from
Aug 4, 2023

Conversation

aeneasr
Copy link
Member

@aeneasr aeneasr commented Aug 4, 2023

@LBBO this new cache logic lead to quite serious problems on the documentation. The reason is that it simply is a catch-all cache which sets an insanely high lifetime. This leads to (in hindsight) very obvious problems:

  • All pages, all HTML, all images, all assets - everything is cached for basically forever
  • If all HTML is cached forever, it may very well refer to old assets which no longer exist both on th server and the client. This breaks the site (see for example API documentation is missing #1489)
  • Because we use cloudflare with edge caching, and it is also explicitly enabled by the public directive. This not only stores outdated assets (such as HTML pages) on the client - it stores them in Cloudflare. This significantly makes the problem of broken pages worse due to a shared state.

Caching is not an easy problem. Never just cache everything forever, and always roll out caches iteratively and monitor the changes correctly.

Write caching rules that make sense. You want to cache javascript assets that have a hash ID? Sure, do that, it's very unlikely that these files will ever change. Caching index.html? Really bad idea, that file will change frequently and the cache MUST NOT be used then.

I should have paid more attention during review but thankfully the issue was noticed relatively quickly - we did have a couple of broken pages though, and page health for all of ory.sh deteriorated from 91% to 50%.

@aeneasr aeneasr requested a review from LBBO August 4, 2023 07:31
@aeneasr aeneasr merged commit 05fe490 into master Aug 4, 2023
9 checks passed
@aeneasr aeneasr deleted the fix-cache branch August 4, 2023 07:33
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.

1 participant