Implemented a wrapper for the memcached PECL client
authorTim Starling <tstarling@wikimedia.org>
Fri, 11 May 2012 05:45:23 +0000 (15:45 +1000)
committerTim Starling <tstarling@wikimedia.org>
Tue, 15 May 2012 00:19:41 +0000 (10:19 +1000)
* Introduced a common base class for the two memcached clients, called
  MemcachedBagOStuff.
* Moved the expiry time normalisation from MemcachedClient.php to
  MemcachedBagOStuff since libmemcached needs the same workaround.

Change-Id: I507d4ec5a7fd863ae64a94f2c453981f9f03746c

includes/AutoLoader.php
includes/DefaultSettings.php
includes/objectcache/MemcachedBagOStuff.php [new file with mode: 0644]
includes/objectcache/MemcachedClient.php
includes/objectcache/MemcachedPeclBagOStuff.php [new file with mode: 0644]
includes/objectcache/MemcachedPhpBagOStuff.php
includes/objectcache/ObjectCache.php

index 6710796..30a18bb 100644 (file)
@@ -57,7 +57,6 @@ $wgAutoloadLocalClasses = array(
        'DeprecatedGlobal' => 'includes/DeprecatedGlobal.php',
        'DerivativeRequest' => 'includes/WebRequest.php',
        'DiffHistoryBlob' => 'includes/HistoryBlob.php',
-
        'DoubleReplacer' => 'includes/StringUtils.php',
        'DummyLinker' => 'includes/Linker.php',
        'Dump7ZipOutput' => 'includes/Export.php',
@@ -668,6 +667,8 @@ $wgAutoloadLocalClasses = array(
        'HashBagOStuff' => 'includes/objectcache/HashBagOStuff.php',
        'MediaWikiBagOStuff' => 'includes/objectcache/SqlBagOStuff.php',
        'MemCachedClientforWiki' => 'includes/objectcache/MemcachedClient.php',
+       'MemcachedBagOStuff' => 'includes/objectcache/MemcachedBagOStuff.php',
+       'MemcachedPeclBagOStuff' => 'includes/objectcache/MemcachedPeclBagOStuff.php',
        'MemcachedPhpBagOStuff' => 'includes/objectcache/MemcachedPhpBagOStuff.php',
        'MultiWriteBagOStuff' => 'includes/objectcache/MultiWriteBagOStuff.php',
        'MWMemcached' => 'includes/objectcache/MemcachedClient.php',
index c210662..2bb7523 100644 (file)
@@ -1592,6 +1592,7 @@ $wgObjectCaches = array(
        'xcache' => array( 'class' => 'XCacheBagOStuff' ),
        'wincache' => array( 'class' => 'WinCacheBagOStuff' ),
        'memcached-php' => array( 'class' => 'MemcachedPhpBagOStuff' ),
+       'memcached-pecl' => array( 'class' => 'MemcachedPeclBagOStuff' ),
        'hash' => array( 'class' => 'HashBagOStuff' ),
 );
 
diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php
new file mode 100644 (file)
index 0000000..49cad8c
--- /dev/null
@@ -0,0 +1,172 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Cache
+ */
+
+/**
+ * Base class for memcached clients.
+ *
+ * @ingroup Cache
+ */
+class MemcachedBagOStuff extends BagOStuff {
+       protected $client;
+
+       /**
+        * Fill in the defaults for any parameters missing from $params, using the
+        * backwards-compatible global variables
+        */
+       protected function applyDefaultParams( $params ) {
+               if ( !isset( $params['servers'] ) ) {
+                       $params['servers'] = $GLOBALS['wgMemCachedServers'];
+               }
+               if ( !isset( $params['debug'] ) ) {
+                       $params['debug'] = $GLOBALS['wgMemCachedDebug'];
+               }
+               if ( !isset( $params['persistent'] ) ) {
+                       $params['persistent'] = $GLOBALS['wgMemCachedPersistent'];
+               }
+               if  ( !isset( $params['compress_threshold'] ) ) {
+                       $params['compress_threshold'] = 1500;
+               }
+               if ( !isset( $params['timeout'] ) ) {
+                       $params['timeout'] = $GLOBALS['wgMemCachedTimeout'];
+               }
+               if ( !isset( $params['connect_timeout'] ) ) {
+                       $params['connect_timeout'] = 0.1;
+               }
+               return $params;
+       }
+
+       /**
+        * @param $key string
+        * @return Mixed
+        */
+       public function get( $key ) {
+               return $this->client->get( $this->encodeKey( $key ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $value
+        * @param $exptime int
+        * @return bool
+        */
+       public function set( $key, $value, $exptime = 0 ) {
+               return $this->client->set( $this->encodeKey( $key ), $value,
+                       $this->fixExpiry( $exptime ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $time int
+        * @return bool
+        */
+       public function delete( $key, $time = 0 ) {
+               return $this->client->delete( $this->encodeKey( $key ), $time );
+       }
+
+       /**
+        * @param $key string
+        * @param $value int
+        * @return Mixed
+        */
+       public function add( $key, $value, $exptime = 0 ) {
+               return $this->client->add( $this->encodeKey( $key ), $value,
+                       $this->fixExpiry( $exptime ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $value int
+        * @param $exptime
+        * @return Mixed
+        */
+       public function replace( $key, $value, $exptime = 0 ) {
+               return $this->client->replace( $this->encodeKey( $key ), $value, 
+                       $this->fixExpiry( $exptime ) );
+       }
+
+       /**
+        * Get the underlying client object. This is provided for debugging
+        * purposes.
+        */
+       public function getClient() {
+               return $this->client;
+       }
+
+       /**
+        * Encode a key for use on the wire inside the memcached protocol.
+        *
+        * We encode spaces and line breaks to avoid protocol errors. We encode
+        * the other control characters for compatibility with libmemcached
+        * verify_key. We leave other punctuation alone, to maximise backwards
+        * compatibility.
+        * @return string
+        */
+       public function encodeKey( $key ) {
+               return preg_replace_callback( '/[\x00-\x20\x25\x7f]+/',
+                       array( $this, 'encodeKeyCallback' ), $key );
+       }
+
+       protected function encodeKeyCallback( $m ) {
+               return rawurlencode( $m[0] );
+       }
+
+       /**
+        * TTLs higher than 30 days will be detected as absolute TTLs
+        * (UNIX timestamps), and will result in the cache entry being
+        * discarded immediately because the expiry is in the past.
+        * Clamp expiries >30d at 30d, unless they're >=1e9 in which
+        * case they are likely to really be absolute (1e9 = 2011-09-09)
+        */
+       function fixExpiry( $expiry ) {
+               if ( $expiry > 2592000 && $expiry < 1000000000 ) {
+                       $expiry = 2592000;
+               }
+               return $expiry;
+       }
+
+       /**
+        * Decode a key encoded with encodeKey(). This is provided as a convenience
+        * function for debugging.
+        *
+        * @param $key string
+        *
+        * @return string
+        */
+       public function decodeKey( $key ) {
+               return urldecode( $key );
+       }
+
+       /**
+        * Send a debug message to the log
+        */
+       protected function debugLog( $text ) {
+               global $wgDebugLogGroups;
+               if( !isset( $wgDebugLogGroups['memcached'] ) ) {
+                       # Prefix message since it will end up in main debug log file
+                       $text = "memcached: $text";
+               }
+               if ( substr( $text, -1 ) !== "\n" ) {
+                       $text .= "\n";
+               }
+               wfDebugLog( 'memcached', $text );
+       }
+}
+
index e816078..9602ffe 100644 (file)
@@ -832,7 +832,7 @@ class MWMemcached {
         * @access private
         */
        function _hashfunc( $key ) {
-               # Hash function must on [0,0x7ffffff]
+               # Hash function must be in [0,0x7ffffff]
                # We take the first 31 bits of the MD5 hash, which unlike the hash
                # function used in a previous version of this client, works
                return hexdec( substr( md5( $key ), 0, 8 ) ) & 0x7fffffff;
@@ -979,15 +979,6 @@ class MWMemcached {
                        $this->stats[$cmd] = 1;
                }
 
-               // TTLs higher than 30 days will be detected as absolute TTLs
-               // (UNIX timestamps), and will result in the cache entry being
-               // discarded immediately because the expiry is in the past.
-               // Clamp expiries >30d at 30d, unless they're >=1e9 in which
-               // case they are likely to really be absolute (1e9 = 2011-09-09)
-               if ( $exp > 2592000 && $exp < 1000000000 ) {
-                       $exp = 2592000;
-               }
-
                $flags = 0;
 
                if ( !is_scalar( $val ) ) {
diff --git a/includes/objectcache/MemcachedPeclBagOStuff.php b/includes/objectcache/MemcachedPeclBagOStuff.php
new file mode 100644 (file)
index 0000000..9c43ede
--- /dev/null
@@ -0,0 +1,200 @@
+<?php
+
+/**
+ * A wrapper class for the PECL memcached client
+ * 
+ * @ingroup Cache
+ */
+class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
+
+       /**
+        * Constructor
+        *
+        * Available parameters are:
+        *   - servers:             The list of IP:port combinations holding the memcached servers.
+        *   - persistent:          Whether to use a persistent connection
+        *   - compress_threshold:  The minimum size an object must be before it is compressed
+        *   - timeout:             The read timeout in microseconds
+        *   - connect_timeout:     The connect timeout in seconds
+        *   - serializer:          May be either "php" or "igbinary". Igbinary produces more compact 
+        *                          values, but serialization is much slower unless the php.ini option
+        *                          igbinary.compact_strings is off.
+        */
+       function __construct( $params ) {
+               $params = $this->applyDefaultParams( $params );
+
+               if ( $params['persistent'] ) {
+                       $this->client = new Memcached( __CLASS__ );
+               } else {
+                       $this->client = new Memcached;
+               }
+
+               if ( !isset( $params['serializer'] ) ) {
+                       $params['serializer'] = 'php';
+               }
+
+               // The compression threshold is an undocumented php.ini option for some 
+               // reason. There's probably not much harm in setting it globally, for 
+               // compatibility with the settings for the PHP client.
+               ini_set( 'memcached.compression_threshold', $params['compress_threshold'] );
+
+               // Set timeouts
+               $this->client->setOption( Memcached::OPT_CONNECT_TIMEOUT, $params['connect_timeout'] * 1000 );
+               $this->client->setOption( Memcached::OPT_SEND_TIMEOUT, $params['timeout'] );
+               $this->client->setOption( Memcached::OPT_RECV_TIMEOUT, $params['timeout'] );
+               $this->client->setOption( Memcached::OPT_POLL_TIMEOUT, $params['timeout'] / 1000 );
+
+               // Set libketama mode since it's recommended by the documentation and 
+               // is as good as any. There's no way to configure libmemcached to use
+               // hashes identical to the ones currently in use by the PHP client, and
+               // even implementing one of the libmemcached hashes in pure PHP for 
+               // forwards compatibility would require MWMemcached::get_sock() to be
+               // rewritten.
+               $this->client->setOption( Memcached::OPT_LIBKETAMA_COMPATIBLE, true );
+
+               // Set the serializer
+               switch ( $params['serializer'] ) {
+                       case 'php':
+                               $this->client->setOption( Memcached::OPT_SERIALIZER, Memcached::SERIALIZER_PHP );
+                               break;
+                       case 'igbinary':
+                               if ( !extension_loaded( 'igbinary' ) ) {
+                                       throw new MWException( __CLASS__.': the igbinary extension is not loaded ' . 
+                                               'but igbinary serialization was requested.' );
+                               }
+                               $this->client->setOption( Memcached::OPT_SERIALIZER, Memcached::SERIALIZER_IGBINARY );
+                               break;
+                       default:
+                               throw new MWException( __CLASS__.': invalid value for serializer parameter' );
+               }
+               foreach ( $params['servers'] as $host ) {
+                       list( $ip, $port ) = IP::splitHostAndPort( $host );
+                       $this->client->addServer( $ip, $port );
+               }
+       }
+
+       /**
+        * @param $key string
+        * @return Mixed
+        */
+       public function get( $key ) {
+               $this->debugLog( "get($key)" );
+               return $this->checkResult( $key, parent::get( $key ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $value
+        * @param $exptime int
+        * @return bool
+        */
+       public function set( $key, $value, $exptime = 0 ) {
+               $this->debugLog( "set($key)" );
+               return $this->checkResult( $key, parent::set( $key, $value, $exptime ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $time int
+        * @return bool
+        */
+       public function delete( $key, $time = 0 ) {
+               $this->debugLog( "delete($key)" );
+               return $this->checkResult( $key, parent::delete( $key, $time ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $value int
+        * @return Mixed
+        */
+       public function add( $key, $value, $exptime = 0 ) {
+               $this->debugLog( "add($key)" );
+               return $this->checkResult( $key, parent::add( $key, $value, $exptime ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $value int
+        * @param $exptime
+        * @return Mixed
+        */
+       public function replace( $key, $value, $exptime = 0 ) {
+               $this->debugLog( "replace($key)" );
+               return $this->checkResult( $key, parent::replace( $key, $value, $exptime ) );
+       }
+
+       /**
+        * @param $key string
+        * @param $value int
+        * @return Mixed
+        */
+       public function incr( $key, $value = 1 ) {
+               $this->debugLog( "incr($key)" );
+               $result = $this->client->increment( $key, $value );
+               return $this->checkResult( $key, $result );
+       }
+
+       /**
+        * @param $key string
+        * @param $value int
+        * @return Mixed
+        */
+       public function decr( $key, $value = 1 ) {
+               $this->debugLog( "decr($key)" );
+               $result = $this->client->decrement( $key, $value );
+               return $this->checkResult( $key, $result );
+       }
+
+       /**
+        * Check the return value from a client method call and take any necessary 
+        * action. Returns the value that the wrapper function should return. At 
+        * present, the return value is always the same as the return value from
+        * the client, but some day we might find a case where it should be 
+        * different.
+        * 
+        * @param $key The key used by the caller, or false if there wasn't one.
+        * @param $result The return value
+        */
+       protected function checkResult( $key, $result ) {
+               if ( $result !== false ) {
+                       return $result;
+               }
+               switch ( $this->client->getResultCode() ) {
+                       case Memcached::RES_SUCCESS:
+                               break;
+                       case Memcached::RES_DATA_EXISTS:
+                       case Memcached::RES_NOTSTORED:
+                       case Memcached::RES_NOTFOUND:
+                               $this->debugLog( "result: " . $this->client->getResultMessage() );
+                               break;
+                       default:
+                               $msg = $this->client->getResultMessage();
+                               if ( $key !== false ) {
+                                       $server = $this->client->getServerByKey( $key );
+                                       $serverName = "{$server['host']}:{$server['port']}";
+                                       $msg = "Memcached error for key \"$key\" on server \"$serverName\": $msg";
+                               } else {
+                                       $msg = "Memcached error: $msg";
+                               }
+                               wfDebugLog( 'memcached-serious', $msg );
+               }
+               return $result;
+       }
+
+       /**
+        * @param $keys Array
+        * @return Array
+        */
+       public function getBatch( array $keys ) {
+               $this->debugLog( 'getBatch(' . implode( ', ', $keys ) . ')' );
+               $callback = array( $this, 'encodeKey' );
+               $result = $this->client->getMulti( array_map( $callback, $keys ) );
+               return $this->checkResult( false, $result );
+       }
+
+       /* NOTE: there is no cas() method here because it is currently not supported 
+        * by the BagOStuff interface and other BagOStuff subclasses, such as 
+        * SqlBagOStuff.
+        */
+}
index 79f3241..eefbb17 100644 (file)
  *
  * @ingroup Cache
  */
-class MemcachedPhpBagOStuff extends BagOStuff {
-
-       /**
-        * @var MemCachedClientforWiki
-        */
-       protected $client;
+class MemcachedPhpBagOStuff extends MemcachedBagOStuff {
 
        /**
         * Constructor.
@@ -47,24 +42,7 @@ class MemcachedPhpBagOStuff extends BagOStuff {
         * @param $params array
         */
        function __construct( $params ) {
-               if ( !isset( $params['servers'] ) ) {
-                       $params['servers'] = $GLOBALS['wgMemCachedServers'];
-               }
-               if ( !isset( $params['debug'] ) ) {
-                       $params['debug'] = $GLOBALS['wgMemCachedDebug'];
-               }
-               if ( !isset( $params['persistent'] ) ) {
-                       $params['persistent'] = $GLOBALS['wgMemCachedPersistent'];
-               }
-               if  ( !isset( $params['compress_threshold'] ) ) {
-                       $params['compress_threshold'] = 1500;
-               }
-               if ( !isset( $params['timeout'] ) ) {
-                       $params['timeout'] = $GLOBALS['wgMemCachedTimeout'];
-               }
-               if ( !isset( $params['connect_timeout'] ) ) {
-                       $params['connect_timeout'] = 0.1;
-               }
+               $params = $this->applyDefaultParams( $params );
 
                $this->client = new MemCachedClientforWiki( $params );
                $this->client->set_servers( $params['servers'] );
@@ -78,14 +56,6 @@ class MemcachedPhpBagOStuff extends BagOStuff {
                $this->client->set_debug( $debug );
        }
 
-       /**
-        * @param $key string
-        * @return Mixed
-        */
-       public function get( $key ) {
-               return $this->client->get( $this->encodeKey( $key ) );
-       }
-
        /**
         * @param $keys Array
         * @return Array
@@ -95,25 +65,6 @@ class MemcachedPhpBagOStuff extends BagOStuff {
                return $this->client->get_multi( array_map( $callback, $keys ) );
        }
 
-       /**
-        * @param $key string
-        * @param $value
-        * @param $exptime int
-        * @return bool
-        */
-       public function set( $key, $value, $exptime = 0 ) {
-               return $this->client->set( $this->encodeKey( $key ), $value, $exptime );
-       }
-
-       /**
-        * @param $key string
-        * @param $time int
-        * @return bool
-        */
-       public function delete( $key, $time = 0 ) {
-               return $this->client->delete( $this->encodeKey( $key ), $time );
-       }
-
        /**
         * @param $key
         * @param $timeout int
@@ -130,26 +81,7 @@ class MemcachedPhpBagOStuff extends BagOStuff {
        public function unlock( $key ) {
                return $this->client->unlock( $this->encodeKey( $key ) );
        }
-
-       /**
-        * @param $key string
-        * @param $value int
-        * @return Mixed
-        */
-       public function add( $key, $value, $exptime = 0 ) {
-               return $this->client->add( $this->encodeKey( $key ), $value, $exptime );
-       }
-
-       /**
-        * @param $key string
-        * @param $value int
-        * @param $exptime
-        * @return Mixed
-        */
-       public function replace( $key, $value, $exptime = 0 ) {
-               return $this->client->replace( $this->encodeKey( $key ), $value, $exptime );
-       }
-
+       
        /**
         * @param $key string
         * @param $value int
@@ -167,45 +99,5 @@ class MemcachedPhpBagOStuff extends BagOStuff {
        public function decr( $key, $value = 1 ) {
                return $this->client->decr( $this->encodeKey( $key ), $value );
        }
-
-       /**
-        * Get the underlying client object. This is provided for debugging
-        * purposes.
-        *
-        * @return MemCachedClientforWiki
-        */
-       public function getClient() {
-               return $this->client;
-       }
-
-       /**
-        * Encode a key for use on the wire inside the memcached protocol.
-        *
-        * We encode spaces and line breaks to avoid protocol errors. We encode
-        * the other control characters for compatibility with libmemcached
-        * verify_key. We leave other punctuation alone, to maximise backwards
-        * compatibility.
-        * @return string
-        */
-       public function encodeKey( $key ) {
-               return preg_replace_callback( '/[\x00-\x20\x25\x7f]+/',
-                       array( $this, 'encodeKeyCallback' ), $key );
-       }
-
-       protected function encodeKeyCallback( $m ) {
-               return rawurlencode( $m[0] );
-       }
-
-       /**
-        * Decode a key encoded with encodeKey(). This is provided as a convenience
-        * function for debugging.
-        *
-        * @param $key string
-        *
-        * @return string
-        */
-       public function decodeKey( $key ) {
-               return urldecode( $key );
-       }
 }
 
index a59ca05..2e00e16 100644 (file)
@@ -132,8 +132,10 @@ class ObjectCache {
 
        /**
         * Factory function that creates a memcached client object.
-        * The idea of this is that it might eventually detect and automatically
-        * support the PECL extension, assuming someone can get it to compile.
+        *
+        * This always uses the PHP client, since the PECL client has a different 
+        * hashing scheme and a different interpretation of the flags bitfield, so 
+        * switching between the two clients randomly would be disasterous.
         *
         * @param $params array
         *