Doctrine 2 - ORM

Doctrine Cache

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.0.1, 2.0.2
  • Fix Version/s: 2.0.2
  • Component/s: None
  • Security Level: All
  • Labels:
    None

Description

I find a problem with doctrine cache key.

It use a namespace and key to save the value and never use the namespace to get it.

I checked that on AbstractQuery::getResultCacheId().

AbstractCache::save > use $this>_getNamespacedId($id)

  1. Text File
    AbstractQuery.php.patch
    13/Feb/11 4:21 PM
    0.5 kB
    Bertrand Zuchuat
  2. File
    apctest.tgz
    13/Feb/11 7:42 AM
    15 kB
    Bertrand Zuchuat
  3. Text File
    yay-caching-works-again.patch
    24/Feb/11 11:29 AM
    0.6 kB
    Alex Woods

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benjamin Eberlei added a comment -

AbstractQuery::getResultCacheId() does not append the namespace. The Cache functions fetch() and contains() do.

Show
Benjamin Eberlei added a comment - AbstractQuery::getResultCacheId() does not append the namespace. The Cache functions fetch() and contains() do.
Hide
Permalink
Alex Woods added a comment - - edited

Actually, I think ALL result caching may be broken since the fix for DDC-892 landed. The code is expecting to fetch an array from the cache with an element in it thats key is $id. Presumably, this is to cater for cases where the hash clashes for different ids. However, the query result is not being inserted into the cache in this format!

This patch should fix the issue.

There's still a less serious issue - and that's that if there are two ids with clashing hashes that are being called repeatedly in quick succession, they're going to keep displacing each other from the cache. An alternative patch could bundle both ids them into the same cache entry:

if ($cached === false)
$cached = array();
$cached[$id] = $result;
$cacheDriver->save($id, $cached, $this->_resultCacheTTL);

However, I think displacing is better than running the risk of having a TTL that is too long be applied as the result of a clash!

EDIT: I'm also wondering why $hash isn't used as the cache key. It looks like the intention was that it should be, but it isn't..

Show
Alex Woods added a comment - - edited Actually, I think ALL result caching may be broken since the fix for DDC-892 landed. The code is expecting to fetch an array from the cache with an element in it thats key is $id. Presumably, this is to cater for cases where the hash clashes for different ids. However, the query result is not being inserted into the cache in this format! This patch should fix the issue. There's still a less serious issue - and that's that if there are two ids with clashing hashes that are being called repeatedly in quick succession, they're going to keep displacing each other from the cache. An alternative patch could bundle both ids them into the same cache entry: if ($cached === false) $cached = array(); $cached [$id] = $result; $cacheDriver->save($id, $cached, $this->_resultCacheTTL); However, I think displacing is better than running the risk of having a TTL that is too long be applied as the result of a clash! EDIT: I'm also wondering why $hash isn't used as the cache key. It looks like the intention was that it should be, but it isn't..
Hide
Permalink
Benjamin Eberlei added a comment -

Fixed, the DDC-892 patch was pretty borked. This does now work

Show
Benjamin Eberlei added a comment - Fixed, the DDC-892 patch was pretty borked. This does now work

People

  • Assignee:
    Benjamin Eberlei
    Reporter:
    Bertrand Zuchuat
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: