Bugfixings for Cache-Lock

- used wrong cachekey in set
- therefore added an abstraction to avoid wrong key concatenation
- forgot to increase the db-version to 1275
This commit is contained in:
Philipp Holzer 2018-07-05 21:47:52 +02:00
parent 3be013361e
commit 34cea93a8b
No known key found for this signature in database
GPG key ID: 58160D7D6AF942B6
7 changed files with 72 additions and 35 deletions

View file

@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM', 'Friendica');
define('FRIENDICA_CODENAME', 'The Tazmans Flax-lily'); define('FRIENDICA_CODENAME', 'The Tazmans Flax-lily');
define('FRIENDICA_VERSION', '2018.08-dev'); define('FRIENDICA_VERSION', '2018.08-dev');
define('DFRN_PROTOCOL_VERSION', '2.23'); define('DFRN_PROTOCOL_VERSION', '2.23');
define('DB_UPDATE_VERSION', 1274); define('DB_UPDATE_VERSION', 1275);
define('NEW_UPDATE_ROUTINE_VERSION', 1170); define('NEW_UPDATE_ROUTINE_VERSION', 1170);
/** /**

View file

@ -0,0 +1,23 @@
<?php
namespace Friendica\Core\Cache;
use Friendica\BaseObject;
/**
* Abstract class for common used functions
*
* Class AbstractCacheDriver
*
* @package Friendica\Core\Cache
*/
abstract class AbstractCacheDriver extends BaseObject implements IMemoryCacheDriver
{
/**
* @param string $key The original key
* @return string The cache key used for the cache
*/
protected function getCacheKey($key) {
return self::getApp()->get_hostname() . ":" . $key;
}
}

View file

@ -12,7 +12,7 @@ use Friendica\Core\Cache;
* *
* @package Friendica\Core\Cache * @package Friendica\Core\Cache
*/ */
class ArrayCache implements IMemoryCacheDriver class ArrayCache extends AbstractCacheDriver
{ {
use TraitCompareDelete; use TraitCompareDelete;

View file

@ -2,7 +2,6 @@
namespace Friendica\Core\Cache; namespace Friendica\Core\Cache;
use Friendica\BaseObject;
use Friendica\Core\Cache; use Friendica\Core\Cache;
/** /**
@ -10,7 +9,7 @@ use Friendica\Core\Cache;
* *
* @author Hypolite Petovan <mrpetovan@gmail.com> * @author Hypolite Petovan <mrpetovan@gmail.com>
*/ */
class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver class MemcacheCacheDriver extends AbstractCacheDriver
{ {
use TraitCompareSet; use TraitCompareSet;
use TraitCompareDelete; use TraitCompareDelete;
@ -39,9 +38,10 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver
public function get($key) public function get($key)
{ {
$return = null; $return = null;
$cachekey = $this->getCacheKey($key);
// We fetch with the hostname as key to avoid problems with other applications // We fetch with the hostname as key to avoid problems with other applications
$cached = $this->memcache->get(self::getApp()->get_hostname() . ':' . $key); $cached = $this->memcache->get($cachekey);
// @see http://php.net/manual/en/memcache.get.php#84275 // @see http://php.net/manual/en/memcache.get.php#84275
if (is_bool($cached) || is_double($cached) || is_long($cached)) { if (is_bool($cached) || is_double($cached) || is_long($cached)) {
@ -65,17 +65,19 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function set($key, $value, $ttl = Cache::FIVE_MINUTES) public function set($key, $value, $ttl = Cache::FIVE_MINUTES)
{ {
$cachekey = $this->getCacheKey($key);
// We store with the hostname as key to avoid problems with other applications // We store with the hostname as key to avoid problems with other applications
if ($ttl > 0) { if ($ttl > 0) {
return $this->memcache->set( return $this->memcache->set(
self::getApp()->get_hostname() . ":" . $key, $cachekey,
serialize($value), serialize($value),
MEMCACHE_COMPRESSED, MEMCACHE_COMPRESSED,
time() + $ttl time() + $ttl
); );
} else { } else {
return $this->memcache->set( return $this->memcache->set(
self::getApp()->get_hostname() . ":" . $key, $cachekey,
serialize($value), serialize($value),
MEMCACHE_COMPRESSED MEMCACHE_COMPRESSED
); );
@ -87,7 +89,8 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function delete($key) public function delete($key)
{ {
return $this->memcache->delete($key); $cachekey = $this->getCacheKey($key);
return $this->memcache->delete($cachekey);
} }
/** /**
@ -103,6 +106,7 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function add($key, $value, $ttl = Cache::FIVE_MINUTES) public function add($key, $value, $ttl = Cache::FIVE_MINUTES)
{ {
return $this->memcache->add(self::getApp()->get_hostname() . ":" . $key, $value, $ttl); $cachekey = $this->getCacheKey($key);
return $this->memcache->add($cachekey, $value, $ttl);
} }
} }

View file

@ -2,7 +2,6 @@
namespace Friendica\Core\Cache; namespace Friendica\Core\Cache;
use Friendica\BaseObject;
use Friendica\Core\Cache; use Friendica\Core\Cache;
/** /**
@ -10,7 +9,7 @@ use Friendica\Core\Cache;
* *
* @author Hypolite Petovan <mrpetovan@gmail.com> * @author Hypolite Petovan <mrpetovan@gmail.com>
*/ */
class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver class MemcachedCacheDriver extends AbstractCacheDriver
{ {
use TraitCompareSet; use TraitCompareSet;
use TraitCompareDelete; use TraitCompareDelete;
@ -38,9 +37,10 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver
public function get($key) public function get($key)
{ {
$return = null; $return = null;
$cachekey = $this->getCacheKey($key);
// We fetch with the hostname as key to avoid problems with other applications // We fetch with the hostname as key to avoid problems with other applications
$value = $this->memcached->get(self::getApp()->get_hostname() . ':' . $key); $value = $this->memcached->get($cachekey);
if ($this->memcached->getResultCode() === \Memcached::RES_SUCCESS) { if ($this->memcached->getResultCode() === \Memcached::RES_SUCCESS) {
$return = $value; $return = $value;
@ -51,16 +51,18 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver
public function set($key, $value, $ttl = Cache::FIVE_MINUTES) public function set($key, $value, $ttl = Cache::FIVE_MINUTES)
{ {
$cachekey = $this->getCacheKey($key);
// We store with the hostname as key to avoid problems with other applications // We store with the hostname as key to avoid problems with other applications
if ($ttl > 0) { if ($ttl > 0) {
return $this->memcached->set( return $this->memcached->set(
self::getApp()->get_hostname() . ':' . $key, $cachekey,
$value, $value,
time() + $ttl time() + $ttl
); );
} else { } else {
return $this->memcached->set( return $this->memcached->set(
self::getApp()->get_hostname() . ':' . $key, $cachekey,
$value $value
); );
} }
@ -69,9 +71,8 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver
public function delete($key) public function delete($key)
{ {
$return = $this->memcached->delete(self::getApp()->get_hostname() . ':' . $key); $cachekey = $this->getCacheKey($key);
return $this->memcached->delete($cachekey);
return $return;
} }
public function clear() public function clear()
@ -89,6 +90,7 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function add($key, $value, $ttl = Cache::FIVE_MINUTES) public function add($key, $value, $ttl = Cache::FIVE_MINUTES)
{ {
return $this->memcached->add(self::getApp()->get_hostname() . ":" . $key, $value, $ttl); $cachekey = $this->getCacheKey($key);
return $this->memcached->add($cachekey, $value, $ttl);
} }
} }

View file

@ -2,7 +2,6 @@
namespace Friendica\Core\Cache; namespace Friendica\Core\Cache;
use Friendica\BaseObject;
use Friendica\Core\Cache; use Friendica\Core\Cache;
/** /**
@ -11,7 +10,7 @@ use Friendica\Core\Cache;
* @author Hypolite Petovan <mrpetovan@gmail.com> * @author Hypolite Petovan <mrpetovan@gmail.com>
* @author Roland Haeder <roland@mxchange.org> * @author Roland Haeder <roland@mxchange.org>
*/ */
class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver class RedisCacheDriver extends AbstractCacheDriver
{ {
/** /**
* @var \Redis * @var \Redis
@ -34,9 +33,10 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver
public function get($key) public function get($key)
{ {
$return = null; $return = null;
$cachekey = $this->getCacheKey($key);
// We fetch with the hostname as key to avoid problems with other applications // We fetch with the hostname as key to avoid problems with other applications
$cached = $this->redis->get(self::getApp()->get_hostname() . ':' . $key); $cached = $this->redis->get($cachekey);
// @see http://php.net/manual/en/redis.get.php#84275 // @see http://php.net/manual/en/redis.get.php#84275
if (is_bool($cached) || is_double($cached) || is_long($cached)) { if (is_bool($cached) || is_double($cached) || is_long($cached)) {
@ -57,16 +57,18 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver
public function set($key, $value, $ttl = Cache::FIVE_MINUTES) public function set($key, $value, $ttl = Cache::FIVE_MINUTES)
{ {
$cachekey = $this->getCacheKey($key);
// We store with the hostname as key to avoid problems with other applications // We store with the hostname as key to avoid problems with other applications
if ($ttl > 0) { if ($ttl > 0) {
return $this->redis->setex( return $this->redis->setex(
self::getApp()->get_hostname() . ":" . $key, $cachekey,
time() + $ttl, time() + $ttl,
serialize($value) serialize($value)
); );
} else { } else {
return $this->redis->set( return $this->redis->set(
self::getApp()->get_hostname() . ":" . $key, $cachekey,
serialize($value) serialize($value)
); );
} }
@ -88,11 +90,13 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function add($key, $value, $ttl = Cache::FIVE_MINUTES) public function add($key, $value, $ttl = Cache::FIVE_MINUTES)
{ {
$cachekey = $this->getCacheKey($key);
if (!is_int($value)) { if (!is_int($value)) {
$value = serialize($value); $value = serialize($value);
} }
return $this->redis->setnx(self::getApp()->get_hostname() . ":" . $key, $value); return $this->redis->setnx($cachekey, $value);
} }
/** /**
@ -100,20 +104,22 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES) public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES)
{ {
$cachekey = $this->getCacheKey($key);
if (!is_int($newValue)) { if (!is_int($newValue)) {
$newValue = serialize($newValue); $newValue = serialize($newValue);
} }
$this->redis->watch(self::getApp()->get_hostname() . ":" . $key); $this->redis->watch($cachekey);
// If the old value isn't what we expected, somebody else changed the key meanwhile // If the old value isn't what we expected, somebody else changed the key meanwhile
if ($this->get($key) === $oldValue) { if ($this->get($cachekey) === $oldValue) {
if ($ttl > 0) { if ($ttl > 0) {
$result = $this->redis->multi() $result = $this->redis->multi()
->setex(self::getApp()->get_hostname() . ":" . $ttl, $key, $newValue) ->setex($cachekey, $ttl, $newValue)
->exec(); ->exec();
} else { } else {
$result = $this->redis->multi() $result = $this->redis->multi()
->set(self::getApp()->get_hostname() . ":" . $key, $newValue) ->set($cachekey, $newValue)
->exec(); ->exec();
} }
return $result !== false; return $result !== false;
@ -126,11 +132,13 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver
*/ */
public function compareDelete($key, $value) public function compareDelete($key, $value)
{ {
$this->redis->watch(self::getApp()->get_hostname() . ":" . $key); $cachekey = $this->getCacheKey($key);
$this->redis->watch($cachekey);
// If the old value isn't what we expected, somebody else changed the key meanwhile // If the old value isn't what we expected, somebody else changed the key meanwhile
if ($this->get($key) === $value) { if ($this->get($key) === $value) {
$result = $this->redis->multi() $result = $this->redis->multi()
->del(self::getApp()->get_hostname() . ":" . $key) ->del($cachekey)
->exec(); ->exec();
return $result !== false; return $result !== false;
} }

View file

@ -29,7 +29,7 @@ class CacheLockDriver extends AbstractLockDriver
$got_lock = false; $got_lock = false;
$start = time(); $start = time();
$cachekey = self::getCacheKey($key); $cachekey = self::getLockKey($key);
do { do {
$lock = $this->cache->get($cachekey); $lock = $this->cache->get($cachekey);
@ -62,7 +62,7 @@ class CacheLockDriver extends AbstractLockDriver
*/ */
public function releaseLock($key) public function releaseLock($key)
{ {
$cachekey = self::getCacheKey($key); $cachekey = self::getLockKey($key);
$this->cache->compareDelete($cachekey, getmypid()); $this->cache->compareDelete($cachekey, getmypid());
$this->markRelease($key); $this->markRelease($key);
@ -73,7 +73,7 @@ class CacheLockDriver extends AbstractLockDriver
*/ */
public function isLocked($key) public function isLocked($key)
{ {
$cachekey = self::getCacheKey($key); $cachekey = self::getLockKey($key);
$lock = $this->cache->get($cachekey); $lock = $this->cache->get($cachekey);
return isset($lock) && ($lock !== false); return isset($lock) && ($lock !== false);
} }
@ -82,7 +82,7 @@ class CacheLockDriver extends AbstractLockDriver
* @param string $key The original key * @param string $key The original key
* @return string The cache key used for the cache * @return string The cache key used for the cache
*/ */
private static function getCacheKey($key) { private static function getLockKey($key) {
return self::getApp()->get_hostname() . ";lock:" . $key; return "lock:" . $key;
} }
} }