Merge "Be more db-friendly when purging expired userrights"

diff --git a/autoload.php b/autoload.php
index fc77fcb..9042f7b 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -1591,6 +1591,7 @@ $wgAutoloadLocalClasses = [
        'UserBlockedError' => __DIR__ . '/includes/exception/UserBlockedError.php',
        'UserCache' => __DIR__ . '/includes/cache/UserCache.php',
        'UserDupes' => __DIR__ . '/maintenance/userDupes.inc',
+       'UserGroupExpiryJob' => __DIR__ . '/includes/jobqueue/jobs/UserGroupExpiryJob.php',
        'UserGroupMembership' => __DIR__ . '/includes/user/UserGroupMembership.php',
        'UserMailer' => __DIR__ . '/includes/mail/UserMailer.php',
        'UserNamePrefixSearch' => __DIR__ . '/includes/user/UserNamePrefixSearch.php',

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 5c3ac06..c856230 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -7459,6 +7459,7 @@ $wgJobClasses = [
        'clearUserWatchlist' => ClearUserWatchlistJob::class,
        'cdnPurge' => CdnPurgeJob::class,
        'enqueue' => EnqueueJob::class, // local queue for multi-DC setups
+       'userGroupExpiry' => UserGroupExpiryJob::class,
        'null' => NullJob::class,
 ];
 

diff --git a/includes/jobqueue/jobs/UserGroupExpiryJob.php b/includes/jobqueue/jobs/UserGroupExpiryJob.php
new file mode 100644 (file)
index 0000000..0945e58
--- /dev/null
+++ b/includes/jobqueue/jobs/UserGroupExpiryJob.php
@@ -0,0 +1,39 @@
+<?php
+/**
+ * Job that purges expired user group memberships.
+ *
+ * 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 3 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
+ * @ingroup JobQueue
+ */
+
+class UserGroupExpiryJob extends Job {
+       public function __construct( $params = false ) {
+               parent::__construct( 'userGroupExpiry', Title::newMainPage(), $params );
+               $this->removeDuplicates = true;
+       }
+
+       /**
+        * Run the job
+        * @return bool Success
+        */
+       public function run() {
+               UserGroupMembership::purgeExpired();
+
+               return true;
+       }
+}

diff --git a/includes/user/UserGroupMembership.php b/includes/user/UserGroupMembership.php
index f771f42..e757e59 100644 (file)
--- a/includes/user/UserGroupMembership.php
+++ b/includes/user/UserGroupMembership.php
@@ -21,6 +21,7 @@
  */
 
 use Wikimedia\Rdbms\IDatabase;
+use MediaWiki\MediaWikiServices;
 
 /**
  * Represents a "user group membership" -- a specific instance of a user belonging
@@ -158,7 +159,7 @@ class UserGroupMembership {
                }
 
                // Purge old, expired memberships from the DB
-               self::purgeExpired( $dbw );
+               JobQueueGroup::singleton()->push( new UserGroupExpiryJob() );
 
                // Check that the values make sense
                if ( $this->group === null ) {
@@ -236,38 +237,59 @@ class UserGroupMembership {
 
        /**
         * Purge expired memberships from the user_groups table
-        *
-        * @param IDatabase|null $dbw
         */
-       public static function purgeExpired( IDatabase $dbw = null ) {
-               if ( wfReadOnly() ) {
+       public static function purgeExpired() {
+               $services = MediaWikiServices::getInstance();
+               if ( $services->getReadOnlyMode()->isReadOnly() ) {
                        return;
                }
 
-               if ( $dbw === null ) {
-                       $dbw = wfGetDB( DB_MASTER );
-               }
+               $lbFactory = $services->getDBLoadBalancerFactory();
+               $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
+               $dbw = $services->getDBLoadBalancer()->getConnection( DB_MASTER );
 
-               DeferredUpdates::addUpdate( new AtomicSectionUpdate(
-                       $dbw,
-                       __METHOD__,
-                       function ( IDatabase $dbw, $fname ) {
-                               $expiryCond = [ 'ug_expiry < ' . $dbw->addQuotes( $dbw->timestamp() ) ];
-                               $res = $dbw->select( 'user_groups', self::selectFields(), $expiryCond, $fname );
+               $lockKey = $dbw->getDomainID() . ':usergroups-prune'; // specific to this wiki
+               $scopedLock = $dbw->getScopedLockAndFlush( $lockKey, __METHOD__, 0 );
+               if ( !$scopedLock ) {
+                       return; // already running
+               }
 
-                               // save an array of users/groups to insert to user_former_groups
-                               $usersAndGroups = [];
+               $now = time();
+               do {
+                       $dbw->startAtomic( __METHOD__ );
+
+                       $res = $dbw->select(
+                               'user_groups',
+                               self::selectFields(),
+                               [ 'ug_expiry < ' . $dbw->addQuotes( $dbw->timestamp( $now ) ) ],
+                               __METHOD__,
+                               [ 'FOR UPDATE', 'LIMIT' => 100 ]
+                       );
+
+                       if ( $res->numRows() > 0 ) {
+                               $insertData = []; // array of users/groups to insert to user_former_groups
+                               $deleteCond = []; // array for deleting the rows that are to be moved around
                                foreach ( $res as $row ) {
-                                       $usersAndGroups[] = [ 'ufg_user' => $row->ug_user, 'ufg_group' => $row->ug_group ];
+                                       $insertData[] = [ 'ufg_user' => $row->ug_user, 'ufg_group' => $row->ug_group ];
+                                       $deleteCond[] = $dbw->makeList(
+                                               [ 'ug_user' => $row->ug_user, 'ug_group' => $row->ug_group ],
+                                               $dbw::LIST_AND
+                                       );
                                }
+                               // Delete the rows we're about to move
+                               $dbw->delete(
+                                       'user_groups',
+                                       $dbw->makeList( $deleteCond, $dbw::LIST_OR ),
+                                       __METHOD__
+                               );
+                               // Push the groups to user_former_groups
+                               $dbw->insert( 'user_former_groups', $insertData, __METHOD__, [ 'IGNORE' ] );
+                       }
 
-                               // delete 'em all
-                               $dbw->delete( 'user_groups', $expiryCond, $fname );
+                       $dbw->endAtomic( __METHOD__ );
 
-                               // and push the groups to user_former_groups
-                               $dbw->insert( 'user_former_groups', $usersAndGroups, __METHOD__, [ 'IGNORE' ] );
-                       }
-               ) );
+                       $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket );
+               } while ( $res->numRows() > 0 );
        }
 
        /**

diff --git a/tests/phpunit/includes/SiteStatsTest.php b/tests/phpunit/includes/SiteStatsTest.php
index cdbf9fd..56bde5d 100644 (file)
--- a/tests/phpunit/includes/SiteStatsTest.php
+++ b/tests/phpunit/includes/SiteStatsTest.php
@@ -11,9 +11,10 @@ class SiteStatsTest extends MediaWikiTestCase {
                $cache = \MediaWiki\MediaWikiServices::getInstance()->getMainWANObjectCache();
                $jobq = JobQueueGroup::singleton();
 
-               // Delete EditPage jobs that might have been left behind by other tests
+               // Delete jobs that might have been left behind by other tests
                $jobq->get( 'htmlCacheUpdate' )->delete();
                $jobq->get( 'recentChangesUpdate' )->delete();
+               $jobq->get( 'userGroupExpiry' )->delete();
                $cache->delete( $cache->makeKey( 'SiteStats', 'jobscount' ) );
 
                $jobq->push( new NullJob( Title::newMainPage(), [] ) );