r83140 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r83139‎ | r83140 | r83141 >
Date:09:37, 3 March 2011
Author:tstarling
Status:resolved (Comments)
Tags:
Comment:
* Rewrote ObjectCache.php to conform to the modern coding style, and to be less convoluted about how CACHE_ANYTHING and CACHE_ACCEL are resolved. Moved most functionality to static members of a new ObjectCache class.
* Moved the global functions to GlobalFunctions.php, where they are now just convenience wrappers. Made them return non-references. Updated callers (none found in extensions).
* Added an advanced configuration method, $wgObjectCaches, which allows a lot more detail in the object cache configuration than $wgMainCacheType.
* Made all object cache classes derive from BagOStuff.
* Split the MWMemcached class into a generic client class and a MediaWiki-specific wrapper class. The wrapper class presents a simple BagOStuff interface to calling code, hiding memcached client internals, and will simplify the task of supporting the PECL extension.
* Added some extra constructor parameters to MWMemcached, configurable via $wgObjectCaches.
* Removed the *_multi() methods from BagOStuff, my grepping indicates that they are not used.
* Rewrote FakeMemCachedClient as a BagOStuff subclass, called EmptyBagOStuff.
* Added an optional "server" parameter to SQLBagOStuff. This allows the server holding the objectcache table to be different from the server holding the core DB.
* Added MultiWriteBagOStuff: a cache class which writes to multiple locations, and reads from them in a defined fallback sequence. This can be used to extend the cache space by adding disk-backed storage to existing in-memory caches.
* Made MWMemcached::get() return false on failure instead of null, to match the BagOStuff documentation and the other BagOStuff subclasses. Anything that was relying on it returning null would have already been broken with SqlBagOStuff.
* Fixed a bug in the memcached client causing keys with spaces or line breaks in them to break the memcached protocol, injecting arbitrary commands or parameters. Since the PECL client apparently also has this flaw, I implemented the fix in the wrapper class.
* Renamed BagOStuff::set_debug() to setDebug(), since we aren't emulating the memcached client anymore
* Fixed spelling error in MWMemcached: persistant -> persistent
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/Setup.php (modified) (history)
  • /trunk/phase3/includes/objectcache/BagOStuff.php (modified) (history)
  • /trunk/phase3/includes/objectcache/EmptyBagOStuff.php (added) (history)
  • /trunk/phase3/includes/objectcache/MemcachedClient.php (modified) (history)
  • /trunk/phase3/includes/objectcache/MemcachedPhpBagOStuff.php (added) (history)
  • /trunk/phase3/includes/objectcache/MultiWriteBagOStuff.php (added) (history)
  • /trunk/phase3/includes/objectcache/ObjectCache.php (modified) (history)
  • /trunk/phase3/includes/objectcache/SqlBagOStuff.php (modified) (history)
  • /trunk/phase3/maintenance/mcc.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/mcc.php
@@ -25,7 +25,7 @@
2626 /** */
2727 require_once( dirname( __FILE__ ) . '/commandLine.inc' );
2828
29 -$mcc = new MWMemcached( array( 'persistant' => true/*, 'debug' => true*/ ) );
 29+$mcc = new MWMemcached( array( 'persistent' => true/*, 'debug' => true*/ ) );
3030 $mcc->set_servers( $wgMemCachedServers );
3131 # $mcc->set_debug( true );
3232
Index: trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php
@@ -70,9 +70,9 @@
7171 $tmpGlobals['wgEnableParserCache'] = false;
7272 $tmpGlobals['wgHooks'] = $wgHooks;
7373 $tmpGlobals['wgDeferredUpdateList'] = array();
74 - $tmpGlobals['wgMemc'] = &wfGetMainCache();
75 - $tmpGlobals['messageMemc'] = &wfGetMessageCacheStorage();
76 - $tmpGlobals['parserMemc'] = &wfGetParserCacheStorage();
 74+ $tmpGlobals['wgMemc'] = wfGetMainCache();
 75+ $tmpGlobals['messageMemc'] = wfGetMessageCacheStorage();
 76+ $tmpGlobals['parserMemc'] = wfGetParserCacheStorage();
7777
7878 // $tmpGlobals['wgContLang'] = new StubContLang;
7979 $tmpGlobals['wgUser'] = new User;
Index: trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php
@@ -42,9 +42,9 @@
4343
4444 $wgEnableParserCache = false;
4545 $wgDeferredUpdateList = array();
46 - $wgMemc = &wfGetMainCache();
47 - $messageMemc = &wfGetMessageCacheStorage();
48 - $parserMemc = &wfGetParserCacheStorage();
 46+ $wgMemc = wfGetMainCache();
 47+ $messageMemc = wfGetMessageCacheStorage();
 48+ $parserMemc = wfGetParserCacheStorage();
4949
5050 // $wgContLang = new StubContLang;
5151 $wgUser = new User;
Index: trunk/phase3/includes/GlobalFunctions.php
@@ -3357,3 +3357,33 @@
33583358 }
33593359 return $ret;
33603360 }
 3361+
 3362+
 3363+/**
 3364+ * Get a cache object.
 3365+ * @param $inputType Cache type, one the the CACHE_* constants.
 3366+ *
 3367+ * @return BagOStuff
 3368+ */
 3369+function wfGetCache( $inputType ) {
 3370+ return ObjectCache::getInstance( $inputType );
 3371+}
 3372+
 3373+/** Get the main cache object */
 3374+function wfGetMainCache() {
 3375+ global $wgMainCacheType;
 3376+ return ObjectCache::getInstance( $wgMainCacheType );
 3377+}
 3378+
 3379+/** Get the cache object used by the message cache */
 3380+function wfGetMessageCacheStorage() {
 3381+ global $wgMessageCacheType;
 3382+ return ObjectCache::getInstance( $wgMessageCacheType );
 3383+}
 3384+
 3385+/** Get the cache object used by the parser cache */
 3386+function wfGetParserCacheStorage() {
 3387+ global $wgParserCacheType;
 3388+ return ObjectCache::getInstance( $wgParserCacheType );
 3389+}
 3390+
Index: trunk/phase3/includes/objectcache/ObjectCache.php
@@ -5,96 +5,90 @@
66 * @file
77 * @ingroup Cache
88 */
 9+class ObjectCache {
 10+ static $instances = array();
911
10 -global $wgCaches;
11 -$wgCaches = array();
 12+ /**
 13+ * Get a cached instance of the specified type of cache object.
 14+ */
 15+ static function getInstance( $id ) {
 16+ if ( isset( self::$instances[$id] ) ) {
 17+ return self::$instances[$id];
 18+ }
1219
13 -/**
14 - * Get a cache object.
15 - * @param $inputType Integer: cache type, one the the CACHE_* constants.
16 - *
17 - * @return BagOStuff
18 - */
19 -function &wfGetCache( $inputType ) {
20 - global $wgCaches, $wgMemCachedServers, $wgMemCachedDebug, $wgMemCachedPersistent;
21 - $cache = false;
 20+ $object = self::newFromId( $id );
 21+ self::$instances[$id] = $object;
 22+ return $object;
 23+ }
2224
23 - if ( $inputType == CACHE_ANYTHING ) {
24 - reset( $wgCaches );
25 - $type = key( $wgCaches );
26 - if ( $type === false || $type === CACHE_NONE ) {
27 - $type = CACHE_DB;
 25+ /**
 26+ * Create a new cache object of the specified type.
 27+ */
 28+ static function newFromId( $id ) {
 29+ global $wgObjectCaches;
 30+
 31+ if ( !isset( $wgObjectCaches[$id] ) ) {
 32+ throw new MWException( "Invalid object cache type \"$id\" requested. " .
 33+ "It is not present in \$wgObjectCaches." );
2834 }
29 - } else {
30 - $type = $inputType;
 35+
 36+ return self::newFromParams( $wgObjectCaches[$id] );
3137 }
3238
33 - if ( $type == CACHE_MEMCACHED ) {
34 - if ( !array_key_exists( CACHE_MEMCACHED, $wgCaches ) ) {
35 - $wgCaches[CACHE_MEMCACHED] = new MemCachedClientforWiki(
36 - array('persistant' => $wgMemCachedPersistent, 'compress_threshold' => 1500 ) );
37 - $wgCaches[CACHE_MEMCACHED]->set_servers( $wgMemCachedServers );
38 - $wgCaches[CACHE_MEMCACHED]->set_debug( $wgMemCachedDebug );
 39+ /**
 40+ * Create a new cache object from parameters
 41+ */
 42+ static function newFromParams( $params ) {
 43+ if ( isset( $params['factory'] ) ) {
 44+ return call_user_func( $params['factory'], $params );
 45+ } elseif ( isset( $params['class'] ) ) {
 46+ $class = $params['class'];
 47+ return new $class( $params );
 48+ } else {
 49+ throw new MWException( "The definition of cache type \"$id\" lacks both " .
 50+ "factory and class parameters." );
3951 }
40 - $cache =& $wgCaches[CACHE_MEMCACHED];
41 - } elseif ( $type == CACHE_ACCEL ) {
42 - if ( !array_key_exists( CACHE_ACCEL, $wgCaches ) ) {
43 - if ( function_exists( 'eaccelerator_get' ) ) {
44 - $wgCaches[CACHE_ACCEL] = new eAccelBagOStuff;
45 - } elseif ( function_exists( 'apc_fetch') ) {
46 - $wgCaches[CACHE_ACCEL] = new APCBagOStuff;
47 - } elseif( function_exists( 'xcache_get' ) ) {
48 - $wgCaches[CACHE_ACCEL] = new XCacheBagOStuff();
49 - } elseif( function_exists( 'wincache_ucache_get' ) ) {
50 - $wgCaches[CACHE_ACCEL] = new WinCacheBagOStuff();
51 - } else {
52 - $wgCaches[CACHE_ACCEL] = false;
53 - }
54 - }
55 - if ( $wgCaches[CACHE_ACCEL] !== false ) {
56 - $cache =& $wgCaches[CACHE_ACCEL];
57 - }
58 - } elseif ( $type == CACHE_DBA ) {
59 - if ( !array_key_exists( CACHE_DBA, $wgCaches ) ) {
60 - $wgCaches[CACHE_DBA] = new DBABagOStuff;
61 - }
62 - $cache =& $wgCaches[CACHE_DBA];
6352 }
6453
65 - if ( $type == CACHE_DB || ( $inputType == CACHE_ANYTHING && $cache === false ) ) {
66 - if ( !array_key_exists( CACHE_DB, $wgCaches ) ) {
67 - $wgCaches[CACHE_DB] = new SqlBagOStuff('objectcache');
 54+ /**
 55+ * Factory function referenced from DefaultSettings.php for CACHE_ANYTHING
 56+ */
 57+ static function newAnything( $params ) {
 58+ global $wgMainCacheType, $wgMessageCacheType, $wgParserCacheType;
 59+ $candidates = array( $wgMainCacheType, $wgMessageCacheType, $wgParserCacheType );
 60+ foreach ( $candidates as $candidate ) {
 61+ if ( $candidate !== CACHE_NONE && $candidate !== CACHE_ANYTHING ) {
 62+ return self::newFromId( $candidate );
 63+ }
6864 }
69 - $cache =& $wgCaches[CACHE_DB];
 65+ return self::newFromId( CACHE_DB );
7066 }
7167
72 - if ( $cache === false ) {
73 - if ( !array_key_exists( CACHE_NONE, $wgCaches ) ) {
74 - $wgCaches[CACHE_NONE] = new FakeMemCachedClient;
 68+ /**
 69+ * Factory function referenced from DefaultSettings.php for CACHE_ACCEL.
 70+ */
 71+ static function newAccelerator( $params ) {
 72+ if ( function_exists( 'eaccelerator_get' ) ) {
 73+ $id = 'eaccelerator';
 74+ } elseif ( function_exists( 'apc_fetch') ) {
 75+ $id = 'apc';
 76+ } elseif( function_exists( 'xcache_get' ) ) {
 77+ $id = 'xcache';
 78+ } elseif( function_exists( 'wincache_ucache_get' ) ) {
 79+ $id = 'wincache';
 80+ } else {
 81+ throw new MWException( "CACHE_ACCEL requested but no suitable object " .
 82+ "cache is present. You may want to install APC." );
7583 }
76 - $cache =& $wgCaches[CACHE_NONE];
 84+ return self::newFromId( $id );
7785 }
7886
79 - return $cache;
 87+ /**
 88+ * Factory function that creates a memcached client object.
 89+ * The idea of this is that it might eventually detect and automatically
 90+ * support the PECL extension, assuming someone can get it to compile.
 91+ */
 92+ static function newMemcached( $params ) {
 93+ return new MemcachedPhpBagOStuff( $params );
 94+ }
8095 }
81 -
82 -/** Get the main cache object */
83 -function &wfGetMainCache() {
84 - global $wgMainCacheType;
85 - $ret =& wfGetCache( $wgMainCacheType );
86 - return $ret;
87 -}
88 -
89 -/** Get the cache object used by the message cache */
90 -function &wfGetMessageCacheStorage() {
91 - global $wgMessageCacheType;
92 - $ret =& wfGetCache( $wgMessageCacheType );
93 - return $ret;
94 -}
95 -
96 -/** Get the cache object used by the parser cache */
97 -function &wfGetParserCacheStorage() {
98 - global $wgParserCacheType;
99 - $ret =& wfGetCache( $wgParserCacheType );
100 - return $ret;
101 -}
Index: trunk/phase3/includes/objectcache/EmptyBagOStuff.php
@@ -0,0 +1,27 @@
 2+<?php
 3+
 4+/**
 5+ * A BagOStuff object with no objects in it. Used to provide a no-op object to calling code.
 6+ *
 7+ * @ingroup Cache
 8+ */
 9+class EmptyBagOStuff extends BagOStuff {
 10+ function get( $key ) {
 11+ return false;
 12+ }
 13+
 14+ function set( $key, $value, $exp = 0 ) {
 15+ return true;
 16+ }
 17+
 18+ function delete( $key, $time = 0 ) {
 19+ return true;
 20+ }
 21+}
 22+
 23+/**
 24+ * Backwards compatibility alias for EmptyBagOStuff
 25+ * @deprecated
 26+ */
 27+class FakeMemCachedClient extends EmptyBagOStuff {
 28+}
Property changes on: trunk/phase3/includes/objectcache/EmptyBagOStuff.php
___________________________________________________________________
Added: svn:eol-style
129 + native
Index: trunk/phase3/includes/objectcache/MemcachedPhpBagOStuff.php
@@ -0,0 +1,116 @@
 2+<?php
 3+
 4+/**
 5+ * A wrapper class for the pure-PHP memcached client, exposing a BagOStuff interface.
 6+ */
 7+class MemcachedPhpBagOStuff extends BagOStuff {
 8+ /**
 9+ * Constructor.
 10+ *
 11+ * Available parameters are:
 12+ * - servers: The list of IP:port combinations holding the memcached servers.
 13+ * - debug: Whether to set the debug flag in the underlying client.
 14+ * - persistent: Whether to use a persistent connection
 15+ * - compress_threshold: The minimum size an object must be before it is compressed
 16+ * - timeout: The read timeout in microseconds
 17+ * - connect_timeout: The connect timeout in seconds
 18+ */
 19+ function __construct( $params ) {
 20+ if ( !isset( $params['servers'] ) ) {
 21+ $params['servers'] = $GLOBALS['wgMemCachedServers'];
 22+ }
 23+ if ( !isset( $params['debug'] ) ) {
 24+ $params['debug'] = $GLOBALS['wgMemCachedDebug'];
 25+ }
 26+ if ( !isset( $params['persistent'] ) ) {
 27+ $params['persistent'] = $GLOBALS['wgMemCachedPersistent'];
 28+ }
 29+ if ( !isset( $params['compress_threshold'] ) ) {
 30+ $params['compress_threshold'] = 1500;
 31+ }
 32+ if ( !isset( $params['timeout'] ) ) {
 33+ $params['timeout'] = $GLOBALS['wgMemCachedTimeout'];
 34+ }
 35+ if ( !isset( $params['connect_timeout'] ) ) {
 36+ $params['connect_timeout'] = 0.1;
 37+ }
 38+
 39+ $this->client = new MemCachedClientforWiki( $params );
 40+ $this->client->set_servers( $params['servers'] );
 41+ $this->client->set_debug( $params['debug'] );
 42+ }
 43+
 44+ public function setDebug( $debug ) {
 45+ $this->client->set_debug( $debug );
 46+ }
 47+
 48+ public function get( $key ) {
 49+ return $this->client->get( $this->encodeKey( $key ) );
 50+ }
 51+
 52+ public function set( $key, $value, $exptime = 0 ) {
 53+ return $this->client->set( $this->encodeKey( $key ), $value, $exptime );
 54+ }
 55+
 56+ public function delete( $key, $time = 0 ) {
 57+ return $this->client->delete( $this->encodeKey( $key ), $time );
 58+ }
 59+
 60+ public function lock( $key, $timeout = 0 ) {
 61+ return $this->client->lock( $this->encodeKey( $key ), $timeout );
 62+ }
 63+
 64+ public function unlock( $key ) {
 65+ return $this->client->unlock( $this->encodeKey( $key ) );
 66+ }
 67+
 68+ public function add( $key, $value, $exptime = 0 ) {
 69+ return $this->client->add( $this->encodeKey( $key ), $value, $exptime );
 70+ }
 71+
 72+ public function replace( $key, $value, $exptime = 0 ) {
 73+ return $this->client->replace( $this->encodeKey( $key ), $value, $exptime );
 74+ }
 75+
 76+ public function incr( $key, $value = 1 ) {
 77+ return $this->client->incr( $this->encodeKey( $key ), $value );
 78+ }
 79+
 80+ public function decr( $key, $value = 1 ) {
 81+ return $this->client->decr( $this->encodeKey( $key ), $value );
 82+ }
 83+
 84+ /**
 85+ * Get the underlying client object. This is provided for debugging
 86+ * purposes.
 87+ */
 88+ public function getClient() {
 89+ return $this->client;
 90+ }
 91+
 92+ /**
 93+ * Encode a key for use on the wire inside the memcached protocol.
 94+ *
 95+ * We encode spaces and line breaks to avoid protocol errors. We encode
 96+ * the other control characters for compatibility with libmemcached
 97+ * verify_key. We leave other punctuation alone, to maximise backwards
 98+ * compatibility.
 99+ */
 100+ public function encodeKey( $key ) {
 101+ return preg_replace_callback( '/[\x00-\x20\x25\x7f]+/',
 102+ array( $this, 'encodeKeyCallback' ), $key );
 103+ }
 104+
 105+ protected function encodeKeyCallback( $m ) {
 106+ return urlencode( $m[0] );
 107+ }
 108+
 109+ /**
 110+ * Decode a key encoded with encodeKey(). This is provided as a convenience
 111+ * function for debugging.
 112+ */
 113+ public function decodeKey( $key ) {
 114+ return urldecode( $key );
 115+ }
 116+}
 117+
Property changes on: trunk/phase3/includes/objectcache/MemcachedPhpBagOStuff.php
___________________________________________________________________
Added: svn:eol-style
1118 + native
Index: trunk/phase3/includes/objectcache/BagOStuff.php
@@ -43,7 +43,7 @@
4444 abstract class BagOStuff {
4545 var $debugMode = false;
4646
47 - public function set_debug( $bool ) {
 47+ public function setDebug( $bool ) {
4848 $this->debugMode = $bool;
4949 }
5050
@@ -87,23 +87,7 @@
8888 }
8989
9090 /* *** Emulated functions *** */
91 - /* Better performance can likely be got with custom written versions */
92 - public function get_multi( $keys ) {
93 - $out = array();
9491
95 - foreach ( $keys as $key ) {
96 - $out[$key] = $this->get( $key );
97 - }
98 -
99 - return $out;
100 - }
101 -
102 - public function set_multi( $hash, $exptime = 0 ) {
103 - foreach ( $hash as $key => $value ) {
104 - $this->set( $key, $value, $exptime );
105 - }
106 - }
107 -
10892 public function add( $key, $value, $exptime = 0 ) {
10993 if ( !$this->get( $key ) ) {
11094 $this->set( $key, $value, $exptime );
@@ -112,18 +96,6 @@
11397 }
11498 }
11599
116 - public function add_multi( $hash, $exptime = 0 ) {
117 - foreach ( $hash as $key => $value ) {
118 - $this->add( $key, $value, $exptime );
119 - }
120 - }
121 -
122 - public function delete_multi( $keys, $time = 0 ) {
123 - foreach ( $keys as $key ) {
124 - $this->delete( $key, $time );
125 - }
126 - }
127 -
128100 public function replace( $key, $value, $exptime = 0 ) {
129101 if ( $this->get( $key ) !== false ) {
130102 $this->set( $key, $value, $exptime );
Index: trunk/phase3/includes/objectcache/SqlBagOStuff.php
@@ -6,21 +6,40 @@
77 * @ingroup Cache
88 */
99 class SqlBagOStuff extends BagOStuff {
10 - var $lb, $db;
 10+ var $lb, $db, $serverInfo;
1111 var $lastExpireAll = 0;
1212
 13+ /**
 14+ * Constructor. Parameters are:
 15+ * - server: A server info structure in the format required by each
 16+ * element in $wgDBServers.
 17+ */
 18+ public function __construct( $params ) {
 19+ if ( isset( $params['server'] ) ) {
 20+ $this->serverInfo = $params['server'];
 21+ $this->serverInfo['load'] = 1;
 22+ }
 23+ }
 24+
1325 protected function getDB() {
1426 if ( !isset( $this->db ) ) {
15 - /* We must keep a separate connection to MySQL in order to avoid deadlocks
16 - * However, SQLite has an opposite behaviour.
17 - * @todo Investigate behaviour for other databases
18 - */
19 - if ( wfGetDB( DB_MASTER )->getType() == 'sqlite' ) {
20 - $this->db = wfGetDB( DB_MASTER );
21 - } else {
22 - $this->lb = wfGetLBFactory()->newMainLB();
 27+ # If server connection info was given, use that
 28+ if ( $this->serverInfo ) {
 29+ $this->lb = new LoadBalancer( array(
 30+ 'servers' => array( $this->serverInfo ) ) );
2331 $this->db = $this->lb->getConnection( DB_MASTER );
2432 $this->db->clearFlag( DBO_TRX );
 33+ } else {
 34+ # We must keep a separate connection to MySQL in order to avoid deadlocks
 35+ # However, SQLite has an opposite behaviour.
 36+ # @todo Investigate behaviour for other databases
 37+ if ( wfGetDB( DB_MASTER )->getType() == 'sqlite' ) {
 38+ $this->db = wfGetDB( DB_MASTER );
 39+ } else {
 40+ $this->lb = wfGetLBFactory()->newMainLB();
 41+ $this->db = $this->lb->getConnection( DB_MASTER );
 42+ $this->db->clearFlag( DBO_TRX );
 43+ }
2544 }
2645 }
2746
Index: trunk/phase3/includes/objectcache/MemcachedClient.php
@@ -51,7 +51,7 @@
5252 * '127.0.0.1:10020'),
5353 * 'debug' => false,
5454 * 'compress_threshold' => 10240,
55 - * 'persistant' => true));
 55+ * 'persistent' => true));
5656 *
5757 * $mc->add('key', array('some', 'array'));
5858 * $mc->replace('key', 'some random string');
@@ -158,12 +158,12 @@
159159 var $_compress_threshold;
160160
161161 /**
162 - * Are we using persistant links?
 162+ * Are we using persistent links?
163163 *
164164 * @var boolean
165165 * @access private
166166 */
167 - var $_persistant;
 167+ var $_persistent;
168168
169169 /**
170170 * If only using one server; contains ip:port to connect to
@@ -245,12 +245,11 @@
246246 * @return mixed
247247 */
248248 public function __construct( $args ) {
249 - global $wgMemCachedTimeout;
250249 $this->set_servers( isset( $args['servers'] ) ? $args['servers'] : array() );
251250 $this->_debug = isset( $args['debug'] ) ? $args['debug'] : false;
252251 $this->stats = array();
253252 $this->_compress_threshold = isset( $args['compress_threshold'] ) ? $args['compress_threshold'] : 0;
254 - $this->_persistant = isset( $args['persistant'] ) ? $args['persistant'] : false;
 253+ $this->_persistent = isset( $args['persistent'] ) ? $args['persistent'] : false;
255254 $this->_compress_enable = true;
256255 $this->_have_zlib = function_exists( 'gzcompress' );
257256
@@ -258,9 +257,9 @@
259258 $this->_host_dead = array();
260259
261260 $this->_timeout_seconds = 0;
262 - $this->_timeout_microseconds = $wgMemCachedTimeout;
 261+ $this->_timeout_microseconds = isset( $args['timeout'] ) ? $args['timeout'] : 100000;
263262
264 - $this->_connect_timeout = 0.01;
 263+ $this->_connect_timeout = isset( $args['connect_timeout'] ) ? $args['connect_timeout'] : 0.1;
265264 $this->_connect_attempts = 2;
266265 }
267266
@@ -433,7 +432,11 @@
434433 }
435434
436435 wfProfileOut( __METHOD__ );
437 - return @$val[$key];
 436+ if ( isset( $val[$key] ) ) {
 437+ return $val[$key];
 438+ } else {
 439+ return false;
 440+ }
438441 }
439442
440443 // }}}
@@ -695,7 +698,7 @@
696699 $errno = $errstr = null;
697700 for( $i = 0; !$sock && $i < $this->_connect_attempts; $i++ ) {
698701 wfSuppressWarnings();
699 - if ( $this->_persistant == 1 ) {
 702+ if ( $this->_persistent == 1 ) {
700703 $sock = pfsockopen( $ip, $port, $errno, $errstr, $timeout );
701704 } else {
702705 $sock = fsockopen( $ip, $port, $errno, $errstr, $timeout );
Index: trunk/phase3/includes/objectcache/MultiWriteBagOStuff.php
@@ -0,0 +1,95 @@
 2+<?php
 3+
 4+/**
 5+ * A cache class that replicates all writes to multiple child caches. Reads
 6+ * are implemented by reading from the caches in the order they are given in
 7+ * the configuration until a cache gives a positive result.
 8+ */
 9+class MultiWriteBagOStuff extends BagOStuff {
 10+ var $caches;
 11+
 12+ /**
 13+ * Constructor. Parameters are:
 14+ *
 15+ * - caches: This should have a numbered array of cache parameter
 16+ * structures, in the style required by $wgObjectCaches. See
 17+ * the documentation of $wgObjectCaches for more detail.
 18+ */
 19+ public function __construct( $params ) {
 20+ if ( !isset( $params['caches'] ) ) {
 21+ throw new MWException( __METHOD__.': the caches parameter is required' );
 22+ }
 23+
 24+ $this->caches = array();
 25+ foreach ( $params['caches'] as $cacheInfo ) {
 26+ $this->caches[] = ObjectCache::newFromParams( $cacheInfo );
 27+ }
 28+ }
 29+
 30+ public function setDebug( $debug ) {
 31+ $this->doWrite( 'setDebug', $debug );
 32+ }
 33+
 34+ public function get( $key ) {
 35+ foreach ( $this->caches as $cache ) {
 36+ $value = $cache->get( $key );
 37+ if ( $value !== false ) {
 38+ return $value;
 39+ }
 40+ }
 41+ return false;
 42+ }
 43+
 44+ public function set( $key, $value, $exptime = 0 ) {
 45+ return $this->doWrite( 'set', $key, $value, $exptime );
 46+ }
 47+
 48+ public function delete( $key, $time = 0 ) {
 49+ return $this->doWrite( 'delete', $key, $time );
 50+ }
 51+
 52+ public function add( $key, $value, $exptime = 0 ) {
 53+ return $this->doWrite( 'add', $key, $value, $exptime );
 54+ }
 55+
 56+ public function replace( $key, $value, $exptime = 0 ) {
 57+ return $this->doWrite( 'replace', $key, $value, $exptime );
 58+ }
 59+
 60+ public function incr( $key, $value = 1 ) {
 61+ return $this->doWrite( 'incr', $key, $value );
 62+ }
 63+
 64+ public function decr( $key, $value = 1 ) {
 65+ return $this->doWrite( 'decr', $key, $value );
 66+ }
 67+
 68+ public function lock( $key, $timeout = 0 ) {
 69+ // Lock only the first cache, to avoid deadlocks
 70+ if ( isset( $this->caches[0] ) ) {
 71+ return $this->caches[0]->lock( $key, $timeout );
 72+ } else {
 73+ return true;
 74+ }
 75+ }
 76+
 77+ public function unlock( $key ) {
 78+ if ( isset( $this->caches[0] ) ) {
 79+ return $this->caches[0]->unlock( $key );
 80+ } else {
 81+ return true;
 82+ }
 83+ }
 84+
 85+ protected function doWrite( $method /*, ... */ ) {
 86+ $ret = true;
 87+ $args = func_get_args();
 88+ array_shift( $args );
 89+
 90+ foreach ( $this->caches as $cache ) {
 91+ $ret = $ret && call_user_func_array( array( $cache, $method ), $args );
 92+ }
 93+ return $ret;
 94+ }
 95+
 96+}
Property changes on: trunk/phase3/includes/objectcache/MultiWriteBagOStuff.php
___________________________________________________________________
Added: svn:eol-style
197 + native
Index: trunk/phase3/includes/Setup.php
@@ -283,7 +283,6 @@
284284 require_once( "$IP/includes/Hooks.php" );
285285 require_once( "$IP/includes/Namespace.php" );
286286 require_once( "$IP/includes/ProxyTools.php" );
287 -require_once( "$IP/includes/objectcache/ObjectCache.php" );
288287 require_once( "$IP/includes/ImageFunctions.php" );
289288 wfProfileOut( $fname . '-includes' );
290289 wfProfileIn( $fname . '-misc1' );
@@ -323,9 +322,9 @@
324323 wfProfileOut( $fname . '-misc1' );
325324 wfProfileIn( $fname . '-memcached' );
326325
327 -$wgMemc =& wfGetMainCache();
328 -$messageMemc =& wfGetMessageCacheStorage();
329 -$parserMemc =& wfGetParserCacheStorage();
 326+$wgMemc = wfGetMainCache();
 327+$messageMemc = wfGetMessageCacheStorage();
 328+$parserMemc = wfGetParserCacheStorage();
330329
331330 wfDebug( 'CACHES: ' . get_class( $wgMemc ) . '[main] ' .
332331 get_class( $messageMemc ) . '[message] ' .
Index: trunk/phase3/includes/AutoLoader.php
@@ -505,11 +505,15 @@
506506 'BagOStuff' => 'includes/objectcache/BagOStuff.php',
507507 'DBABagOStuff' => 'includes/objectcache/DBABagOStuff.php',
508508 'eAccelBagOStuff' => 'includes/objectcache/eAccelBagOStuff.php',
509 - 'FakeMemCachedClient' => 'includes/objectcache/ObjectCache.php',
 509+ 'EmptyBagOStuff' => 'includes/objectcache/EmptyBagOStuff.php',
 510+ 'FakeMemCachedClient' => 'includes/objectcache/EmptyBagOStuff.php',
510511 'HashBagOStuff' => 'includes/objectcache/HashBagOStuff.php',
511512 'MWMemcached' => 'includes/objectcache/MemcachedClient.php',
512513 'MediaWikiBagOStuff' => 'includes/objectcache/SqlBagOStuff.php',
513514 'MemCachedClientforWiki' => 'includes/objectcache/MemcachedClient.php',
 515+ 'MemcachedPhpBagOStuff' => 'includes/objectcache/MemcachedPhpBagOStuff.php',
 516+ 'MultiWriteBagOStuff' => 'includes/objectcache/MultiWriteBagOStuff.php',
 517+ 'ObjectCache' => 'includes/objectcache/ObjectCache.php',
514518 'SqlBagOStuff' => 'includes/objectcache/SqlBagOStuff.php',
515519 'WinCacheBagOStuff' => 'includes/objectcache/WinCacheBagOStuff.php',
516520 'XCacheBagOStuff' => 'includes/objectcache/XCacheBagOStuff.php',
Index: trunk/phase3/includes/DefaultSettings.php
@@ -1481,6 +1481,8 @@
14821482 * - CACHE_DBA: Use PHP's DBA extension to store in a DBM-style
14831483 * database. This is slow, and is not recommended for
14841484 * anything other than debugging.
 1485+ * - (other): A string may be used which identifies a cache
 1486+ * configuration in $wgObjectCaches.
14851487 *
14861488 * @see $wgMessageCacheType, $wgParserCacheType
14871489 */
@@ -1503,6 +1505,36 @@
15041506 $wgParserCacheType = CACHE_ANYTHING;
15051507
15061508 /**
 1509+ * Advanced object cache configuration.
 1510+ *
 1511+ * Use this to define the class names and constructor parameters which are used
 1512+ * for the various cache types. Custom cache types may be defined here and
 1513+ * referenced from $wgMainCacheType, $wgMessageCacheType or $wgParserCacheType.
 1514+ *
 1515+ * The format is an associative array where the key is a cache identifier, and
 1516+ * the value is an associative array of parameters. The "class" parameter is the
 1517+ * class name which will be used. Alternatively, a "factory" parameter may be
 1518+ * given, giving a callable function which will generate a suitable cache object.
 1519+ *
 1520+ * The other parameters are dependent on the class used.
 1521+ */
 1522+$wgObjectCaches = array(
 1523+ CACHE_NONE => array( 'class' => 'FakeMemCachedClient' ),
 1524+ CACHE_DB => array( 'class' => 'SqlBagOStuff', 'table' => 'objectcache' ),
 1525+ CACHE_DBA => array( 'class' => 'DBABagOStuff' ),
 1526+
 1527+ CACHE_ANYTHING => array( 'factory' => 'ObjectCache::newAnything' ),
 1528+ CACHE_ACCEL => array( 'factory' => 'ObjectCache::newAccelerator' ),
 1529+ CACHE_MEMCACHED => array( 'factory' => 'ObjectCache::newMemcached' ),
 1530+
 1531+ 'eaccelerator' => array( 'class' => 'eAccelBagOStuff' ),
 1532+ 'apc' => array( 'class' => 'APCBagOStuff' ),
 1533+ 'xcache' => array( 'class' => 'XCacheBagOStuff' ),
 1534+ 'wincache' => array( 'class' => 'WinCacheBagOStuff' ),
 1535+ 'memcached-php' => array( 'class' => 'MemcachedPhpBagOStuff' ),
 1536+);
 1537+
 1538+/**
15071539 * The expiry time for the parser cache, in seconds. The default is 86.4k
15081540 * seconds, otherwise known as a day.
15091541 */

Sign-offs

UserFlagDate
😂inspected15:36, 3 March 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r83144Followup r83140: FakeMemCachedClient -> EmptyBagOStuff in testsdemon12:55, 3 March 2011
r83147* When CACHE_ANYTHING is requested, return the cached instance, don't constru...tstarling15:24, 3 March 2011
r83208* Added an Ehcache client....tstarling06:01, 4 March 2011
r83416Follow up r83140. Add a clear() method to ObjectCache so that it can be used ...platonides23:07, 6 March 2011
r83418r82867 converted $wgCaches into a class instance. Update the parsertests....platonides23:15, 6 March 2011
r83419$wgCaches removed in r83140. Installer.php no longer uses $errs and $mainList...platonides23:22, 6 March 2011
r83878Fix r83140: 'ObjectCache::newAnything' is not a valid callback, use array( 'O...catrope10:27, 14 March 2011
r84727Merged in the ObjectCache refactor and the Ehcache client. MFT r83135, r83136...tstarling03:28, 25 March 2011
r89106Followup r83140, fix undefined $idreedy14:28, 29 May 2011
r105419(bug 32853) DBA cache broken in MW 1.18...hashar11:04, 7 December 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r83136More renames and splits for objectcache reorganisation.tstarling04:48, 3 March 2011

Comments

#Comment by Nikerabbit (talk | contribs)   09:56, 3 March 2011
  • Made MWMemcached::get() return false on failure instead of null, to match the BagOStuff documentation and the other BagOStuff subclasses. Anything that was relying on it returning null would have already been broken with SqlBagOStuff.

This is good! It has caused so many subtle bugs.

#Comment by IAlex (talk | contribs)   10:07, 3 March 2011
+	CACHE_NONE => array( 'class' => 'FakeMemCachedClient' ),

Shouldn't the class be EmptyBagOStuff ?

#Comment by Tim Starling (talk | contribs)   10:54, 3 March 2011

It should. There are quite a lot of references to FakeMemCachedClient in the core. I wanted to fix them in a separate commit to make review easier.

#Comment by 😂 (talk | contribs)   12:57, 3 March 2011

Fixed the instances in tests/ in r83144.

#Comment by Siebrand (talk | contribs)   15:25, 3 March 2011

Causes PHP Fatal error: Call to a member function set() on a non-object in /www/w/includes/MemcachedSessions.php on line 67. Tim can kind of reproduce this. His latest comments from IRC: "My test wiki just started reproducing it. Actually I think it is PHP's fault after all. I'm reproducing it on 5.3.4. What I don't understand is how it could ever work."

Marking FIXME.

#Comment by Tim Starling (talk | contribs)   15:25, 3 March 2011

Should be fixed as of r83147.

#Comment by Tim Starling (talk | contribs)   15:52, 3 March 2011

On request shutdown, PHP does things in the following order:

  1. register_shutdown_function() functions
  2. zend_call_destructors()
  3. zend_deactivate_modules()

zend_call_destructors() goes through the $GLOBALS symbol table and identifies every global variable that holds an object which has a reference count of 1. It removes these globals. It doesn't decrement any reference counts. Any globals which are not objects, or have a reference count greater than 1, are left in the symbol table. So if we have a variable $x:

$x = new Foo;

That will be deleted, unless we protect it by making a reference to it from somewhere:

$y =& $x;

After this perplexing action, zend_deactivate_modules() calls php_session_save_current_state() which calls our memcached save handler, which uses $wgMemc. Because this revision removed all references to $wgMemc, being obsolete holdovers from PHP 4, this exposed $wgMemc to zend_call_destructors(), meaning that it was not present in the symbol table when memsess_write() was called.

#Comment by Siebrand (talk | contribs)   16:01, 3 March 2011

Thanks for digging, Tim. Back to new.

#Comment by Tim Starling (talk | contribs)   00:04, 4 March 2011
#Comment by Siebrand (talk | contribs)   23:04, 5 March 2011

Marked WONTFIX upstream. Made a choice between two evils, apparently.

#Comment by Tim Starling (talk | contribs)   00:19, 6 March 2011

I talked to Johannes on IRC about it. I think I convinced him that something is not quite right about this code. He said he'd have another look at it if I wrote it up for php internals.

Filing bugs against PHP is more of a starting point than an ending point. Most of the bugs that I've gotten fixed in PHP started out being closed as some flavour of "bogus".

#Comment by Reedy (talk | contribs)   10:51, 25 March 2011

newFromParams() in ObjectCache.php:

Line 55 is using an undefined variable $id

Status & tagging log