-
Notifications
You must be signed in to change notification settings - Fork 384
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
Eliminate post-processor cache #4178
Comments
@westonruter Would you consider removing post-processor related cache as well, or let them expire? |
Removing the cache entries? There wouldn't be a way to iterate over all of them, so no, there's nothing we can do to delete them. They'll be deleted when they expire. |
Yep that's true... I see no problem with them expiring anyways. |
The wiki page on post-processor cache may need to be updated to indicate it has been removed. |
Feature description
It has become clear that the post-processor caching is not working. It is currently only offered when the user has an external object cache, and when it is available then very frequently users will see this notice appear on the AMP settings screen:
The reality is that pages with highly-variable content are extremely common, whether that be random numbers, timestamps, or nonces. Therefore I believe this is a feature that should be removed since it is not effective.
We may also want to eliminate support for sending 304 Not Modified responses (#1481). Even though this doesn't directly cause data to be put in to a cache, it can be wasted processing time to compute the
ETag
if it changes with each request.PRs related to post-processor response caching:
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation brief
QA testing instructions
Demo
Changelog entry
The text was updated successfully, but these errors were encountered: