From what i can see in the sess_write function will always set the $user->cache to 0|zero since the $user will always start with an anonomous user.

The problem here with having $user->cache timestamp to always 0|zero, is that when
the evaluation in the cache_get function, will never evaluate to true, returning old cache data
even for authenticated users.

I refer to this part:

 if ($user->cache > $cache->created) {
        // This cache data is too old and thus not valid for us, ignore it.
        return 0;
      }

in the cache_get function http://api.drupal.org/api/function/cache_get.

One easy solution is to set the $user->cache = time() after the drupal_unpack($user) in the sess_read function in sessions.inc.

Comments

Anonymous’s picture

Anonymous (not verified) commented

Subscribing

catch’s picture

catch
he/him
Primary language English
commented
Version: 6.x-dev » 7.x-dev

Marking as duplicate of http://drupal.org/node/277155 was duplciate, and bumping this to 7.x so it can be fixed there first.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Closed (duplicate)

Actually marking it duplicate.

damien tournoud’s picture

damien tournoud commented
Status: Closed (duplicate) » Active

This is in fact the oldest issue.

About the issue itself: I guess this is by design. $user->cache is only used during a cache_clear_all() call to simulate that the cache has been flushed for the current user. Other users will still see old data until cache_lifetime seconds have been elapsed.

I'm very tempted to mark that as 'by design', but others may want to comment.

deplifer’s picture

deplifer commented

This forces modules to use be dependent:

* on the cache_clear_all() generally called only on a node insert/update/delete $op.
* Or to have cron running, and purge the cache objects themself, which needs to be updated on a basis different than the cache_lifetime itself.
* Then it should also be clear in the cache_set that only CACHE_PERMANENT|CACHE_TEMPORARY, are working.

This i not practical, since the cache_clear_all().
Forces alot of cache objects to be flushed at the same time, creating a heavy impact on the site if you have alot of stored cache objects.

Therefore allowing the independent use of unix timestamp, would allow that objects are updated independent of a call to the cache_clear_all() and that some of content(blocks) on the site are updated on a more regular basis. Then a call to cache_clear_all() is having a less heavier impact on the overall site, since not all objects are dependent on a call to this function.

damien tournoud’s picture

damien tournoud commented

Therefore allowing the independent use of unix timestamp, would allow that objects are updated independent of a call to the cache_clear_all().

This would require a substantial rework of the minimum cache life architecture. Currently, the minimum cache life relies on cron (implementation details: system_cron() calls cache_clear_all()) to invalidate objects older than expire + minimum_cachelife. It will not work if there is no cron running.

On the other hand your solution can't work because $user->cache is only meant to simulate that a call to cache_clear_all() has an immediate impact for the current user. Settign $user->cache to time() in sess_read() would essentially remove all caching.

deplifer’s picture

deplifer commented

I agree but the only work around would be to change the $expire variable when it is set to CACHE_TEMPORARY for the cache_set() function.

  $expire = ( ($expire == CACHE_TEMPORARY) ? ( time() + variable_get('cache_lifetime', 0)) : $expire);

The CACHE_PERMANENT block in the cache_get function is always evaluated before the $user->cache > $cache->created.

This allows the authenticated user to have some blocks, to be updated without cron or on a more reularg basis.

The anonymous users would still receive the cached pages.

And when the cache'd page is expired some blocks of content which has already been cached for a authenticated user, wouldnt need to be rebuild if the authenticated user already has cached that section of the page.

The code above would also make cached pages be flushed on a more independent basis.
Since say pages are cached for 30 minutes, then a page A could be cached 5 minutes before page B and also would be flushed 5 minutes before page B. If they are accessed on a regulary basis in those 30minutes.

Now say you have 10thousand pages or more flushed at the same time, and you have the same amount of anonymous and authenticated users accessing the pages that have been flushed at the same time or in a 1-3minutes intervall, this is spike which would be more normalised if the cache objects are flushed more or less on a independent basis with own timestamps.

damien tournoud’s picture

damien tournoud commented
Status: Active » Closed (works as designed)

@deplifer: I do agree that the management of cache_lifetime could be improved. But the current architecture is probably there for a reason and we need people more aware of the design constraints involved to comment on that.

I only wanted to make it clear that was you suggested in the first message is wrong. I suggest you open a separate issue to discuss improving the cache_lifetime management.

deplifer’s picture

deplifer commented

* Forum Topic ?

or

* Feature Request ?

damien tournoud’s picture

damien tournoud commented

I suggest you open a feature request.

deplifer’s picture

deplifer commented

http://drupal.org/node/277229