From 3f7e4f5bb67c67586423d9e1105ce274b83767c3 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Thu, 28 Jun 2018 22:57:17 +0200 Subject: [PATCH] redesign of locking & caching - New Factory "CacheDriverFactory" for Cache and Locks - Adding Redis/Memcached Locking - Moved Lock to Core - other improvements --- src/Core/Cache.php | 27 +--- src/Core/Cache/CacheDriverFactory.php | 48 ++++++ src/Core/Lock.php | 145 ++++++++++++++++++ src/Core/Lock/AbstractLockDriver.php | 57 +++++++ .../Lock/CacheLockDriver.php} | 51 +++--- .../Lock/DatabaseLockDriver.php | 20 ++- src/{Util => Core}/Lock/ILockDriver.php | 10 +- src/Core/Lock/SemaphoreLockDriver.php | 68 ++++++++ src/Core/Worker.php | 2 +- src/Protocol/OStatus.php | 2 +- src/Util/Lock.php | 97 ------------ src/Util/Lock/SemaphoreLockDriver.php | 82 ---------- 12 files changed, 369 insertions(+), 240 deletions(-) create mode 100644 src/Core/Cache/CacheDriverFactory.php create mode 100644 src/Core/Lock.php create mode 100644 src/Core/Lock/AbstractLockDriver.php rename src/{Util/Lock/MemcacheLockDriver.php => Core/Lock/CacheLockDriver.php} (61%) rename src/{Util => Core}/Lock/DatabaseLockDriver.php (79%) rename src/{Util => Core}/Lock/ILockDriver.php (72%) create mode 100644 src/Core/Lock/SemaphoreLockDriver.php delete mode 100644 src/Util/Lock.php delete mode 100644 src/Util/Lock/SemaphoreLockDriver.php diff --git a/src/Core/Cache.php b/src/Core/Cache.php index 4202db325c..cc77d9bfd0 100644 --- a/src/Core/Cache.php +++ b/src/Core/Cache.php @@ -4,8 +4,7 @@ */ namespace Friendica\Core; -use Friendica\Core\Cache; -use Friendica\Core\Config; +use Friendica\Core\Cache\CacheDriverFactory; /** * @brief Class for storing data for a short time @@ -24,31 +23,13 @@ class Cache extends \Friendica\BaseObject /** * @var Cache\ICacheDriver */ - static $driver = null; + private static $driver = null; public static function init() { - switch(Config::get('system', 'cache_driver', 'database')) { - case 'memcache': - $memcache_host = Config::get('system', 'memcache_host', '127.0.0.1'); - $memcache_port = Config::get('system', 'memcache_port', 11211); + $driver_name = Config::get('system', 'cache_driver', 'database'); - self::$driver = new Cache\MemcacheCacheDriver($memcache_host, $memcache_port); - break; - case 'memcached': - $memcached_hosts = Config::get('system', 'memcached_hosts', [['127.0.0.1', 11211]]); - - self::$driver = new Cache\MemcachedCacheDriver($memcached_hosts); - break; - case 'redis': - $redis_host = Config::get('system', 'redis_host', '127.0.0.1'); - $redis_port = Config::get('system', 'redis_port', 6379); - - self::$driver = new Cache\RedisCacheDriver($redis_host, $redis_port); - break; - default: - self::$driver = new Cache\DatabaseCacheDriver(); - } + self::$driver = CacheDriverFactory::create($driver_name); } /** diff --git a/src/Core/Cache/CacheDriverFactory.php b/src/Core/Cache/CacheDriverFactory.php new file mode 100644 index 0000000000..d15b8e9858 --- /dev/null +++ b/src/Core/Cache/CacheDriverFactory.php @@ -0,0 +1,48 @@ +getMessage()); + } + } + + // 2. Try to use Cache Locking (don't use the DB-Cache Locking because it works different!) + $cache_driver = Config::get('system', 'cache_driver', 'database'); + if ($cache_driver != 'database') { + try { + $lock_driver = CacheDriverFactory::create($cache_driver); + self::$driver = new Lock\CacheLockDriver($lock_driver); + return; + } catch (\Exception $exception) { + logger('Using Cache driver for locking failed: ' . $exception->getMessage()); + } + } + + // 3. Use Database Locking as a Fallback + self::$driver = new Lock\DatabaseLockDriver(); + } + + /** + * Returns the current cache driver + * + * @return Lock\ILockDriver; + */ + private static function getDriver() + { + if (self::$driver === null) { + self::init(); + } + + return self::$driver; + } + + /** + * @brief Acquires a lock for a given name + * + * @param string $key Name of the lock + * @param integer $timeout Seconds until we give up + * + * @return boolean Was the lock successful? + */ + public static function acquireLock($key, $timeout = 120) + { + return self::getDriver()->acquireLock($key, $timeout); + } + + /** + * @brief Releases a lock if it was set by us + * + * @param string $key Name of the lock + * @return mixed + */ + public static function releaseLock($key) + { + return self::getDriver()->releaseLock($key); + } + + /** + * @brief Releases all lock that were set by us + * @return void + */ + public static function releaseAll() + { + self::getDriver()->releaseAll(); + } +} diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php new file mode 100644 index 0000000000..15820c3782 --- /dev/null +++ b/src/Core/Lock/AbstractLockDriver.php @@ -0,0 +1,57 @@ +acquireLock[$key]); + } + + /** + * @brief Mark a locally acquired lock + * + * @param string $key The Name of the lock + */ + protected function markAcquire(string $key) { + $this->acquiredLocks[$key] = true; + } + + /** + * @brief Mark a release of a locally acquired lock + * + * @param string $key The Name of the lock + */ + protected function markRelease(string $key) { + unset($this->acquiredLocks[$key]); + } + + /** + * @brief Releases all lock that were set by us + * + * @return void + */ + public function releaseAll() { + foreach ($this->acquiredLocks as $acquiredLock) { + $this->releaseLock($acquiredLock); + } + } +} diff --git a/src/Util/Lock/MemcacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php similarity index 61% rename from src/Util/Lock/MemcacheLockDriver.php rename to src/Core/Lock/CacheLockDriver.php index d0789a95c4..0adca140d1 100644 --- a/src/Util/Lock/MemcacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -1,12 +1,26 @@ cache = $cache; + } + /** * * @brief Sets a lock for a given name @@ -16,7 +30,7 @@ class MemcacheLockDriver implements ILockDriver * * @return boolean Was the lock successful? */ - public function acquireLock($key, $timeout = 120) + public function acquireLock(string $key, int $timeout = 120) { $got_lock = false; $start = time(); @@ -24,9 +38,7 @@ class MemcacheLockDriver implements ILockDriver $cachekey = get_app()->get_hostname() . ";lock:" . $key; do { - // We only lock to be sure that nothing happens at exactly the same time - dba::lock('locks'); - $lock = Cache::get($cachekey); + $lock = $this->cache->get($cachekey); if (!is_bool($lock)) { $pid = (int)$lock; @@ -38,17 +50,17 @@ class MemcacheLockDriver implements ILockDriver } } if (is_bool($lock)) { - Cache::set($cachekey, getmypid(), 300); + $this->cache->set($cachekey, getmypid(), 300); $got_lock = true; } - dba::unlock(); - if (!$got_lock && ($timeout > 0)) { usleep(rand(10000, 200000)); } } while (!$got_lock && ((time() - $start) < $timeout)); + $this->markAcquire($key); + return $got_lock; } @@ -59,28 +71,19 @@ class MemcacheLockDriver implements ILockDriver * * @return mixed */ - public function releaseLock($key) + public function releaseLock(string $key) { $cachekey = get_app()->get_hostname() . ";lock:" . $key; - $lock = Cache::get($cachekey); + $lock = $this->cache->get($cachekey); if (!is_bool($lock)) { if ((int)$lock == getmypid()) { - Cache::delete($cachekey); + $this->cache->delete($cachekey); } } - return; - } + $this->markRelease($key); - /** - * @brief Removes all lock that were set by us - * - * @return void - */ - public function releaseAll() - { - // We cannot delete all cache entries, but this doesn't matter with memcache return; } } diff --git a/src/Util/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php similarity index 79% rename from src/Util/Lock/DatabaseLockDriver.php rename to src/Core/Lock/DatabaseLockDriver.php index b2e8f50279..f9878340cf 100644 --- a/src/Util/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -1,6 +1,6 @@ true, 'pid' => getmypid()], ['name' => $key]); $got_lock = true; } - } elseif (!DBM::is_result($lock)) { + } else { dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid()]); $got_lock = true; } @@ -54,6 +54,8 @@ class DatabaseLockDriver implements ILockDriver } } while (!$got_lock && ((time() - $start) < $timeout)); + $this->markAcquire($key); + return $got_lock; } @@ -64,9 +66,11 @@ class DatabaseLockDriver implements ILockDriver * * @return mixed */ - public function releaseLock($key) + public function releaseLock(string $key) { - dba::update('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); + dba::delete('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); + + $this->releaseLock($key); return; } @@ -78,6 +82,8 @@ class DatabaseLockDriver implements ILockDriver */ public function releaseAll() { - dba::update('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); + dba::delete('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); + + $this->acquiredLocks = []; } } diff --git a/src/Util/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php similarity index 72% rename from src/Util/Lock/ILockDriver.php rename to src/Core/Lock/ILockDriver.php index 7f5a359946..39e4ba8e89 100644 --- a/src/Util/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -1,6 +1,6 @@ acquiredLocks[$key] = sem_get(self::semaphoreKey($key)); + if ($this->acquiredLocks[$key]) { + return sem_acquire($this->acquiredLocks[$key], ($timeout == 0)); + } + } + + /** + * @brief Removes a lock if it was set by us + * + * @param string $key Name of the lock + * + * @return mixed + */ + public function releaseLock(string $key) + { + if (empty($this->acquiredLocks[$key])) { + return false; + } else { + $success = @sem_release($this->acquiredLocks[$key]); + unset($this->acquiredLocks[$key]); + return $success; + } + } +} diff --git a/src/Core/Worker.php b/src/Core/Worker.php index ef7873fcd4..cbf2ae8bd9 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -7,10 +7,10 @@ namespace Friendica\Core; use Friendica\Core\Addon; use Friendica\Core\Config; use Friendica\Core\System; +use Friendica\Core\Lock; use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; use Friendica\Util\Network; use dba; diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index a1223b7b68..288cbfed1b 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -9,6 +9,7 @@ use Friendica\Content\Text\HTML; use Friendica\Core\Cache; use Friendica\Core\Config; use Friendica\Core\L10n; +use Friendica\Core\Lock; use Friendica\Core\System; use Friendica\Database\DBM; use Friendica\Model\Contact; @@ -19,7 +20,6 @@ use Friendica\Model\User; use Friendica\Network\Probe; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; use Friendica\Util\Network; use Friendica\Util\XML; use dba; diff --git a/src/Util/Lock.php b/src/Util/Lock.php deleted file mode 100644 index b6a4a97f18..0000000000 --- a/src/Util/Lock.php +++ /dev/null @@ -1,97 +0,0 @@ -=')) { - self::$driver = new Lock\SemaphoreLockDriver(); - } elseif (Config::get('system', 'cache_driver', 'database') == 'memcache') { - self::$driver = new Lock\MemcacheLockDriver(); - } else { - self::$driver = new Lock\DatabaseLockDriver(); - } - } - } - - /** - * Returns the current cache driver - * - * @return Lock\ILockDriver; - */ - private static function getDriver() - { - if (self::$driver === null) { - self::init(); - } - - return self::$driver; - } - - /** - * @brief Acquires a lock for a given name - * - * @param string $key Name of the lock - * @param integer $timeout Seconds until we give up - * - * @return boolean Was the lock successful? - */ - public static function acquireLock($key, $timeout = 120) - { - return self::getDriver()->acquireLock($key, $timeout); - } - - /** - * @brief Releases a lock if it was set by us - * - * @param string $key Name of the lock - * @return mixed - */ - public static function releaseLock($key) - { - return self::getDriver()->releaseLock($key); - } - - /** - * @brief Releases all lock that were set by us - * @return void - */ - public static function releaseAll() - { - self::getDriver()->releaseAll(); - } -} diff --git a/src/Util/Lock/SemaphoreLockDriver.php b/src/Util/Lock/SemaphoreLockDriver.php deleted file mode 100644 index 1cd3af1411..0000000000 --- a/src/Util/Lock/SemaphoreLockDriver.php +++ /dev/null @@ -1,82 +0,0 @@ -=')) { - self::$semaphore[$key] = sem_get(self::semaphoreKey($key)); - if (self::$semaphore[$key]) { - return sem_acquire(self::$semaphore[$key], ($timeout == 0)); - } - } - } - - /** - * @brief Removes a lock if it was set by us - * - * @param string $key Name of the lock - * - * @return mixed - */ - public function releaseLock($key) - { - if (function_exists('sem_get') && version_compare(PHP_VERSION, '5.6.1', '>=')) { - if (empty(self::$semaphore[$key])) { - return false; - } else { - $success = @sem_release(self::$semaphore[$key]); - unset(self::$semaphore[$key]); - return $success; - } - } - } - - /** - * @brief Removes all lock that were set by us - * - * @return void - */ - public function releaseAll() - { - // not needed/supported - return; - } -}