Skip to content

Clear WP object cache periodically on media regenerate/import.#62

Merged
schlessera merged 3 commits intomasterfrom
flush-wp-object-cache
Mar 20, 2018
Merged

Clear WP object cache periodically on media regenerate/import.#62
schlessera merged 3 commits intomasterfrom
flush-wp-object-cache

Conversation

@gitlost
Copy link
Copy Markdown
Contributor

@gitlost gitlost commented Jan 15, 2018

For discussion.

Noticed this old report of memory issues using wp media regenerate in slack by @dws122 https://wordpress.slack.com/archives/C02RP4T41/p1511409150000032 and thought it might make sense to call wp_clear_object_cache() periodically like wp import and wp export do.

Testing of wp media regenerate suggests it clears around 3 or 4 MB each time, using the same 500 interval as wp import. Testing of wp media import was less convincing, but seemed to clear around 1 MB each time.

@schlessera
Copy link
Copy Markdown
Member

Oh... I just looked at how the current cache clearing works, and I think that might potentially produce disasters on a high-traffic production site.

For some high-traffic sites, the cache is vital to keep the site online, and a cache invalidation will lead to a cache stampede. If we're forcing such a cache invalidation, without even making that obvious in the documentation or giving people a switch to turn it off, we risk breaking such sites. We will not only mess with the in-memory cache, but also with the persistent (Memcached/Redis/...) cache.

I'd like to first investigate whether we cannot just replace the global variable $wp_object_cache that contains the current WP_Object_Cache instance with a sort of dummy cache object that just not stores anything for these commands.

@schlessera schlessera added the command:media-regenerate Related to 'media regenerate' command label Jan 16, 2018
@gitlost
Copy link
Copy Markdown
Contributor Author

gitlost commented Jan 16, 2018

Good points ta.

I'd like to first investigate whether we cannot just replace the global variable $wp_object_cache that contains the current WP_Object_Cache instance with a sort of dummy cache object

That sounds like a great solution, easy to implement and likely to have speed benefits as well. I'll close this PR and create an issue for it on wp-cli/wp-cli.

@gitlost gitlost closed this Jan 16, 2018
@gitlost gitlost deleted the flush-wp-object-cache branch January 16, 2018 10:59
@danielbachhuber danielbachhuber restored the flush-wp-object-cache branch February 28, 2018 13:37
@danielbachhuber danielbachhuber added this to the 1.1.5 milestone Feb 28, 2018
@danielbachhuber
Copy link
Copy Markdown
Member

For some high-traffic sites, the cache is vital to keep the site online, and a cache invalidation will lead to a cache stampede. If we're forcing such a cache invalidation, without even making that obvious in the documentation or giving people a switch to turn it off, we risk breaking such sites. We will not only mess with the in-memory cache, but also with the persistent (Memcached/Redis/...) cache.

Because the underlying utility, Utils\wp_clear_object_cache(), doesn't flush persistent cache, this isn't applicable.

@danielbachhuber danielbachhuber requested a review from a team February 28, 2018 13:54
@danielbachhuber
Copy link
Copy Markdown
Member

Requesting a second review from @schlessera

@schlessera schlessera merged commit 30dfca6 into master Mar 20, 2018
@schlessera schlessera deleted the flush-wp-object-cache branch March 20, 2018 15:40
@schlessera
Copy link
Copy Markdown
Member

I agree that this solves the problem immediately, instead of risking to introduce potential issues through a new mechanism.

However, I will keep the idea of a dummy cache object in mind, as it would move the special cache handling logic from multiple different commands in multiple different repositories into one single, isolated location.

danielbachhuber pushed a commit that referenced this pull request Nov 18, 2022
Clear WP object cache periodically on media regenerate/import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:media-regenerate Related to 'media regenerate' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants