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

Eliminate post-processor cache #4178

Closed
westonruter opened this issue Jan 25, 2020 · 4 comments · Fixed by #4391
Closed

Eliminate post-processor cache #4178

westonruter opened this issue Jan 25, 2020 · 4 comments · Fixed by #4391
Assignees
Labels
Caching P1 Medium priority
Projects
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Jan 25, 2020

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 post-processor cache was disabled due to detecting randomly generated content found on on this web page. Randomly generated content was detected on this web page. To avoid filling up the cache with unusable content, the AMP plugin's post-processor cache was automatically disabled.	Read more. This will enable post-processor caching to speed up processing an AMP response after WordPress renders a template.

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

@westonruter westonruter added P1 Medium priority Caching labels Jan 25, 2020
@westonruter westonruter added this to Backlog in Ongoing Jan 25, 2020
@westonruter westonruter added this to the v1.5 milestone Feb 13, 2020
@kmyram kmyram moved this from Backlog to To Do in Ongoing Mar 12, 2020
@westonruter westonruter self-assigned this Mar 12, 2020
@westonruter westonruter assigned pierlon and unassigned westonruter Mar 14, 2020
@westonruter westonruter changed the title Eliminate post-processor cache in favor of detecting and encouraging use of page caching Eliminate post-processor cache in favor of detecting Mar 14, 2020
@pierlon pierlon moved this from To Do to In Progress in Ongoing Mar 14, 2020
@pierlon
Copy link
Contributor

pierlon commented Mar 15, 2020

@westonruter Would you consider removing post-processor related cache as well, or let them expire?

@westonruter
Copy link
Member Author

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.

@pierlon pierlon moved this from In Progress to Ready for Review in Ongoing Mar 15, 2020
@pierlon
Copy link
Contributor

pierlon commented Mar 15, 2020

Yep that's true... I see no problem with them expiring anyways.

@westonruter westonruter moved this from Ready for Review to Done in Ongoing Mar 16, 2020
@pierlon
Copy link
Contributor

pierlon commented Mar 16, 2020

The wiki page on post-processor cache may need to be updated to indicate it has been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Caching P1 Medium priority
Projects
Ongoing
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants