r101492 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101491‎ | r101492 | r101493 >
Date:16:10, 1 November 2011
Author:mah
Status:reverted (Comments)
Tags:
Comment:
Apply Vitaliy Filippov's patch from Bug #29283 with some small style mods
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/LocalisationCache.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/LocalisationCache.php
@@ -159,8 +159,13 @@
160160 case 'db':
161161 $storeClass = 'LCStore_DB';
162162 break;
 163+ case 'accel':
163164 case 'detect':
164 - $storeClass = $wgCacheDirectory ? 'LCStore_CDB' : 'LCStore_DB';
 165+ if ( !( wfGetCache( CACHE_ACCEL ) instanceof FakeMemCachedClient ) ) {
 166+ $storeClass = 'LCStore_Accel';
 167+ } else {
 168+ $storeClass = $wgCacheDirectory ? 'LCStore_CDB' : 'LCStore_DB';
 169+ }
165170 break;
166171 default:
167172 throw new MWException(
@@ -348,7 +353,9 @@
349354 }
350355
351356 $deps = $this->store->get( $code, 'deps' );
352 - if ( $deps === null ) {
 357+ $keys = $this->store->get( $code, 'list', 'messages' );
 358+ // 'list:messages' sometimes expires separately of 'deps' in LCStore_Accel
 359+ if ( $deps === null || $keys === null ) {
353360 wfDebug( __METHOD__."($code): cache missing, need to make one\n" );
354361 return true;
355362 }
@@ -830,6 +837,54 @@
831838 }
832839
833840 /**
 841+ * LCStore implementation which uses PHP accelerator to store data.
 842+ * This will work if one of XCache, eAccelerator, or APC cacher is configured.
 843+ * (See ObjectCache.php)
 844+ */
 845+class LCStore_Accel implements LCStore {
 846+ var $currentLang;
 847+ var $keys;
 848+
 849+ public function __construct() {
 850+ $this->cache = wfGetCache( CACHE_ACCEL );
 851+ }
 852+
 853+ public function get( $code, $key ) {
 854+ $k = wfMemcKey( 'l10n', $code, 'k', $key );
 855+ return $this->cache->get( $k );
 856+ }
 857+
 858+ public function startWrite( $code ) {
 859+ $k = wfMemcKey( 'l10n', $code, 'l' );
 860+ $keys = $this->cache->get( $k );
 861+ if ( $keys ) {
 862+ foreach ( $keys as $k ) {
 863+ $this->cache->delete( $k );
 864+ }
 865+ }
 866+ $this->currentLang = $code;
 867+ $this->keys = array();
 868+ }
 869+
 870+ public function finishWrite() {
 871+ if ( $this->currentLang ) {
 872+ $k = wfMemcKey( 'l10n', $this->currentLang, 'l' );
 873+ $this->cache->set( $k, array_keys( $this->keys ) );
 874+ }
 875+ $this->currentLang = null;
 876+ $this->keys = array();
 877+ }
 878+
 879+ public function set( $key, $value ) {
 880+ if ( $this->currentLang ) {
 881+ $k = wfMemcKey( 'l10n', $this->currentLang, 'k', $key );
 882+ $this->keys[$k] = true;
 883+ $this->cache->set( $k, $value );
 884+ }
 885+ }
 886+}
 887+
 888+/**
834889 * LCStore implementation which uses the standard DB functions to store data.
835890 * This will work on any MediaWiki installation.
836891 */
@@ -1126,4 +1181,4 @@
11271182 $this->unload( $code );
11281183 }
11291184 }
1130 -}
 1185+}
\ No newline at end of file
Index: trunk/phase3/includes/AutoLoader.php
@@ -127,6 +127,7 @@
128128 'IndexPager' => 'includes/Pager.php',
129129 'Interwiki' => 'includes/interwiki/Interwiki.php',
130130 'IP' => 'includes/IP.php',
 131+ 'LCStore_Accel' => 'includes/LocalisationCache.php',
131132 'LCStore_CDB' => 'includes/LocalisationCache.php',
132133 'LCStore_DB' => 'includes/LocalisationCache.php',
133134 'LCStore_Null' => 'includes/LocalisationCache.php',

Follow-up revisions

RevisionCommit summaryAuthorDate
r101496followup 101492 — make it actually work.mah16:29, 1 November 2011
r101500Revert r101492, broken, see CR. Also revert followup r101496.catrope16:51, 1 November 2011
r101507Re-apply Vitaliy Filippov's patch from Bug #29283 with a modification...mah18:31, 1 November 2011
r102304re Bug #29283, r101507: Apply Vitaliy Filippov's followup fix:...mah17:50, 7 November 2011

Comments

#Comment by Catrope (talk | contribs)   16:32, 1 November 2011
+	public function get( $code, $key ) {
+		$k = wfMemcKey( 'l10n', $code, 'k', $key );
+		return $this->cache->get( $k );
+	}
+

Doesn't the LCStore_Accel class just duplicate the one for memcached but with a different value for $this->cache? If so, it would be nice to use inheritance to remove the code duplication.

#Comment by Catrope (talk | contribs)   16:36, 1 November 2011

Running MW in an environment without APC this gives me the following error. Reverting this revision fixes it.

CACHE_ACCEL requested but no suitable object cache is present. You may want to install APC.

Backtrace:

#0 [internal function]: ObjectCache::newAccelerator(Array)
#1 /home/catrope/mediawiki/trunk/phase3/includes/objectcache/ObjectCache.php(62): call_user_func('ObjectCache::ne...', Array)
#2 /home/catrope/mediawiki/trunk/phase3/includes/objectcache/ObjectCache.php(50): ObjectCache::newFromParams(Array)
#3 /home/catrope/mediawiki/trunk/phase3/includes/objectcache/ObjectCache.php(23): ObjectCache::newFromId(3)
#4 /home/catrope/mediawiki/trunk/phase3/includes/GlobalFunctions.php(3569): ObjectCache::getInstance(3)
#5 /home/catrope/mediawiki/trunk/phase3/includes/LocalisationCache.php(164): wfGetCache(3)
#6 /home/catrope/mediawiki/trunk/phase3/languages/Language.php(271): LocalisationCache->__construct(Array)
#7 /home/catrope/mediawiki/trunk/phase3/languages/Language.php(284): Language::getLocalisationCache()
#8 /home/catrope/mediawiki/trunk/phase3/languages/Language.php(176): Language->__construct()
#9 /home/catrope/mediawiki/trunk/phase3/languages/Language.php(146): Language::newFromCode('en')
#10 /home/catrope/mediawiki/trunk/phase3/includes/Setup.php(434): Language::factory('en')
#11 /home/catrope/mediawiki/trunk/phase3/includes/WebStart.php(157): require_once('/home/catrope/m...')
#12 /home/catrope/mediawiki/trunk/phase3/index.php(54): require('/home/catrope/m...')
#13 {main}
#Comment by Catrope (talk | contribs)   16:42, 1 November 2011

Mark told me to try r101496 but that didn't help.

I had $wgMessageCacheType = CACHE_ANYTHING (the default), but even setting it to CACHE_MEMCACHED doesn't make this error go away.

#Comment by Exobuzz (talk | contribs)   06:51, 5 December 2011

I get this error on the 1.18 release when using dumpBackup.php

#Comment by Exobuzz (talk | contribs)   07:00, 5 December 2011

The error I have I think is unrelated sorry. ignore ill post bugzilla bug.

Status & tagging log