DDC-1026: Doctrine Cache #1616

Closed
doctrinebot opened this Issue Feb 10, 2011 · 8 comments

2 participants

@doctrinebot

Jira issue originally created by user garfield-fr:

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)

@doctrinebot

Comment created by @beberlei:

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

@doctrinebot

Comment created by garfield-fr:

This is my test project with Symfony2.
Load vendor with "git submodule update"

Thanks

@doctrinebot

Comment created by garfield-fr:

First query (no data on apc cache): http://www.screencast.com/t/kMZ4SSeORH and second (data on apc cache): http://screencast.com/t/uBO42F21

@doctrinebot

Comment created by garfield-fr:

I find the problem with cache. On function execute in AbstractQuery.php, if the cache is not an array, the return failed. In my result, i have only a string.
I have changed return $cached[$id]; with return is_array($cached) ? $cached[$id] : cached;

@doctrinebot

Comment created by awoods:

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..

@doctrinebot

Comment created by @beberlei:

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

@doctrinebot

Issue was closed with resolution "Fixed"

@beberlei beberlei was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot added this to the 2.0.2 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment