From: Aaron Schulz Date: Wed, 19 Oct 2016 03:55:35 +0000 (-0700) Subject: Make updateCategoryCounts() have better lag checks X-Git-Tag: 1.31.0-rc.0~5007^2 X-Git-Url: http://git.cyclocoop.org/url?a=commitdiff_plain;h=e3b7bf4d0ad7945d9cd9f900fc10400faff232dd;p=lhc%2Fweb%2Fwiklou.git Make updateCategoryCounts() have better lag checks * Add the lag checks to LinksUpdate. Previously, only LinksDeletionUpdate had any such checks. * Remove the transaction hook usage, since the only two callers are already lag/contention aware. Deferring them just makes the wait checks pointless and they might end up happening all at once. * Also set the visibility on some neigboring methods. * Clean up LinksUpdate $existing variables in passing. Instead of overriding the same variable, use a differently named variable to avoid mistakes. Bug: T95501 Change-Id: I43e3af17399417cbf0ab4e5e7d1f2bd518fa7e90 --- diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php index c7d378e926..229a9a258f 100644 --- a/includes/deferred/LinksUpdate.php +++ b/includes/deferred/LinksUpdate.php @@ -205,64 +205,85 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { protected function doIncrementalUpdate() { # Page links - $existing = $this->getExistingLinks(); - $this->linkDeletions = $this->getLinkDeletions( $existing ); - $this->linkInsertions = $this->getLinkInsertions( $existing ); + $existingPL = $this->getExistingLinks(); + $this->linkDeletions = $this->getLinkDeletions( $existingPL ); + $this->linkInsertions = $this->getLinkInsertions( $existingPL ); $this->incrTableUpdate( 'pagelinks', 'pl', $this->linkDeletions, $this->linkInsertions ); # Image links - $existing = $this->getExistingImages(); - $imageDeletes = $this->getImageDeletions( $existing ); - $this->incrTableUpdate( 'imagelinks', 'il', $imageDeletes, - $this->getImageInsertions( $existing ) ); + $existingIL = $this->getExistingImages(); + $imageDeletes = $this->getImageDeletions( $existingIL ); + $this->incrTableUpdate( + 'imagelinks', + 'il', + $imageDeletes, + $this->getImageInsertions( $existingIL ) ); # Invalidate all image description pages which had links added or removed - $imageUpdates = $imageDeletes + array_diff_key( $this->mImages, $existing ); + $imageUpdates = $imageDeletes + array_diff_key( $this->mImages, $existingIL ); $this->invalidateImageDescriptions( $imageUpdates ); # External links - $existing = $this->getExistingExternals(); - $this->incrTableUpdate( 'externallinks', 'el', $this->getExternalDeletions( $existing ), - $this->getExternalInsertions( $existing ) ); + $existingEL = $this->getExistingExternals(); + $this->incrTableUpdate( + 'externallinks', + 'el', + $this->getExternalDeletions( $existingEL ), + $this->getExternalInsertions( $existingEL ) ); # Language links - $existing = $this->getExistingInterlangs(); - $this->incrTableUpdate( 'langlinks', 'll', $this->getInterlangDeletions( $existing ), - $this->getInterlangInsertions( $existing ) ); + $existingLL = $this->getExistingInterlangs(); + $this->incrTableUpdate( + 'langlinks', + 'll', + $this->getInterlangDeletions( $existingLL ), + $this->getInterlangInsertions( $existingLL ) ); # Inline interwiki links - $existing = $this->getExistingInterwikis(); - $this->incrTableUpdate( 'iwlinks', 'iwl', $this->getInterwikiDeletions( $existing ), - $this->getInterwikiInsertions( $existing ) ); + $existingIW = $this->getExistingInterwikis(); + $this->incrTableUpdate( + 'iwlinks', + 'iwl', + $this->getInterwikiDeletions( $existingIW ), + $this->getInterwikiInsertions( $existingIW ) ); # Template links - $existing = $this->getExistingTemplates(); - $this->incrTableUpdate( 'templatelinks', 'tl', $this->getTemplateDeletions( $existing ), - $this->getTemplateInsertions( $existing ) ); + $existingTL = $this->getExistingTemplates(); + $this->incrTableUpdate( + 'templatelinks', + 'tl', + $this->getTemplateDeletions( $existingTL ), + $this->getTemplateInsertions( $existingTL ) ); # Category links - $existing = $this->getExistingCategories(); - $categoryDeletes = $this->getCategoryDeletions( $existing ); - $this->incrTableUpdate( 'categorylinks', 'cl', $categoryDeletes, - $this->getCategoryInsertions( $existing ) ); - - # Invalidate all categories which were added, deleted or changed (set symmetric difference) - $categoryInserts = array_diff_assoc( $this->mCategories, $existing ); + $existingCL = $this->getExistingCategories(); + $categoryDeletes = $this->getCategoryDeletions( $existingCL ); + $this->incrTableUpdate( + 'categorylinks', + 'cl', + $categoryDeletes, + $this->getCategoryInsertions( $existingCL ) ); + $categoryInserts = array_diff_assoc( $this->mCategories, $existingCL ); $categoryUpdates = $categoryInserts + $categoryDeletes; - $this->invalidateCategories( $categoryUpdates ); - $this->updateCategoryCounts( $categoryInserts, $categoryDeletes ); # Page properties - $existing = $this->getExistingProperties(); - $this->propertyDeletions = $this->getPropertyDeletions( $existing ); - $this->incrTableUpdate( 'page_props', 'pp', $this->propertyDeletions, - $this->getPropertyInsertions( $existing ) ); + $existingPP = $this->getExistingProperties(); + $this->propertyDeletions = $this->getPropertyDeletions( $existingPP ); + $this->incrTableUpdate( + 'page_props', + 'pp', + $this->propertyDeletions, + $this->getPropertyInsertions( $existingPP ) ); # Invalidate the necessary pages - $this->propertyInsertions = array_diff_assoc( $this->mProperties, $existing ); + $this->propertyInsertions = array_diff_assoc( $this->mProperties, $existingPP ); $changed = $this->propertyDeletions + $this->propertyInsertions; $this->invalidateProperties( $changed ); + # Invalidate all categories which were added, deleted or changed (set symmetric difference) + $this->invalidateCategories( $categoryUpdates ); + $this->updateCategoryCounts( $categoryInserts, $categoryDeletes ); + # Refresh links of all pages including this page # This will be in a separate transaction if ( $this->mRecursive ) { @@ -324,7 +345,7 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { /** * @param array $cats */ - function invalidateCategories( $cats ) { + private function invalidateCategories( $cats ) { PurgeJobUtils::invalidatePages( $this->getDB(), NS_CATEGORY, array_keys( $cats ) ); } @@ -333,17 +354,31 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { * @param array $added Associative array of category name => sort key * @param array $deleted Associative array of category name => sort key */ - function updateCategoryCounts( $added, $deleted ) { - $a = WikiPage::factory( $this->mTitle ); - $a->updateCategoryCounts( - array_keys( $added ), array_keys( $deleted ) - ); + private function updateCategoryCounts( array $added, array $deleted ) { + global $wgUpdateRowsPerQuery; + + $wp = WikiPage::factory( $this->mTitle ); + $factory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); + + foreach ( array_chunk( array_keys( $added ), $wgUpdateRowsPerQuery ) as $addBatch ) { + $wp->updateCategoryCounts( $addBatch, [], $this->mId ); + $factory->commitAndWaitForReplication( + __METHOD__, $this->ticket, [ 'wiki' => $this->getDB()->getWikiID() ] + ); + } + + foreach ( array_chunk( array_keys( $deleted ), $wgUpdateRowsPerQuery ) as $deleteBatch ) { + $wp->updateCategoryCounts( [], $deleteBatch, $this->mId ); + $factory->commitAndWaitForReplication( + __METHOD__, $this->ticket, [ 'wiki' => $this->getDB()->getWikiID() ] + ); + } } /** * @param array $images */ - function invalidateImageDescriptions( $images ) { + private function invalidateImageDescriptions( $images ) { PurgeJobUtils::invalidatePages( $this->getDB(), NS_FILE, array_keys( $images ) ); } diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 3dc41fba58..9aa8503422 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -3547,107 +3547,103 @@ class WikiPage implements Page, IDBAccessObject { * Update all the appropriate counts in the category table, given that * we've added the categories $added and deleted the categories $deleted. * + * This should only be called from deferred updates or jobs to avoid contention. + * * @param array $added The names of categories that were added * @param array $deleted The names of categories that were deleted * @param integer $id Page ID (this should be the original deleted page ID) */ public function updateCategoryCounts( array $added, array $deleted, $id = 0 ) { $id = $id ?: $this->getId(); + $ns = $this->getTitle()->getNamespace(); + + $addFields = [ 'cat_pages = cat_pages + 1' ]; + $removeFields = [ 'cat_pages = cat_pages - 1' ]; + if ( $ns == NS_CATEGORY ) { + $addFields[] = 'cat_subcats = cat_subcats + 1'; + $removeFields[] = 'cat_subcats = cat_subcats - 1'; + } elseif ( $ns == NS_FILE ) { + $addFields[] = 'cat_files = cat_files + 1'; + $removeFields[] = 'cat_files = cat_files - 1'; + } + $dbw = wfGetDB( DB_MASTER ); - $method = __METHOD__; - // Do this at the end of the commit to reduce lock wait timeouts - $dbw->onTransactionPreCommitOrIdle( - function () use ( $dbw, $added, $deleted, $id, $method ) { - $ns = $this->getTitle()->getNamespace(); - - $addFields = [ 'cat_pages = cat_pages + 1' ]; - $removeFields = [ 'cat_pages = cat_pages - 1' ]; - if ( $ns == NS_CATEGORY ) { - $addFields[] = 'cat_subcats = cat_subcats + 1'; - $removeFields[] = 'cat_subcats = cat_subcats - 1'; - } elseif ( $ns == NS_FILE ) { - $addFields[] = 'cat_files = cat_files + 1'; - $removeFields[] = 'cat_files = cat_files - 1'; - } - if ( count( $added ) ) { - $existingAdded = $dbw->selectFieldValues( - 'category', - 'cat_title', - [ 'cat_title' => $added ], - $method - ); + if ( count( $added ) ) { + $existingAdded = $dbw->selectFieldValues( + 'category', + 'cat_title', + [ 'cat_title' => $added ], + __METHOD__ + ); - // For category rows that already exist, do a plain - // UPDATE instead of INSERT...ON DUPLICATE KEY UPDATE - // to avoid creating gaps in the cat_id sequence. - if ( count( $existingAdded ) ) { - $dbw->update( - 'category', - $addFields, - [ 'cat_title' => $existingAdded ], - $method - ); - } + // For category rows that already exist, do a plain + // UPDATE instead of INSERT...ON DUPLICATE KEY UPDATE + // to avoid creating gaps in the cat_id sequence. + if ( count( $existingAdded ) ) { + $dbw->update( + 'category', + $addFields, + [ 'cat_title' => $existingAdded ], + __METHOD__ + ); + } - $missingAdded = array_diff( $added, $existingAdded ); - if ( count( $missingAdded ) ) { - $insertRows = []; - foreach ( $missingAdded as $cat ) { - $insertRows[] = [ - 'cat_title' => $cat, - 'cat_pages' => 1, - 'cat_subcats' => ( $ns == NS_CATEGORY ) ? 1 : 0, - 'cat_files' => ( $ns == NS_FILE ) ? 1 : 0, - ]; - } - $dbw->upsert( - 'category', - $insertRows, - [ 'cat_title' ], - $addFields, - $method - ); - } + $missingAdded = array_diff( $added, $existingAdded ); + if ( count( $missingAdded ) ) { + $insertRows = []; + foreach ( $missingAdded as $cat ) { + $insertRows[] = [ + 'cat_title' => $cat, + 'cat_pages' => 1, + 'cat_subcats' => ( $ns == NS_CATEGORY ) ? 1 : 0, + 'cat_files' => ( $ns == NS_FILE ) ? 1 : 0, + ]; } + $dbw->upsert( + 'category', + $insertRows, + [ 'cat_title' ], + $addFields, + __METHOD__ + ); + } + } - if ( count( $deleted ) ) { - $dbw->update( - 'category', - $removeFields, - [ 'cat_title' => $deleted ], - $method - ); - } + if ( count( $deleted ) ) { + $dbw->update( + 'category', + $removeFields, + [ 'cat_title' => $deleted ], + __METHOD__ + ); + } - foreach ( $added as $catName ) { - $cat = Category::newFromName( $catName ); - Hooks::run( 'CategoryAfterPageAdded', [ $cat, $this ] ); - } + foreach ( $added as $catName ) { + $cat = Category::newFromName( $catName ); + Hooks::run( 'CategoryAfterPageAdded', [ $cat, $this ] ); + } - foreach ( $deleted as $catName ) { - $cat = Category::newFromName( $catName ); - Hooks::run( 'CategoryAfterPageRemoved', [ $cat, $this, $id ] ); - } + foreach ( $deleted as $catName ) { + $cat = Category::newFromName( $catName ); + Hooks::run( 'CategoryAfterPageRemoved', [ $cat, $this, $id ] ); + } - // Refresh counts on categories that should be empty now, to - // trigger possible deletion. Check master for the most - // up-to-date cat_pages. - if ( count( $deleted ) ) { - $rows = $dbw->select( - 'category', - [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ], - [ 'cat_title' => $deleted, 'cat_pages <= 0' ], - $method - ); - foreach ( $rows as $row ) { - $cat = Category::newFromRow( $row ); - $cat->refreshCounts(); - } - } - }, - __METHOD__ - ); + // Refresh counts on categories that should be empty now, to + // trigger possible deletion. Check master for the most + // up-to-date cat_pages. + if ( count( $deleted ) ) { + $rows = $dbw->select( + 'category', + [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ], + [ 'cat_title' => $deleted, 'cat_pages <= 0' ], + __METHOD__ + ); + foreach ( $rows as $row ) { + $cat = Category::newFromRow( $row ); + $cat->refreshCounts(); + } + } } /**