Fix duplicate removal. Was completely broken, with a "job_id IS NULL" condition in...

diff --git a/includes/job/JobQueue.php b/includes/job/JobQueue.php
index 949dfe5..517b94f 100644 (file)
--- a/includes/job/JobQueue.php
+++ b/includes/job/JobQueue.php
@@ -68,6 +68,7 @@ abstract class Job {
                /* Ensure we "own" this row */
                $dbw->delete( 'job', array( 'job_id' => $row->job_id ), __METHOD__ );
                $affected = $dbw->affectedRows();
+               $dbw->commit();
 
                if ( $affected == 0 ) {
                        wfProfileOut( __METHOD__ );
@@ -81,13 +82,7 @@ abstract class Job {
                $job = Job::factory( $row->job_cmd, $title, Job::extractBlob( $row->job_params ),
                        $row->job_id );
 
-               $dbw->delete( 'job', $job->insertFields(), __METHOD__ );
-               $affected = $dbw->affectedRows();
-               $dbw->commit();
-
-               if ( $affected ) {
-                       wfIncrStats( 'job-dup-delete', $affected );
-               }
+               $job->removeDuplicates();
 
                wfProfileOut( __METHOD__ );
                return $job;
@@ -176,15 +171,7 @@ abstract class Job {
                $job = Job::factory( $row->job_cmd, $title, Job::extractBlob( $row->job_params ), $row->job_id );
 
                // Remove any duplicates it may have later in the queue
-               // Deadlock prone section
-               $dbw->begin();
-               $dbw->delete( 'job', $job->insertFields(), __METHOD__ );
-               $affected = $dbw->affectedRows();
-               $dbw->commit();
-
-               if ( $affected ) {
-                       wfIncrStats( 'job-dup-delete', $affected );
-               }
+               $job->removeDuplicates();
 
                wfProfileOut( __METHOD__ );
                return $job;
@@ -308,6 +295,27 @@ abstract class Job {
                );
        }
 
+       /**
+        * Remove jobs in the job queue which are duplicates of this job.
+        * This is deadlock-prone and so starts its own transaction.
+        */
+       function removeDuplicates() {
+               if ( !$this->removeDuplicates ) {
+                       return;
+               }
+
+               $fields = $this->insertFields();
+               unset( $fields['job_id'] );
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->begin();
+               $dbw->delete( 'job', $fields, __METHOD__ );
+               $affected = $dbw->affectedRows();
+               $dbw->commit();
+               if ( $affected ) {
+                       wfIncrStats( 'job-dup-delete', $affected );
+               }
+       }
+
        function toString() {
                $paramString = '';
                if ( $this->params ) {