From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 24 May 2017 17:50:05 +0000 (-0700)
Subject: Avoid treating mcrouter set()s as failing due to AllAsyncRoute
X-Git-Tag: 1.31.0-rc.0~3148^2
X-Git-Url: http://git.cyclocoop.org/%22.%20generer_url_ecrire%28%22sites_tous%22%2C%22%22%29.%20%22?a=commitdiff_plain;h=1180a859ba6cad9d72f276af855afb76d948fe01;p=lhc%2Fweb%2Fwiklou.git

Avoid treating mcrouter set()s as failing due to AllAsyncRoute

Per https://github.com/facebook/mcrouter/wiki/List-of-Route-Handles
a NullRoute response is always given for DELETE and SET. The former
already is already handled by MediaWiki treating NOT_FOUND as success.

Change-Id: I79c26bcd6b8ffe7eea73e0d45badcc4ed63f05e6
---

diff --git a/includes/libs/objectcache/MemcachedClient.php b/includes/libs/objectcache/MemcachedClient.php
index c3fcab94bd..a94f86ae4d 100644
--- a/includes/libs/objectcache/MemcachedClient.php
+++ b/includes/libs/objectcache/MemcachedClient.php
@@ -1114,9 +1114,13 @@ class MemcachedClient {
 		if ( $this->_debug ) {
 			$this->_debugprint( sprintf( "%s %s (%s)", $cmd, $key, $line ) );
 		}
-		if ( $line == "STORED" ) {
+		if ( $line === "STORED" ) {
+			return true;
+		} elseif ( $line === "NOT_STORED" && $cmd === "set" ) {
+			// "Not stored" is always used as the mcrouter response with AllAsyncRoute
 			return true;
 		}
+
 		return false;
 	}
 
diff --git a/includes/libs/objectcache/MemcachedPeclBagOStuff.php b/includes/libs/objectcache/MemcachedPeclBagOStuff.php
index 5983c1b825..c568e7b4f6 100644
--- a/includes/libs/objectcache/MemcachedPeclBagOStuff.php
+++ b/includes/libs/objectcache/MemcachedPeclBagOStuff.php
@@ -149,7 +149,12 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
 
 	public function set( $key, $value, $exptime = 0, $flags = 0 ) {
 		$this->debugLog( "set($key)" );
-		return $this->checkResult( $key, parent::set( $key, $value, $exptime ) );
+		$result = parent::set( $key, $value, $exptime );
+		if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTSTORED ) {
+			// "Not stored" is always used as the mcrouter response with AllAsyncRoute
+			return true;
+		}
+		return $this->checkResult( $key, $result );
 	}
 
 	protected function cas( $casToken, $key, $value, $exptime = 0 ) {
@@ -163,9 +168,8 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
 		if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTFOUND ) {
 			// "Not found" is counted as success in our interface
 			return true;
-		} else {
-			return $this->checkResult( $key, $result );
 		}
+		return $this->checkResult( $key, $result );
 	}
 
 	public function add( $key, $value, $exptime = 0 ) {