From 78aad9802d467443996923f88ae7fd8c152572e0 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 1 Apr 2014 10:52:24 -0400 Subject: [PATCH] Include parsed revision ID in parser cache One theory for what's behind bug 46014 is that the vandal submits the edit, then someone (maybe the vandal) gets into the branch of Article::view that uses PoolWorkArticleView, then ClueBot comes along and reverts before the PoolWorkArticleView actually executes. Once that PoolWorkArticleView actually does execute, it overwrites the parser cache entry from ClueBot's revert with the one from the old edit. To detect this sort of thing, let's include the revision id in the parser cache entry and consider it expired if that doesn't match. Which makes sense to do anyway. And for good measure, let's have PoolWorkArticleView not save to the parser cache if !$isCurrent. Bug: 46014 Change-Id: Ifcc4d2f67f3b77f990eb2fa45417a25bd6c7b790 --- includes/WikiPage.php | 9 ++++-- includes/jobqueue/jobs/RefreshLinksJob.php | 4 ++- includes/parser/CacheTime.php | 35 +++++++++++++++++++++- includes/parser/ParserCache.php | 30 +++++++++++++++++-- 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/includes/WikiPage.php b/includes/WikiPage.php index af74f47b04..18409b0a7b 100644 --- a/includes/WikiPage.php +++ b/includes/WikiPage.php @@ -2070,6 +2070,7 @@ class WikiPage implements Page, IDBAccessObject { $edit = (object)array(); $edit->revid = $revid; + $edit->timestamp = wfTimestampNow(); $edit->pstContent = $content ? $content->preSaveTransform( $this->mTitle, $user, $popts ) : null; @@ -2127,7 +2128,9 @@ class WikiPage implements Page, IDBAccessObject { // Save it to the parser cache if ( $wgEnableParserCache ) { $parserCache = ParserCache::singleton(); - $parserCache->save( $editInfo->output, $this, $editInfo->popts ); + $parserCache->save( + $editInfo->output, $this, $editInfo->popts, $editInfo->timestamp, $editInfo->revid + ); } // Update the links tables and other secondary data @@ -3605,9 +3608,9 @@ class PoolWorkArticleView extends PoolCounterWork { $this->page->getTitle()->getPrefixedDBkey() ) ); } - if ( $this->cacheable && $this->parserOutput->isCacheable() ) { + if ( $this->cacheable && $this->parserOutput->isCacheable() && $isCurrent ) { ParserCache::singleton()->save( - $this->parserOutput, $this->page, $this->parserOptions, $cacheTime ); + $this->parserOutput, $this->page, $this->parserOptions, $cacheTime, $this->revid ); } // Make sure file cache is not used on uncacheable content. diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php index 3bcb4fc36e..f82af273d8 100644 --- a/includes/jobqueue/jobs/RefreshLinksJob.php +++ b/includes/jobqueue/jobs/RefreshLinksJob.php @@ -163,7 +163,9 @@ class RefreshLinksJob extends Job { && $parserOutput->isCacheable() ) { $ctime = wfTimestamp( TS_MW, (int)$start ); // cache time - ParserCache::singleton()->save( $parserOutput, $page, $parserOptions, $ctime ); + ParserCache::singleton()->save( + $parserOutput, $page, $parserOptions, $ctime, $revision->getId() + ); } } diff --git a/includes/parser/CacheTime.php b/includes/parser/CacheTime.php index e1b3f28deb..6dda64cf0d 100644 --- a/includes/parser/CacheTime.php +++ b/includes/parser/CacheTime.php @@ -35,7 +35,8 @@ class CacheTime { var $mVersion = Parser::VERSION, # Compatibility check $mCacheTime = '', # Time when this object was generated, or -1 for uncacheable. Used in ParserCache. $mCacheExpiry = null, # Seconds after which the object should expire, use 0 for uncachable. Used in ParserCache. - $mContainsOldMagic; # Boolean variable indicating if the input contained variables like {{CURRENTDAY}} + $mContainsOldMagic, # Boolean variable indicating if the input contained variables like {{CURRENTDAY}} + $mCacheRevisionId = null; # Revision ID that was parsed /** * @return string TS_MW timestamp @@ -69,6 +70,22 @@ class CacheTime { return wfSetVar( $this->mCacheTime, $t ); } + /** + * @since 1.23 + * @return int|null Revision id, if any was set + */ + function getCacheRevisionId() { + return $this->mCacheRevisionId; + } + + /** + * @since 1.23 + * @param $id int Revision id + */ + function setCacheRevisionId( $id ) { + $this->mCacheRevisionId = $id; + } + /** * Sets the number of seconds after which this object should expire. * This value is used with the ParserCache. @@ -152,4 +169,20 @@ class CacheTime { version_compare( $this->mVersion, Parser::VERSION, "lt" ); } + /** + * Return true if this cached output object is for a different revision of + * the page. + * + * @todo We always return false if $this->getCacheRevisionId() is null; + * this prevents invalidating the whole parser cache when this change is + * deployed. Someday that should probably be changed. + * + * @since 1.23 + * @param int $id the affected article's current revision id + * @return Boolean + */ + public function isDifferentRevision( $id ) { + $cached = $this->getCacheRevisionId(); + return $cached !== null && $id !== $cached; + } } diff --git a/includes/parser/ParserCache.php b/includes/parser/ParserCache.php index 040c74f809..4c7084e45c 100644 --- a/includes/parser/ParserCache.php +++ b/includes/parser/ParserCache.php @@ -146,6 +146,12 @@ class ParserCache { $cacheTime = $optionsKey->getCacheTime(); wfDebug( "Parser options key expired, touched " . $article->getTouched() . ", epoch $wgCacheEpoch, cached $cacheTime\n" ); return false; + } elseif ( $optionsKey->isDifferentRevision( $article->getLatest() ) ) { + wfIncrStats( "pcache_miss_revid" ); + $revId = $article->getLatest(); + $cachedRevId = $optionsKey->getCacheRevisionId(); + wfDebug( "ParserOutput key is for an old revision, latest $revId, cached $cachedRevId\n" ); + return false; } // $optionsKey->mUsedOptions is set by save() by calling ParserOutput::getUsedOptions() @@ -211,6 +217,12 @@ class ParserCache { $cacheTime = $value->getCacheTime(); wfDebug( "ParserOutput key expired, touched $touched, epoch $wgCacheEpoch, cached $cacheTime\n" ); $value = false; + } elseif ( $value->isDifferentRevision( $article->getLatest() ) ) { + wfIncrStats( "pcache_miss_revid" ); + $revId = $article->getLatest(); + $cachedRevId = $value->getCacheRevisionId(); + wfDebug( "ParserOutput key is for an old revision, latest $revId, cached $cachedRevId\n" ); + $value = false; } else { wfIncrStats( "pcache_hit" ); } @@ -224,11 +236,16 @@ class ParserCache { * @param WikiPage $page * @param ParserOptions $popts * @param string $cacheTime Time when the cache was generated + * @param int $revId Revision ID that was parsed */ - public function save( $parserOutput, $page, $popts, $cacheTime = null ) { + public function save( $parserOutput, $page, $popts, $cacheTime = null, $revId = null ) { $expire = $parserOutput->getCacheExpiry(); if ( $expire > 0 ) { $cacheTime = $cacheTime ?: wfTimestampNow(); + if ( !$revId ) { + $revision = $page->getRevision(); + $revId = $revision ? $revision->getId() : null; + } $optionsKey = new CacheTime; $optionsKey->mUsedOptions = $parserOutput->getUsedOptions(); @@ -236,6 +253,8 @@ class ParserCache { $optionsKey->setCacheTime( $cacheTime ); $parserOutput->setCacheTime( $cacheTime ); + $optionsKey->setCacheRevisionId( $revId ); + $parserOutput->setCacheRevisionId( $revId ); $optionsKey->setContainsOldMagic( $parserOutput->containsOldMagic() ); @@ -245,8 +264,13 @@ class ParserCache { // Save the timestamp so that we don't have to load the revision row on view $parserOutput->setTimestamp( $page->getTimestamp() ); - $parserOutput->mText .= "\n\n"; - wfDebug( "Saved in parser cache with key $parserOutputKey and timestamp $cacheTime\n" ); + $msg = "Saved in parser cache with key $parserOutputKey" . + " and timestamp $cacheTime" . + " and revision id $revId" . + "\n"; + + $parserOutput->mText .= "\n\n"; + wfDebug( $msg ); // Save the parser output $this->mMemc->set( $parserOutputKey, $parserOutput, $expire ); -- 2.20.1