Make Database disconnect and error suppression more robust
[lhc/web/wiklou.git] / includes / db / Database.php
index d4edf01..4e358d4 100644 (file)
@@ -58,6 +58,8 @@ abstract class DatabaseBase implements IDatabase {
        protected $mTrxPreCommitCallbacks = [];
        /** @var array[] List of (callable, method name) */
        protected $mTrxEndCallbacks = [];
+       /** @var bool Whether to suppress triggering of post-commit callbacks */
+       protected $suppressPostCommitCallbacks = false;
 
        protected $mTablePrefix;
        protected $mSchema;
@@ -694,9 +696,6 @@ abstract class DatabaseBase implements IDatabase {
        }
 
        public function close() {
-               if ( count( $this->mTrxIdleCallbacks ) ) { // sanity
-                       throw new MWException( "Transaction idle callbacks still pending." );
-               }
                if ( $this->mConn ) {
                        if ( $this->trxLevel() ) {
                                if ( !$this->mTrxAutomatic ) {
@@ -709,6 +708,8 @@ abstract class DatabaseBase implements IDatabase {
 
                        $closed = $this->closeConnection();
                        $this->mConn = false;
+               } elseif ( $this->mTrxIdleCallbacks || $this->mTrxEndCallbacks ) { // sanity
+                       throw new MWException( "Transaction callbacks still pending." );
                } else {
                        $closed = true;
                }
@@ -782,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 );
@@ -809,7 +811,10 @@ 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 ) ) {
+               # 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)" );
                        $this->mTrxAutomatic = true;
                }
@@ -861,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;
@@ -899,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 );
                }
@@ -917,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" );
@@ -2387,14 +2429,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
@@ -2450,6 +2497,10 @@ abstract class DatabaseBase implements IDatabase {
                return false;
        }
 
+       public function serverIsReadOnly() {
+               return false;
+       }
+
        final public function onTransactionResolution( callable $callback ) {
                if ( !$this->mTrxLevel ) {
                        throw new DBUnexpectedError( $this, "No transaction is active." );
@@ -2460,7 +2511,7 @@ abstract class DatabaseBase implements IDatabase {
        final public function onTransactionIdle( callable $callback ) {
                $this->mTrxIdleCallbacks[] = [ $callback, wfGetCaller() ];
                if ( !$this->mTrxLevel ) {
-                       $this->runOnTransactionIdleCallbacks();
+                       $this->runOnTransactionIdleCallbacks( self::TRIGGER_IDLE );
                }
        }
 
@@ -2468,31 +2519,59 @@ abstract class DatabaseBase implements IDatabase {
                if ( $this->mTrxLevel ) {
                        $this->mTrxPreCommitCallbacks[] = [ $callback, wfGetCaller() ];
                } else {
-                       $this->onTransactionIdle( $callback ); // this will trigger immediately
+                       // If no transaction is active, then make one for this callback
+                       $this->begin( __METHOD__ );
+                       try {
+                               call_user_func( $callback );
+                               $this->commit( __METHOD__ );
+                       } catch ( Exception $e ) {
+                               $this->rollback( __METHOD__ );
+                               throw $e;
+                       }
                }
        }
 
        /**
-        * Actually any "on transaction idle" callbacks.
+        * Whether to disable running of post-commit callbacks
+        *
+        * This method should not be used outside of Database/LoadBalancer
+        *
+        * @param bool $suppress
+        * @since 1.28
+        */
+       final public function setPostCommitCallbackSupression( $suppress ) {
+               $this->suppressPostCommitCallbacks = $suppress;
+       }
+
+       /**
+        * Actually run and consume any "on transaction idle/resolution" callbacks.
         *
+        * This method should not be used outside of Database/LoadBalancer
+        *
+        * @param integer $trigger IDatabase::TRIGGER_* constant
         * @since 1.20
+        * @throws Exception
         */
-       protected function runOnTransactionIdleCallbacks() {
-               $autoTrx = $this->getFlag( DBO_TRX ); // automatic begin() enabled?
+       public function runOnTransactionIdleCallbacks( $trigger ) {
+               if ( $this->suppressPostCommitCallbacks ) {
+                       return;
+               }
 
+               $autoTrx = $this->getFlag( DBO_TRX ); // automatic begin() enabled?
+               /** @var Exception $e */
                $e = $ePrior = null; // last exception
                do { // callbacks may add callbacks :)
                        $callbacks = array_merge(
                                $this->mTrxIdleCallbacks,
                                $this->mTrxEndCallbacks // include "transaction resolution" callbacks
                        );
-                       $this->mTrxIdleCallbacks = []; // recursion guard
-                       $this->mTrxEndCallbacks = []; // recursion guard
+                       $this->mTrxIdleCallbacks = []; // consumed (and recursion guard)
+                       $this->mTrxEndCallbacks = []; // consumed (recursion guard)
                        foreach ( $callbacks as $callback ) {
                                try {
                                        list( $phpCallback ) = $callback;
                                        $this->clearFlag( DBO_TRX ); // make each query its own transaction
-                                       call_user_func( $phpCallback );
+                                       call_user_func_array( $phpCallback, [ $trigger ] );
                                        if ( $autoTrx ) {
                                                $this->setFlag( DBO_TRX ); // restore automatic begin()
                                        } else {
@@ -2518,15 +2597,17 @@ abstract class DatabaseBase implements IDatabase {
        }
 
        /**
-        * Actually any "on transaction pre-commit" callbacks.
+        * Actually run and consume any "on transaction pre-commit" callbacks.
+        *
+        * This method should not be used outside of Database/LoadBalancer
         *
         * @since 1.22
         */
-       protected function runOnTransactionPreCommitCallbacks() {
+       public function runOnTransactionPreCommitCallbacks() {
                $e = $ePrior = null; // last exception
                do { // callbacks may add callbacks :)
                        $callbacks = $this->mTrxPreCommitCallbacks;
-                       $this->mTrxPreCommitCallbacks = []; // recursion guard
+                       $this->mTrxPreCommitCallbacks = []; // consumed (and recursion guard)
                        foreach ( $callbacks as $callback ) {
                                try {
                                        list( $phpCallback ) = $callback;
@@ -2561,12 +2642,12 @@ abstract class DatabaseBase implements IDatabase {
 
        final public function endAtomic( $fname = __METHOD__ ) {
                if ( !$this->mTrxLevel ) {
-                       throw new DBUnexpectedError( $this, 'No atomic transaction is open.' );
+                       throw new DBUnexpectedError( $this, "No atomic transaction is open (got $fname)." );
                }
                if ( !$this->mTrxAtomicLevels ||
                        array_pop( $this->mTrxAtomicLevels ) !== $fname
                ) {
-                       throw new DBUnexpectedError( $this, 'Invalid atomic section ended.' );
+                       throw new DBUnexpectedError( $this, "Invalid atomic section ended (got $fname)." );
                }
 
                if ( !$this->mTrxAtomicLevels && $this->mTrxAutomaticAtomic ) {
@@ -2603,13 +2684,13 @@ abstract class DatabaseBase implements IDatabase {
                                        "$fname: Transaction already in progress (from {$this->mTrxFname}), " .
                                                " performing implicit commit!"
                                );
-                       } else {
+                       } elseif ( $this->mTrxDoneWrites ) {
                                // The transaction was automatic and has done write operations
-                               if ( $this->mTrxDoneWrites ) {
-                                       wfLogDBError( "$fname: Automatic transaction with writes in progress" .
+                               throw new DBUnexpectedError(
+                                       $this,
+                                       "$fname: Automatic transaction with writes in progress" .
                                                " (from {$this->mTrxFname}), performing implicit commit!\n"
-                                       );
-                               }
+                               );
                        }
 
                        $this->runOnTransactionPreCommitCallbacks();
@@ -2621,7 +2702,7 @@ abstract class DatabaseBase implements IDatabase {
                                        $this->mServer, $this->mDBname, $this->mTrxShortId, $writeTime );
                        }
 
-                       $this->runOnTransactionIdleCallbacks();
+                       $this->runOnTransactionIdleCallbacks( self::TRIGGER_COMMIT );
                }
 
                // Avoid fatals if close() was called
@@ -2634,8 +2715,6 @@ abstract class DatabaseBase implements IDatabase {
                $this->mTrxAutomatic = false;
                $this->mTrxAutomaticAtomic = false;
                $this->mTrxAtomicLevels = [];
-               $this->mTrxIdleCallbacks = [];
-               $this->mTrxPreCommitCallbacks = [];
                $this->mTrxShortId = wfRandomString( 12 );
                $this->mTrxWriteDuration = 0.0;
                $this->mTrxWriteCallers = [];
@@ -2681,7 +2760,10 @@ abstract class DatabaseBase implements IDatabase {
                                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!" );
+                               throw new DBUnexpectedError(
+                                       $this,
+                                       "$fname: Explicit commit of implicit transaction."
+                               );
                        }
                }
 
@@ -2697,7 +2779,7 @@ abstract class DatabaseBase implements IDatabase {
                                $this->mServer, $this->mDBname, $this->mTrxShortId, $writeTime );
                }
 
-               $this->runOnTransactionIdleCallbacks();
+               $this->runOnTransactionIdleCallbacks( self::TRIGGER_COMMIT );
        }
 
        /**
@@ -2737,7 +2819,7 @@ abstract class DatabaseBase implements IDatabase {
 
                $this->mTrxIdleCallbacks = []; // clear
                $this->mTrxPreCommitCallbacks = []; // clear
-               $this->runOnTransactionIdleCallbacks();
+               $this->runOnTransactionIdleCallbacks( self::TRIGGER_ROLLBACK );
        }
 
        /**
@@ -2748,11 +2830,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
@@ -2854,6 +2945,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;
        }