Remove web-request usages of deadlockLoop()
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 27 Jul 2016 01:44:41 +0000 (18:44 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Wed, 27 Jul 2016 01:44:41 +0000 (18:44 -0700)
Add comment discouraging use of the method. There are no problems
with deadlocks/timeouts in these code paths according to WMF logs.

Change-Id: I5b21cc423df584efa881361063000e01932cc2ea

includes/db/Database.php
includes/import/WikiImporter.php
includes/import/WikiRevision.php

index 7d04cac..2e3e225 100644 (file)
@@ -2388,14 +2388,19 @@ abstract class DatabaseBase implements IDatabase {
         * queries. If a deadlock occurs during the processing, the transaction
         * will be rolled back and the callback function will be called again.
         *
+        * Avoid using this method outside of Job or Maintenance classes.
+        *
         * Usage:
         *   $dbw->deadlockLoop( callback, ... );
         *
         * Extra arguments are passed through to the specified callback function.
+        * This method requires that no transactions are already active to avoid
+        * causing premature commits or exceptions.
         *
         * Returns whatever the callback function returned on its successful,
         * iteration, or false on error, for example if the retry limit was
         * reached.
+        *
         * @return mixed
         * @throws DBUnexpectedError
         * @throws Exception
index 826bb59..406667e 100644 (file)
@@ -332,8 +332,7 @@ class WikiImporter {
                }
 
                try {
-                       $dbw = wfGetDB( DB_MASTER );
-                       return $dbw->deadlockLoop( [ $revision, 'importOldRevision' ] );
+                       return $revision->importOldRevision();
                } catch ( MWContentSerializationException $ex ) {
                        $this->notice( 'import-error-unserialize',
                                $revision->getTitle()->getPrefixedText(),
@@ -351,8 +350,7 @@ class WikiImporter {
         * @return bool
         */
        public function importLogItem( $revision ) {
-               $dbw = wfGetDB( DB_MASTER );
-               return $dbw->deadlockLoop( [ $revision, 'importLogItem' ] );
+               return $revision->importLogItem();
        }
 
        /**
@@ -361,8 +359,7 @@ class WikiImporter {
         * @return bool
         */
        public function importUpload( $revision ) {
-               $dbw = wfGetDB( DB_MASTER );
-               return $dbw->deadlockLoop( [ $revision, 'importUpload' ] );
+               return $revision->importUpload();
        }
 
        /**
index 356a79f..d78d61a 100644 (file)
@@ -574,7 +574,7 @@ class WikiRevision {
                if ( !$this->getTitle() ) {
                        wfDebug( __METHOD__ . ": skipping invalid {$this->type}/{$this->action} log time, timestamp " .
                                $this->timestamp . "\n" );
-                       return;
+                       return false;
                }
                # Check if it exists already
                // @todo FIXME: Use original log ID (better for backups)
@@ -594,7 +594,7 @@ class WikiRevision {
                        wfDebug( __METHOD__
                                . ": skipping existing item for Log:{$this->type}/{$this->action}, timestamp "
                                . $this->timestamp . "\n" );
-                       return;
+                       return false;
                }
                $log_id = $dbw->nextSequenceValue( 'logging_log_id_seq' );
                $data = [
@@ -610,6 +610,8 @@ class WikiRevision {
                        'log_params' => $this->params
                ];
                $dbw->insert( 'logging', $data, __METHOD__ );
+
+               return true;
        }
 
        /**