WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 3 years ago

Last modified 3 years ago

#5389 closed defect (bug) (fixed)

Suspend and resume object cache

Reported by: ryan Owned by:
Milestone: 3.3 Priority: normal
Severity: normal Version: 2.5
Component: Cache API Keywords: has-patch
Focuses: Cc:

Description (last modified by ryan)

We sometimes need to suspend object cache operations, notably during installation and importing. During install we want to know we are working with the raw DB values. During import we want to avoid set/get trips to the cache (especially when using the memcached backend) for a potentially huge number of new objects -- objects that probably won't be requested again. We also want to avoid the overhead of caching imported objects in memory. With big imports we can bust the php memory limit.

In some places, we check if WP_IMPORTING or WP_INSTALLING is set and disable cache sets and gets. Not all cache backends do this however. Instead of referencing global defines, let's formalize this with two function calls, wp_suspend_cache() and wp_resume_cache(). When the cache is suspended, all cache gets return false and all sets/adds return true without actually setting anything. Deletes should probably still be processed. It is up to the caller to make sure it doesn't do things that would make the cache inconsistent with the DB while the cache is suspended. Given our current model where we usually delete on change and add on read, this shouldn't be a problem.

Attachments (4)

cache_suspend.diff (2.3 KB) - added by ryan 6 years ago.
5389.patch (3.5 KB) - added by SergeyBiryukov 3 years ago.
5389.static.diff (1.5 KB) - added by duck_ 3 years ago.
5389.static.2.diff (1.5 KB) - added by duck_ 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 markjaquith6 years ago

Cool by me.

comment:2 ryan6 years ago

  • Description modified (diff)

ryan6 years ago

comment:3 ryan6 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

Didn't really help import performance so not doing this.

comment:4 Nazgul6 years ago

  • Milestone 2.5 deleted

comment:5 nacin3 years ago

  • Keywords has-patch added; cache removed
  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Per duck_, for 10k posts, peak memory went from ~200 MB to 120 MB using the WordPress importer.

Closing #15812 as a duplicate and re-opening.

comment:6 ryan3 years ago

The old patch of mine suspended the cache completely, including gets. The new one just suspends adding new stuff. I think I prefer the new patch. Advantage of having the suspend function outside of cache.php is that you know it will always exist. It means a new global though.

comment:7 johnbillion3 years ago

  • Cc johnbillion@… added

comment:9 nacin3 years ago

  • Component changed from General to Cache
  • Keywords 3.2-early added
  • Milestone changed from Awaiting Review to Future Release

comment:11 follow-up: duck_3 years ago

Was thinking about this again and two ideas for slightly different approaches.

Could use a filter instead of a global: if ( apply_filters( 'suspend_caching', false .... Downside being more overhead in a filter than another check.

Or take Ryan's old patch, move the functions out of cache.php and still allow gets. Therefore not introducing a new global, reusing $wp_object_cache instead. I prefer this approach.

Will re-test and upload patch later.

comment:12 joostdevalk3 years ago

I would be very much in favor of getting this feature in ASAP. My particular problem being with get_permalink, as that throws the post you're getting the permalink for into cache. This causes a huge memory usage when you're trying to generate a 1,000 post sitemap, and all you really need is the URL...

SergeyBiryukov3 years ago

comment:13 in reply to: ↑ 11 SergeyBiryukov3 years ago

Replying to duck_:

Or take Ryan's old patch, move the functions out of cache.php and still allow gets.

Implemented in 5389.patch.

duck_3 years ago

comment:14 duck_3 years ago

  • Keywords 3.2-early removed
  • Milestone changed from Future Release to 3.3

Or it could be done with a static variable, see 5389.static.diff.

duck_3 years ago

comment:15 ryan3 years ago

5389.static.2.diff is fine by me.

comment:16 nacin3 years ago

Let's go with wp_cache_add_suspend or wp_cache_suspend_adds?

comment:17 duck_3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [18681]:

Introduce wp_suspend_cache_addition() to allow reduced memory usage when cache additions aren't useful. Fixes #5389.

comment:18 SergeyBiryukov3 years ago

Shouldn't we now use wp_suspend_cache_addition() during installation and importing, as suggested in the ticket description and in cache_suspend.diff? Or is that already done elsewhere?

Note: See TracTickets for help on using tickets.