rdbms: fix transaction flushing in Database::close
[lhc/web/wiklou.git] / includes / libs / rdbms / database / Database.php
index 53810da..8d38fb0 100644 (file)
@@ -101,6 +101,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected $queryLogger;
        /** @var callback Error logging callback */
        protected $errorLogger;
+       /** @var callback Deprecation logging callback */
+       protected $deprecationLogger;
 
        /** @var resource|null Database connection */
        protected $conn = null;
@@ -149,6 +151,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @var Exception|null The last error that caused the status to become STATUS_TRX_ERROR
         */
        protected $trxStatusCause;
+       /**
+        * @var array|null If wasKnownStatementRollbackError() prevented trxStatus from being set,
+        *  the relevant details are stored here.
+        */
+       protected $trxStatusIgnoredCause;
        /**
         * Either 1 if a transaction is active or 0 otherwise.
         * The other Trx fields may not be meaningfull if this is 0.
@@ -312,6 +319,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->connLogger = $params['connLogger'];
                $this->queryLogger = $params['queryLogger'];
                $this->errorLogger = $params['errorLogger'];
+               $this->deprecationLogger = $params['deprecationLogger'];
 
                if ( isset( $params['nonNativeInsertSelectBatchSize'] ) ) {
                        $this->nonNativeInsertSelectBatchSize = $params['nonNativeInsertSelectBatchSize'];
@@ -396,6 +404,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *      includes the agent as a SQL comment.
         *   - trxProfiler: Optional TransactionProfiler instance.
         *   - errorLogger: Optional callback that takes an Exception and logs it.
+        *   - deprecationLogger: Optional callback that takes a string and logs it.
         *   - cliMode: Whether to consider the execution context that of a CLI script.
         *   - agent: Optional name used to identify the end-user in query profiling/logging.
         *   - srvCache: Optional BagOStuff instance to an APC-style cache.
@@ -437,6 +446,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
                                };
                        }
+                       if ( !isset( $p['deprecationLogger'] ) ) {
+                               $p['deprecationLogger'] = function ( $msg ) {
+                                       trigger_error( $msg, E_USER_DEPRECATED );
+                               };
+                       }
 
                        /** @var Database $conn */
                        $conn = new $class( $p );
@@ -884,34 +898,48 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                if ( $this->conn ) {
                        // Resolve any dangling transaction first
                        if ( $this->trxLevel ) {
-                               // Meaningful transactions should ideally have been resolved by now
-                               if ( $this->writesOrCallbacksPending() ) {
-                                       $this->queryLogger->warning(
-                                               __METHOD__ . ": writes or callbacks still pending.",
-                                               [ 'trace' => ( new RuntimeException() )->getTraceAsString() ]
-                                       );
+                               if ( $this->trxAtomicLevels ) {
                                        // Cannot let incomplete atomic sections be committed
-                                       if ( $this->trxAtomicLevels ) {
-                                               $levels = $this->flatAtomicSectionList();
-                                               $exception = new DBUnexpectedError(
-                                                       $this,
-                                                       __METHOD__ . ": atomic sections $levels are still open."
-                                               );
-                                       // Check if it is possible to properly commit and trigger callbacks
-                                       } elseif ( $this->trxEndCallbacksSuppressed ) {
+                                       $levels = $this->flatAtomicSectionList();
+                                       $exception = new DBUnexpectedError(
+                                               $this,
+                                               __METHOD__ . ": atomic sections $levels are still open."
+                                       );
+                               } elseif ( $this->trxAutomatic ) {
+                                       // Only the connection manager can commit non-empty DBO_TRX transactions
+                                       if ( $this->writesOrCallbacksPending() ) {
                                                $exception = new DBUnexpectedError(
                                                        $this,
-                                                       __METHOD__ . ': callbacks are suppressed; cannot properly commit.'
+                                                       __METHOD__ .
+                                                       ": mass commit/rollback of peer transaction required (DBO_TRX set)."
                                                );
                                        }
+                               } elseif ( $this->trxLevel ) {
+                                       // Commit explicit transactions as if this was commit()
+                                       $this->queryLogger->warning(
+                                               __METHOD__ . ": writes or callbacks still pending.",
+                                               [ 'trace' => ( new RuntimeException() )->getTraceAsString() ]
+                                       );
                                }
+
+                               if ( $this->trxEndCallbacksSuppressed ) {
+                                       $exception = $exception ?: new DBUnexpectedError(
+                                               $this,
+                                               __METHOD__ . ': callbacks are suppressed; cannot properly commit.'
+                                       );
+                               }
+
                                // Commit or rollback the changes and run any callbacks as needed
                                if ( $this->trxStatus === self::STATUS_TRX_OK && !$exception ) {
-                                       $this->commit( __METHOD__, self::TRANSACTION_INTERNAL );
+                                       $this->commit(
+                                               __METHOD__,
+                                               $this->trxAutomatic ? self::FLUSHING_INTERNAL : self::FLUSHING_ONE
+                                       );
                                } else {
-                                       $this->rollback( __METHOD__, self::TRANSACTION_INTERNAL );
+                                       $this->rollback( __METHOD__, self::FLUSHING_INTERNAL );
                                }
                        }
+
                        // Close the actual connection in the binding handle
                        $closed = $this->closeConnection();
                        $this->conn = false;
@@ -1061,6 +1089,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        public function query( $sql, $fname = __METHOD__, $tempIgnore = false ) {
                $this->assertTransactionStatus( $sql, $fname );
 
+               # Avoid fatals if close() was called
+               $this->assertOpen();
+
                $priorWritesPending = $this->writesOrCallbacksPending();
                $this->lastQuery = $sql;
 
@@ -1111,9 +1142,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->queryLogger->debug( "{$this->dbName} {$commentedSql}" );
                }
 
-               # Avoid fatals if close() was called
-               $this->assertOpen();
-
                # Send the query to the server and fetch any corresponding errors
                $ret = $this->doProfiledQuery( $sql, $commentedSql, $isNonTempWrite, $fname );
                $lastError = $this->lastError();
@@ -1139,26 +1167,29 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
 
                if ( $ret === false ) {
-                       if ( $this->trxLevel && !$this->wasKnownStatementRollbackError() ) {
-                               # Either the query was aborted or all queries after BEGIN where aborted.
-                               if ( $this->explicitTrxActive() || $priorWritesPending ) {
-                                       # In the first case, the only options going forward are (a) ROLLBACK, or
-                                       # (b) ROLLBACK TO SAVEPOINT (if one was set). If the later case, the only
-                                       # option is ROLLBACK, since the snapshots would have been released.
-                                       if ( is_object( $tempIgnore ) ) {
-                                               // Ugly hack to know that savepoints are in use for postgres
-                                               // FIXME: remove this and make DatabasePostgres use ATOMIC_CANCELABLE
-                                       } else {
+                       if ( $this->trxLevel ) {
+                               if ( !$this->wasKnownStatementRollbackError() ) {
+                                       # Either the query was aborted or all queries after BEGIN where aborted.
+                                       if ( $this->explicitTrxActive() || $priorWritesPending ) {
+                                               # In the first case, the only options going forward are (a) ROLLBACK, or
+                                               # (b) ROLLBACK TO SAVEPOINT (if one was set). If the later case, the only
+                                               # option is ROLLBACK, since the snapshots would have been released.
                                                $this->trxStatus = self::STATUS_TRX_ERROR;
                                                $this->trxStatusCause =
                                                        $this->makeQueryException( $lastError, $lastErrno, $sql, $fname );
                                                $tempIgnore = false; // cannot recover
+                                       } else {
+                                               # Nothing prior was there to lose from the transaction,
+                                               # so just roll it back.
+                                               $this->doRollback( __METHOD__ . " ($fname)" );
+                                               $this->trxStatus = self::STATUS_TRX_OK;
                                        }
+                                       $this->trxStatusIgnoredCause = null;
                                } else {
-                                       # Nothing prior was there to lose from the transaction,
-                                       # so just roll it back.
-                                       $this->doRollback( __METHOD__ . " ($fname)" );
-                                       $this->trxStatus = self::STATUS_TRX_OK;
+                                       # We're ignoring an error that caused just the current query to be aborted.
+                                       # But log the cause so we can log a deprecation notice if a
+                                       # caller actually does ignore it.
+                                       $this->trxStatusIgnoredCause = [ $lastError, $lastErrno, $fname ];
                                }
                        }
 
@@ -1269,16 +1300,24 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @throws DBTransactionStateError
         */
        private function assertTransactionStatus( $sql, $fname ) {
-               if (
-                       $this->trxStatus < self::STATUS_TRX_OK &&
-                       $this->getQueryVerb( $sql ) !== 'ROLLBACK' // transaction/savepoint
-               ) {
+               if ( $this->getQueryVerb( $sql ) === 'ROLLBACK' ) { // transaction/savepoint
+                       return;
+               }
+
+               if ( $this->trxStatus < self::STATUS_TRX_OK ) {
                        throw new DBTransactionStateError(
                                $this,
                                "Cannot execute query from $fname while transaction status is ERROR. ",
                                [],
                                $this->trxStatusCause
                        );
+               } elseif ( $this->trxStatus === self::STATUS_TRX_OK && $this->trxStatusIgnoredCause ) {
+                       list( $iLastError, $iLastErrno, $iFname ) = $this->trxStatusIgnoredCause;
+                       call_user_func( $this->deprecationLogger,
+                               "Caller from $fname ignored an error originally raised from $iFname: " .
+                               "[$iLastErrno] $iLastError"
+                       );
+                       $this->trxStatusIgnoredCause = null;
                }
        }
 
@@ -1320,7 +1359,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        private function handleSessionLoss() {
                // Clean up tracking of session-level things...
                // https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html
-               // https://www.postgresql.org/docs/9.1/static/sql-createtable.html (ignoring ON COMMIT)
+               // https://www.postgresql.org/docs/9.2/static/sql-createtable.html (ignoring ON COMMIT)
                $this->sessionTempTables = [];
                // https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock
                // https://www.postgresql.org/docs/9.4/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
@@ -3468,6 +3507,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                } elseif ( $savepointId !== 'n/a' ) {
                        $this->doRollbackToSavepoint( $savepointId, $fname );
                        $this->trxStatus = self::STATUS_TRX_OK; // no exception; recovered
+                       $this->trxStatusIgnoredCause = null;
                }
 
                $this->affectedRowCount = 0; // for the sake of consistency
@@ -3487,6 +3527,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        final public function begin( $fname = __METHOD__, $mode = self::TRANSACTION_EXPLICIT ) {
+               static $modes = [ self::TRANSACTION_EXPLICIT, self::TRANSACTION_INTERNAL ];
+               if ( !in_array( $mode, $modes, true ) ) {
+                       throw new DBUnexpectedError( $this, "$fname: invalid mode parameter '$mode'." );
+               }
+
                // Protect against mismatched atomic section, transaction nesting, and snapshot loss
                if ( $this->trxLevel ) {
                        if ( $this->trxAtomicLevels ) {
@@ -3510,6 +3555,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                $this->doBegin( $fname );
                $this->trxStatus = self::STATUS_TRX_OK;
+               $this->trxStatusIgnoredCause = null;
                $this->trxAtomicCounter = 0;
                $this->trxTimestamp = microtime( true );
                $this->trxFname = $fname;
@@ -3544,9 +3590,14 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxLevel = 1;
        }
 
-       final public function commit( $fname = __METHOD__, $flush = '' ) {
+       final public function commit( $fname = __METHOD__, $flush = self::FLUSHING_ONE ) {
+               static $modes = [ self::FLUSHING_ONE, self::FLUSHING_ALL_PEERS, self::FLUSHING_INTERNAL ];
+               if ( !in_array( $flush, $modes, true ) ) {
+                       throw new DBUnexpectedError( $this, "$fname: invalid flush parameter '$flush'." );
+               }
+
                if ( $this->trxLevel && $this->trxAtomicLevels ) {
-                       // There are still atomic sections open. This cannot be ignored
+                       // There are still atomic sections open; this cannot be ignored
                        $levels = $this->flatAtomicSectionList();
                        throw new DBUnexpectedError(
                                $this,