jenkins-bot has submitted this change and it was merged. Change subject: Revert "Removed doCascadeProtectionUpdates method to avoid DB writes on page views" ......................................................................
Revert "Removed doCascadeProtectionUpdates method to avoid DB writes on page views" due to breakage at least in phpunit tests for mysql: https://travis-ci.org/wikimedia/mediawiki-extensions-Wikibase/jobs/51490784 This reverts commit 132f7bb89f38780f1433caff8ec643e91a850a8a. Change-Id: I85d19ab5ad30e8d13a956d7b7467a94c9e73219d --- M includes/DefaultSettings.php M includes/cache/BacklinkCache.php M includes/deferred/HTMLCacheUpdate.php M includes/deferred/LinksUpdate.php M includes/jobqueue/jobs/RefreshLinksJob.php M includes/page/Article.php M includes/page/WikiPage.php M includes/parser/ParserOutput.php M includes/poolcounter/PoolWorkArticleView.php 9 files changed, 61 insertions(+), 103 deletions(-) Approvals: Hoo man: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index bddccec..d4cdf9e 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -6422,7 +6422,6 @@ 'PublishStashedFile' => 'PublishStashedFileJob', 'ThumbnailRender' => 'ThumbnailRenderJob', 'recentChangesUpdate' => 'RecentChangesUpdateJob', - 'refreshLinksPrioritized' => 'RefreshLinksJob', // for cascading protection 'null' => 'NullJob' ); diff --git a/includes/cache/BacklinkCache.php b/includes/cache/BacklinkCache.php index c3aefc5..c6d9a18 100644 --- a/includes/cache/BacklinkCache.php +++ b/includes/cache/BacklinkCache.php @@ -487,45 +487,4 @@ return array( 'numRows' => $numRows, 'batches' => $batches ); } - - /** - * Get a Title iterator for cascade-protected template/file use backlinks - * - * @return TitleArray - * @since 1.25 - */ - public function getCascadeProtectedLinks() { - // This method is used to make redudant jobs anyway, so its OK to use - // a slave. Also, the set of cascade protected pages tends to be stable. - $dbr = $this->getDB(); - - $queries = array(); - // @note: UNION filters any duplicate pages - $queries[] = $dbr->selectSQLText( - array( 'templatelinks', 'page_restrictions', 'page' ), - array( 'page_namespace', 'page_title', 'page_id' ), - array( - 'tl_namespace' => $this->title->getNamespace(), - 'tl_title' => $this->title->getDBkey(), - 'tl_from = pr_page', - 'pr_cascade' => 1, - 'page_id = tl_from' - ) - ); - $queries[] = $dbr->selectSQLText( - array( 'imagelinks', 'page_restrictions', 'page' ), - array( 'page_namespace', 'page_title', 'page_id' ), - array( - 'il_to' => $this->title->getDBkey(), - 'il_from = pr_page', - 'pr_cascade' => 1, - 'page_id = il_from' - ) - ); - - return TitleArray::newFromResult( $dbr->query( - $dbr->unionQueries( $queries, false ), - __METHOD__ - ) ); - } } diff --git a/includes/deferred/HTMLCacheUpdate.php b/includes/deferred/HTMLCacheUpdate.php index 862ac27..e02cfbc 100644 --- a/includes/deferred/HTMLCacheUpdate.php +++ b/includes/deferred/HTMLCacheUpdate.php @@ -43,6 +43,7 @@ } public function doUpdate() { + $job = new HTMLCacheUpdateJob( $this->mTitle, array( @@ -62,5 +63,6 @@ $job->run(); // just do the purge query now } ); } + } } diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php index e4f00e7..9c377df 100644 --- a/includes/deferred/LinksUpdate.php +++ b/includes/deferred/LinksUpdate.php @@ -228,24 +228,12 @@ * Which means do LinksUpdate on all pages that include the current page, * using the job queue. */ - protected function queueRecursiveJobs() { + function queueRecursiveJobs() { self::queueRecursiveJobsForTable( $this->mTitle, 'templatelinks' ); if ( $this->mTitle->getNamespace() == NS_FILE ) { // Process imagelinks in case the title is or was a redirect self::queueRecursiveJobsForTable( $this->mTitle, 'imagelinks' ); } - - $bc = $this->mTitle->getBacklinkCache(); - // Get jobs for cascade-protected backlinks for a high priority queue. - // If meta-templates change to using a new template, the new template - // should be implicitly protected as soon as possible, if applicable. - // These jobs duplicate a subset of the above ones, but can run sooner. - // Which ever runs first generally no-ops the other one. - $jobs = array(); - foreach ( $bc->getCascadeProtectedLinks() as $title ) { - $jobs[] = new RefreshLinksJob( $title, array( 'prioritize' => true ) ); - } - JobQueueGroup::singleton()->push( $jobs ); } /** @@ -265,7 +253,6 @@ "refreshlinks:{$table}:{$title->getPrefixedText()}" ) ); - JobQueueGroup::singleton()->push( $job ); JobQueueGroup::singleton()->deduplicateRootJob( $job ); } diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php index 1252b0b..5d95792 100644 --- a/includes/jobqueue/jobs/RefreshLinksJob.php +++ b/includes/jobqueue/jobs/RefreshLinksJob.php @@ -39,10 +39,6 @@ function __construct( $title, $params = '' ) { parent::__construct( 'refreshLinks', $title, $params ); - // A separate type is used just for cascade-protected backlinks - if ( !empty( $this->params['prioritize'] ) ) { - $this->command .= 'Prioritized'; - } // Base backlink update jobs and per-title update jobs can be de-duplicated. // If template A changes twice before any jobs run, a clean queue will have: // (A base, A base) @@ -104,10 +100,6 @@ return true; } - /** - * @param Title $title - * @return bool - */ protected function runForTitle( Title $title = null ) { $linkCache = LinkCache::singleton(); $linkCache->clear(); diff --git a/includes/page/Article.php b/includes/page/Article.php index 83c3241..59f2ae7 100644 --- a/includes/page/Article.php +++ b/includes/page/Article.php @@ -707,7 +707,7 @@ } # Get the ParserOutput actually *displayed* here. - # Note that $this->mParserOutput is the *current*/oldid version output. + # Note that $this->mParserOutput is the *current* version output. $pOutput = ( $outputDone instanceof ParserOutput ) ? $outputDone // object fetched by hook : $this->mParserOutput; diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index fe61f6f..d30f589 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -3378,35 +3378,70 @@ } /** - * Opportunistically enqueue link update jobs given fresh parser output if useful + * Updates cascading protections * - * @param ParserOutput $parserOutput Current version page output - * @return bool Whether a job was pushed - * @since 1.25 + * @param ParserOutput $parserOutput ParserOutput object for the current version */ - public function triggerOpportunisticLinksUpdate( ParserOutput $parserOutput ) { - if ( wfReadOnly() ) { - return false; + public function doCascadeProtectionUpdates( ParserOutput $parserOutput ) { + if ( wfReadOnly() || !$this->mTitle->areRestrictionsCascading() ) { + return; } - if ( $this->mTitle->areRestrictionsCascading() ) { - // If the page is cascade protecting, the links should really be up-to-date - $params = array( 'prioritize' => true ); - } elseif ( $parserOutput->hasDynamicContent() ) { - // Assume the output contains time/random based magic words - $params = array(); - } else { - // If the inclusions are deterministic, the edit-triggered link jobs are enough - return false; + // templatelinks or imagelinks tables may have become out of sync, + // especially if using variable-based transclusions. + // For paranoia, check if things have changed and if + // so apply updates to the database. This will ensure + // that cascaded protections apply as soon as the changes + // are visible. + + // Get templates from templatelinks and images from imagelinks + $id = $this->getId(); + + $dbLinks = array(); + + $dbr = wfGetDB( DB_SLAVE ); + $res = $dbr->select( array( 'templatelinks' ), + array( 'tl_namespace', 'tl_title' ), + array( 'tl_from' => $id ), + __METHOD__ + ); + + foreach ( $res as $row ) { + $dbLinks["{$row->tl_namespace}:{$row->tl_title}"] = true; } - // Check if the last link refresh was before page_touched - if ( $this->getLinksTimestamp() < $this->getTouched() ) { - JobQueueGroup::singleton()->push( new RefreshLinksJob( $this->mTitle, $params ) ); - return true; + $dbr = wfGetDB( DB_SLAVE ); + $res = $dbr->select( array( 'imagelinks' ), + array( 'il_to' ), + array( 'il_from' => $id ), + __METHOD__ + ); + + foreach ( $res as $row ) { + $dbLinks[NS_FILE . ":{$row->il_to}"] = true; } - return false; + // Get templates and images from parser output. + $poLinks = array(); + foreach ( $parserOutput->getTemplates() as $ns => $templates ) { + foreach ( $templates as $dbk => $id ) { + $poLinks["$ns:$dbk"] = true; + } + } + foreach ( $parserOutput->getImages() as $dbk => $id ) { + $poLinks[NS_FILE . ":$dbk"] = true; + } + + // Get the diff + $links_diff = array_diff_key( $poLinks, $dbLinks ); + + if ( count( $links_diff ) > 0 ) { + // Whee, link updates time. + // Note: we are only interested in links here. We don't need to get + // other DataUpdate items from the parser output. + $u = new LinksUpdate( $this->mTitle, $parserOutput, false ); + $u->doUpdate(); + } } /** diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php index da7842a..e9e72be 100644 --- a/includes/parser/ParserOutput.php +++ b/includes/parser/ParserOutput.php @@ -880,22 +880,6 @@ } /** - * Check whether the cache TTL was lowered due to dynamic content - * - * When content is determined by more than hard state (e.g. page edits), - * such as template/file transclusions based on the current timestamp or - * extension tags that generate lists based on queries, this return true. - * - * @return bool - * @since 1.25 - */ - public function hasDynamicContent() { - global $wgParserCacheExpireTime; - - return $this->getCacheExpiry() < $wgParserCacheExpireTime; - } - - /** * Get or set the prevent-clickjacking flag * * @since 1.24 diff --git a/includes/poolcounter/PoolWorkArticleView.php b/includes/poolcounter/PoolWorkArticleView.php index 54cbb27..da20f94 100644 --- a/includes/poolcounter/PoolWorkArticleView.php +++ b/includes/poolcounter/PoolWorkArticleView.php @@ -159,7 +159,7 @@ } if ( $isCurrent ) { - $this->page->triggerOpportunisticLinksUpdate( $this->parserOutput ); + $this->page->doCascadeProtectionUpdates( $this->parserOutput ); } return true; -- To view, visit https://gerrit.wikimedia.org/r/191870 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I85d19ab5ad30e8d13a956d7b7467a94c9e73219d Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Aude <aude.w...@gmail.com> Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org> Gerrit-Reviewer: Aude <aude.w...@gmail.com> Gerrit-Reviewer: Cscott <canan...@wikimedia.org> Gerrit-Reviewer: Hoo man <h...@online.de> Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com> Gerrit-Reviewer: TTO <at.li...@live.com.au> Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
