From bf8c90538dff567efc455fafa18ac2dc4015b246 Mon Sep 17 00:00:00 2001
From: Marius Hoch <hoo@online.de>
Date: Tue, 31 Jan 2017 10:39:56 +0100
Subject: [PATCH] Make RecentChangesUpdateJob::updateActiveUsers more robust

Do work we can outside of the lock (purge no longer active users)
and fail more gracefully if the lock is taken (if this is the case,
we're probably on a high traffic wiki, so this job is going to
run very often anyway, so no need to warn).

Given this is an asynchronous job people can't expect this to be
always fully consistent with the actual state of the RC table anyway.

Note: This will still sometimes log the warning (if we have a race
between Database::lockIsFree and Database::lock), but that should be
negligible.

Bug: T156638
Change-Id: I2e49a8e12bc64156d64a32ccf54911a76087346a
---
 .../jobqueue/jobs/RecentChangesUpdateJob.php  | 31 ++++++++++++-------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/includes/jobqueue/jobs/RecentChangesUpdateJob.php b/includes/jobqueue/jobs/RecentChangesUpdateJob.php
index 0e90674a2f..5c733088ee 100644
--- a/includes/jobqueue/jobs/RecentChangesUpdateJob.php
+++ b/includes/jobqueue/jobs/RecentChangesUpdateJob.php
@@ -128,8 +128,10 @@ class RecentChangesUpdateJob extends Job {
 				$dbw->setSessionOptions( [ 'connTimeout' => 900 ] );
 
 				$lockKey = wfWikiID() . '-activeusers';
-				if ( !$dbw->lock( $lockKey, __METHOD__, 1 ) ) {
-					return; // exclusive update (avoids duplicate entries)
+				if ( !$dbw->lockIsFree( $lockKey, __METHOD__ ) || !$dbw->lock( $lockKey, __METHOD__, 1 ) ) {
+					// Exclusive update (avoids duplicate entries)… it's usually fine to just drop out here,
+					// if the Job is already running.
+					return;
 				}
 
 				$nowUnix = time();
@@ -168,15 +170,6 @@ class RecentChangesUpdateJob extends Job {
 					$names[$row->rc_user_text] = $row->lastedittime;
 				}
 
-				// Rotate out users that have not edited in too long (according to old data set)
-				$dbw->delete( 'querycachetwo',
-					[
-						'qcc_type' => 'activeusers',
-						'qcc_value < ' . $dbw->addQuotes( $nowUnix - $days * 86400 ) // TS_UNIX
-					],
-					__METHOD__
-				);
-
 				// Find which of the recently active users are already accounted for
 				if ( count( $names ) ) {
 					$res = $dbw->select( 'querycachetwo',
@@ -184,9 +177,13 @@ class RecentChangesUpdateJob extends Job {
 						[
 							'qcc_type' => 'activeusers',
 							'qcc_namespace' => NS_USER,
-							'qcc_title' => array_keys( $names ) ],
+							'qcc_title' => array_keys( $names ),
+							'qcc_value >= ' . $dbw->addQuotes( $nowUnix - $days * 86400 ), // TS_UNIX
+						 ],
 						__METHOD__
 					);
+					// Note: In order for this to be actually consistent, we would need
+					// to update these rows with the new lastedittime.
 					foreach ( $res as $row ) {
 						unset( $names[$row->user_name] );
 					}
@@ -224,6 +221,16 @@ class RecentChangesUpdateJob extends Job {
 				);
 
 				$dbw->unlock( $lockKey, __METHOD__ );
+
+				// Rotate out users that have not edited in too long (according to old data set)
+				$dbw->delete( 'querycachetwo',
+					[
+						'qcc_type' => 'activeusers',
+						'qcc_value < ' . $dbw->addQuotes( $nowUnix - $days * 86400 ) // TS_UNIX
+					],
+					__METHOD__
+				);
+
 			},
 			__METHOD__
 		);
-- 
2.20.1