Port Persistent Object Cache Health Check from performance plugin to core

[furi3r]

As part of the performance module life cycle, we want to port this site health check to the core.
​https://github.com/WordPress/performance/tree/trunk/modules/object-cache/persistent-object-cache-health-check

[hellofromTonya]

#56041 was marked as a duplicate.

[furi3r]

Performance issue GH link: ​https://github.com/WordPress/performance/issues/391

Adding Persistent Object cache check to Site Health.

Trac ticket: #56040

[audrasjb]

I and thanks for the PR! I reported a few Docblock issues to fix before it's ready to ship. Thanks :)

Thanks for the nitpicks @audrasjb, I've fixed them.

I think this one is ready to be merged.
cc: @felixarntz

@tillkruss @Clorith I've implemented the changes.

1. Removed badge color for different statuses.
2. Added ignore comment for interpolation/CS.
3. Do we agree that the tests stay direct instead of async? the query is light and fast.
4. Do we agree on the extra filters to allow more flexibility to hosting companies?

Thanks!

I think this is good to go now! cc: @Clorith @tillkruss @felixarntz

LGTM

@Clorith Can you please give this another review this week or early next week? It now has 3 approvals, from my perspective it's ready to commit.

[flixos90]

In 53955:

Site Health: Introduce persistent object cache check.

This changeset adds a new persistent_object_cache check which determines whether the site uses a persistent object cache, and if not, recommends it if it is beneficial for the site. A support resource to learn more about object caching has been created and is linked in the check.

A few filters are included for customization of the check, aimed primarily at hosting providers to provide more specific information in regards to their environment:

• site_status_persistent_object_cache_url filters the URL to learn more about object caching, so that e.g. a hosting-specific object caching support resource could be linked.
• site_status_persistent_object_cache_notes filters the notes added to the check description, so that more fine tuned information on object caching based on the environment can be provided.
• site_status_should_suggest_persistent_object_cache is a short-circuit filter which allows using entirely custom logic to determine whether a persistent object cache would make sense for the site.
• site_status_persistent_object_cache_thresholds filters the thresholds in the default logic to determine whether a persistent object cache would make sense for the site, which is based on the amount of data in the database.

Note that due to the nature of this check it is only run in production environments.

Props furi3r, tillkruss, spacedmonkey, audrasjb, Clorith.
Fixes #56040.

@manuelRod Committed in https://core.trac.wordpress.org/changeset/53955 🎉

[flixos90]

Reopening this to write a dev note eventually. It probably makes sense to write a single combined dev note for this together with #56041, which is likely to be committed this week as well.

@furi3r Is that something you would be interested in working on? Definitely happy to support/review as needed. The primary purpose of the dev note is to explain the feature to developers, specifically focusing on the customization points (e.g. explaining the filters, with brief code examples etc.).

[furi3r]

@flixos90 thanks for the commit! Yes, I will try to get a draft before I'm going on holiday next week.

[furi3r]

@flixos90 one thing I've just realized, the committed code is linking to ​https://wordpress.org/support/article/optimization/#object-caching

While I can see that anchor has been updated to ​https://wordpress.org/support/article/optimization/#persistent-object-cache

It needs to be updated.

[flixos90]

Great catch @furi3r, I will prepare a follow-up commit to fix the link. Thank you!

[flixos90]

In 54042:

Site Health: Update persistent object cache check documentation URL.

The anchor for the relevant documentation section had changed since the original code had been written.

Props furi3r.
See #56040.

[SergeyBiryukov]

The test added in [53955] appears to occasionally fail:

• See ​this test run on GitHub Actions.
• Or ​this test run on the Host Test Results page.

Could there be a race condition somewhere?

[flixos90]

@SergeyBiryukov Thanks, will look into this first thing tomorrow.

[flixos90]

In 54053:

Site Health: Refine persistent object cache check tests.

This changeset makes these tests more reliable by having them less affected by external environment factors, fixing occasional failures.

See #56040.

[flixos90]

In 54054:

Site Health: Ensure persistent object cache check short-circuit filter also short-circuits multisite.

Follow up to [54053].

See #56040.

[flixos90]

@SergeyBiryukov With the additional commits above, the occasional test problems here should be addressed now. Please flag here if you notice additional failures. Thank you!

[desrosj]

It looks like everything is wrapped up on this with the exception of the dev note. Let's close this out with beta 1 today. The dev note can be tracked with needs-dev-note on closed tickets.

[spacedmonkey]

There is a draft of a dev note available ​here.

[desrosj]

@spacedmonkey Thanks for drafting that! I had a rough outline written down, but had not written it out. I'll take a look in the next few days!

[spacedmonkey]

Replying to desrosj:

@spacedmonkey Thanks for drafting that! I had a rough outline written down, but had not written it out. I'll take a look in the next few days!

For the record, @furi3r did all the hard work and @flixos90 did the review. As far as I understand it is basically good to publish.

[milana_cap]

Dev note: ​https://make.wordpress.org/core/2022/10/06/new-cache-site-health-checks-in-wordpress-6-1/

[KnowingArt_com]

As a WordPress host, there's nothing "healthy" about this new recommendation.

Even the memecached documentation agrees with me:

"If you have a database host, give as much ram as possible to the database. When cache misses do happen, you'll get more benefit from ensuring your indexes and data are already in memory."
​https://github.com/memcached/memcached/wiki/Hardware

Read the memcached documentation, and use common sense. In most cases, it makes a more sense to allocate "extra" memory to other areas: Innodb, database connections, more php-fpm children, etc. I've been hosting WordPress for almost 20 years, that's how you do it.

More caching is rarely the solution, you're just asking for trouble.

To spin up an additional memcached server that will eat up all free memory, that is a recipe for disaster, as the memcached documentation itself warns about.

"Will take as much memory as you give it."

Sounds like a good way to end up with an unstable system. Oh wait, that's not a bug, it's a feature LOL.

"Assign physical memory, with a few percent extra, to a memcached server. Do not over-allocate memory and expect swap to save you. Performance will be very, very poor. Take extra care to monitor if your server is using swap, and tune if necessary." ​https://github.com/memcached/memcached/wiki/Hardware

And you're actually recommending we use this? Do you really expect hosts to have a separate memcached server, with special swap and network requirements, just for memcached? Who is going to pay for and administer that?

And will this make blogs faster? No, because adding more software and unnecessary network traffic never makes things faster or more reliable. Even the memcached documentation warns about it:

"Network requirements will vary greatly by the average size of your memcached items. Your application should aim to keep them small, as it can mean the difference between being fine with gigabit inter-switch uplinks, or being completely toast."

Yeah, that's a "healthy" recommendation. Sarcasm off.

On top of that, the recommended plugins look to be almost experimental.

Do you think I am going to configure and manage TWO databases (Redis + MySQL) to satisfy this little nag message? I already spent years of my life getting MySQL to work well. There is no way I am going to reinvent the wheel (as if WordPress hosting is not complicated enough) to add Redis into the mix, when MySQL has all kinds of caches that work perfectly fine.

If people have slow WordPress performance, it's NOT because they lack an object cache.

I am really sick of all the silly "health" recommendations that are shoved in the face of my clients on the dashboard. I can't help but wonder if there are any conflicts of interest here. Who really benefits from an object cache? Not the blogger. Not the host. Who then?

Show me any proof memory is better allocated to an object cache and not the database or php-fpm. And even if you can prove it on a poorly configured system, we're talking about a few milliseconds.

[KnowingArt_com]

Who do I invoice every time I add the following line to a new blog:

<?php
add_filter( 'site_status_should_suggest_persistent_object_cache', '__return_false' );

[knutsp]

Good points. Cache isn't for ground speed. With a large database indexes and data can not stay in memory. With high traffic, it saves database queries. So it's about what Health Check tests before recommending, I havn't studied yet. But I have seen a lot of small and low traffic sites getting this recommendation, I think it's a bad approach and needs discussion. But

This ticket was closed on a completed milestone.
If you have a bug or enhancement to report, please open a new ticket. Be sure to mention this ticket, #56040.

[KnowingArt_com]

I don't see this as a software problem that can be solved with a ticket, as much as it is a vision/perspective problem. WordPress is popular today because it was easy to install and because PHP was universally supported by the thousands of hosting companies out there. More or less that is still true today.

If this change is allowed to continue, you're essentially changing the installation requirements to include Redis (because apparently nobody is using the memcache plugin.) Adding a whole new database as a requirement for WordPress is not a small thing. It is difficult enough maintaining MySQL/MariaDB. Small example, a new version of MySQL came out recently that required upgrading all WordPress databases to a new format. ​https://pjbrunet.com/upgrade-from-10-7-4-to-10-8-3-crashes-mysql-mariadb/

As a WordPress host, I have many pages of notes that keep growing over the years, and that includes tons of MySQL documentation, which involves memory management, caching, monitoring, swapping, backups, etc. It's not easy. I'm not looking to make everything more complicated than necessary by adding Redis, unless there is a very good reason. And it's not just me. Working for a billion-dollar startup, I saw firsthand that MySQL is a huge painpoint for other companies, even companies using 3rd party services like Pantheon.

In a perfect world, WordPress would not need a database and would manage its own data, but I have sacrificed a significant portion of my life to learn to use and maintain MySQL/MariaDB, and to keep it running however I can, which is frankly why I had to learn Linux in the first place.

I disagree that large blogs "can not stay in memory." If that is happening, the hosting company is overselling its hardware. For example, a blog with 30,000 posts will easily fit into small amount of InnoDB memory without swapping. It's just text. Even in extreme cases, when I had a blog with millions of posts in MySQL, the entire database fit into a few gigabytes of ram.

I hear what you're saying, I think. But IMO you don't want bloggers nagging their sysadmin to install Redis. That is not a small request, and the benefit is not really there. In most cases it will create worse problems.

I think I agree with you this kind of recommendation only makes sense in certain scenarios, like organizations with unlimited budgets. (Or "This nag is sponsored by Google Cloud, yay more technology to buy!") But I don't think traffic has anything to do with it. Doesn't the "Query Monitor" plugin already monitor database performance?

If somebody is actually struggling with a slow database, I'd assume their host is "overselling" the hardware, or is at the wrong data center, or has a lot to learn about the basics: php-fpm, mysql, hosting, linux, virtualization, etc.

Replying to knutsp:

Good points. Cache isn't for ground speed. With a large database indexes and data can not stay in memory. With high traffic, it saves database queries. So it's about what Health Check tests before recommending, I havn't studied yet. But I have seen a lot of small and low traffic sites getting this recommendation, I think it's a bad approach and needs discussion. But

This ticket was closed on a completed milestone.
If you have a bug or enhancement to report, please open a new ticket. Be sure to mention this ticket, #56040.