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
This commit is contained in:
Aaron Schulz 2016-10-18 20:55:35 -07:00
parent 875ccb3625
commit e3b7bf4d0a
2 changed files with 160 additions and 129 deletions

View file

@ -205,64 +205,85 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate {
protected function doIncrementalUpdate() { protected function doIncrementalUpdate() {
# Page links # Page links
$existing = $this->getExistingLinks(); $existingPL = $this->getExistingLinks();
$this->linkDeletions = $this->getLinkDeletions( $existing ); $this->linkDeletions = $this->getLinkDeletions( $existingPL );
$this->linkInsertions = $this->getLinkInsertions( $existing ); $this->linkInsertions = $this->getLinkInsertions( $existingPL );
$this->incrTableUpdate( 'pagelinks', 'pl', $this->linkDeletions, $this->linkInsertions ); $this->incrTableUpdate( 'pagelinks', 'pl', $this->linkDeletions, $this->linkInsertions );
# Image links # Image links
$existing = $this->getExistingImages(); $existingIL = $this->getExistingImages();
$imageDeletes = $this->getImageDeletions( $existing ); $imageDeletes = $this->getImageDeletions( $existingIL );
$this->incrTableUpdate( 'imagelinks', 'il', $imageDeletes, $this->incrTableUpdate(
$this->getImageInsertions( $existing ) ); 'imagelinks',
'il',
$imageDeletes,
$this->getImageInsertions( $existingIL ) );
# Invalidate all image description pages which had links added or removed # 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 ); $this->invalidateImageDescriptions( $imageUpdates );
# External links # External links
$existing = $this->getExistingExternals(); $existingEL = $this->getExistingExternals();
$this->incrTableUpdate( 'externallinks', 'el', $this->getExternalDeletions( $existing ), $this->incrTableUpdate(
$this->getExternalInsertions( $existing ) ); 'externallinks',
'el',
$this->getExternalDeletions( $existingEL ),
$this->getExternalInsertions( $existingEL ) );
# Language links # Language links
$existing = $this->getExistingInterlangs(); $existingLL = $this->getExistingInterlangs();
$this->incrTableUpdate( 'langlinks', 'll', $this->getInterlangDeletions( $existing ), $this->incrTableUpdate(
$this->getInterlangInsertions( $existing ) ); 'langlinks',
'll',
$this->getInterlangDeletions( $existingLL ),
$this->getInterlangInsertions( $existingLL ) );
# Inline interwiki links # Inline interwiki links
$existing = $this->getExistingInterwikis(); $existingIW = $this->getExistingInterwikis();
$this->incrTableUpdate( 'iwlinks', 'iwl', $this->getInterwikiDeletions( $existing ), $this->incrTableUpdate(
$this->getInterwikiInsertions( $existing ) ); 'iwlinks',
'iwl',
$this->getInterwikiDeletions( $existingIW ),
$this->getInterwikiInsertions( $existingIW ) );
# Template links # Template links
$existing = $this->getExistingTemplates(); $existingTL = $this->getExistingTemplates();
$this->incrTableUpdate( 'templatelinks', 'tl', $this->getTemplateDeletions( $existing ), $this->incrTableUpdate(
$this->getTemplateInsertions( $existing ) ); 'templatelinks',
'tl',
$this->getTemplateDeletions( $existingTL ),
$this->getTemplateInsertions( $existingTL ) );
# Category links # Category links
$existing = $this->getExistingCategories(); $existingCL = $this->getExistingCategories();
$categoryDeletes = $this->getCategoryDeletions( $existing ); $categoryDeletes = $this->getCategoryDeletions( $existingCL );
$this->incrTableUpdate( 'categorylinks', 'cl', $categoryDeletes, $this->incrTableUpdate(
$this->getCategoryInsertions( $existing ) ); 'categorylinks',
'cl',
# Invalidate all categories which were added, deleted or changed (set symmetric difference) $categoryDeletes,
$categoryInserts = array_diff_assoc( $this->mCategories, $existing ); $this->getCategoryInsertions( $existingCL ) );
$categoryInserts = array_diff_assoc( $this->mCategories, $existingCL );
$categoryUpdates = $categoryInserts + $categoryDeletes; $categoryUpdates = $categoryInserts + $categoryDeletes;
$this->invalidateCategories( $categoryUpdates );
$this->updateCategoryCounts( $categoryInserts, $categoryDeletes );
# Page properties # Page properties
$existing = $this->getExistingProperties(); $existingPP = $this->getExistingProperties();
$this->propertyDeletions = $this->getPropertyDeletions( $existing ); $this->propertyDeletions = $this->getPropertyDeletions( $existingPP );
$this->incrTableUpdate( 'page_props', 'pp', $this->propertyDeletions, $this->incrTableUpdate(
$this->getPropertyInsertions( $existing ) ); 'page_props',
'pp',
$this->propertyDeletions,
$this->getPropertyInsertions( $existingPP ) );
# Invalidate the necessary pages # 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; $changed = $this->propertyDeletions + $this->propertyInsertions;
$this->invalidateProperties( $changed ); $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 # Refresh links of all pages including this page
# This will be in a separate transaction # This will be in a separate transaction
if ( $this->mRecursive ) { if ( $this->mRecursive ) {
@ -324,7 +345,7 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate {
/** /**
* @param array $cats * @param array $cats
*/ */
function invalidateCategories( $cats ) { private function invalidateCategories( $cats ) {
PurgeJobUtils::invalidatePages( $this->getDB(), NS_CATEGORY, array_keys( $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 $added Associative array of category name => sort key
* @param array $deleted Associative array of category name => sort key * @param array $deleted Associative array of category name => sort key
*/ */
function updateCategoryCounts( $added, $deleted ) { private function updateCategoryCounts( array $added, array $deleted ) {
$a = WikiPage::factory( $this->mTitle ); global $wgUpdateRowsPerQuery;
$a->updateCategoryCounts(
array_keys( $added ), array_keys( $deleted ) $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 * @param array $images
*/ */
function invalidateImageDescriptions( $images ) { private function invalidateImageDescriptions( $images ) {
PurgeJobUtils::invalidatePages( $this->getDB(), NS_FILE, array_keys( $images ) ); PurgeJobUtils::invalidatePages( $this->getDB(), NS_FILE, array_keys( $images ) );
} }

View file

@ -3547,107 +3547,103 @@ class WikiPage implements Page, IDBAccessObject {
* Update all the appropriate counts in the category table, given that * Update all the appropriate counts in the category table, given that
* we've added the categories $added and deleted the categories $deleted. * 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 $added The names of categories that were added
* @param array $deleted The names of categories that were deleted * @param array $deleted The names of categories that were deleted
* @param integer $id Page ID (this should be the original deleted page ID) * @param integer $id Page ID (this should be the original deleted page ID)
*/ */
public function updateCategoryCounts( array $added, array $deleted, $id = 0 ) { public function updateCategoryCounts( array $added, array $deleted, $id = 0 ) {
$id = $id ?: $this->getId(); $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 ); $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' ]; if ( count( $added ) ) {
$removeFields = [ 'cat_pages = cat_pages - 1' ]; $existingAdded = $dbw->selectFieldValues(
if ( $ns == NS_CATEGORY ) { 'category',
$addFields[] = 'cat_subcats = cat_subcats + 1'; 'cat_title',
$removeFields[] = 'cat_subcats = cat_subcats - 1'; [ 'cat_title' => $added ],
} elseif ( $ns == NS_FILE ) { __METHOD__
$addFields[] = 'cat_files = cat_files + 1'; );
$removeFields[] = 'cat_files = cat_files - 1';
// 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__
);
}
}
if ( count( $added ) ) { if ( count( $deleted ) ) {
$existingAdded = $dbw->selectFieldValues( $dbw->update(
'category', 'category',
'cat_title', $removeFields,
[ 'cat_title' => $added ], [ 'cat_title' => $deleted ],
$method __METHOD__
); );
}
// For category rows that already exist, do a plain foreach ( $added as $catName ) {
// UPDATE instead of INSERT...ON DUPLICATE KEY UPDATE $cat = Category::newFromName( $catName );
// to avoid creating gaps in the cat_id sequence. Hooks::run( 'CategoryAfterPageAdded', [ $cat, $this ] );
if ( count( $existingAdded ) ) { }
$dbw->update(
'category',
$addFields,
[ 'cat_title' => $existingAdded ],
$method
);
}
$missingAdded = array_diff( $added, $existingAdded ); foreach ( $deleted as $catName ) {
if ( count( $missingAdded ) ) { $cat = Category::newFromName( $catName );
$insertRows = []; Hooks::run( 'CategoryAfterPageRemoved', [ $cat, $this, $id ] );
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 ) ) { // Refresh counts on categories that should be empty now, to
$dbw->update( // trigger possible deletion. Check master for the most
'category', // up-to-date cat_pages.
$removeFields, if ( count( $deleted ) ) {
[ 'cat_title' => $deleted ], $rows = $dbw->select(
$method 'category',
); [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ],
} [ 'cat_title' => $deleted, 'cat_pages <= 0' ],
__METHOD__
foreach ( $added as $catName ) { );
$cat = Category::newFromName( $catName ); foreach ( $rows as $row ) {
Hooks::run( 'CategoryAfterPageAdded', [ $cat, $this ] ); $cat = Category::newFromRow( $row );
} $cat->refreshCounts();
}
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__
);
} }
/** /**