diff --git a/Cm/Cache/Backend/Redis.php b/Cm/Cache/Backend/Redis.php index 041738b..9467615 100644 --- a/Cm/Cache/Backend/Redis.php +++ b/Cm/Cache/Backend/Redis.php @@ -145,11 +145,11 @@ protected function getClientOptions($options = array()) $clientOptions->forceStandalone = isset($options['force_standalone']) && $options['force_standalone']; $clientOptions->connectRetries = isset($options['connect_retries']) ? (int) $options['connect_retries'] : self::DEFAULT_CONNECT_RETRIES; $clientOptions->readTimeout = isset($options['read_timeout']) ? (float) $options['read_timeout'] : null; - $clientOptions->password = isset($options['password']) ? $options['password'] : null; - $clientOptions->username = isset($options['username']) ? $options['username'] : null; + $clientOptions->password = $options['password'] ?? null; + $clientOptions->username = $options['username'] ?? null; $clientOptions->database = isset($options['database']) ? (int) $options['database'] : 0; - $clientOptions->persistent = isset($options['persistent']) ? $options['persistent'] : ''; - $clientOptions->timeout = isset($options['timeout']) ? $options['timeout'] : self::DEFAULT_CONNECT_TIMEOUT; + $clientOptions->persistent = $options['persistent'] ?? ''; + $clientOptions->timeout = $options['timeout'] ?? self::DEFAULT_CONNECT_TIMEOUT; return $clientOptions; } @@ -157,6 +157,8 @@ protected function getClientOptions($options = array()) * Construct Zend_Cache Redis backend * @param array $options * @throws Zend_Cache_Exception + * @throws CredisException + * @noinspection PhpMissingParentConstructorInspection */ public function __construct($options = array()) { @@ -210,7 +212,7 @@ public function __construct($options = array()) $this->_redis = $redisMaster; break 2; - } catch (\Exception $e) { + } catch (Exception $e) { unset($sentinelClient); $exception = $e; } @@ -225,16 +227,16 @@ public function __construct($options = array()) $slaves = $sentinel->getSlaveClients($sentinelMaster); if ($slaves) { if ($options['load_from_slaves'] == 2) { - array_push($slaves, $this->_redis); // Also send reads to the master + $slaves[] = $this->_redis; // Also send reads to the master } $slaveSelect = isset($options['slave_select_callable']) && is_callable($options['slave_select_callable']) ? $options['slave_select_callable'] : null; if ($slaveSelect) { $slave = $slaveSelect($slaves, $this->_redis); } else { - $slaveKey = array_rand($slaves, 1); + $slaveKey = array_rand($slaves); $slave = $slaves[$slaveKey]; /* @var $slave Credis_Client */ } - if ($slave instanceof Credis_Client && $slave != $this->_redis) { + if ($slave instanceof Credis_Client && $slave !== $this->_redis) { try { $this->_applyClientOptions($slave, true); $this->_slave = $slave; @@ -255,7 +257,7 @@ public function __construct($options = array()) // Direct connection to single Redis server and optional slaves else { - $port = isset($options['port']) ? $options['port'] : 6379; + $port = $options['port'] ?? 6379; $this->_redis = new Credis_Client($options['server'], $port, $this->_clientOptions->timeout, $this->_clientOptions->persistent); $this->_applyClientOptions($this->_redis); @@ -268,7 +270,7 @@ public function __construct($options = array()) $clientOptions = $this->getClientOptions($options['load_from_slave'] + $options); $totalServers = 2; } else { // Multiple slaves - $slaveKey = array_rand($options['load_from_slave'], 1); + $slaveKey = array_rand($options['load_from_slave']); $slave = $options['load_from_slave'][$slaveKey]; $server = $slave['server']; $port = $slave['port']; @@ -283,7 +285,7 @@ public function __construct($options = array()) // If multiple addresses are given, split and choose a random one if (strpos($server, ',') !== false) { $slaves = preg_split('/\s*,\s*/', $server, -1, PREG_SPLIT_NO_EMPTY); - $slaveKey = array_rand($slaves, 1); + $slaveKey = array_rand($slaves); $server = $slaves[$slaveKey]; $port = null; $totalServers = count($slaves) + 1; @@ -291,7 +293,7 @@ public function __construct($options = array()) $totalServers = 2; } } - // Skip setting up slave if master is not write only and it is randomly chosen to be the read server + // Skip setting up slave if master is not write only, and it is randomly chosen to be the read server $masterWriteOnly = isset($options['master_write_only']) ? (int) $options['master_write_only'] : false; if (is_string($server) && $server && ! (!$masterWriteOnly && rand(1, $totalServers) === 1)) { try { @@ -341,15 +343,15 @@ public function __construct($options = array()) } elseif (function_exists('lz4_compress')) { $version = phpversion("lz4"); if (version_compare($version, "0.3.0") < 0) { - $this->_compressTags = $this->_compressTags > 1 ? true : false; - $this->_compressData = $this->_compressData > 1 ? true : false; + $this->_compressTags = $this->_compressTags > 1; + $this->_compressData = $this->_compressData > 1; } $this->_compressionLib = 'l4z'; } elseif (function_exists('zstd_compress')) { $version = phpversion("zstd"); if (version_compare($version, "0.4.13") < 0) { - $this->_compressTags = $this->_compressTags > 1 ? true : false; - $this->_compressData = $this->_compressData > 1 ? true : false; + $this->_compressTags = $this->_compressTags > 1; + $this->_compressData = $this->_compressData > 1; } $this->_compressionLib = 'zstd'; } elseif (function_exists('lzf_compress')) { @@ -396,6 +398,10 @@ public function __construct($options = array()) * Apply common configuration to client instances. * * @param Credis_Client $client + * @param bool $forceSelect + * @param null|stdClass $clientOptions + * @throws CredisException + * @throws Zend_Cache_Exception */ protected function _applyClientOptions(Credis_Client $client, $forceSelect = false, $clientOptions = null) { @@ -428,8 +434,10 @@ protected function _applyClientOptions(Credis_Client $client, $forceSelect = fal } /** - * @deprecated - Previously this setup an instance of Credis_Cluster but this class was not complete or flawed * @param $options + * @throws CredisException + * @throws Zend_Cache_Exception + * @deprecated - Previously this setup an instance of Credis_Cluster but this class was not complete or flawed */ protected function _setupReadWriteCluster($options) { @@ -442,8 +450,8 @@ protected function _setupReadWriteCluster($options) $this->_redis = new Credis_Client( $masterNode['host'], $masterNode['port'], - isset($masterNode['timeout']) ? $masterNode['timeout'] : 2.5, - isset($masterNode['persistent']) ? $masterNode['persistent'] : '' + $masterNode['timeout'] ?? 2.5, + $masterNode['persistent'] ?? '' ); $this->_applyClientOptions($this->_redis); break; @@ -451,13 +459,13 @@ protected function _setupReadWriteCluster($options) } if (!empty($options['cluster']['slave'])) { - $slaveKey = array_rand($options['cluster']['slave'], 1); + $slaveKey = array_rand($options['cluster']['slave']); $slave = $options['cluster']['slave'][$slaveKey]; $this->_slave = new Credis_Client( $slave['host'], $slave['port'], - isset($slave['timeout']) ? $slave['timeout'] : 2.5, - isset($slave['persistent']) ? $slave['persistent'] : '' + $slave['timeout'] ?? 2.5, + $slave['persistent'] ?? '' ); $this->_applyClientOptions($this->_redis, true); } @@ -469,6 +477,7 @@ protected function _setupReadWriteCluster($options) * @param string $id Cache id * @param boolean $doNotTestCacheValidity If set to true, the cache validity won't be tested * @return bool|string + * @throws CredisException */ public function load($id, $doNotTestCacheValidity = false) { @@ -531,17 +540,17 @@ public function test($id) { // Don't use slave for this since `test` is usually used for locking $mtime = $this->_redis->hGet(self::PREFIX_KEY.$id, self::FIELD_MTIME); - return ($mtime ? $mtime : false); + return ($mtime ? (int)$mtime : false); } /** - * Get the life time + * Get the lifetime * - * if $specificLifetime is not false, the given specific life time is used + * if $specificLifetime is not false, the given specific lifetime is used * else, the global lifetime is used * * @param int $specificLifetime - * @return int Cache life time + * @return int Cache lifetime */ public function getLifetime($specificLifetime) { @@ -714,31 +723,17 @@ public function remove($id) $result = $this->_redis->exec(); - return isset($result[0]) ? (bool)$result[0] : false; + return isset($result[0]) && (bool)$result[0]; } /** * @param array $tags + * @throws Zend_Cache_Exception */ protected function _removeByNotMatchingTags($tags) { $ids = $this->getIdsNotMatchingTags($tags); - if ($ids) { - $ids = array_chunk($ids, $this->_removeChunkSize); - foreach ($ids as $idsChunk) { - $this->_redis->pipeline()->multi(); - - // Remove data - $this->_redis->unlink($this->_preprocessIds($idsChunk)); - - // Remove ids from list of all ids - if ($this->_notMatchingTags) { - $this->_redis->sRem(self::SET_IDS, $idsChunk); - } - - $this->_redis->exec(); - } - } + $this->_removeByIds($ids); } /** @@ -747,6 +742,14 @@ protected function _removeByNotMatchingTags($tags) protected function _removeByMatchingTags($tags) { $ids = $this->getIdsMatchingTags($tags); + $this->_removeByIds($ids); + } + + /** + * @param array $ids + */ + protected function _removeByIds($ids) + { if ($ids) { $ids = array_chunk($ids, $this->_removeChunkSize); foreach ($ids as $idsChunk) { @@ -804,7 +807,7 @@ protected function _removeByMatchingAnyTags($tags) if ($ids) { $ids = array_chunk($ids, $this->_removeChunkSize); - foreach ($ids as $index => $idsChunk) { + foreach ($ids as $idsChunk) { // Remove data $this->_redis->unlink($this->_preprocessIds($idsChunk)); @@ -946,10 +949,10 @@ protected function _collectGarbage() unset($expired); } + // TODO // Clean up global list of ids for ids with no tag - if ($this->_notMatchingTags) { - // TODO - } +// if ($this->_notMatchingTags) { +// } } /** @@ -1041,9 +1044,9 @@ public function setDirectives($directives) * Mainly a workaround for the issues that arise due to the fact that * Magento's Enterprise_PageCache module doesn't set any expiry. * - * @param int $specificLifetime - * @param string $id - * @return int Cache life time + * @param int $lifetime + * @param string $id + * @return int Cache lifetime */ protected function _getAutoExpiringLifetime($lifetime, $id) { @@ -1127,6 +1130,7 @@ public function getIdsMatchingTags($tags = array()) * * @param array $tags array of tags * @return array array of not matching cache ids (string) + * @throws Zend_Cache_Exception */ public function getIdsNotMatchingTags($tags = array()) { @@ -1180,15 +1184,17 @@ public function getInfo() */ public function getFillingPercentage() { - $maxMem = $this->_redis->config('GET', 'maxmemory'); + try { + $maxMem = $this->_redis->config('GET', 'maxmemory'); + } catch (CredisException $e) { + throw new Zend_Cache_Exception($e->getMessage(), 0, $e); + } if (0 == (int) $maxMem['maxmemory']) { return 1; } $info = $this->_redis->info(); return (int) round( - ($info['used_memory']/$maxMem['maxmemory']*100), - 0, - PHP_ROUND_HALF_UP + ($info['used_memory']/$maxMem['maxmemory']*100) ); } @@ -1200,13 +1206,17 @@ public function getFillingPercentage() */ public function getHitMissPercentage() { - $info = $this->_redis->info(); + try { + $info = $this->_redis->info(); + } catch (CredisException $e) { + throw new Zend_Cache_Exception($e->getMessage(), 0, $e); + } $hits = $info['keyspace_hits']; $misses = $info['keyspace_misses']; $total = $misses+$hits; $percentage = 0; if ($total > 0) { - $percentage = round($hits*100/$total, 0); + $percentage = round($hits*100/$total); } return $percentage; } diff --git a/README.md b/README.md index 76a6570..27ddd04 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Works with any Zend Framework project including all versions of Magento! - Uses the [phpredis PECL extension](https://github.com/nicolasff/phpredis) for best performance (requires **master** branch or tagged version newer than Aug 19 2011). - Falls back to standalone PHP if phpredis isn't available using the [Credis](https://github.com/colinmollenhour/credis) library. - - Tagging is fully supported, implemented using the Redis "set" and "hash" datatypes for efficient tag management. + - Tagging is fully supported, implemented using the Redis "set" and "hash" data types for efficient tag management. - Key expiration is handled automatically by Redis. - Supports unix socket connection for even better performance on a single machine. - Supports configurable compression for memory savings. Can choose between gzip, lzf and snappy and can change configuration without flushing cache. @@ -182,9 +182,9 @@ to read from rather than using md5 hash of the keys. # TUNING - - The recommended "maxmemory-policy" is "volatile-lru". All tag metadata is non-volatile so it is + - The recommended "maxmemory-policy" is "volatile-lru". All tag metadata is non-volatile, so it is recommended to use key expirations unless non-volatile keys are absolutely necessary so that tag - data cannot get evicted. So, be sure that the "maxmemory" is high enough to accommodate all of + data cannot get evicted. So, be sure that the "maxmemory" is high enough to accommodate all the tag data and non-volatile data with enough room left for the volatile key data as well. - Automatic cleaning is optional and not recommended since it is slow and uses lots of memory. - Occasional (e.g. once a day) garbage collection is recommended if the entire cache is infrequently cleared and @@ -201,8 +201,8 @@ to read from rather than using md5 hash of the keys. - Enable persistent connections. Make sure that if you have multiple configurations connecting the persistent string is unique for each configuration so that "select" commands don't cause conflicts. - Increase your server's `lua-time-limit` if you are getting "BUSY" errors. This setting can also cause Redis Sentinel - to invoke failovers when you would probably prefer to let the Lua script finish and have clients wait a little longer. - - Use the `stats.php` script to inspect your cache to find oversized or wasteful cache tags. + to invoke fail-overs when you would probably prefer to let the Lua script finish and have clients wait a little longer. + - Use the `stats.php` script to inspect your cache to find over-sized or wasteful cache tags. ### Example Garbage Collection Script (Magento) diff --git a/stats.php b/stats.php index 1a829b0..901948b 100644 --- a/stats.php +++ b/stats.php @@ -1,5 +1,7 @@ select($db); @@ -90,10 +86,10 @@ function printStats($data, $key, $limit) } // Top 20 by total size -printStats($tagStats, 'total size', $limit, true); +printStats($tagStats, 'total size', $limit); // Top 20 by average size -printStats($tagStats, 'avg size', $limit, true); +printStats($tagStats, 'avg size', $limit); // Top 20 by count -printStats($tagStats, 'count', $limit, true); +printStats($tagStats, 'count', $limit); diff --git a/tests/CommonBackendTest.php b/tests/CommonBackendTest.php index e08d062..c57a2b5 100644 --- a/tests/CommonBackendTest.php +++ b/tests/CommonBackendTest.php @@ -15,10 +15,10 @@ public function __construct($name = null, array $data = array(), $dataName = '') parent::__construct($name, $data, $dataName); } - public function setUp($notag = false): void + public function setUp($noTag = false): void { $this->_instance->setDirectives(array('logging' => false)); - if ($notag) { + if ($noTag) { $this->_instance->save('bar : data to cache', 'bar'); $this->_instance->save('bar2 : data to cache', 'bar2'); $this->_instance->save('bar3 : data to cache', 'bar3'); @@ -36,25 +36,14 @@ public function tearDown(): void public function testConstructorBadOption(): void { - $this->expectNotToPerformAssertions(); - try { - $class = $this->_className; - $test = new $class(array(1 => 'bar')); - } catch (Zend_Cache_Exception $e) { - return; - } - $this->fail('Zend_Cache_Exception was expected but not thrown'); + $this->expectException('Zend_Cache_Exception'); + new Cm_Cache_Backend_Redis(array(1 => 'bar')); } public function testSetDirectivesBadArgument(): void { - $this->expectNotToPerformAssertions(); - try { - $this->_instance->setDirectives('foo'); - } catch (Zend_Cache_Exception $e) { - return; - } - $this->fail('Zend_Cache_Exception was expected but not thrown'); + $this->expectException('Zend_Cache_Exception'); + $this->_instance->setDirectives('foo'); } public function testSetDirectivesBadDirective(): void @@ -67,13 +56,8 @@ public function testSetDirectivesBadDirective(): void public function testSetDirectivesBadDirective2(): void { - $this->expectNotToPerformAssertions(); - try { - $this->_instance->setDirectives(array('foo' => true, 12 => 3600)); - } catch (Zend_Cache_Exception $e) { - return; - } - $this->fail('Zend_Cache_Exception was expected but not thrown'); + $this->expectException('Zend_Cache_Exception'); + $this->_instance->setDirectives(array('foo' => true, 12 => 3600)); } public function testSaveCorrectCall(): void diff --git a/tests/CommonExtendedBackendTest.php b/tests/CommonExtendedBackendTest.php index 8d09e67..d00532c 100644 --- a/tests/CommonExtendedBackendTest.php +++ b/tests/CommonExtendedBackendTest.php @@ -7,14 +7,9 @@ abstract class CommonExtendedBackendTest extends CommonBackendTest { private $_capabilities; - public function __construct($name = null, array $data = array(), $dataName = '') + public function setUp($noTag = false): void { - parent::__construct($name); - } - - public function setUp($notag = false): void - { - parent::setUp($notag); + parent::setUp($noTag); $this->_capabilities = $this->_instance->getCapabilities(); } @@ -28,7 +23,7 @@ public function testGetFillingPercentage(): void public function testGetFillingPercentageOnEmptyBackend(): void { - $this->_instance->clean(Zend_Cache::CLEANING_MODE_ALL); + $this->_instance->clean(); $res = $this->_instance->getFillingPercentage(); $this->assertTrue(is_integer($res)); $this->assertTrue($res >= 0); @@ -55,7 +50,7 @@ public function testGetTags(): void return; } $res = $this->_instance->getTags(); - $this->assertEquals(4, count($res)); + $this->assertCount(4, $res); $this->assertTrue(in_array('tag1', $res)); $this->assertTrue(in_array('tag2', $res)); $this->assertTrue(in_array('tag3', $res)); @@ -93,7 +88,7 @@ public function testGetIdsMatchingTags3(): void return; } $res = $this->_instance->getIdsMatchingTags(array('tag9999')); - $this->assertTrue(count($res) == 0); + $this->assertEmpty($res); } @@ -115,7 +110,7 @@ public function testGetIdsNotMatchingTags(): void return; } $res = $this->_instance->getIdsNotMatchingTags(array('tag3')); - $this->assertEquals(0, count($res)); + $this->assertCount(0, $res); } public function testGetIdsNotMatchingTags2(): void @@ -141,14 +136,14 @@ public function testGetIdsNotMatchingTags3(): void $this->assertTrue(in_array('bar3', $res)); } - public function testGetMetadatas($notag = false) + public function testGetMetadatas($noTag = false) { $res = $this->_instance->getMetadatas('bar'); $this->assertTrue(isset($res['tags'])); $this->assertTrue(isset($res['mtime'])); $this->assertTrue(isset($res['expire'])); - if ($notag) { - $this->assertTrue(count($res['tags']) == 0); + if ($noTag) { + $this->assertEmpty($res['tags']); } else { $this->assertTrue(count($res['tags']) == 2); $this->assertTrue(in_array('tag3', $res['tags'])); diff --git a/tests/RedisBackendTest.php b/tests/RedisBackendTest.php index e4aff14..ad9c6e0 100644 --- a/tests/RedisBackendTest.php +++ b/tests/RedisBackendTest.php @@ -50,7 +50,7 @@ public function __construct($name = null, array $data = array(), $dataName = '') parent::__construct('Cm_Cache_Backend_Redis', $data, $dataName); } - public function setUp($notag = false): void + public function setUp($noTag = false): void { $this->_instance = new Cm_Cache_Backend_Redis(array( 'server' => getenv('REDIS_SERVER') ?: 'localhost', @@ -65,9 +65,9 @@ public function setUp($notag = false): void 'auto_expire_lifetime' => $this->autoExpireLifetime, 'auto_expire_refresh_on_load' => $this->autoExpireRefreshOnLoad, )); - $this->_instance->clean(Zend_Cache::CLEANING_MODE_ALL); + $this->_instance->clean(); $this->_instance->___scriptFlush(); - parent::setUp($notag); + parent::setUp($noTag); } public function tearDown(): void @@ -114,20 +114,20 @@ public function testExpiredCleanup(): void public function testGetIdsMatchingAnyTags(): void { $res = $this->_instance->getIdsMatchingAnyTags(array('tag999')); - $this->assertEquals(0, count($res)); + $this->assertCount(0, $res); } public function testGetIdsMatchingAnyTags2(): void { $res = $this->_instance->getIdsMatchingAnyTags(array('tag1', 'tag999')); - $this->assertEquals(1, count($res)); + $this->assertCount(1, $res); $this->assertTrue(in_array('bar2', $res)); } public function testGetIdsMatchingAnyTags3(): void { $res = $this->_instance->getIdsMatchingAnyTags(array('tag3', 'tag999')); - $this->assertEquals(3, count($res)); + $this->assertCount(3, $res); $this->assertTrue(in_array('bar', $res)); $this->assertTrue(in_array('bar2', $res)); $this->assertTrue(in_array('bar3', $res)); @@ -136,7 +136,7 @@ public function testGetIdsMatchingAnyTags3(): void public function testGetIdsMatchingAnyTags4(): void { $res = $this->_instance->getIdsMatchingAnyTags(array('tag1', 'tag4')); - $this->assertEquals(2, count($res)); + $this->assertCount(2, $res); $this->assertTrue(in_array('bar', $res)); $this->assertTrue(in_array('bar2', $res)); } @@ -181,7 +181,7 @@ public function testCleanModeMatchingAnyTags5(): void } $this->assertGreaterThan(self::LUA_MAX_C_STACK, count($this->_instance->getIdsMatchingAnyTags($tags))); $this->_instance->clean(Zend_Cache::CLEANING_MODE_MATCHING_ANY_TAG, $tags); - $this->assertEquals(0, count($this->_instance->getIdsMatchingAnyTags($tags))); + $this->assertCount(0, $this->_instance->getIdsMatchingAnyTags($tags)); } public function testCleanModeMatchingAnyTags6(): void @@ -192,9 +192,9 @@ public function testCleanModeMatchingAnyTags6(): void } $this->_instance->save('foo', 'foo', $tags); $_tags = array(end($tags)); - $this->assertEquals(1, count($this->_instance->getIdsMatchingAnyTags($_tags))); + $this->assertCount(1, $this->_instance->getIdsMatchingAnyTags($_tags)); $this->_instance->clean(Zend_Cache::CLEANING_MODE_MATCHING_ANY_TAG, $_tags); - $this->assertEquals(0, count($this->_instance->getIdsMatchingAnyTags($_tags))); + $this->assertCount(0, $this->_instance->getIdsMatchingAnyTags($_tags)); } public function testScriptsCaching(): void