objectcache: Allow bounded HashBagOStuff sizes and limit it in WANObjectCache

Change-Id: Icca2474b1ea6feb7134f8958aecf79aa51b7f71e

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index 94bc95f..bdcf180 100644 (file)
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -20,6 +20,7 @@
  * @file
  * @ingroup Cache
  */
+use Wikimedia\Assert\Assert;
 
 /**
  * This is a test of the interface, mainly. It stores things in an associative
@@ -28,18 +29,28 @@
  * @ingroup Cache
  */
 class HashBagOStuff extends BagOStuff {
-       /** @var array */
-       protected $bag;
+       /** @var mixed[] */
+       protected $bag = array();
+       /** @var integer Max entries allowed */
+       protected $maxCacheKeys;
 
+       const KEY_VAL = 0;
+       const KEY_EXP = 1;
+
+       /**
+        * @param array $params Additional parameters include:
+        *   - maxKeys : only allow this many keys (using oldest-first eviction)
+        */
        function __construct( $params = array() ) {
                parent::__construct( $params );
-               $this->bag = array();
+
+               $this->maxCacheKeys = isset( $params['maxKeys'] ) ? $params['maxKeys'] : INF;
+               Assert::parameter( $this->maxCacheKeys > 0, 'maxKeys', 'must be above zero' );
        }
 
        protected function expire( $key ) {
-               $et = $this->bag[$key][1];
-
-               if ( ( $et == 0 ) || ( $et > time() ) ) {
+               $et = $this->bag[$key][self::KEY_EXP];
+               if ( $et == 0 || $et > time() ) {
                        return false;
                }
 
@@ -57,15 +68,25 @@ class HashBagOStuff extends BagOStuff {
                        return false;
                }
 
-               return $this->bag[$key][0];
+               return $this->bag[$key][self::KEY_VAL];
        }
 
        public function set( $key, $value, $exptime = 0, $flags = 0 ) {
-               $this->bag[$key] = array( $value, $this->convertExpiry( $exptime ) );
+               $this->bag[$key] = array(
+                       self::KEY_VAL => $value,
+                       self::KEY_EXP => $this->convertExpiry( $exptime )
+               );
+
+               if ( count( $this->bag ) > $this->maxCacheKeys ) {
+                       reset( $this->bag );
+                       $evictKey = key( $this->bag );
+                       unset( $this->bag[$evictKey] );
+               }
+
                return true;
        }
 
-       function delete( $key ) {
+       public function delete( $key ) {
                unset( $this->bag[$key] );
 
                return true;

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index 6ef2bce..c04d4b6 100644 (file)
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -131,6 +131,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
        const PURGE_VAL_PREFIX = 'PURGED:';
 
+       const MAX_PC_KEYS = 1000; // max keys to keep in process cache
+
        /**
         * @param array $params
         *   - cache   : BagOStuff object
@@ -142,7 +144,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                $this->cache = $params['cache'];
                $this->pool = $params['pool'];
                $this->relayer = $params['relayer'];
-               $this->procCache = new HashBagOStuff();
+               $this->procCache = new HashBagOStuff( array( 'maxKeys' => self::MAX_PC_KEYS ) );
                $this->setLogger( isset( $params['logger'] ) ? $params['logger'] : new NullLogger() );
        }
 

diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
index 94b74cb..55f71cc 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
@@ -240,4 +240,17 @@ class BagOStuffTest extends MediaWikiTestCase {
                $this->assertType( 'ScopedCallback', $value1, 'First reentrant call returned lock' );
                $this->assertType( 'ScopedCallback', $value1, 'Second reentrant call returned lock' );
        }
+
+       public function testHashBagEviction() {
+               $cache = new HashBagOStuff( array( 'maxKeys' => 10 ) );
+               for ( $i=0; $i<10; ++$i ) {
+                       $cache->set( "key$i", 1 );
+                       $this->assertEquals( 1, $cache->get( "key$i" ) );
+               }
+               for ( $i=10; $i<20; ++$i ) {
+                       $cache->set( "key$i", 1 );
+                       $this->assertEquals( 1, $cache->get( "key$i" ) );
+                       $this->assertEquals( false, $cache->get( "key" . $i - 10 ) );
+               }
+       }
 }