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 c3fcab9..a94f86a 100644 (file)
--- 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 5983c1b..c568e7b 100644 (file)
--- 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 ) {