Remove direct rollback() calls from some places

Rely on the mass-rollback logic in MWExceptionHandler instead.
This results in a better chance of atomicity.

Change-Id: I2eb5661d4acc105a1323d69c5463268c234bd745

diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php
index 361058b..65638f2 100644 (file)
--- a/includes/FileDeleteForm.php
+++ b/includes/FileDeleteForm.php
@@ -189,31 +189,24 @@ class FileDeleteForm {
                        );
                        $page = WikiPage::factory( $title );
                        $dbw = wfGetDB( DB_MASTER );
-                       try {
-                               $dbw->startAtomic( __METHOD__ );
-                               // delete the associated article first
-                               $error = '';
-                               $deleteStatus = $page->doDeleteArticleReal( $reason, $suppress, 0, false, $error, $user );
-                               // doDeleteArticleReal() returns a non-fatal error status if the page
-                               // or revision is missing, so check for isOK() rather than isGood()
-                               if ( $deleteStatus->isOK() ) {
-                                       $status = $file->delete( $reason, $suppress, $user );
-                                       if ( $status->isOK() ) {
-                                               $status->value = $deleteStatus->value; // log id
-                                               $dbw->endAtomic( __METHOD__ );
-                                       } else {
-                                               // Page deleted but file still there? rollback page delete
-                                               wfGetLBFactory()->rollbackMasterChanges( __METHOD__ );
-                                       }
-                               } else {
-                                       // Done; nothing changed
+                       $dbw->startAtomic( __METHOD__ );
+                       // delete the associated article first
+                       $error = '';
+                       $deleteStatus = $page->doDeleteArticleReal( $reason, $suppress, 0, false, $error, $user );
+                       // doDeleteArticleReal() returns a non-fatal error status if the page
+                       // or revision is missing, so check for isOK() rather than isGood()
+                       if ( $deleteStatus->isOK() ) {
+                               $status = $file->delete( $reason, $suppress, $user );
+                               if ( $status->isOK() ) {
+                                       $status->value = $deleteStatus->value; // log id
                                        $dbw->endAtomic( __METHOD__ );
+                               } else {
+                                       // Page deleted but file still there? rollback page delete
+                                       wfGetLBFactory()->rollbackMasterChanges( __METHOD__ );
                                }
-                       } catch ( Exception $e ) {
-                               // Rollback before returning to prevent UI from displaying
-                               // incorrect "View or restore N deleted edits?"
-                               $dbw->rollback( __METHOD__ );
-                               throw $e;
+                       } else {
+                               // Done; nothing changed
+                               $dbw->endAtomic( __METHOD__ );
                        }
                }
 

diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php
index 479ec32..b9f4592 100644 (file)
--- a/includes/jobqueue/JobQueueDB.php
+++ b/includes/jobqueue/JobQueueDB.php
@@ -245,10 +245,7 @@ class JobQueueDB extends JobQueue {
                                count( $rowSet ) + count( $rowList ) - count( $rows )
                        );
                } catch ( DBError $e ) {
-                       if ( $flags & self::QOS_ATOMIC ) {
-                               $dbw->rollback( $method );
-                       }
-                       throw $e;
+                       $this->throwDBException( $e );
                }
                if ( $flags & self::QOS_ATOMIC ) {
                        $dbw->endAtomic( $method );

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index 5556dd8..84936a4 100644 (file)
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -695,6 +695,7 @@ class SqlBagOStuff extends BagOStuff {
         *
         * @param DBError $exception
         * @param int $serverIndex
+        * @throws Exception
         */
        protected function handleWriteError( DBError $exception, $serverIndex ) {
                if ( $exception instanceof DBConnectionError ) {
@@ -702,6 +703,13 @@ class SqlBagOStuff extends BagOStuff {
                }
                if ( $exception->db && $exception->db->wasReadOnlyError() ) {
                        if ( $exception->db->trxLevel() ) {
+                               if ( !$this->serverInfos ) {
+                                       // Errors like deadlocks and connection drops already cause rollback.
+                                       // For consistency, we have no choice but to throw an error and trigger
+                                       // complete rollback if the main DB is also being used as the cache DB.
+                                       throw $exception;
+                               }
+
                                try {
                                        $exception->db->rollback( __METHOD__ );
                                } catch ( DBError $e ) {

diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index 0344756..033eb67 100644 (file)
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -1770,7 +1770,6 @@ class WikiPage implements Page, IDBAccessObject {
                        $revisionId = $revision->insertOn( $dbw );
                        // Update page_latest and friends to reflect the new revision
                        if ( !$this->updateRevisionOn( $dbw, $revision, null, $meta['oldIsRedirect'] ) ) {
-                               $dbw->rollback( __METHOD__ ); // sanity; this should never happen
                                throw new MWException( "Failed to update page row to use new revision." );
                        }
 
@@ -1920,7 +1919,6 @@ class WikiPage implements Page, IDBAccessObject {
                $revisionId = $revision->insertOn( $dbw );
                // Update the page record with revision data
                if ( !$this->updateRevisionOn( $dbw, $revision, 0 ) ) {
-                       $dbw->rollback( __METHOD__ ); // sanity; this should never happen
                        throw new MWException( "Failed to update page row to use new revision." );
                }