Opened 9 years ago
Closed 8 years ago
#36251 closed defect (bug) (fixed)
Allowed memory size exhausted on wp_update_term
Reported by: | rahal.aboulfeth | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 3.9 |
Component: | Taxonomy | Keywords: | |
Focuses: | performance | Cc: |
Description
Hello,
I have a category with a lot of posts ( more than 70000 ) , and I can't update it from the dashboard because of a Allowed memory exhaustion error.
After reviewing wp_update_term code, I noticed this section line 3282 in taxonomy.php :
<?php // Clean the relationship caches for all object types using this term. $objects = $wpdb->get_col( $wpdb->prepare( "SELECT object_id FROM $wpdb->term_relationships WHERE term_taxonomy_id = %d", $tt_id ) ); $tax_object = get_taxonomy( $taxonomy ); foreach ( $tax_object->object_type as $object_type ) { clean_object_term_cache( $objects, $object_type ); }
In my case objects is an array with more than 70000 rows, and this is probably what caused the memory exhaustion. I commented this section and the update started working again.
Now, I don't really get why we should clean all those caches.
I think the same behavior is also present in wp_delete_term.
Thank you
Attachments (1)
Change History (7)
#1
@
9 years ago
- Component changed from General to Taxonomy
- Summary changed from Allowed memory size exhausted on up_update_term to Allowed memory size exhausted on wp_update_term
#3
@
8 years ago
- Focuses administration removed
- Milestone changed from Awaiting Review to Future Release
- Version changed from 4.4.2 to 3.9
It's necessary to bust the caches on term deletion. If a post has terms a
and b
, and b
is deleted, you have to remove the association with b
from the cache. I don't think there's a way around this.
The cache busting in wp_update_term()
was introduced as part of #22526. The reason why it's needed is the {$taxonomy}_relationships
cache contains arrays describing the entire term object, rather than simple term IDs. I think it was probably originally built this way because we didn't have decent cache support for single terms. Now we do, so we can probably explore changing how the object relationship cache works. If it stores only term IDs, and fetches the terms themselves from their own caches, it should no longer be necessary to call clean_object_term_cache()
when updating a term.
#4
@
8 years ago
- Milestone changed from Future Release to 4.6
36251.diff is a first attempt at fixing the problem. Notes:
- Anywhere where the
{$taxonomy}_relationships
cache is set, use an array of term IDs rather than full term data. - When term data is pulled from the cache (
get_object_term_cache()
), convert term IDs to term data arrays. There is a bit of juggling involved.wp_cache_get()
gives[1, 2, 3]
. In order to get terms 1, 2, and 3 from the cache, we useget_term( 1 )
etc, which returns aWP_Term
object. But functions callingget_object_term_cache()
expect an array rather thanWP_Term
, so we return the$data
property from the term objects, and let the functions that callget_object_term_cache()
convert those arrays back intoWP_Term
objects. It's a bit clunky, but required for backward compatibility. - Some unit tests are no longer relevant, so they've been updated for the new technique.
This seems to me a much improved strategy over what's currently in place. By reducing redundancy in the term data we cache (since things like name
, etc are only cached with the individual object), we reduce the size of the cache, and fix bugs like #22526 along the way.
The one remaining compatibility issue I can think of is that there may be plugins calling wp_cache_set( $object_id, "{$taxonomy}_relationships" )
directly, rather than update_object_term_cache()
. If plugins manually put term arrays rather than term IDs into the cache, things will break. Generally, we don't go out of our way to maintain backward compatibility with the structure of the data we keep in our internal caches, so maybe this is not a concern. If it is, get_object_term_cache()
could include a check to see whether the cached data is in the legacy format, and convert as necessary.
Feedback welcome.
Hello,
Any ideas how to fix this?