From 1180a859ba6cad9d72f276af855afb76d948fe01 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 24 May 2017 10:50:05 -0700 Subject: [PATCH] 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 --- includes/libs/objectcache/MemcachedClient.php | 6 +++++- includes/libs/objectcache/MemcachedPeclBagOStuff.php | 10 +++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) 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 ) { -- 2.20.1