object cache's terms bucket potentially contains junk

[Denis-de-Bernardy]

We've a few occurrences of things such as:

wp_cache_add( $cache_key, $terms, 'terms' );

... in taxonomy.php. These are not flushed when the terms cache gets flushed, and end up stored in memcached indefinitely. We should at least add a timeout, maybe after a day.

Better yet, we should flush them properly.

[Denis-de-Bernardy]

see also: r9102, r8225, etc.

[Denis-de-Bernardy]

there potentially is a better way to do this. such as is done in get_pages():

	$key = md5( serialize( compact(array_keys($defaults)) ) );
	if ( $cache = wp_cache_get( 'get_pages', 'posts' ) ) {
		if ( is_array($cache) && isset( $cache[ $key ] ) ) {
			$pages = apply_filters('get_pages', $cache[ $key ], $r );
			return $pages;
		}
	}

[filosofo]

I don't think the get_pages() approach scales very well. I've noticed several times that sites with a large number of pages at points exhaust the available memory in one of the cache methods. Since for a typical WP site there are many more terms than pages, I'd expect this to crop up more often if used for terms.

Why not do something like I suggested a while ago: #4476. Delete cache items by group name. Then the clean_term_cache() could just flush all members of the "terms" group. Problem solved, right?

[ryan]

The last_changed method used by terms was done to avoid the problems of the pages style caching where the arrays could get huge. Currently, the last_changed method relies on old cache keys eventually being pushed out of the cache. Using a timeout to reclaim them faster seems fine.

I've yet to see flush by group (or even just flush) implemented solidly for all of the various cache backends for both WP and WPMU.

[Denis-de-Bernardy]

Replying to filosofo:

Why not do something like I suggested a while ago: #4476. Delete cache items by group name. Then the clean_term_cache() could just flush all members of the "terms" group. Problem solved, right?

It would, if memcached allowed to partially flush keys using wildcards. Trouble is, memcached offers only two option, mark this very specific key as obsolete, and mark all keys as obsolete.

there are a few bits in backpress' memcached that allow to to flush an entire bucket, but I'm 100% sure that it is not concurrent proof: (I interacted the pecl package's author last year in order to convince him to implement locks for memcached-based session handling. It's the only thing in the memcached pecl with any type of locking.)

@Ryan: the file I sent you the other day implements flush (as in, full flush) quite fine. But no partial flush.

[Denis-de-Bernardy]

I've been working with the patch applied for a couple days. No side effects apart from the obvious extra query per day.

@Ryan: do you need it massaged any further?

[hakre]

I'd like to rate this as tested. Denis can you report that it made another 12 days working properly?

[Denis-de-Bernardy]

I'm afraid I can't, since I svn switched back to the 2.9 branch upon seeing a few scary looking changes creeping in. But definitely tested, yes.

[shidouhikari]

But memcached doesn't flush stuff after some time?

I use WP File Cache plugin and by default it gets filemtime to know when cache was created, and if it's older than 1h cache is deleted and returns false. I changed default to 1 day and works fine.

It's really scary having data eternally cached by default. Even more when wp_cache_add() only adds data to cache if it's not already cached, and wp_cache_replace() also only replace if data is already cached.

A bug in any data editing code that doesn't delete cache may make old data be used forever.

[Denis-de-Bernardy]

But memcached doesn't flush stuff after some time?

Not really... It kicks old stuff out of the cache automatically when it runs out of memory. But there's no timeout per se if you don't pass one.

Related: #14713

[markjaquith]

(In [15583]) Do not cache terms indefinitely... use TTL of one day. fixes #11431. props Denis-de-Bernardy