From 836ad263c3509f14beea4adea912e4e437668f82 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 14 Apr 2015 17:53:53 -0700 Subject: [PATCH] Made wl_notificationtimestamp updates able to use queues * This adds a wgActivityUpdatesUseJobQueue setting, which lets these updates work via the job queue, rather than direct DB master updates. Bug: T91284 Change-Id: Ie60e20162fd833e64d81763a6aa1dc3faf2162f3 --- includes/DefaultSettings.php | 10 +++ includes/User.php | 4 +- includes/WatchedItem.php | 75 ++++++++++++++------ includes/jobqueue/jobs/ActivityUpdateJob.php | 75 ++++++++++++++++++++ 4 files changed, 143 insertions(+), 21 deletions(-) create mode 100644 includes/jobqueue/jobs/ActivityUpdateJob.php diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index a16a1f02b6..a9e16539dc 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -1587,6 +1587,15 @@ $wgEnotifMaxRecips = 500; */ $wgEnotifUseJobQ = false; +/** + * Use the job queue for user activity updates like updating "last visited" + * fields for email notifications of page changes. This should only be enabled + * if the jobs have a dedicated runner to avoid update lag. + * + * @since 1.26 + */ +$wgActivityUpdatesUseJobQueue = false; + /** * Use real name instead of username in e-mail "from" field. */ @@ -6467,6 +6476,7 @@ $wgJobClasses = array( 'ThumbnailRender' => 'ThumbnailRenderJob', 'recentChangesUpdate' => 'RecentChangesUpdateJob', 'refreshLinksPrioritized' => 'RefreshLinksJob', // for cascading protection + 'activityUpdateJob' => 'ActivityUpdateJob', 'enqueue' => 'EnqueueJob', // local queue for multi-DC setups 'null' => 'NullJob' ); diff --git a/includes/User.php b/includes/User.php index c3d4a65c6d..48372ca37a 100644 --- a/includes/User.php +++ b/includes/User.php @@ -3455,7 +3455,9 @@ class User implements IDBAccessObject { $force = 'force'; } - $this->getWatchedItem( $title )->resetNotificationTimestamp( $force, $oldid ); + $this->getWatchedItem( $title )->resetNotificationTimestamp( + $force, $oldid, WatchedItem::DEFERRED + ); } /** diff --git a/includes/WatchedItem.php b/includes/WatchedItem.php index 524e701b69..73b0b937b0 100644 --- a/includes/WatchedItem.php +++ b/includes/WatchedItem.php @@ -27,20 +27,6 @@ * @ingroup Watchlist */ class WatchedItem { - /** - * Constant to specify that user rights 'editmywatchlist' and - * 'viewmywatchlist' should not be checked. - * @since 1.22 - */ - const IGNORE_USER_RIGHTS = 0; - - /** - * Constant to specify that user rights 'editmywatchlist' and - * 'viewmywatchlist' should be checked. - * @since 1.22 - */ - const CHECK_USER_RIGHTS = 1; - /** @var Title */ public $mTitle; @@ -59,6 +45,31 @@ class WatchedItem { /** @var string */ private $timestamp; + /** + * Constant to specify that user rights 'editmywatchlist' and + * 'viewmywatchlist' should not be checked. + * @since 1.22 + */ + const IGNORE_USER_RIGHTS = 0; + + /** + * Constant to specify that user rights 'editmywatchlist' and + * 'viewmywatchlist' should be checked. + * @since 1.22 + */ + const CHECK_USER_RIGHTS = 1; + + /** + * Do DB master updates right now + * @since 1.26 + */ + const IMMEDIATE = 0; + /** + * Do DB master updates via the job queue + * @since 1.26 + */ + const DEFERRED = 1; + /** * Create a WatchedItem object with the given user and title * @since 1.22 $checkRights parameter added @@ -208,8 +219,13 @@ class WatchedItem { * @param bool $force Whether to force the write query to be executed even if the * page is not watched or the notification timestamp is already NULL. * @param int $oldid The revision id being viewed. If not given or 0, latest revision is assumed. + * @mode int $mode WatchedItem::DEFERRED/IMMEDIATE */ - public function resetNotificationTimestamp( $force = '', $oldid = 0 ) { + public function resetNotificationTimestamp( + $force = '', $oldid = 0, $mode = self::IMMEDIATE + ) { + global $wgActivityUpdatesUseJobQueue; + // Only loggedin user can have a watchlist if ( wfReadOnly() || $this->mUser->isAnon() || !$this->isAllowed( 'editmywatchlist' ) ) { return; @@ -258,11 +274,30 @@ class WatchedItem { } } - // If the page is watched by the user (or may be watched), update the timestamp on any - // any matching rows - $dbw = wfGetDB( DB_MASTER ); - $dbw->update( 'watchlist', array( 'wl_notificationtimestamp' => $notificationTimestamp ), - $this->dbCond(), __METHOD__ ); + // If the page is watched by the user (or may be watched), update the timestamp + if ( $mode === self::DEFERRED && $wgActivityUpdatesUseJobQueue ) { + JobQueueGroup::singleton()->push( + EnqueueJob::newFromLocalJobs( new JobSpecification( + 'activityUpdateJob', + array( + 'type' => 'updateWatchlistNotification', + 'userid' => $this->getUserId(), + 'notifTime' => $notificationTimestamp, + 'curTime' => time() + ), + array( 'removeDuplicates' => true ), + $title + ) ) + ); + } else { + $dbw = wfGetDB( DB_MASTER ); + $dbw->update( 'watchlist', + array( 'wl_notificationtimestamp' => $notificationTimestamp ), + $this->dbCond(), + __METHOD__ + ); + } + $this->timestamp = null; } diff --git a/includes/jobqueue/jobs/ActivityUpdateJob.php b/includes/jobqueue/jobs/ActivityUpdateJob.php new file mode 100644 index 0000000000..495bda995b --- /dev/null +++ b/includes/jobqueue/jobs/ActivityUpdateJob.php @@ -0,0 +1,75 @@ +removeDuplicates = true; + } + + public function run() { + if ( $this->params['type'] === 'updateWatchlistNotification' ) { + $this->updateWatchlistNotification(); + } else { + throw new Exception( "Invalid 'type' parameter '{$this->params['type']}'." ); + } + + return true; + } + + protected function updateWatchlistNotification() { + $casTimestamp = ( $this->params['notifTime'] !== null ) + ? $this->params['notifTime'] + : $this->params['curTime']; + + $dbw = wfGetDB( DB_MASTER ); + $dbw->update( 'watchlist', + array( + 'wl_notificationtimestamp' => $dbw->timestampOrNull( $this->params['notifTime'] ) + ), + array( + 'wl_user' => $this->params['userid'], + 'wl_namespace' => $this->title->getNamespace(), + 'wl_title' => $this->title->getDBkey(), + // Add a "check and set" style comparison to handle conflicts. + // The inequality always avoids updates when the current value + // is already NULL per ANSI SQL. This is desired since NULL means + // that the user is "caught up" on edits already. When the field + // is non-NULL, make sure not to set it back in time or set it to + // NULL when newer revisions were in fact added to the page. + 'wl_notificationtimestamp < ' . $dbw->addQuotes( $dbw->timestamp( $casTimestamp ) ) + ), + __METHOD__ + ); + } +} -- 2.20.1