Remove the in-memory cache #6037
Conversation
|
| // be cached _downstream_. This is governed by cache headers, which are | ||
| // configured by the service, the user's request, and the server's default | ||
| // cache length. | ||
| module.exports = class NonMemoryCachingBaseService extends BaseService { |
There was a problem hiding this comment.
This class only existed to dodge the cache behavior which is being removed herewith. The two services using this can now use BaseService directly.
There was a problem hiding this comment.
For years now we've used Cloudflare as a CDN, so trying to cache onboard is less useful than before
Also helps that we require traffic to ingress via cloudflare (no more possibility of hitting badge servers directly).
Changes LGTM, though image the merging and deployment of this one will be done at a predetermined time so the experiment can be executed in isolation for clean observation.
I assume we'll want to keep an eye on the overall request load and uncached requests hitting the backend, with a reference to trending response time to see what, if any, impact this has from the removal.
May be interesting to see if there's any minor drop in memory usage too, though don't expect much on that front
|
I just deployed the last commit to master and could merge and deploy this one next. We could give it the weekend and maybe Monday morning for observation, and roll back if necessary. |
Shields has long had an in-memory cache with some complex logic for determining when the cached values are used and when they are flushed. At the time this was implemented, squeezing cache performance was helpful since there was no downstream cache. For years now we've used Cloudflare as a CDN, so trying to cache onboard is less useful than before. Furthermore, since the cache is very small and only used in fairly restrictive ways, it probably has very little impact on performance.
This feels like "step one" in addressing #3368 and #4655, as it's disabling most of the complicated behavior in the existing request-handling code.
In addition, #3368 is a dependency for #3328, and given issues like badges/sc#5 (a DOS vulnerability in Shields), ScoutCamp feels like a liability. Getting the train rolling toward #3328 feels useful.
There was some discussion on this in the ops meeting today. This change is slightly experimental (I think there is maybe a 1% chance it causes a serious problem) but it's easy enough to roll back if it does.
Ref #3368