Detect available object caching and show recommended plugin in admin dashboard. #6493

dhaval-parekh · 2021-07-28T15:56:53Z

Summary

Fixes #5780

Checklist

My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

github-actions · 2021-08-05T12:52:09Z

Plugin builds for f198357 are ready 🛎️!

Download development build
Download production build

pierlon

I made a few comments but I like where this is heading 😄

src/Admin/SiteHealth.php

pierlon · 2021-08-17T06:35:57Z

src/Admin/SiteHealth.php

+		/*
+		 * Try to connect redis server.
+		 */
+		$redis_server = [


I see that W3 Total Cache only checks if the Redis class exists.

Any reason to go further and attempt to ping the Redis server? It seems like a brittle way of determining if there is indeed a Redis server as there may be many things that could cause it not to respond such as a firewall or iptables rule, for instance. It may also come off as hostile to some persons as it's attempting to make a connection that the user did not deliberately agree to.

@pierlon As I understood, We do need to check services with connecting instead of checking if a class of extension exists. And if some services are not able to connect then it's better not to recommend anything.

Right, but attempting to connect and fail would do more harm than good here and is more likely to produce more false positives. We should be only checking if a user's environment is capable of using Redis cache, not if we can connect to a Redis server.

pierlon · 2021-08-17T06:40:45Z

src/Admin/SiteHealth.php

+	 */
+	public function is_site_has_memcached() {
+
+		if ( ! ( class_exists( 'Memcache' ) || class_exists( 'Memcached' ) ) ) {


W3 Total Cache also checks for these ✅

pierlon · 2021-08-17T06:42:26Z

src/Admin/SiteHealth.php

+			return false;
+		}
+
+		$has_object_cache = false;


Similar to my comment above, I don't think it our place to attempt to connect to an external service. Checking if the Memcache or Memcached class exists seems to be a good enough heuristic measure.

src/Admin/SiteHealth.php

pierlon · 2021-08-17T06:52:42Z

src/Admin/SiteHealth.php

+			$available_cache = $this->check_available_cache();
+
+			/* translators: plugin recommendation markup */
+			$plugin_placeholder = _x( 'We recommend to use %s plugin.', 'plugin recommendation markup', 'amp' );


We may want to not recommend any specific plugin, but instead point them to documentation that may guide them how to configure object caching for their site. As mentioned in the issue by @westonruter:

Instead of mentioning the specific plugins in the Site Health info recommendation, the list of backend-specific plugins can be in a documentation page on amp-wp.org. This documentation page can pull together the various pieces of information about object caching spread out across WordPress.org, including:

https://make.wordpress.org/hosting/handbook/handbook/performance/#object-cache

https://developer.wordpress.org/reference/classes/wp_object_cache/#persistent-caching

Ultimately, the user should ask their host for the recommended way to enable object caching first before they do it themselves. So our recommendation should be to first contact the host, and then continue with our guidance to try the plugins we've identified which may work with their site.

See support topics about object caching:

https://wordpress.org/support/topic/how-enable-persistent-object-in-amp-plugin/

https://wordpress.org/support/topic/amp-error-25/

https://wordpress.org/support/topic/persistent-object-cache-is-not-enabled-2/

https://wordpress.org/support/topic/persistent-object-caching-is-not-enabled-2/

https://wordpress.org/support/topic/persistent-object-cache-is-not-enabled-amp/

Anything we can do to make it easier for users to enable object caching the better.

What about linking off to search results for object caching plugins using a given backend? For example:

https://wordpress.org/plugins/search/redis+object+cache/

https://wordpress.org/plugins/search/memcached+object+cache/

https://wordpress.org/plugins/search/apcu+object+cache/

And then we can include with that a link to our object caching guide so they can see which plugins specifically we've tested: https://amp-wp.org/documentation/getting-started/amp-site-setup/persistent-object-caching/#i-need-help-setting-up-my-persistent-object-cache

At the very lest we can just say: “It looks like you have X available on your server.” where X is the backend service and then they will provide them with a starting point for which plugins to look for.

pierlon

Apart from attempting to externally connect to Redis and Memcache server instances, this looks OK.

…disable

…ce/5780-object-cache-guide * 'develop' of github.com:ampproject/amp-wp: (43 commits) Make placeholder dependency on video ID more explicit Fix adding start to placeholder link; improve coverage Let placeholder link point to YouTube permalink and not iframe src Prevent generating closing IMG tags Use coversDefaultClass Remove condition which cannot be reached since video ID is always provided Opt to pass-through iframe for unrecognized YouTube URL rather than convert to link Remove unreachable code Give more explicit array phpdoc type Fix issues identified by phpstan Try installing phpstan via shivammathur/setup-php Try downloading a specific version of phpstan instead of latest Fix yml syntax error Try removing phpstan before composer install Print phpstan version when running on GHA Revert "Revert phpstan ignores" Try adding phpstan version to composer cache key Revert phpstan ignores update unit test cases Sanitize attribute name Use original YouTube URL as placeholder when sanitizing raw embed Fix phpcs alignment warning Use constant instead of member variable Let Document::fromNode() obtain the ownerDocument Deduplicate iframe_props into constant ...

westonruter · 2021-09-10T05:14:45Z

@dhaval-parekh I've made some improvements to the test. It now shows multiple persistent object caching services when they are available and then links to the Add Plugins screen in the admin:

Could you please add some test coverage?

westonruter · 2021-09-10T05:29:46Z

(Without rebasing please 😊)

dhaval-parekh self-assigned this Aug 3, 2021

dhaval-parekh marked this pull request as ready for review August 5, 2021 12:47

dhaval-parekh requested review from westonruter and pierlon August 5, 2021 12:47

dhaval-parekh removed the request for review from westonruter August 10, 2021 07:28

pierlon suggested changes Aug 17, 2021

View reviewed changes

dhaval-parekh force-pushed the performance/5780-object-cache-guide branch 2 times, most recently from fd132ed to 271e75c Compare August 20, 2021 09:02

dhaval-parekh requested a review from pierlon August 20, 2021 10:07

dhaval-parekh force-pushed the performance/5780-object-cache-guide branch from 271e75c to ceca448 Compare August 26, 2021 06:04

pierlon reviewed Sep 2, 2021

View reviewed changes

pierlon requested a review from westonruter September 2, 2021 12:08

dhaval-parekh added 7 commits September 6, 2021 13:28

Add helper function to check available object cache mechanism

511184c

Add plugin recommendation in site health notice when object cache is …

d402a59

…disable

Fix fatal error during testing memcache

52c9fb8

Update unit test cases

cb4ae49

Apply suggestion from PR

422d8bb

update message for plugin recommandation

4b3fb94

Update the logic to check object cache availability, Fix phpstan issues

852a5a4

dhaval-parekh force-pushed the performance/5780-object-cache-guide branch from ceca448 to 852a5a4 Compare September 6, 2021 08:24

dhaval-parekh requested a review from pierlon September 6, 2021 08:37

westonruter added this to the v2.2 milestone Sep 10, 2021

westonruter added 3 commits September 9, 2021 20:51

Refactor how object cache services are listed

60ae5ff

Let object cache links lead to Add Plugins screen

0be15d4

dhaval-parekh added 2 commits September 10, 2021 19:18

Add unit test cases

a90e07b

Update unit test case

406ccac

Improve toggling ext-object cache in tests

f198357

westonruter approved these changes Sep 10, 2021

View reviewed changes

westonruter merged commit f819294 into develop Sep 10, 2021

westonruter deleted the performance/5780-object-cache-guide branch September 10, 2021 16:13

westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Detect available object caching and show recommended plugin in admin dashboard. #6493

Detect available object caching and show recommended plugin in admin dashboard. #6493

Uh oh!

dhaval-parekh commented Jul 28, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Aug 5, 2021 •

edited

Loading

Uh oh!

pierlon left a comment

Uh oh!

Uh oh!

pierlon Aug 17, 2021

Uh oh!

dhaval-parekh Aug 20, 2021

Uh oh!

pierlon Sep 2, 2021

Uh oh!

pierlon Aug 17, 2021

Uh oh!

pierlon Aug 17, 2021 •

edited

Loading

Uh oh!

Uh oh!

pierlon Aug 17, 2021

Uh oh!

westonruter Aug 19, 2021

Uh oh!

pierlon left a comment

Uh oh!

westonruter commented Sep 10, 2021

Uh oh!

westonruter commented Sep 10, 2021

Uh oh!

Uh oh!

Detect available object caching and show recommended plugin in admin dashboard. #6493

Detect available object caching and show recommended plugin in admin dashboard. #6493

Uh oh!

Conversation

dhaval-parekh commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

github-actions bot commented Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierlon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pierlon Aug 17, 2021

Choose a reason for hiding this comment

Uh oh!

dhaval-parekh Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

pierlon Sep 2, 2021

Choose a reason for hiding this comment

Uh oh!

pierlon Aug 17, 2021

Choose a reason for hiding this comment

Uh oh!

pierlon Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pierlon Aug 17, 2021

Choose a reason for hiding this comment

Uh oh!

westonruter Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

pierlon left a comment

Choose a reason for hiding this comment

Uh oh!

westonruter commented Sep 10, 2021

Uh oh!

westonruter commented Sep 10, 2021

Uh oh!

Uh oh!

dhaval-parekh commented Jul 28, 2021 •

edited

Loading

github-actions bot commented Aug 5, 2021 •

edited

Loading

pierlon Aug 17, 2021 •

edited

Loading