Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/316733
Change subject: Make updateCategoryCounts() lag checks better
......................................................................
Make updateCategoryCounts() lag checks better
* 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.
Bug: T95501
Change-Id: I43e3af17399417cbf0ab4e5e7d1f2bd518fa7e90
---
M includes/deferred/LinksUpdate.php
M includes/page/WikiPage.php
2 files changed, 111 insertions(+), 101 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/33/316733/1
diff --git a/includes/deferred/LinksUpdate.php
b/includes/deferred/LinksUpdate.php
index c7d378e..6899c19 100644
--- a/includes/deferred/LinksUpdate.php
+++ b/includes/deferred/LinksUpdate.php
@@ -246,12 +246,6 @@
$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 );
- $categoryUpdates = $categoryInserts + $categoryDeletes;
- $this->invalidateCategories( $categoryUpdates );
- $this->updateCategoryCounts( $categoryInserts, $categoryDeletes
);
-
# Page properties
$existing = $this->getExistingProperties();
$this->propertyDeletions = $this->getPropertyDeletions(
$existing );
@@ -262,6 +256,12 @@
$this->propertyInsertions = array_diff_assoc(
$this->mProperties, $existing );
$changed = $this->propertyDeletions + $this->propertyInsertions;
$this->invalidateProperties( $changed );
+
+ # Invalidate all categories which were added, deleted or
changed (set symmetric difference)
+ $categoryInserts = array_diff_assoc( $this->mCategories,
$existing );
+ $categoryUpdates = $categoryInserts + $categoryDeletes;
+ $this->invalidateCategories( $categoryUpdates );
+ $this->updateCategoryCounts( $categoryInserts, $categoryDeletes
);
# Refresh links of all pages including this page
# This will be in a separate transaction
@@ -324,7 +324,7 @@
/**
* @param array $cats
*/
- function invalidateCategories( $cats ) {
+ private function invalidateCategories( $cats ) {
PurgeJobUtils::invalidatePages( $this->getDB(), NS_CATEGORY,
array_keys( $cats ) );
}
@@ -333,17 +333,31 @@
* @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 3dc41fb..9aa8503 100644
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -3547,107 +3547,103 @@
* 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__
+ );
+
+ // 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 ) ) {
- $existingAdded =
$dbw->selectFieldValues(
- 'category',
- 'cat_title',
- [ 'cat_title' => $added ],
- $method
- );
+ if ( count( $deleted ) ) {
+ $dbw->update(
+ 'category',
+ $removeFields,
+ [ 'cat_title' => $deleted ],
+ __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
- );
- }
+ foreach ( $added as $catName ) {
+ $cat = Category::newFromName( $catName );
+ Hooks::run( 'CategoryAfterPageAdded', [ $cat, $this ] );
+ }
- $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
- );
- }
- }
+ foreach ( $deleted as $catName ) {
+ $cat = Category::newFromName( $catName );
+ Hooks::run( 'CategoryAfterPageRemoved', [ $cat, $this,
$id ] );
+ }
- 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 ( $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();
+ }
+ }
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/316733
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I43e3af17399417cbf0ab4e5e7d1f2bd518fa7e90
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
