filecache_cron has

    if ($cache->expire == CACHE_TEMPORARY ||
        $cache->expire < REQUEST_TIME) {
      @unlink($filename);
    }

But cache_set's documentation states

* @param $expire
 *   (optional) One of the following values:
 *   - CACHE_PERMANENT: Indicates that the item should never be removed unless
 *     explicitly told to using cache_clear_all() with a cache ID.
 *   - CACHE_TEMPORARY: Indicates that the item should be removed at the next
 *     general cache wipe.
 *   - A Unix timestamp: Indicates that the item should be kept at least until
 *     the given time, after which it behaves like CACHE_TEMPORARY.
 *

And I believe that a cron run should not do a general cache wipe (only a cache_clear_all should), but only clear the files that have expired.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sneakyvv’s picture

Patch attached, which takes cache_lifetime into account on cron, instead of deleting all CACHE_TEMPORARY files.

Sneakyvv’s picture

Version: 7.x-1.0-beta2 » 7.x-1.x-dev

Patch applies to latest dev as well

Sneakyvv’s picture

Status: Active » Needs review

Triggered test bot

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

I confirm this problem, & also that this patch applies cleanly to the latest dev & solves the problem. RTBC.

That being said, I wonder why this cache backend is concerning itself with cache expiration anyway. Wouldn't it make more sense to leave cache expiration to a module like Cache Expiration & confine this module to cache storage? Cache expiration is already a hard problem, so why make it more complicated by having multiple modules influencing it?

What would be harmed by ripping out filecache_cron() entirely?

Sneakyvv’s picture

@rhclayto: Cache expiration is a module which allows you to expire the cache in a better fashion, i.e. not via a global minimum lifetime setting. So now, file cache works without an extra module. Of course it could depend on for example cache expiration...

Btw, I'm using file cache in combination with cache_lifetime_options, which allows me to cache pages up to a year and expire them via cache expiration.

PS: I've noticed my previous patch had one small issue. The '+=' would cause the cache to expire one second too early, since CACHE_TEMPORARY is -1.

  • ogi committed c914b00 on 7.x-1.x authored by Sneakyvv
    Issue #2530522 by Sneakyvv: Clearing CACHE_TEMPORARY on cron.
    
ogi’s picture

Status: Reviewed & tested by the community » Fixed

Thank you for providing patch. I decided to make a release for this one.

  • ogi committed 427bfc4 on 7.x-1.x
    Issue #2530522 by Sneakyvv: Clearing CACHE_TEMPORARY on cron
    

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.