Add IDatabase::getScopedLockAndFlush() method

This method is less manual and avoids the usual pitfalls of
not unlocking for a return statement or not flushing out any
prior transaction.

Change-Id: Ib1681244767de860105a68210e181e2f024ee525

diff --git a/includes/db/DBConnRef.php b/includes/db/DBConnRef.php
index f09de4f..264ee11 100644 (file)
--- a/includes/db/DBConnRef.php
+++ b/includes/db/DBConnRef.php
@@ -505,6 +505,10 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
+       public function getScopedLockAndFlush( $lockKey, $fname, $timeout ) {
+               return $this->__call( __FUNCTION__, func_get_args() );
+       }
+
        public function namedLocksEnqueue() {
                return $this->__call( __FUNCTION__, func_get_args() );
        }

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 1835958..dc1570e 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3167,6 +3167,21 @@ abstract class DatabaseBase implements IDatabase {
                return true;
        }
 
+       public function getScopedLockAndFlush( $lockKey, $fname, $timeout ) {
+               if ( !$this->lock( $lockKey, $fname, $timeout ) ) {
+                       return null;
+               }
+
+               $that = $this;
+               $unlocker = new ScopedCallback( function () use ( $that, $lockKey, $fname ) {
+                       $that->unlock( $lockKey, $fname );
+               } );
+
+               $this->commit( __METHOD__, 'flush' );
+
+               return $unlocker;
+       }
+
        public function namedLocksEnqueue() {
                return false;
        }

diff --git a/includes/db/IDatabase.php b/includes/db/IDatabase.php
index cecb643..1a6e779 100644 (file)
--- a/includes/db/IDatabase.php
+++ b/includes/db/IDatabase.php
@@ -1493,8 +1493,8 @@ interface IDatabase {
         * Named locks are not related to transactions
         *
         * @param string $lockName Name of lock to aquire
-        * @param string $method Name of method calling us
-        * @param int $timeout
+        * @param string $method Name of the calling method
+        * @param int $timeout Acquisition timeout in seconds
         * @return bool
         */
        public function lock( $lockName, $method, $timeout = 5 );
@@ -1505,7 +1505,7 @@ interface IDatabase {
         * Named locks are not related to transactions
         *
         * @param string $lockName Name of lock to release
-        * @param string $method Name of method calling us
+        * @param string $method Name of the calling method
         *
         * @return int Returns 1 if the lock was released, 0 if the lock was not established
         * by this thread (in which case the lock is not released), and NULL if the named
@@ -1513,6 +1513,25 @@ interface IDatabase {
         */
        public function unlock( $lockName, $method );
 
+       /**
+        * Acquire a named lock, flush any transaction, and return an RAII style unlocker object
+        *
+        * This is suitiable for transactions that need to be serialized using cooperative locks,
+        * where each transaction can see each others' changes. Any transaction is flushed to clear
+        * out stale REPEATABLE-READ snapshot data. Once the returned object falls out of PHP scope,
+        * the lock will be released.
+        *
+        * If the lock acquisition failed, then no transaction flush happens, and null is returned.
+        *
+        * @param string $lockKey Name of lock to release
+        * @param string $fname Name of the calling method
+        * @param int $timeout Acquisition timeout in seconds
+        * @return ScopedCallback|null
+        * @throws DBUnexpectedError
+        * @since 1.27
+        */
+       public function getScopedLockAndFlush( $lockKey, $fname, $timeout );
+
        /**
         * Check to see if a named lock used by lock() use blocking queues
         *

diff --git a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
index 98c87a5..a6869c1 100644 (file)
--- a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
+++ b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
@@ -51,20 +51,13 @@ class CategoryMembershipChangeJob extends Job {
                $dbw = wfGetDB( DB_MASTER );
 
                // Use a named lock so that jobs for this page see each others' changes
-               $fname = __METHOD__;
                $lockKey = "CategoryMembershipUpdates:{$page->getId()}";
-               if ( !$dbw->lock( $lockKey, $fname, 10 ) ) {
+               $scopedLock = $dbw->getScopedLockAndFlush( $lockKey, __METHOD__, 10 );
+               if ( !$scopedLock ) {
                        $this->setLastError( "Could not acquire lock '$lockKey'" );
                        return false;
                }
 
-               $unlocker = new ScopedCallback( function () use ( $dbw, $lockKey, $fname ) {
-                       $dbw->unlock( $lockKey, $fname );
-               } );
-
-               // Sanity: clear any DB transaction snapshot
-               $dbw->commit( __METHOD__, 'flush' );
-
                $cutoffUnix = wfTimestamp( TS_UNIX, $this->params['revTimestamp'] );
                // Using ENQUEUE_FUDGE_SEC handles jobs inserted out of revision order due to the delay
                // between COMMIT and actual enqueueing of the CategoryMembershipChangeJob job.
@@ -121,8 +114,6 @@ class CategoryMembershipChangeJob extends Job {
                        $this->notifyUpdatesForRevision( $page, Revision::newFromRow( $row ) );
                }
 
-               ScopedCallback::consume( $unlocker );
-
                return true;
        }