r81741 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81740‎ | r81741 | r81742 >
Date:15:02, 8 February 2011
Author:brion
Status:reverted (Comments)
Tags:
Comment:
Provisional workaround for parser cache interaction with r70783

New format info getting saved for one set of options would invalidate *all* old caches in effect, causing a lot more clears and reparses than expected.
Fix from Platonides in IRC
Modified paths:
  • /branches/wmf/1.17wmf1/includes/parser/ParserCache.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.17wmf1/includes/parser/ParserCache.php
——@@ -136,6 +136,11 @@
137137
138138 $value = $this->mMemc->get( $parserOutputKey );
139139 if ( !$value ) {
 140+ wfDebug( "New format parser cache miss.\n" );
 141+ $parserOutputKey = $this->getParserOutputKey( $article, ParserOptions::legacyOptions() );
 142+ $value = $this->mMemc->get( $parserOutputKey );
 143+ }
 144+ if ( !$value ) {
140145 wfDebug( "Parser cache miss.\n" );
141146 wfIncrStats( "pcache_miss_absent" );
142147 wfProfileOut( __METHOD__ );

Follow-up revisions

RevisionCommit summaryAuthorDate
r83013Revert r81741 (temporary 1.17 transition workaround to avoid parser cache cle...tstarling12:52, 1 March 2011
r83476Merge corrected r81741 and resolve the TODO....platonides19:13, 7 March 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r70783Use only the page relevant pieces in the parser cache key. Eg. two users with...platonides21:53, 9 August 2010

Comments

#Comment by Happy-melon (talk | contribs)   15:08, 8 February 2011

This presumably needs to be added to trunk as well?

#Comment by 😂 (talk | contribs)   15:13, 8 February 2011

Adding new 'mergetotrunk' keyword. I imagine we'll have several fixes checked in over the next 24hrs that should make it back up the tree.

#Comment by Tim Starling (talk | contribs)   12:33, 10 February 2011

That's what "trunk" is for.

#Comment by Platonides (talk | contribs)   11:13, 22 February 2011

I'm not convinced about it being appropiate for trunk, as most sites won't need it, not even wmf in a few days. However, we have no way to know when $wgParserCacheExpireTime after update has elapsed :(

#Comment by Tim Starling (talk | contribs)   12:50, 1 March 2011

Parser::legacyOptions() returns an array, and getParserOutputKey() expects a string as a second parameter. Domas noticed a lot of keys of the form "enwiki:pcache:idhash:7029871-0!Array" in tcpdump and raised the alarm. I'm just going to revert this for now on the site.

#Comment by Platonides (talk | contribs)   17:58, 2 March 2011

Great way to try improving cache hits without doing anything.

It should have been 

$this->getParserOutputKey( $article, $popts->optionsHash( ParserOptions::legacyOptions() ) );

Although all entries this was targetting are probably expired by now.


All those gets would have been misses, so luckily, no real harm was done.

Status & tagging log