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

increase max-age for download and social badges; affects [twitch] #6567

Merged
merged 2 commits into from Jun 2, 2021

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Jun 1, 2021

We serve a lot of download and social badges (collectively download and social categories account for ~25% of the traffic we serve from Heroku) and we serve them with the default max-age (2 mins).
In a lot of cases the upstream providers cache these totals and we also often present them as rounded numbers e.g:

  • downloads | 20k
  • stars | 3k

I think we can get away with cacheing these for longer and serve more of these badge requests from downstream cache without noticeably downgrading service for most users. Lets try setting them to 15 mins and see how it goes.
I've decided to manually exclude twitch status from this and actually set a shorter max-age on it as "live now" seems like it need to be more immediate to be useful. I think all other download and social badges can use the new default.

@chris48s chris48s added the operations Hosting, monitoring, and reliability for the production badge servers label Jun 1, 2021
@shields-ci
Copy link

shields-ci commented Jun 1, 2021

Warnings
⚠️ This PR modified service code for twitch but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 22b8bb7

@calebcartwright
Copy link
Member

I'm onboard for changing the value here but I'm torn about the 15 minute value. It's true in cases with higher numbers that the incremental diffs within a given 15 minutes block would likely be obfuscated by the rounding, but I think that only applies once one gets to badge messages that exceed 1k

Conversely, if a package only has 50 downloads or a user 25 followers, then it's highly unlikely for that number to change dramatically, and be observed, within that cache window.

However, (as I continue debating myself 😄) it seems surprising at first blush that we'd have a longer cache period for download badges vs. version badges, given that the former seems more dynamic than the latter.

I'm not necessarily opposed to trying out the 15 minute value and won't block if others are content to proceed, but want to ponder on this a bit before approving myself

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Okay didn't need as long as I thought I might 😆

Figure it's better to try it and see if there's any actual reports or concerns instead of just hypothesizing

@chris48s
Copy link
Member Author

chris48s commented Jun 2, 2021

Its a good point. In many cases we would expect these values to change more often than version.

I think the reason it is different in my mind is because I see number of downloads/stars/etc as more of an "indicative" statistic. 491 downloads/followers is "about 500" and 508 is also "about 500". Versions are discrete - version 1.2 is a completely different thing to version 1.3.

That said, I'm certainly not married to 15 minutes. Its a number I made up and we can make it smaller if turns out to be a poor choice :)

@repo-ranger repo-ranger bot merged commit 830bb15 into badges:master Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operations Hosting, monitoring, and reliability for the production badge servers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants