Doctrine 1
  1. Doctrine 1
  2. DC-644

_getCacheKeys() exhausts memory

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Caching
    • Labels:
      None
    • Environment:
      Doctrine is installed as a Symfony plugin. Using the latest Symfony from SVN.

      Description

      My scripts have excessive memory consumption and I've often saw in my logs:

      PHP Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 2097152 bytes) in /proj/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Cache/Apc.php on line 111

      Looking into the code I've found which function to blame:

      protected function _getCacheKeys()
      {
      $ci = apc_cache_info('user');
      $keys = array();

      foreach ($ci['cache_list'] as $entry)

      Unknown macro: { $keys[] = $entry['info']; ######### THIS IS THE LINE }

      return $keys;
      }

      My server extensively uses APC caching and it's normal to have many cache keys.
      Obviously retrieving ALL of them is time and memory consuming.
      As I'm not well versed with Doctrine's code, I didn't want to dive further in.

      Is there another way to avoid this pitfall?

        Activity

        Hide
        Jonathan H. Wage added a comment -

        Let me know what you come up with and we'll have a look at including it in the next 1.2.x release.

        Show
        Jonathan H. Wage added a comment - Let me know what you come up with and we'll have a look at including it in the next 1.2.x release.
        Hide
        Amir W added a comment -

        Bypassing the extra array is still not good enough and IMHO the whole idea of deleteBy() should NOT be used if many such requests could be made, as is my case.

        What I've done now is what I mentioned before with a patch that is quite ugly.

        In Doctrine/Query/Abstract.php right after the line

        $cacheDriver->save($hash, $cached, $this->getResultCacheLifeSpan());
        

        I've added

                        if (!empty($GLOBALS['rcache_users_in_query'])) {
                        	MyCache::keepRelatedCacheKey($GLOBALS['rcache_users_in_query'], $hash);
                        }
        

        Which saves another cache key which holds the hash tags that would have to be deleted on an update.
        My global variable is actually an array as a Doctrine query result may be associated with more than one user and possibly other parameters.
        Before calling the $q->execute(), I simply update this variable.

        When a user on my system does the update, I then delete all relevant Doctrine keys with something like

        		if (is_null($cacheDriver)) $cacheDriver = Doctrine_Manager::getInstance()->getAttribute(Doctrine_Core::ATTR_RESULT_CACHE);
        		
        		foreach($arKeys as $key) {
        			$cacheDriver->delete($key);			
        		}
        

        and then delete my other cache key.

        This solution works well for me. Sorry I cannot make a nice Doctrine patch for it as I'm not well versed with your code. I still believe it should be supported by Doctrine with an optional extra parameter for $q->useResultCache()

        Thanks

        Show
        Amir W added a comment - Bypassing the extra array is still not good enough and IMHO the whole idea of deleteBy() should NOT be used if many such requests could be made, as is my case. What I've done now is what I mentioned before with a patch that is quite ugly. In Doctrine/Query/Abstract.php right after the line $cacheDriver->save($hash, $cached, $ this ->getResultCacheLifeSpan()); I've added if (!empty($GLOBALS['rcache_users_in_query'])) { MyCache::keepRelatedCacheKey($GLOBALS['rcache_users_in_query'], $hash); } Which saves another cache key which holds the hash tags that would have to be deleted on an update. My global variable is actually an array as a Doctrine query result may be associated with more than one user and possibly other parameters. Before calling the $q->execute(), I simply update this variable. When a user on my system does the update, I then delete all relevant Doctrine keys with something like if (is_null($cacheDriver)) $cacheDriver = Doctrine_Manager::getInstance()->getAttribute(Doctrine_Core::ATTR_RESULT_CACHE); foreach($arKeys as $key) { $cacheDriver->delete($key); } and then delete my other cache key. This solution works well for me. Sorry I cannot make a nice Doctrine patch for it as I'm not well versed with your code. I still believe it should be supported by Doctrine with an optional extra parameter for $q->useResultCache() Thanks
        Hide
        David Abdemoulaie added a comment -

        Hi Amir,

        Zend_Cache does not implement tagging for either APC or Memcached backends, see the documentation. It also likely never will, all requests for this functionality have been closed with Wont Fix.

        I don't think the deleteBy methods should have ever been implemented. When initially implemented they cached a "doctrine_cache_keys" variable to store the keys known to Doctrine. This however led to a crippling bug that would crash my production servers after a few hours. Not even a friendly "out of memory" limit, but a slowdown and eventual crash. Please see DDC-460 for details. Note that I don't use the magic delete methods, just simple saves with timeouts and this was affecting me.

        I fixed the solution as you've seen using the _getCacheKeys() method. I don't believe this functionality should have ever been added to Doctrine to begin with, but this is what we have to work with. It should be the responsibility of the cache store to handle tagging and such, not poorly hacked on with application code.

        As it stands, the current implementation doesn't affect people who aren't even using this functionality, as it should be. As Jon suggested, you shouldn't be using this in the context of a page request. Use a CLI script or work on another solution. Your idea of tracking your keys in application code is a good idea, but it doesn't belong in Doctrine imo.

        Show
        David Abdemoulaie added a comment - Hi Amir, Zend_Cache does not implement tagging for either APC or Memcached backends, see the documentation . It also likely never will, all requests for this functionality have been closed with Wont Fix. I don't think the deleteBy methods should have ever been implemented. When initially implemented they cached a "doctrine_cache_keys" variable to store the keys known to Doctrine. This however led to a crippling bug that would crash my production servers after a few hours. Not even a friendly "out of memory" limit, but a slowdown and eventual crash. Please see DDC-460 for details. Note that I don't use the magic delete methods, just simple saves with timeouts and this was affecting me. I fixed the solution as you've seen using the _getCacheKeys() method. I don't believe this functionality should have ever been added to Doctrine to begin with, but this is what we have to work with. It should be the responsibility of the cache store to handle tagging and such, not poorly hacked on with application code. As it stands, the current implementation doesn't affect people who aren't even using this functionality, as it should be. As Jon suggested, you shouldn't be using this in the context of a page request. Use a CLI script or work on another solution. Your idea of tracking your keys in application code is a good idea, but it doesn't belong in Doctrine imo.
        Hide
        Amir W added a comment -

        Thanks David for your comment.

        I agree with you that my implementation should not belong in Doctrine and that tagging should have been a part of the cache backends.

        Continuing with the same logic you've presented, deleteBy...() functionality **should be removed** from Doctrine if it causes the system to crash as it does so in an obnoxious way so that it would take too long for most developers to notice this is where the problem lies. It has certainly taken too much of my time and efforts and I'd rather save the pain from others.

        Show
        Amir W added a comment - Thanks David for your comment. I agree with you that my implementation should not belong in Doctrine and that tagging should have been a part of the cache backends. Continuing with the same logic you've presented, deleteBy...() functionality ** should be removed ** from Doctrine if it causes the system to crash as it does so in an obnoxious way so that it would take too long for most developers to notice this is where the problem lies. It has certainly taken too much of my time and efforts and I'd rather save the pain from others.
        Hide
        Carsten Henkelmann added a comment -

        We had the exact same problem. We used a "deleteAll()" of a ApcCache object and ran into the "allowed memory size exhausted" pitfall. We helped ourselves with a new class that extends ApcCache and uses the simpler apc_clear_cache function.

         
        namespace Foo\Cache;
        
        class ApcCache extends \Doctrine\Common\Cache\ApcCache
        {
            /**
             * Delete all cache entries. Memory saving version...
             *
             * @return bool
             */
            public function deleteAll()
            {
                return apc_clear_cache('user');
            }
        }
        
         
        use Foo\Cache\ApcCache as Apc;
        ...
        $this->_apc = new Apc();
        $this->_apc->deleteAll();
        

        This doesn't return the ids of the deleted entries like the original function but we don't need that. So this works fine for us.

        Show
        Carsten Henkelmann added a comment - We had the exact same problem. We used a "deleteAll()" of a ApcCache object and ran into the "allowed memory size exhausted" pitfall. We helped ourselves with a new class that extends ApcCache and uses the simpler apc_clear_cache function. namespace Foo\Cache; class ApcCache extends \Doctrine\Common\Cache\ApcCache { /** * Delete all cache entries. Memory saving version... * * @return bool */ public function deleteAll() { return apc_clear_cache('user'); } } use Foo\Cache\ApcCache as Apc; ... $this->_apc = new Apc(); $this->_apc->deleteAll(); This doesn't return the ids of the deleted entries like the original function but we don't need that. So this works fine for us.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Amir W
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: