-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
tweak default cache lengths by category #10919
Conversation
'platform-support': 300, | ||
size: 300, | ||
version: 300, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My logic here is that the cadence with which platform support and size change is probably about once per release, so lets treat them the same as version numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that platform-support tend to change less then version and size, as most versions would not change supported platform or change major version, but its an improvement from 120s so we might just start with that.
chat: 1800, | ||
downloads: 1800, | ||
rating: 1800, | ||
social: 1800, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these categories primarily show what I've described as "indicative metrics"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
30min seems fair.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, hope this reduces resource use with minimal effect on client usage perception
Quick refresher on how we set max-age:
_cacheLength
specific to that service_cacheLength
we fall back and use the default for that category. This is the code I am changing in this PRdefaultCacheLengthSeconds
(which we set to 120 seconds in production)The basic principles of setting defaults by category are:
metric()
. It is less crucial for an indicative metric to be fresh.In this PR I am suggesting we tweak the category defaults a bit and add some categories that were previously using the global default of 2 mins. The broad result of all of this being: Cache more things for longer. I am focussing on categories that make up a large proportion of our traffic.
We can always continue to tweak these thresholds.
Also:
_cacheLength
on individual badge classes - those continue to override the defaults by category