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

Flush per site cache #33

Closed
wants to merge 2 commits into from
Closed

Flush per site cache #33

wants to merge 2 commits into from

Conversation

Sneezry
Copy link

@Sneezry Sneezry commented Jul 20, 2016

Redis Object Cache provides a Flush Cache button, however, it clears the entire DB. We have a multisite WordPress with thousands of blogs, and to flush all cache may lead performance problem to us. So I make some changes to flush per site cache. This feature is only available for network sites. A Flush Cache link will be added in each site line in Network Sites menu.

flush_cache_action

@tillkruss
Copy link
Member

tillkruss commented Jul 21, 2016

Thanks for the PR!

How long have you been using this in production already?

@Sneezry
Copy link
Author

Sneezry commented Jul 22, 2016

We haven't used this in production, because if we update the plugin the feature will be lost, and that's the reason I create this pull request. However, the code has passed our code review :)

@tillkruss
Copy link
Member

I'd be open to merging this after you tested this feature thoroughly. Any chance you guys could manually apply the changes and see how it goes?

@Sneezry
Copy link
Author

Sneezry commented Jul 23, 2016

OK, we will test it on our dev environment for days, and if it works fine, we will deploy it on MSDN blog production. I will inform you when we decide to deploy it on production, and you may consider when to merge it then :)

@Sneezry
Copy link
Author

Sneezry commented Aug 4, 2016

The plugin has been tested on PPE (test env) for a half month, and we have deployed it on our production today. I move "Flush Cache" link from custom column to row action as shown in our production below:

download

@tillkruss
Copy link
Member

Nice, keep me posted.

@lkraav
Copy link
Contributor

lkraav commented Aug 22, 2016

This is terrific, I'm super interested.

@tillkruss
Copy link
Member

@Sneezry: Can you fix the conflicts?

@Sneezry
Copy link
Author

Sneezry commented Sep 28, 2016

OK, I'll fix it in few days.

@tillkruss
Copy link
Member

This would be nice to have as well: https://make.wordpress.org/core/2016/10/04/custom-bulk-actions/

@tillkruss
Copy link
Member

We just implemented selective cache key flushing that uses the WP_CACHE_KEY_SALT constant in #53 and I have to say I'm not sure if a per-site cache flush makes sense to me.

It seems like a cache-invalidation nightmare to me because of the global groups, like the users table. If someone can describe a scenario in which they would greatly benefit from a per-site flush and they have a solid solution for this let me know. Otherwise prefer not to merge this.

@Sneezry
Copy link
Author

Sneezry commented Nov 28, 2016 via email

@svandragt
Copy link

So are you saying you're thinking of moving away from this PR in favour of a cache salt approach (#53)? In a multisite scenario then would one have to all the site-id to the cache salt to have functionality similar to this?

Being able to clear a cache for a single site in a high traffic multisite environment is a definite use case that we have as flushing the db causes a large performance drop. Typically in this environment visitors do not login with WordPress therefore not flushing the users table is ok.

However I can also see implementations where this could be a problem, so perhaps this PR simple needs a setting/option for flushing the 'shared cache' along with a site flush so that we can have that control.

@tillkruss
Copy link
Member

tillkruss commented Nov 28, 2016

@svandragt: No, I'd still like to see better multisite support in the plugin, but it has to be implemented in a way that works great out-of-the-box for "regular" users and we can "unlock" different scenarios with WP_REDIS_* constants.

A few thoughts:

By default the entire cache should be flushed, multisite or not.

Next level could be a constant that enables per-site flushing (under "Sites"), which would also flush all shared/global groups. However it must still be possible to flush the entire cache somewhere. Maybe Network -> Settings -> Redis?

There should be WP CLI commands for flushing...:

  • ...the entire cache
  • ...the cache of a single network site
  • ...the global groups

If another separate constant is added that prevents the global groups from being flushed in multisite environments, then there should be a button to flush global groups somewhere. Maybe as well under Network -> Settings -> Redis?

@ravloony
Copy link
Contributor

If someone can describe a scenario in which they would greatly benefit from a per-site flush and they have a solid solution for this let me know. Otherwise prefer not to merge this.

We are using a multisite setup (~4000 websites and counting) where we each site has an internal url of the form slug.ourcompany.com until the site moves to production, at which point our provisioner application edits all urls in the database for that site to change them to the real url. This implies a cache flush.

Obviously, with that many sites, a cache flush is be pretty damaging to performance. As it stands we are holding of our impending doom by cunningly queueing connections in nginx so the database never gets overwhelmed, but ideally we would like a per site flush to be possible.

One option I am toying with is to set the WP_CACHE_KEY_SALT to md5($_SERVER['SERVER_NAME']) and using WP_REDIS_SELECTIVE_FLUSH, but I am not yet sure of the implications. For one thing it may make it difficult to use wp-cli.

@tillkruss
Copy link
Member

@ravloony: That should work fine, better than flushing all sites at once. Depends how many hits you have per second on those 4000 sites.

I never use hashes as prefixes, because it makes impossible to find manually, instead I usually use a sanitized string of the domain, like example_com: or beta_example_com:.

@tillkruss
Copy link
Member

tillkruss commented Apr 11, 2017

To be honest, I don't like the approach of this PR, using WP_REDIS_SELECTIVE_FLUSH in combination with WP_CACHE_KEY_SALT seems smarter.

The "Flush Cache" UI link is great, tho. We should just use the selective flush constant and allow cache flushing via network admin and normal admin UI. If anyone wants to make a PR, that'd be great!
I think it would be nicer if the a "Flush Site"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants