This should reduce excess contention and lock timeouts.
Previously, it used a pre-commit hook which ran just before the
end of the DB transaction round.
Also removed unused User::incEditCountImmediate() method.
Bug: T202715
Depends-on: I6d239a5ea286afb10d9e317b2ee1436de60f7e4f
Depends-on: I0ad3d17107efc7b0e59f1dd54d5733cd1572a2b7
Change-Id: I0d6d7ddd91bbb21995142808248d162e05696d47
'UserBlockedError' => __DIR__ . '/includes/exception/UserBlockedError.php',
'UserCache' => __DIR__ . '/includes/cache/UserCache.php',
'UserDupes' => __DIR__ . '/maintenance/userDupes.inc',
+ 'UserEditCountUpdate' => __DIR__ . '/includes/deferred/UserEditCountUpdate.php',
'UserGroupExpiryJob' => __DIR__ . '/includes/jobqueue/jobs/UserGroupExpiryJob.php',
'UserGroupMembership' => __DIR__ . '/includes/user/UserGroupMembership.php',
'UserMailer' => __DIR__ . '/includes/mail/UserMailer.php',
<?php
/**
- * Interface that deferrable updates can implement. DeferredUpdates uses this to merge
- * all pending updates of PHP class into a single update by calling merge().
+ * Interface that deferrable updates can implement to signal that updates can be combined.
+ *
+ * DeferredUpdates uses this to merge all pending updates of PHP class into a single update
+ * by calling merge(). Note that upon merge(), the combined update goes to the back of the FIFO
+ * queue so that such updates occur after related non-mergeable deferred updates. For example,
+ * suppose updates that purge URLs can be merged, and the calling pattern is:
+ * - a) DeferredUpdates::addUpdate( $purgeCdnUrlsA );
+ * - b) DeferredUpdates::addUpdate( $deleteContentUrlsB );
+ * - c) DeferredUpdates::addUpdate( $purgeCdnUrlsB )
+ *
+ * The purges for urls A and B will all happen after the $deleteContentUrlsB update.
*
* @since 1.27
*/
--- /dev/null
+<?php
+/**
+ * User edit count incrementing.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+use Wikimedia\Assert\Assert;
+use MediaWiki\MediaWikiServices;
+
+/**
+ * Handles increment the edit count for a given set of users
+ */
+class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate {
+ /** @var array[] Map of (user ID => ('increment': int, 'instances': User[])) */
+ private $infoByUser;
+
+ /**
+ * @param User $user
+ * @param int $increment
+ */
+ public function __construct( User $user, $increment ) {
+ if ( !$user->getId() ) {
+ throw new RuntimeException( "Got user ID of zero" );
+ }
+ $this->infoByUser = [
+ $user->getId() => [ 'increment' => $increment, 'instances' => [ $user ] ]
+ ];
+ }
+
+ public function merge( MergeableUpdate $update ) {
+ /** @var UserEditCountUpdate $update */
+ Assert::parameterType( __CLASS__, $update, '$update' );
+
+ foreach ( $update->infoByUser as $userId => $info ) {
+ if ( !isset( $this->infoByUser[$userId] ) ) {
+ $this->infoByUser[$userId] = [ 'increment' => 0, 'instances' => [] ];
+ }
+ // Merge the increment amount
+ $this->infoByUser[$userId]['increment'] += $info['increment'];
+ // Merge the list of User instances to update in doUpdate()
+ foreach ( $info['instances'] as $user ) {
+ if ( !in_array( $user, $this->infoByUser[$userId]['instances'], true ) ) {
+ $this->infoByUser[$userId]['instances'][] = $user;
+ }
+ }
+ }
+ }
+
+ /**
+ * Purges the list of URLs passed to the constructor.
+ */
+ public function doUpdate() {
+ foreach ( $this->infoByUser as $userId => $info ) {
+ $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+ $dbw = $lb->getConnection( DB_MASTER );
+
+ $dbw->startAtomic( __METHOD__ ); // define minimum row lock duration
+ $dbw->update(
+ 'user',
+ [ 'user_editcount=user_editcount+' . (int)$info['increment'] ],
+ [ 'user_id' => $userId, 'user_editcount IS NOT NULL' ],
+ __METHOD__
+ );
+ /** @var User[] $affectedInstances */
+ $affectedInstances = $info['instances'];
+ // Lazy initialization check...
+ if ( $dbw->affectedRows() == 0 ) {
+ // No rows will be "affected" if user_editcount is NULL.
+ // Get the generic "replica" connection to see if it actually uses the master.
+ $dbr = $lb->getConnection( DB_REPLICA );
+ if ( $dbr !== $dbw ) {
+ // This method runs after the new revisions were committed.
+ // Wait for the replica to catch up so they will all be counted.
+ $dbr->flushSnapshot( __METHOD__ );
+ $lb->safeWaitForMasterPos( $dbr );
+ }
+ $affectedInstances[0]->initEditCountInternal();
+ }
+ $newCount = (int)$dbw->selectField(
+ 'user',
+ [ 'user_editcount' ],
+ [ 'user_id' => $userId ],
+ __METHOD__
+ );
+ $dbw->endAtomic( __METHOD__ );
+
+ // Update the edit count in the instance caches. This is mostly useful
+ // for maintenance scripts, where deferred updates might run immediately
+ // and user instances might be reused for a long time.
+ foreach ( $affectedInstances as $affectedInstance ) {
+ $affectedInstance->setEditCountInternal( $newCount );
+ }
+ // Clear the edit count in user cache too
+ $affectedInstances[0]->invalidateCache();
+ }
+ }
+}
if ( $count === null ) {
// it has not been initialized. do so.
- $count = $this->initEditCount();
+ $count = $this->initEditCountInternal();
}
$this->mEditCount = $count;
}
}
/**
- * Deferred version of incEditCountImmediate()
- *
- * This function, rather than incEditCountImmediate(), should be used for
- * most cases as it avoids potential deadlocks caused by concurrent editing.
+ * Schedule a deferred update to update the user's edit count
*/
public function incEditCount() {
- wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle(
- function () {
- $this->incEditCountImmediate();
- },
- __METHOD__
+ if ( $this->isAnon() ) {
+ return; // sanity
+ }
+
+ DeferredUpdates::addUpdate(
+ new UserEditCountUpdate( $this, 1 ),
+ DeferredUpdates::POSTSEND
);
}
/**
- * Increment the user's edit-count field.
- * Will have no effect for anonymous users.
- * @since 1.26
+ * This method should not be called outside User/UserEditCountUpdate
+ *
+ * @param int $count
*/
- public function incEditCountImmediate() {
- if ( $this->isAnon() ) {
- return;
- }
-
- $dbw = wfGetDB( DB_MASTER );
- // No rows will be "affected" if user_editcount is NULL
- $dbw->update(
- 'user',
- [ 'user_editcount=user_editcount+1' ],
- [ 'user_id' => $this->getId(), 'user_editcount IS NOT NULL' ],
- __METHOD__
- );
- // Lazy initialization check...
- if ( $dbw->affectedRows() == 0 ) {
- // Now here's a goddamn hack...
- $dbr = wfGetDB( DB_REPLICA );
- if ( $dbr !== $dbw ) {
- // If we actually have a replica DB server, the count is
- // at least one behind because the current transaction
- // has not been committed and replicated.
- $this->mEditCount = $this->initEditCount( 1 );
- } else {
- // But if DB_REPLICA is selecting the master, then the
- // count we just read includes the revision that was
- // just added in the working transaction.
- $this->mEditCount = $this->initEditCount();
- }
- } else {
- if ( $this->mEditCount === null ) {
- $this->getEditCount();
- $dbr = wfGetDB( DB_REPLICA );
- $this->mEditCount += ( $dbr !== $dbw ) ? 1 : 0;
- } else {
- $this->mEditCount++;
- }
- }
- // Edit count in user cache too
- $this->invalidateCache();
+ public function setEditCountInternal( $count ) {
+ $this->mEditCount = $count;
}
/**
* Initialize user_editcount from data out of the revision table
*
+ * This method should not be called outside User/UserEditCountUpdate
+ *
* @param int $add Edits to add to the count from the revision table
* @return int Number of edits
*/
- protected function initEditCount( $add = 0 ) {
+ public function initEditCountInternal( $add = 0 ) {
// Pull from a replica DB to be less cruel to servers
// Accuracy isn't the point anyway here
$dbr = wfGetDB( DB_REPLICA );
// Now let's also increment our editcount
$this->editPage( ucfirst( __FUNCTION__ ), '' );
+ $user->clearInstanceCache();
$this->assertFalse( $this->doCheckCache( $user ),
"Cache should be invalidated when it's old and the user has an intervening edit" );
}
// increase the edit count
$user->incEditCount();
+ $user->clearInstanceCache();
$this->assertEquals(
4,