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

site: keep expired cache when background refresh failed #100

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

unknwon
Copy link
Member

@unknwon unknwon commented Aug 24, 2023

We have been pretty picky about freshness of the cached content (5m) and would aggressively delete the cache even if the background refresh failed.

It has the big assumption that the content host (GitHub.com) would never go down, and creates deep coupling of our docs site liveness directly with the liveness of the GitHub.com.

This PR removes that in the spirit of favoring stableness over freshness. It is better to have content got stale, than having the entire site down. Especially in the former case, the content would not be possible to be refreshed if the content host is also down in the first place.

Copy link

@rafax rafax left a comment

Choose a reason for hiding this comment

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

LGTM, do we expose the "freshness" of the cache in some way? Would it be possible to alert on cache getting stale - before the PR, staleness would be visible in docsite status?

@rafax
Copy link

rafax commented Aug 24, 2023

This PR looks good to me, and should address large part of the Github induced instability - one other thing we could consider is sharing the cache between replicas (so new replicas can come up during upstream outages), but I defer to you on how hard that would be.

@unknwon
Copy link
Member Author

unknwon commented Aug 24, 2023

do we expose the "freshness" of the cache in some way? Would it be possible to alert on cache getting stale - before the PR, staleness would be visible in docsite status?

We can add an endpoint to expose, the staleness wasn't visible in typical sense, but by responding server error to all requests (aka. down time) 😅

@unknwon unknwon merged commit 073eb22 into main Aug 24, 2023
3 checks passed
@unknwon unknwon deleted the jc/keep-expired-cache branch August 24, 2023 17:20
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.

None yet

2 participants