Merge "Make doAtomicSection() return the callback result"
[lhc/web/wiklou.git] / includes / db / Database.php
index ba25183..940c3f7 100644 (file)
@@ -703,7 +703,7 @@ abstract class DatabaseBase implements IDatabase {
                                                " performing implicit commit before closing connection!" );
                                }
 
-                               $this->commit( __METHOD__, 'flush' );
+                               $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
                        }
 
                        $closed = $this->closeConnection();
@@ -783,6 +783,7 @@ abstract class DatabaseBase implements IDatabase {
        public function query( $sql, $fname = __METHOD__, $tempIgnore = false ) {
                global $wgUser;
 
+               $priorWritesPending = $this->writesOrCallbacksPending();
                $this->mLastQuery = $sql;
 
                $isWriteQuery = $this->isWriteQuery( $sql );
@@ -810,8 +811,11 @@ abstract class DatabaseBase implements IDatabase {
                // Or, for one-word queries (like "BEGIN" or COMMIT") add it to the end (bug 42598)
                $commentedSql = preg_replace( '/\s|$/', " /* $fname $userName */ ", $sql, 1 );
 
-               if ( !$this->mTrxLevel && $this->getFlag( DBO_TRX ) && $this->isTransactableQuery( $sql ) ) {
-                       $this->begin( __METHOD__ . " ($fname)" );
+               # Start implicit transactions that wrap the request if DBO_TRX is enabled
+               if ( !$this->mTrxLevel && $this->getFlag( DBO_TRX )
+                       && $this->isTransactableQuery( $sql )
+               ) {
+                       $this->begin( __METHOD__ . " ($fname)", self::TRANSACTION_INTERNAL );
                        $this->mTrxAutomatic = true;
                }
 
@@ -862,31 +866,23 @@ abstract class DatabaseBase implements IDatabase {
 
                # Try reconnecting if the connection was lost
                if ( false === $ret && $this->wasErrorReissuable() ) {
-                       # Transaction is gone; this can mean lost writes or REPEATABLE-READ snapshots
-                       $hadTrx = $this->mTrxLevel;
-                       # T127428: for non-write transactions, a disconnect and a COMMIT are similar:
-                       # neither changed data and in both cases any read snapshots are reset anyway.
-                       $isNoopCommit = ( !$this->writesOrCallbacksPending() && $sql === 'COMMIT' );
-                       # Update state tracking to reflect transaction loss
-                       $this->mTrxLevel = 0;
-                       $this->mTrxIdleCallbacks = []; // bug 65263
-                       $this->mTrxPreCommitCallbacks = []; // bug 65263
-                       wfDebug( "Connection lost, reconnecting...\n" );
-                       # Stash the last error values since ping() might clear them
+                       $recoverable = $this->canRecoverFromDisconnect( $sql, $priorWritesPending );
+                       # Stash the last error values before anything might clear them
                        $lastError = $this->lastError();
                        $lastErrno = $this->lastErrno();
-                       if ( $this->ping() ) {
+                       # Update state tracking to reflect transaction loss due to disconnection
+                       $this->handleTransactionLoss();
+                       wfDebug( "Connection lost, reconnecting...\n" );
+                       if ( $this->reconnect() ) {
                                wfDebug( "Reconnected\n" );
-                               $server = $this->getServer();
-                               $msg = __METHOD__ . ": lost connection to $server; reconnected";
+                               $msg = __METHOD__ . ": lost connection to {$this->getServer()}; reconnected";
                                wfDebugLog( 'DBPerformance', "$msg:\n" . wfBacktrace( true ) );
 
-                               if ( ( $hadTrx && !$isNoopCommit ) || $this->mNamedLocksHeld ) {
-                                       # Leave $ret as false and let an error be reported.
-                                       # Callers may catch the exception and continue to use the DB.
-                                       $this->reportQueryError( $lastError, $lastErrno, $sql, $fname, $tempIgnore );
+                               if ( !$recoverable ) {
+                                       # Callers may catch the exception and continue to use the DB
+                                       $this->reportQueryError( $lastError, $lastErrno, $sql, $fname );
                                } else {
-                                       # Should be safe to silently retry (no trx/callbacks/locks)
+                                       # Should be safe to silently retry the query
                                        $startTime = microtime( true );
                                        $ret = $this->doQuery( $commentedSql );
                                        $queryRuntime = microtime( true ) - $startTime;
@@ -900,6 +896,17 @@ abstract class DatabaseBase implements IDatabase {
                }
 
                if ( false === $ret ) {
+                       # Deadlocks cause the entire transaction to abort, not just the statement.
+                       # http://dev.mysql.com/doc/refman/5.7/en/innodb-error-handling.html
+                       # https://www.postgresql.org/docs/9.1/static/explicit-locking.html
+                       if ( $this->wasDeadlock() ) {
+                               if ( $this->explicitTrxActive() || $priorWritesPending ) {
+                                       $tempIgnore = false; // not recoverable
+                               }
+                               # Update state tracking to reflect transaction loss
+                               $this->handleTransactionLoss();
+                       }
+
                        $this->reportQueryError(
                                $this->lastError(), $this->lastErrno(), $sql, $fname, $tempIgnore );
                }
@@ -918,6 +925,40 @@ abstract class DatabaseBase implements IDatabase {
                return $res;
        }
 
+       private function canRecoverFromDisconnect( $sql, $priorWritesPending ) {
+               # Transaction dropped; this can mean lost writes, or REPEATABLE-READ snapshots.
+               # Dropped connections also mean that named locks are automatically released.
+               # Only allow error suppression in autocommit mode or when the lost transaction
+               # didn't matter anyway (aside from DBO_TRX snapshot loss).
+               if ( $this->mNamedLocksHeld ) {
+                       return false; // possible critical section violation
+               } elseif ( $sql === 'COMMIT' ) {
+                       return !$priorWritesPending; // nothing written anyway? (T127428)
+               } elseif ( $sql === 'ROLLBACK' ) {
+                       return true; // transaction lost...which is also what was requested :)
+               } elseif ( $this->explicitTrxActive() ) {
+                       return false; // don't drop atomocity
+               } elseif ( $priorWritesPending ) {
+                       return false; // prior writes lost from implicit transaction
+               }
+
+               return true;
+       }
+
+       private function handleTransactionLoss() {
+               $this->mTrxLevel = 0;
+               $this->mTrxIdleCallbacks = []; // bug 65263
+               $this->mTrxPreCommitCallbacks = []; // bug 65263
+               try {
+                       // Handle callbacks in mTrxEndCallbacks
+                       $this->runOnTransactionIdleCallbacks( self::TRIGGER_ROLLBACK );
+                       return null;
+               } catch ( Exception $e ) {
+                       // Already logged; move on...
+                       return $e;
+               }
+       }
+
        public function reportQueryError( $error, $errno, $sql, $fname, $tempIgnore = false ) {
                if ( $this->ignoreErrors() || $tempIgnore ) {
                        wfDebug( "SQL ERROR (ignored): $error\n" );
@@ -1823,7 +1864,7 @@ abstract class DatabaseBase implements IDatabase {
        /**
         * Gets an array of aliased table names
         *
-        * @param array $tables Array( [alias] => table )
+        * @param array $tables [ [alias] => table ]
         * @return string[] See tableNameWithAlias()
         */
        public function tableNamesWithAlias( $tables ) {
@@ -1857,7 +1898,7 @@ abstract class DatabaseBase implements IDatabase {
        /**
         * Gets an array of aliased field names
         *
-        * @param array $fields Array( [alias] => field )
+        * @param array $fields [ [alias] => field ]
         * @return string[] See fieldNameWithAlias()
         */
        public function fieldNamesWithAlias( $fields ) {
@@ -2162,7 +2203,7 @@ abstract class DatabaseBase implements IDatabase {
 
                $useTrx = !$this->mTrxLevel;
                if ( $useTrx ) {
-                       $this->begin( $fname );
+                       $this->begin( $fname, self::TRANSACTION_INTERNAL );
                }
                try {
                        # Update any existing conflicting row(s)
@@ -2180,7 +2221,7 @@ abstract class DatabaseBase implements IDatabase {
                        throw $e;
                }
                if ( $useTrx ) {
-                       $this->commit( $fname );
+                       $this->commit( $fname, self::TRANSACTION_INTERNAL );
                }
 
                return $ok;
@@ -2479,7 +2520,7 @@ abstract class DatabaseBase implements IDatabase {
                        $this->mTrxPreCommitCallbacks[] = [ $callback, wfGetCaller() ];
                } else {
                        // If no transaction is active, then make one for this callback
-                       $this->begin( __METHOD__ );
+                       $this->begin( __METHOD__, self::TRANSACTION_INTERNAL );
                        try {
                                call_user_func( $callback );
                                $this->commit( __METHOD__ );
@@ -2509,6 +2550,7 @@ abstract class DatabaseBase implements IDatabase {
         *
         * @param integer $trigger IDatabase::TRIGGER_* constant
         * @since 1.20
+        * @throws Exception
         */
        public function runOnTransactionIdleCallbacks( $trigger ) {
                if ( $this->suppressPostCommitCallbacks ) {
@@ -2516,7 +2558,7 @@ abstract class DatabaseBase implements IDatabase {
                }
 
                $autoTrx = $this->getFlag( DBO_TRX ); // automatic begin() enabled?
-
+               /** @var Exception $e */
                $e = $ePrior = null; // last exception
                do { // callbacks may add callbacks :)
                        $callbacks = array_merge(
@@ -2586,7 +2628,7 @@ abstract class DatabaseBase implements IDatabase {
 
        final public function startAtomic( $fname = __METHOD__ ) {
                if ( !$this->mTrxLevel ) {
-                       $this->begin( $fname );
+                       $this->begin( $fname, self::TRANSACTION_INTERNAL );
                        $this->mTrxAutomatic = true;
                        // If DBO_TRX is set, a series of startAtomic/endAtomic pairs will result
                        // in all changes being in one transaction to keep requests transactional.
@@ -2609,58 +2651,43 @@ abstract class DatabaseBase implements IDatabase {
                }
 
                if ( !$this->mTrxAtomicLevels && $this->mTrxAutomaticAtomic ) {
-                       $this->commit( $fname, 'flush' );
+                       $this->commit( $fname, self::FLUSHING_INTERNAL );
                }
        }
 
        final public function doAtomicSection( $fname, callable $callback ) {
                $this->startAtomic( $fname );
                try {
-                       call_user_func_array( $callback, [ $this, $fname ] );
+                       $res = call_user_func_array( $callback, [ $this, $fname ] );
                } catch ( Exception $e ) {
                        $this->rollback( $fname );
                        throw $e;
                }
                $this->endAtomic( $fname );
+
+               return $res;
        }
 
-       final public function begin( $fname = __METHOD__ ) {
-               if ( $this->mTrxLevel ) { // implicit commit
+       final public function begin( $fname = __METHOD__, $mode = self::TRANSACTION_EXPLICIT ) {
+               // Protect against mismatched atomic section, transaction nesting, and snapshot loss
+               if ( $this->mTrxLevel ) {
                        if ( $this->mTrxAtomicLevels ) {
-                               // If the current transaction was an automatic atomic one, then we definitely have
-                               // a problem. Same if there is any unclosed atomic level.
                                $levels = implode( ', ', $this->mTrxAtomicLevels );
-                               throw new DBUnexpectedError(
-                                       $this,
-                                       "Got explicit BEGIN from $fname while atomic section(s) $levels are open."
-                               );
+                               $msg = "Got explicit BEGIN from $fname while atomic section(s) $levels are open.";
+                               throw new DBUnexpectedError( $this, $msg );
                        } elseif ( !$this->mTrxAutomatic ) {
-                               // We want to warn about inadvertently nested begin/commit pairs, but not about
-                               // auto-committing implicit transactions that were started by query() via DBO_TRX
-                               throw new DBUnexpectedError(
-                                       $this,
-                                       "$fname: Transaction already in progress (from {$this->mTrxFname}), " .
-                                               " performing implicit commit!"
-                               );
-                       } elseif ( $this->mTrxDoneWrites ) {
-                               // The transaction was automatic and has done write operations
-                               throw new DBUnexpectedError(
-                                       $this,
-                                       "$fname: Automatic transaction with writes in progress" .
-                                               " (from {$this->mTrxFname}), performing implicit commit!\n"
-                               );
-                       }
-
-                       $this->runOnTransactionPreCommitCallbacks();
-                       $writeTime = $this->pendingWriteQueryDuration();
-                       $this->doCommit( $fname );
-                       if ( $this->mTrxDoneWrites ) {
-                               $this->mDoneWrites = microtime( true );
-                               $this->getTransactionProfiler()->transactionWritingOut(
-                                       $this->mServer, $this->mDBname, $this->mTrxShortId, $writeTime );
+                               $msg = "$fname: Explicit transaction already active (from {$this->mTrxFname}).";
+                               throw new DBUnexpectedError( $this, $msg );
+                       } else {
+                               // @TODO: make this an exception at some point
+                               $msg = "$fname: Implicit transaction already active (from {$this->mTrxFname}).";
+                               wfLogDBError( $msg );
+                               return; // join the main transaction set
                        }
-
-                       $this->runOnTransactionIdleCallbacks( self::TRIGGER_COMMIT );
+               } elseif ( $this->getFlag( DBO_TRX ) && $mode !== self::TRANSACTION_INTERNAL ) {
+                       // @TODO: make this an exception at some point
+                       wfLogDBError( "$fname: Implicit transaction expected (DBO_TRX set)." );
+                       return; // let any writes be in the main transaction
                }
 
                // Avoid fatals if close() was called
@@ -2700,25 +2727,27 @@ abstract class DatabaseBase implements IDatabase {
                        $levels = implode( ', ', $this->mTrxAtomicLevels );
                        throw new DBUnexpectedError(
                                $this,
-                               "Got COMMIT while atomic sections $levels are still open"
+                               "Got COMMIT while atomic sections $levels are still open."
                        );
                }
 
-               if ( $flush === 'flush' ) {
+               if ( $flush === self::FLUSHING_INTERNAL || $flush === self::FLUSHING_ALL_PEERS ) {
                        if ( !$this->mTrxLevel ) {
                                return; // nothing to do
                        } elseif ( !$this->mTrxAutomatic ) {
                                throw new DBUnexpectedError(
                                        $this,
-                                       "$fname: Flushing an explicit transaction, getting out of sync!"
+                                       "$fname: Flushing an explicit transaction, getting out of sync."
                                );
                        }
                } else {
                        if ( !$this->mTrxLevel ) {
-                               wfWarn( "$fname: No transaction to commit, something got out of sync!" );
+                               wfWarn( "$fname: No transaction to commit, something got out of sync." );
                                return; // nothing to do
                        } elseif ( $this->mTrxAutomatic ) {
-                               wfWarn( "$fname: Explicit commit of implicit transaction. Something may be out of sync!" );
+                               // @TODO: make this an exception at some point
+                               wfLogDBError( "$fname: Explicit commit of implicit transaction." );
+                               return; // wait for the main transaction set commit round
                        }
                }
 
@@ -2751,14 +2780,19 @@ abstract class DatabaseBase implements IDatabase {
        }
 
        final public function rollback( $fname = __METHOD__, $flush = '' ) {
-               if ( $flush !== 'flush' ) {
+               if ( $flush === self::FLUSHING_INTERNAL || $flush === self::FLUSHING_ALL_PEERS ) {
                        if ( !$this->mTrxLevel ) {
-                               wfWarn( "$fname: No transaction to rollback, something got out of sync!" );
                                return; // nothing to do
                        }
                } else {
                        if ( !$this->mTrxLevel ) {
+                               wfWarn( "$fname: No transaction to rollback, something got out of sync." );
                                return; // nothing to do
+                       } elseif ( $this->getFlag( DBO_TRX ) ) {
+                               throw new DBUnexpectedError(
+                                       $this,
+                                       "$fname: Expected mass rollback of all peer databases (DBO_TRX set)."
+                               );
                        }
                }
 
@@ -2785,11 +2819,20 @@ abstract class DatabaseBase implements IDatabase {
         */
        protected function doRollback( $fname ) {
                if ( $this->mTrxLevel ) {
-                       $this->query( 'ROLLBACK', $fname, true );
+                       # Disconnects cause rollback anyway, so ignore those errors
+                       $ignoreErrors = true;
+                       $this->query( 'ROLLBACK', $fname, $ignoreErrors );
                        $this->mTrxLevel = 0;
                }
        }
 
+       /**
+        * @return bool
+        */
+       protected function explicitTrxActive() {
+               return $this->mTrxLevel && ( $this->mTrxAtomicLevels || !$this->mTrxAutomatic );
+       }
+
        /**
         * Creates a new table with structure copied from existing table
         * Note that unlike most database abstraction functions, this function does not
@@ -2891,6 +2934,19 @@ abstract class DatabaseBase implements IDatabase {
        }
 
        public function ping() {
+               try {
+                       // This will reconnect if possible, or error out if not
+                       $this->query( "SELECT 1 AS ping", __METHOD__ );
+                       return true;
+               } catch ( DBError $e ) {
+                       return false;
+               }
+       }
+
+       /**
+        * @return bool
+        */
+       protected function reconnect() {
                # Stub. Not essential to override.
                return true;
        }
@@ -3235,11 +3291,11 @@ abstract class DatabaseBase implements IDatabase {
                }
 
                $unlocker = new ScopedCallback( function () use ( $lockKey, $fname ) {
-                       $this->commit( __METHOD__, 'flush' );
+                       $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
                        $this->unlock( $lockKey, $fname );
                } );
 
-               $this->commit( __METHOD__, 'flush' );
+               $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
 
                return $unlocker;
        }