Merge "jobqueue: Use explicit retry when refreshLinks can't get a lock"

diff --git a/includes/deferred/LinksDeletionUpdate.php b/includes/deferred/LinksDeletionUpdate.php
index f370e43..5ab83c6 100644 (file)
--- a/includes/deferred/LinksDeletionUpdate.php
+++ b/includes/deferred/LinksDeletionUpdate.php
@@ -71,6 +71,9 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate {
                        // Make sure all links update threads see the changes of each other.
                        // This handles the case when updates have to batched into several COMMITs.
                        $scopedLock = LinksUpdate::acquirePageLock( $this->getDB(), $id );
+                       if ( !$scopedLock ) {
+                               throw new RuntimeException( "Could not acquire lock for page ID '{$id}'." );
+                       }
                }
 
                $title = $this->page->getTitle();

diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php
index 5b1be6d..dbe387b 100644 (file)
--- a/includes/deferred/LinksUpdate.php
+++ b/includes/deferred/LinksUpdate.php
@@ -21,6 +21,7 @@
  */
 
 use Wikimedia\Rdbms\IDatabase;
+use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use Wikimedia\ScopedCallback;
 
@@ -163,6 +164,9 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate {
                        // Make sure all links update threads see the changes of each other.
                        // This handles the case when updates have to batched into several COMMITs.
                        $scopedLock = self::acquirePageLock( $this->getDB(), $this->mId );
+                       if ( !$scopedLock ) {
+                               throw new RuntimeException( "Could not acquire lock for page ID '{$this->mId}'." );
+                       }
                }
 
                // Avoid PHP 7.1 warning from passing $this by reference
@@ -190,15 +194,19 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate {
         * @param IDatabase $dbw
         * @param int $pageId
         * @param string $why One of (job, atomicity)
-        * @return ScopedCallback
-        * @throws RuntimeException
+        * @return ScopedCallback|null
         * @since 1.27
         */
        public static function acquirePageLock( IDatabase $dbw, $pageId, $why = 'atomicity' ) {
                $key = "LinksUpdate:$why:pageid:$pageId";
                $scopedLock = $dbw->getScopedLockAndFlush( $key, __METHOD__, 15 );
                if ( !$scopedLock ) {
-                       throw new RuntimeException( "Could not acquire lock '$key'." );
+                       $logger = LoggerFactory::getInstance( 'SecondaryDataUpdate' );
+                       $logger->info( "Could not acquire lock '{key}' for page ID '{page_id}'.", [
+                               'key' => $key,
+                               'page_id' => $pageId,
+                       ] );
+                       return null;
                }
 
                return $scopedLock;

diff --git a/includes/jobqueue/jobs/DeleteLinksJob.php b/includes/jobqueue/jobs/DeleteLinksJob.php
index 0a14192..8328abf 100644 (file)
--- a/includes/jobqueue/jobs/DeleteLinksJob.php
+++ b/includes/jobqueue/jobs/DeleteLinksJob.php
@@ -47,6 +47,10 @@ class DeleteLinksJob extends Job {
 
                // Serialize links updates by page ID so they see each others' changes
                $scopedLock = LinksUpdate::acquirePageLock( wfGetDB( DB_MASTER ), $pageId, 'job' );
+               if ( $scopedLock === null ) {
+                       $this->setLastError( 'LinksUpdate already running for this page, try again later.' );
+                       return false;
+               }
 
                if ( WikiPage::newFromID( $pageId, WikiPage::READ_LATEST ) ) {
                        // The page was restored somehow or something went wrong

diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php
index 8854c65..aa8e121 100644 (file)
--- a/includes/jobqueue/jobs/RefreshLinksJob.php
+++ b/includes/jobqueue/jobs/RefreshLinksJob.php
@@ -146,6 +146,11 @@ class RefreshLinksJob extends Job {
                $dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
                /** @noinspection PhpUnusedLocalVariableInspection */
                $scopedLock = LinksUpdate::acquirePageLock( $dbw, $page->getId(), 'job' );
+               if ( $scopedLock === null ) {
+                       // Another job is already updating the page, likely for an older revision (T170596).
+                       $this->setLastError( 'LinksUpdate already running for this page, try again later.' );
+                       return false;
+               }
                // Get the latest ID *after* acquirePageLock() flushed the transaction.
                // This is used to detect edits/moves after loadPageData() but before the scope lock.
                // The works around the chicken/egg problem of determining the scope lock key.