Details

    • Type: Bug Bug
    • Status: 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. AbstractQuery.php.patch
        0.5 kB
        Bertrand Zuchuat
      2. apctest.tgz
        15 kB
        Bertrand Zuchuat
      3. yay-caching-works-again.patch
        0.6 kB
        Alex Woods

        Activity

        Hide
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: