Merge pull request #47259 from nextcloud/memcache-ncad

feat: add negative compare-and-delete to imemcache
nextcloud · Aug 21, 2024 · c3ad469 · c3ad469
2 parents 6214e3a + 20dbb6c
commit c3ad469
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 2 deletions.
diff --git a/lib/private/Memcache/CADTrait.php b/lib/private/Memcache/CADTrait.php
@@ -35,4 +35,21 @@ public function cad($key, $old) {
 			return false;
 		}
 	}
+
+	public function ncad(string $key, mixed $old): bool {
+		//no native cad, emulate with locking
+		if ($this->add($key . '_lock', true)) {
+			$value = $this->get($key);
+			if ($value !== null && $value !== $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/LoggerWrapperCache.php b/lib/private/Memcache/LoggerWrapperCache.php
@@ -149,6 +149,17 @@ public function cad($key, $old) {
 		return $this->wrappedCache->cad($key, $old);
 	}
 
+	/** @inheritDoc */
+	public function ncad(string $key, mixed $old): bool {
+		file_put_contents(
+			$this->logFile,
+			$this->getNameSpace() . '::ncad::' . $key . "\n",
+			FILE_APPEND
+		);
+
+		return $this->wrappedCache->cad($key, $old);
+	}
+
 	/** @inheritDoc */
 	public function setTTL(string $key, int $ttl) {
 		$this->wrappedCache->setTTL($key, $ttl);

diff --git a/lib/private/Memcache/NullCache.php b/lib/private/Memcache/NullCache.php
@@ -43,6 +43,11 @@ public function cad($key, $old) {
 		return true;
 	}
 
+	public function ncad(string $key, mixed $old): bool {
+		return true;
+	}
+
+
 	public function clear($prefix = '') {
 		return true;
 	}

diff --git a/lib/private/Memcache/ProfilerWrapperCache.php b/lib/private/Memcache/ProfilerWrapperCache.php
@@ -166,6 +166,18 @@ public function cad($key, $old) {
 		return $ret;
 	}
 
+	/** @inheritDoc */
+	public function ncad(string $key, mixed $old): bool {
+		$start = microtime(true);
+		$ret = $this->wrappedCache->ncad($key, $old);
+		$this->data['queries'][] = [
+			'start' => $start,
+			'end' => microtime(true),
+			'op' => $this->getPrefix() . '::ncad::' . $key,
+		];
+		return $ret;
+	}
+
 	/** @inheritDoc */
 	public function setTTL(string $key, int $ttl) {
 		$this->wrappedCache->setTTL($key, $ttl);

diff --git a/lib/private/Memcache/Redis.php b/lib/private/Memcache/Redis.php
@@ -23,6 +23,10 @@ class Redis extends Cache implements IMemcacheTTL {
 			'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end',
 			'cf0e94b2e9ffc7e04395cf88f7583fc309985910',
 		],
+		'ncad' => [
+			'if redis.call("get", KEYS[1]) ~= ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end',
+			'75526f8048b13ce94a41b58eee59c664b4990ab2',
+		],
 		'caSetTtl' => [
 			'if redis.call("get", KEYS[1]) == ARGV[1] then redis.call("expire", KEYS[1], ARGV[2]) return 1 else return 0 end',
 			'fa4acbc946d23ef41d7d3910880b60e6e4972d72',
@@ -164,6 +168,12 @@ public function cad($key, $old) {
 		return $this->evalLua('cad', [$key], [$old]) > 0;
 	}
 
+	public function ncad(string $key, mixed $old): bool {
+		$old = self::encodeValue($old);
+
+		return $this->evalLua('ncad', [$key], [$old]) > 0;
+	}
+
 	public function setTTL($key, $ttl) {
 		if ($ttl === 0) {
 			// having infinite TTL can lead to leaked keys as the prefix changes with version upgrades

diff --git a/lib/public/IMemcache.php b/lib/public/IMemcache.php
@@ -56,21 +56,37 @@ public function dec($key, $step = 1);
 	/**
 	 * Compare and set
 	 *
+	 *  Set $key to $new only if it's current value is $new
+	 *
 	 * @param string $key
 	 * @param mixed $old
 	 * @param mixed $new
-	 * @return bool
+	 * @return bool true if the value was successfully set or false if $key wasn't set to $old
 	 * @since 8.1.0
 	 */
 	public function cas($key, $old, $new);
 
 	/**
 	 * Compare and delete
 	 *
+	 *  Delete $key if the stored value is equal to $old
+	 *
 	 * @param string $key
 	 * @param mixed $old
-	 * @return bool
+	 * @return bool true if the value was successfully deleted or false if $key wasn't set to $old
 	 * @since 8.1.0
 	 */
 	public function cad($key, $old);
+
+	/**
+	 * Negative compare and delete
+	 *
+	 * Delete $key if the stored value is not equal to $old
+	 *
+	 * @param string $key
+	 * @param mixed $old
+	 * @return bool true if the value was successfully deleted or false if $key was set to $old or is not set
+	 * @since 30.0.0
+	 */
+	public function ncad(string $key, mixed $old): bool;
 }
diff --git a/tests/lib/Memcache/Cache.php b/tests/lib/Memcache/Cache.php
@@ -109,6 +109,10 @@ public function testCasChanged() {
 		$this->assertEquals('bar1', $this->instance->get('foo'));
 	}
 
+	public function testCasNotSet() {
+		$this->assertFalse($this->instance->cas('foo', 'bar', 'asd'));
+	}
+
 	public function testCadNotChanged() {
 		$this->instance->set('foo', 'bar');
 		$this->assertTrue($this->instance->cad('foo', 'bar'));
@@ -121,6 +125,25 @@ public function testCadChanged() {
 		$this->assertTrue($this->instance->hasKey('foo'));
 	}
 
+	public function testCadNotSet() {
+		$this->assertFalse($this->instance->cad('foo', 'bar'));
+	}
+
+	public function testNcadNotChanged() {
+		$this->instance->set('foo', 'bar');
+		$this->assertFalse($this->instance->ncad('foo', 'bar'));
+		$this->assertTrue($this->instance->hasKey('foo'));
+	}
+
+	public function testNcadChanged() {
+		$this->instance->set('foo', 'bar1');
+		$this->assertTrue($this->instance->ncad('foo', 'bar'));
+		$this->assertFalse($this->instance->hasKey('foo'));
+	}
+
+	public function testNcadNotSet() {
+		$this->assertFalse($this->instance->ncad('foo', 'bar'));
+	}
 
 	protected function tearDown(): void {
 		if ($this->instance) {