From 1bc56a99e9e368091753ff79facf930336478a35 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 8 Jun 2015 15:22:20 +0200 Subject: [PATCH 1/4] compare-and-set and compare-and-delete using lua scripts for redis --- lib/private/memcache/redis.php | 52 ++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/private/memcache/redis.php b/lib/private/memcache/redis.php index cfc35dcc37..1ef9417eaf 100644 --- a/lib/private/memcache/redis.php +++ b/lib/private/memcache/redis.php @@ -26,13 +26,15 @@ namespace OC\Memcache; use OCP\IMemcache; class Redis extends Cache implements IMemcache { - use CASTrait; - /** * @var \Redis $cache */ private static $cache = null; + private static $casScript; + + private static $cadScript; + public function __construct($prefix = '') { parent::__construct($prefix); if (is_null(self::$cache)) { @@ -57,6 +59,29 @@ class Redis extends Cache implements IMemcache { self::$cache->connect($host, $port, $timeout); + self::$casScript = self::$cache->script('load', ' +local key = ARGV[1] +local old = ARGV[2] +local new = ARGV[3] + +if redis.call(\'get\', key) == old then + redis.call(\'set\', key, new) + return true +end + +return false'); + + self::$cadScript = self::$cache->script('load', ' +local key = ARGV[1] +local old = ARGV[2] + +if redis.call(\'get\', key) == old then + redis.call(\'del\', key) + return true +end + +return false'); + if (isset($config['dbindex'])) { self::$cache->select($config['dbindex']); } @@ -150,6 +175,29 @@ class Redis extends Cache implements IMemcache { return self::$cache->decrBy($this->getNamespace() . $key, $step); } + /** + * Compare and set + * + * @param string $key + * @param mixed $old + * @param mixed $new + * @return bool + */ + public function cas($key, $old, $new) { + return (bool) self::$cache->evalSha(self::$casScript, [$this->getNamespace() . $key, json_encode($old), json_encode($new)]); + } + + /** + * Compare and delete + * + * @param string $key + * @param mixed $old + * @return bool + */ + public function cad($key, $old) { + return (bool)self::$cache->evalSha(self::$cadScript, [$this->getNamespace() . $key, json_encode($old)]); + } + static public function isAvailable() { return extension_loaded('redis') && version_compare(phpversion('redis'), '2.2.5', '>='); From ac9f998abdceb21fbbda4e8dee0e8b16c2f5ec4e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 8 Jun 2015 15:25:23 +0200 Subject: [PATCH 2/4] add compare-and-delete to the memcache interface --- lib/private/memcache/apc.php | 2 ++ lib/private/memcache/arraycache.php | 2 ++ lib/private/memcache/cadtrait.php | 53 +++++++++++++++++++++++++++++ lib/private/memcache/memcached.php | 2 ++ lib/private/memcache/nullcache.php | 4 +++ lib/private/memcache/xcache.php | 2 ++ lib/public/imemcache.php | 10 ++++++ tests/lib/memcache/cache.php | 12 +++++++ 8 files changed, 87 insertions(+) create mode 100644 lib/private/memcache/cadtrait.php diff --git a/lib/private/memcache/apc.php b/lib/private/memcache/apc.php index 50b942e729..f768cdc1c6 100644 --- a/lib/private/memcache/apc.php +++ b/lib/private/memcache/apc.php @@ -31,6 +31,8 @@ class APC extends Cache implements IMemcache { cas as casEmulated; } + use CADTrait; + public function get($key) { $result = apc_fetch($this->getPrefix() . $key, $success); if (!$success) { diff --git a/lib/private/memcache/arraycache.php b/lib/private/memcache/arraycache.php index 2b1b87a9eb..8a3fdd2f7c 100644 --- a/lib/private/memcache/arraycache.php +++ b/lib/private/memcache/arraycache.php @@ -28,6 +28,8 @@ class ArrayCache extends Cache implements IMemcache { /** @var array Array with the cached data */ protected $cachedData = array(); + use CADTrait; + /** * {@inheritDoc} */ diff --git a/lib/private/memcache/cadtrait.php b/lib/private/memcache/cadtrait.php new file mode 100644 index 0000000000..e9836e2404 --- /dev/null +++ b/lib/private/memcache/cadtrait.php @@ -0,0 +1,53 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Memcache; + +trait CADTrait { + abstract public function get($key); + + abstract public function remove($key); + + abstract public function add($key, $value, $ttl = 0); + + /** + * Compare and delete + * + * @param string $key + * @param mixed $old + * @return bool + */ + public function cad($key, $old) { + //no native cas, emulate with locking + if ($this->add($key . '_lock', true)) { + if ($this->get($key) === $old) { + $this->remove($key); + $this->remove($key . '_lock'); + return true; + } else { + $this->remove($key . '_lock'); + return false; + } + } else { + return false; + } + } +} diff --git a/lib/private/memcache/memcached.php b/lib/private/memcache/memcached.php index cf1d651b55..1503851fd7 100644 --- a/lib/private/memcache/memcached.php +++ b/lib/private/memcache/memcached.php @@ -34,6 +34,8 @@ class Memcached extends Cache implements IMemcache { */ private static $cache = null; + use CADTrait; + public function __construct($prefix = '') { parent::__construct($prefix); if (is_null(self::$cache)) { diff --git a/lib/private/memcache/nullcache.php b/lib/private/memcache/nullcache.php index 77eadba4eb..f971ffc9b2 100644 --- a/lib/private/memcache/nullcache.php +++ b/lib/private/memcache/nullcache.php @@ -55,6 +55,10 @@ class NullCache extends Cache implements \OCP\IMemcache { return true; } + public function cad($key, $old) { + return true; + } + public function clear($prefix = '') { return true; } diff --git a/lib/private/memcache/xcache.php b/lib/private/memcache/xcache.php index 0be79d06ed..a6265ed562 100644 --- a/lib/private/memcache/xcache.php +++ b/lib/private/memcache/xcache.php @@ -34,6 +34,8 @@ use OCP\IMemcache; class XCache extends Cache implements IMemcache { use CASTrait; + use CADTrait; + /** * entries in XCache gets namespaced to prevent collisions between ownCloud instances and users */ diff --git a/lib/public/imemcache.php b/lib/public/imemcache.php index f8b898e54c..a1a00791b6 100644 --- a/lib/public/imemcache.php +++ b/lib/public/imemcache.php @@ -76,4 +76,14 @@ interface IMemcache extends ICache { * @since 8.1.0 */ public function cas($key, $old, $new); + + /** + * Compare and delete + * + * @param string $key + * @param mixed $old + * @return bool + * @since 8.1.0 + */ + public function cad($key, $old); } diff --git a/tests/lib/memcache/cache.php b/tests/lib/memcache/cache.php index 9d977cf024..3ff72ee931 100644 --- a/tests/lib/memcache/cache.php +++ b/tests/lib/memcache/cache.php @@ -103,6 +103,18 @@ abstract class Cache extends \Test_Cache { $this->assertEquals('bar1', $this->instance->get('foo')); } + public function testCadNotChanged() { + $this->instance->set('foo', 'bar'); + $this->assertTrue($this->instance->cad('foo', 'bar')); + $this->assertFalse($this->instance->hasKey('foo')); + } + + public function testCadChanged() { + $this->instance->set('foo', 'bar1'); + $this->assertFalse($this->instance->cad('foo', 'bar')); + $this->assertTrue($this->instance->hasKey('foo')); + } + protected function tearDown() { if ($this->instance) { From 520a74187639bbe73bbc4336f349586c8e19e06e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 8 Jun 2015 15:27:36 +0200 Subject: [PATCH 3/4] clear memcache keys in locking --- lib/private/lock/memcachelockingprovider.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index 3f32ab1d8c..85b62c8340 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -91,9 +91,10 @@ class MemcacheLockingProvider implements ILockingProvider { if (isset($this->acquiredLocks['shared'][$path]) and $this->acquiredLocks['shared'][$path] > 0) { $this->memcache->dec($path); $this->acquiredLocks['shared'][$path]--; + $this->memcache->cad($path, 0); } } else if ($type === self::LOCK_EXCLUSIVE) { - $this->memcache->cas($path, 'exclusive', 0); + $this->memcache->cad($path, 'exclusive'); unset($this->acquiredLocks['exclusive'][$path]); } } From a9ff242f6abd54c9f8d55d5818b658b76d7807b3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 8 Jun 2015 18:01:49 +0200 Subject: [PATCH 4/4] switch to using watch to implement cas and cad on redis --- lib/private/memcache/redis.php | 52 ++++++++++++++-------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/lib/private/memcache/redis.php b/lib/private/memcache/redis.php index 1ef9417eaf..30619c356b 100644 --- a/lib/private/memcache/redis.php +++ b/lib/private/memcache/redis.php @@ -31,10 +31,6 @@ class Redis extends Cache implements IMemcache { */ private static $cache = null; - private static $casScript; - - private static $cadScript; - public function __construct($prefix = '') { parent::__construct($prefix); if (is_null(self::$cache)) { @@ -59,29 +55,6 @@ class Redis extends Cache implements IMemcache { self::$cache->connect($host, $port, $timeout); - self::$casScript = self::$cache->script('load', ' -local key = ARGV[1] -local old = ARGV[2] -local new = ARGV[3] - -if redis.call(\'get\', key) == old then - redis.call(\'set\', key, new) - return true -end - -return false'); - - self::$cadScript = self::$cache->script('load', ' -local key = ARGV[1] -local old = ARGV[2] - -if redis.call(\'get\', key) == old then - redis.call(\'del\', key) - return true -end - -return false'); - if (isset($config['dbindex'])) { self::$cache->select($config['dbindex']); } @@ -184,7 +157,18 @@ return false'); * @return bool */ public function cas($key, $old, $new) { - return (bool) self::$cache->evalSha(self::$casScript, [$this->getNamespace() . $key, json_encode($old), json_encode($new)]); + if (!is_int($new)) { + $new = json_encode($new); + } + self::$cache->watch($this->getNamespace() . $key); + if ($this->get($key) === $old) { + $result = self::$cache->multi() + ->set($this->getNamespace() . $key, $new) + ->exec(); + return ($result === false) ? false : true; + } + self::$cache->unwatch(); + return false; } /** @@ -195,12 +179,20 @@ return false'); * @return bool */ public function cad($key, $old) { - return (bool)self::$cache->evalSha(self::$cadScript, [$this->getNamespace() . $key, json_encode($old)]); + self::$cache->watch($this->getNamespace() . $key); + if ($this->get($key) === $old) { + $result = self::$cache->multi() + ->del($this->getNamespace() . $key) + ->exec(); + return ($result === false) ? false : true; + } + self::$cache->unwatch(); + return false; } static public function isAvailable() { return extension_loaded('redis') - && version_compare(phpversion('redis'), '2.2.5', '>='); + && version_compare(phpversion('redis'), '2.2.5', '>='); } }