From 7cddc22fb80a1cc2d44ecefa6e9d756b498e3363 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 29 Oct 2015 14:23:32 -0700
Subject: [PATCH] objectcache: Allow bounded HashBagOStuff sizes and limit it
 in WANObjectCache

Change-Id: Icca2474b1ea6feb7134f8958aecf79aa51b7f71e
---
 includes/libs/objectcache/HashBagOStuff.php   | 39 ++++++++++++++-----
 includes/libs/objectcache/WANObjectCache.php  |  4 +-
 .../libs/objectcache/BagOStuffTest.php        | 13 +++++++
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index 94bc95fe39..bdcf18017f 100644
--- 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 6ef2bce0b7..c04d4b6be4 100644
--- 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 94b74cb651..55f71cc4f0 100644
--- 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 ) );
+		}
+	}
 }
-- 
2.20.1