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

Encourage use of page caching #4386

Closed
westonruter opened this issue Mar 14, 2020 · 20 comments · Fixed by #6456
Closed

Encourage use of page caching #4386

westonruter opened this issue Mar 14, 2020 · 20 comments · Fixed by #6456
Assignees
Labels
Caching Changelogged Whether the issue/PR has been added to release notes. Epic Groomed P0 High priority Performance Punted WS:Perf Work stream for Metrics, Performance and Optimizer WS:UX Work stream for UX/Front-end
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Mar 14, 2020

Feature description

The post processor cache was removed in #4178 due to being ineffective.

Instead of a post-processor cache, the AMP plugin should instead be detecting whether the site has page caching enabled. Having page caching (and object caching) is essential for any WordPress site that needs to scale. Whether this be provided by a plugin like WP Super Cache or a reverse proxy layer like Varnish, the AMP plugin should be detecting whether page caching is enabled, and if not, direct the user to the best page caching solution for their environment.

For some hosts, the solution is just to turn on page caching in the dashboard. For other environments, it is recommending a plugin based on whether a persistent caching service is available (e.g. Memcached or Redis). For others, it's whether some PHP extension is available (e.g. APCu).

See also #1216 for host-specific instructions for object caching.

This will involve evaluating caching plugins for AMP compatibility so we can direct users to install ones that we know will work properly.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  1. Implement a strong/accurate caching detection mechanism
  2. Provided guidance should include:
    -- General information in terms of caching approaches and associated plugins
    -- General information about caching services provided by hosting services
    -- Provide a filter that hosting providers can leverage hydrate the general guidance
  3. The guidance provided should be concise with links to external resources for details

Design

Site Health

Implementation brief

For detection of page caching, one solution is to explicitly check for specific plugins being active and configured to enable page caching. This is feasible although tedious. Nevertheless, it won't help if there is an external caching layer in the network stack (e.g. Varnish). Therefore, a more general approach to detect page caching being enabled would be:

  1. Add a template_redirect hook which short-circuits with printing wp_rand() when a given query parameter is present.
  2. Performs a fetch request without credentials to that URL 3 times (or more). Beware of hosts that will interpret this as DDoS attack.
  3. If any of the responses come back identical to any of the others, page caching is enabled.

The results should then be displayed in Site Health.

QA testing instructions

Demo

Changelog entry

@westonruter westonruter added this to the v1.6 milestone Mar 14, 2020
@westonruter
Copy link
Member Author

@westonruter westonruter added the P0 High priority label Mar 20, 2020
@westonruter westonruter modified the milestones: v1.6, v1.5.1 Mar 20, 2020
@amedina
Copy link
Member

amedina commented Mar 20, 2020

Additional inquiry:

  • Is it possible for the plugin to detect accurately the hosting provider the site is being served from? This would determine hosting-specific guidance (e.g. WP Engine)
  • Is it possible to define one of the AC based on the accuracy of the detection approach specified in the implementation brief? E.g. page/object caching is successfully detected 100%
  • There is already an approach in place for detecting object caching, correct?

@amedina
Copy link
Member

amedina commented Mar 20, 2020

Want to populate a table like this to converge to the full guidance that will be provided by the plugin:

Screen Shot 2020-03-20 at 1 06 57 PM

@westonruter
Copy link
Member Author

westonruter commented Mar 20, 2020

  • Is it possible for the plugin to detect accurately the hosting provider the site is being served from? This would determine hosting-specific guidance (e.g. WP Engine)

I think so, but it would be a unique heuristic for each host. On Pantheon, for example, the test would be isset( $_ENV['PANTHEON_ENVIRONMENT'] ). We'll need to work with each host to find the best detection mechanism and then which instructions (or link to instructions) for then how to enable object caching.

  • There is already an approach in place for detecting object caching, correct?

Yes, it is wp_using_ext_object_cache().

@westonruter
Copy link
Member Author

westonruter commented Mar 20, 2020

Perhaps my intention to do host detection is a hopeless/brittle effort, and we should just provide general guidance but have a filter which hosts could use to direct users to docs that are specific to their hosting scenario . Something that was brought up is the case of hosts that do whitelabel reselling, where it would be bad if the underlying host was mentioned in the instructions. The default guidance could have a link to a page on amp-wp.org that privides the host-specific instructions as well as general instructions.

@westonruter
Copy link
Member Author

See also #6432, where we can detect cache varying by mobile user agents to determine if we can default to server-side redirection.

@westonruter westonruter added P0 High priority and removed P1 Medium priority labels Jul 3, 2021
@westonruter
Copy link
Member Author

Note that if we detect page caching is enabled, we'll need to instruct the user to flush the cache after changing the template mode. See #4843.

@jwold
Copy link
Collaborator

jwold commented Jul 23, 2021

@westonruter and @dhaval-parekh I've started going through this and want to make sure I understand the flow. Following is a diagram showing the statuses and actions I imagine could exist:

#4386 Encourage Page Caching

  1. Is this an accurate flow?
  2. Is the flow for object cache and page cache different?

@westonruter
Copy link
Member Author

Thanks for the diagram.

We won't be able to reliably know whether the host includes the option for object caching or page caching. So we can't direct the user to where to go. When we provide provide a list of possible plugins to use, we'll also need to indicate that they should contact their host for the best option(s).

As for compatibility, that will be a bit tricky to detect. If not using a caching plugin, then we won't be able to detect what is being used (as it is in another layer of the network stack). If it is a caching plugin, we could check to see if they have one of the specific plugins we've tested and if not, mention that we haven't tested with that plugin. But perhaps it's better to initially just say, “We've detected you have page caching enabled. If you're experiencing any issues, please refer to our guide.”

@jwold
Copy link
Collaborator

jwold commented Jul 26, 2021

Thanks for the feedback! So we'd want to simplify a few things. Is this an accurate representation of what you suggested above?

#4386 Encourage Page Caching

@westonruter
Copy link
Member Author

Yes. That looks right. In both cases, we'll end up linking them off to our guide. What will differ between the two paths is the status of the check, whether it is a PASS or FAIL.

2. Is the flow for object cache and page cache different?

I missed this. The flow is essentially the same, although it is much simpler to detect whether object caching is implemented. We already have that implemented. What's missing is the recommended plugins, based on what capabilities their server has (e.g. Redis, Memcached, etc).

@maitreyie-chavan maitreyie-chavan added the WS:UX Work stream for UX/Front-end label Aug 19, 2021
@kenrodriguesdesign
Copy link

Hey @westonruter, I'm working on the UI for this and in regards to your comment:

We could use WP_CACHE constant as detection for whether page caching is enabled (or rather if advanced-cache.php drop-in exists). For sites with an external caching layer, then we may need to have a button in the Site Health test for the user to say “I do have page caching!” which can then suppress the test failure. In a subsequent release, we could improve the detection for external caching layers and reset the flag.

I went ahead and created this and wanted to know your thoughts about this.
#4386

@westonruter
Copy link
Member Author

I'll note that the updated logic for detecting page caching is being made more robust in #6456 so we don't have to rely on WP_CACHE. Nevertheless, having such a button will provide a way to dismiss this site health test. We'll need to make sure that we have a link to the page caching documentation featured here.

Note there is an existing such button for another Site Health test for the stylesheet transient caching:

image

@kenrodriguesdesign
Copy link

Duly noted about the updated logic.

I went ahead and updated the design by adding the page caching link. (Content needs to be reviewed)
I would also recommend using the primary button(present in design) instead of the secondary button as shown in the previous comments.
Screenshot 2021-08-26 at 10 35 31 PM

Let me know what you think

@westonruter
Copy link
Member Author

Yeah, looks good to me.

@delawski
Copy link
Collaborator

QA Passed

When tested without any caching plugin, Site Health displays a message that page caching was not detected. After installing and activating a caching plugin, page caching is detected on Site Health.

No caching plugin WP Super Cache enabled
Screenshot 2021-11-19 at 11 35 22 Screenshot 2021-11-19 at 11 52 06

Tested with AMP 2.2.0-alpha-20211117T230738Z-094aef8

@westonruter
Copy link
Member Author

I've also proposed this Site Health test for core: https://core.trac.wordpress.org/ticket/54423

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Caching Changelogged Whether the issue/PR has been added to release notes. Epic Groomed P0 High priority Performance Punted WS:Perf Work stream for Metrics, Performance and Optimizer WS:UX Work stream for UX/Front-end
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants