[stable25] redis: use atomic operations everywhere

lib/private/Memcache/Redis.php

@@ -32,51 +32,77 @@
 use OCP\IMemcacheTTL;
 
 class Redis extends Cache implements IMemcacheTTL {
+	/** name => [script, sha1] */
+	public const LUA_SCRIPTS = [
+		'dec' => [
+			'if redis.call("exists", KEYS[1]) == 1 then return redis.call("decrby", KEYS[1], ARGV[1]) else return "NEX" end',
+			'720b40cb66cef1579f2ef16ec69b3da8c85510e9',
+		],
+		'cas' => [
+			'if redis.call("get", KEYS[1]) == ARGV[1] then redis.call("set", KEYS[1], ARGV[2]) return 1 else return 0 end',
+			'94eac401502554c02b811e3199baddde62d976d4',
+		],
+		'cad' => [
+			'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end',
+			'cf0e94b2e9ffc7e04395cf88f7583fc309985910',
+		],
+	];
+
 	/**
-	 * @var \Redis $cache
+	 * @var \Redis|\RedisCluster $cache
 	 */
 	private static $cache = null;
 
 	public function __construct($prefix = '', string $logFile = '') {
 		parent::__construct($prefix);
+	}
+
+	/**
+	 * @return \Redis|\RedisCluster|null
+	 * @throws \Exception
+	 */
+	public function getCache() {
 		if (is_null(self::$cache)) {
 			self::$cache = \OC::$server->getGetRedisFactory()->getInstance();
 		}
+		return self::$cache;
 	}
 
 	public function get($key) {
-		$result = self::$cache->get($this->getPrefix() . $key);
-		if ($result === false && !self::$cache->exists($this->getPrefix() . $key)) {
+		$result = $this->getCache()->get($this->getPrefix() . $key);
+		if ($result === false) {
 			return null;
-		} else {
-			return json_decode($result, true);
 		}
+
+		return self::decodeValue($result);
 	}
 
 	public function set($key, $value, $ttl = 0) {
+		$value = self::encodeValue($value);
 		if ($ttl > 0) {
-			return self::$cache->setex($this->getPrefix() . $key, $ttl, json_encode($value));
+			return $this->getCache()->setex($this->getPrefix() . $key, $ttl, $value);
 		} else {
-			return self::$cache->set($this->getPrefix() . $key, json_encode($value));
+			return $this->getCache()->set($this->getPrefix() . $key, $value);
 		}
 	}
 
 	public function hasKey($key) {
-		return (bool)self::$cache->exists($this->getPrefix() . $key);
+		return (bool)$this->getCache()->exists($this->getPrefix() . $key);
 	}
 
 	public function remove($key) {
-		if (self::$cache->del($this->getPrefix() . $key)) {
+		if ($this->getCache()->del($this->getPrefix() . $key)) {
 			return true;
 		} else {
 			return false;
 		}
 	}
 
 	public function clear($prefix = '') {
+		// TODO: this is slow and would fail with Redis cluster
 		$prefix = $this->getPrefix() . $prefix . '*';
-		$keys = self::$cache->keys($prefix);
-		$deleted = self::$cache->del($keys);
+		$keys = $this->getCache()->keys($prefix);
+		$deleted = $this->getCache()->del($keys);
 
 		return (is_array($keys) && (count($keys) === $deleted));
 	}
@@ -90,17 +116,14 @@
 	 * @return bool
 	 */
 	public function add($key, $value, $ttl = 0) {
-		// don't encode ints for inc/dec
-		if (!is_int($value)) {
-			$value = json_encode($value);
-		}
+		$value = self::encodeValue($value);
 
 		$args = ['nx'];
 		if ($ttl !== 0 && is_int($ttl)) {
 			$args['ex'] = $ttl;
 		}
 
-		return self::$cache->set($this->getPrefix() . $key, $value, $args);
+		return $this->getCache()->set($this->getPrefix() . $key, $value, $args);
 	}
 
 	/**
@@ -111,7 +134,7 @@
 	 * @return int | bool
 	 */
 	public function inc($key, $step = 1) {
-		return self::$cache->incrBy($this->getPrefix() . $key, $step);
+		return $this->getCache()->incrBy($this->getPrefix() . $key, $step);
 	}
 
 	/**
@@ -122,10 +145,8 @@
 	 * @return int | bool
 	 */
 	public function dec($key, $step = 1) {
-		if (!$this->hasKey($key)) {
-			return false;
-		}
-		return self::$cache->decrBy($this->getPrefix() . $key, $step);
+		$res = $this->evalLua('dec', [$key], [$step]);
+		return ($res === 'NEX') ? false : $res;
 	}
 
 	/**
@@ -137,18 +158,10 @@
 	 * @return bool
 	 */
 	public function cas($key, $old, $new) {
-		if (!is_int($new)) {
-			$new = json_encode($new);
-		}
-		self::$cache->watch($this->getPrefix() . $key);
-		if ($this->get($key) === $old) {
-			$result = self::$cache->multi()
-				->set($this->getPrefix() . $key, $new)
-				->exec();
-			return $result !== false;
-		}
-		self::$cache->unwatch();
-		return false;
+		$old = self::encodeValue($old);
+		$new = self::encodeValue($new);
+
+		return $this->evalLua('cas', [$key], [$old, $new]) > 0;
 	}
 
 	/**
@@ -159,22 +172,37 @@
 	 * @return bool
 	 */
 	public function cad($key, $old) {
-		self::$cache->watch($this->getPrefix() . $key);
-		if ($this->get($key) === $old) {
-			$result = self::$cache->multi()
-				->del($this->getPrefix() . $key)
-				->exec();
-			return $result !== false;
-		}
-		self::$cache->unwatch();
-		return false;
+		$old = self::encodeValue($old);
+
+		return $this->evalLua('cad', [$key], [$old]) > 0;
 	}
 
 	public function setTTL($key, $ttl) {
-		self::$cache->expire($this->getPrefix() . $key, $ttl);
+		$this->getCache()->expire($this->getPrefix() . $key, $ttl);
 	}
 
 	public static function isAvailable(): bool {
 		return \OC::$server->getGetRedisFactory()->isAvailable();
 	}
+
+	protected function evalLua(string $scriptName, array $keys, array $args) {
+		$keys = array_map(fn ($key) => $this->getPrefix() . $key, $keys);
+		$args = array_merge($keys, $args);
+		$script = self::LUA_SCRIPTS[$scriptName];
+
+		$result = $this->getCache()->evalSha($script[1], $args, count($keys));
+		if (false === $result) {
+			$result = $this->getCache()->eval($script[0], $args, count($keys));
+		}
+
+		return $result;
+	}
+
+	protected static function encodeValue(mixed $value): string {
+		return is_int($value) ? (string) $value : json_encode($value);
+	}
+
+	protected static function decodeValue(string $value): mixed {
+		return is_numeric($value) ? (int) $value : json_decode($value, true);
+	}
 }

tests/lib/Memcache/RedisTest.php

@@ -52,4 +52,10 @@ protected function setUp(): void {
 		parent::setUp();
 		$this->instance = new \OC\Memcache\Redis($this->getUniqueID());
 	}
+
+	public function testScriptHashes() {
+		foreach (\OC\Memcache\Redis::LUA_SCRIPTS as $script) {
+			$this->assertEquals(sha1($script[0]), $script[1]);
+		}
+	}
 }