redis: use atomic operations everywhere

This removes a lot of acrobatics in the code and does each operation
atomically using a lua script. This also reduces several round trips
to the server, and the cost of compiling the scripts can be ignored,
since these are very small.

Notably, since all operations work only on a single key (except clear,
which is broken anyway and shouldn't be used), they will continue to
function and be atomic for Redis cluster.

Signed-off-by: Varun Patil <[email protected]>

Author: pulsejet

lib/private/Memcache/Redis.php

@@ -54,18 +54,19 @@ public function getCache() {
 
 	public function get($key) {
 		$result = $this->getCache()->get($this->getPrefix() . $key);
-		if ($result === false && !$this->getCache()->exists($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 $this->getCache()->setex($this->getPrefix() . $key, $ttl, json_encode($value));
+			return $this->getCache()->setex($this->getPrefix() . $key, $ttl, $value);
 		} else {
-			return $this->getCache()->set($this->getPrefix() . $key, json_encode($value));
+			return $this->getCache()->set($this->getPrefix() . $key, $value);
 		}
 	}
 
@@ -82,6 +83,7 @@ public function remove($key) {
 	}
 
 	public function clear($prefix = '') {
+		// TODO: this is slow and would fail with Redis cluster
 		$prefix = $this->getPrefix() . $prefix . '*';
 		$keys = $this->getCache()->keys($prefix);
 		$deleted = $this->getCache()->del($keys);
@@ -98,17 +100,14 @@ public function clear($prefix = '') {
 	 * @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 $this->getCache()->set($this->getPrefix() . $key, (string)$value, $args);
+		return $this->getCache()->set($this->getPrefix() . $key, $value, $args);
 	}
 
 	/**
@@ -130,10 +129,17 @@ public function inc($key, $step = 1) {
 	 * @return int | bool
 	 */
 	public function dec($key, $step = 1) {
-		if (!$this->hasKey($key)) {
-			return false;
-		}
-		return $this->getCache()->decrBy($this->getPrefix() . $key, $step);
+		$res = $this->getCache()->eval(
+			'if redis.call("exists", KEYS[1]) == 1 then
+				return redis.call("decrby", KEYS[1], ARGV[1])
+			else
+				return "NEX"
+			end',
+			[$this->getPrefix() . $key, $step],
+			1
+		);
+
+		return ($res === 'NEX') ? false : $res;
 	}
 
 	/**
@@ -145,18 +151,19 @@ public function dec($key, $step = 1) {
 	 * @return bool
 	 */
 	public function cas($key, $old, $new) {
-		if (!is_int($new)) {
-			$new = json_encode($new);
-		}
-		$this->getCache()->watch($this->getPrefix() . $key);
-		if ($this->get($key) === $old) {
-			$result = $this->getCache()->multi()
-				->set($this->getPrefix() . $key, $new)
-				->exec();
-			return $result !== false;
-		}
-		$this->getCache()->unwatch();
-		return false;
+		$old = self::encodeValue($old);
+		$new = self::encodeValue($new);
+
+		return $this->getCache()->eval(
+			'if redis.call("get", KEYS[1]) == ARGV[1] then
+				redis.call("set", KEYS[1], ARGV[2])
+				return 1
+			else
+				return 0
+			end',
+			[$this->getPrefix() . $key, $old, $new],
+			1
+		) > 0;
 	}
 
 	/**
@@ -167,15 +174,17 @@ public function cas($key, $old, $new) {
 	 * @return bool
 	 */
 	public function cad($key, $old) {
-		$this->getCache()->watch($this->getPrefix() . $key);
-		if ($this->get($key) === $old) {
-			$result = $this->getCache()->multi()
-				->unlink($this->getPrefix() . $key)
-				->exec();
-			return $result !== false;
-		}
-		$this->getCache()->unwatch();
-		return false;
+		$old = self::encodeValue($old);
+
+		return $this->getCache()->eval(
+			'if redis.call("get", KEYS[1]) == ARGV[1] then
+				return redis.call("del", KEYS[1])
+			else
+				return 0
+			end',
+			[$this->getPrefix() . $key, $old],
+			1
+		) > 0;
 	}
 
 	public function setTTL($key, $ttl) {
@@ -185,4 +194,12 @@ public function setTTL($key, $ttl) {
 	public static function isAvailable(): bool {
 		return \OC::$server->getGetRedisFactory()->isAvailable();
 	}
+
+	protected static function encodeValue($value) {
+		return is_int($value) ? (string) $value : json_encode($value);
+	}
+
+	protected static function decodeValue($value) {
+		return is_numeric($value) ? (int) $value : json_decode($value, true);
+	}
 }