Merge "Fix link to Help:Contents in UPGRADE file"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 11 Aug 2016 09:10:44 +0000 (09:10 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 11 Aug 2016 09:10:44 +0000 (09:10 +0000)
includes/db/Database.php
includes/db/DatabaseMysql.php
includes/db/DatabaseMysqlBase.php
includes/db/DatabaseMysqli.php
includes/jobqueue/JobRunner.php
includes/specialpage/SpecialPage.php
includes/specialpage/SpecialPageFactory.php
includes/utils/AutoloadGenerator.php
tests/phpunit/includes/db/DatabaseMysqlBaseTest.php

index ba25183..4e358d4 100644 (file)
@@ -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,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;
                }
@@ -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" );
@@ -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(
@@ -2718,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."
+                               );
                        }
                }
 
@@ -2785,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
@@ -2891,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;
        }
index 5b15147..87330b0 100644 (file)
@@ -201,10 +201,4 @@ class DatabaseMysql extends DatabaseMysqlBase {
 
                return mysql_real_escape_string( $s, $conn );
        }
-
-       protected function mysqlPing() {
-               $conn = $this->getBindingHandle();
-
-               return mysql_ping( $conn );
-       }
 }
index 798815d..6380fe7 100644 (file)
@@ -570,14 +570,7 @@ abstract class DatabaseMysqlBase extends Database {
         * @return string
         */
        function strencode( $s ) {
-               $sQuoted = $this->mysqlRealEscapeString( $s );
-
-               if ( $sQuoted === false ) {
-                       $this->ping();
-                       $sQuoted = $this->mysqlRealEscapeString( $s );
-               }
-
-               return $sQuoted;
+               return $this->mysqlRealEscapeString( $s );
        }
 
        /**
@@ -606,18 +599,7 @@ abstract class DatabaseMysqlBase extends Database {
                return strlen( $name ) && $name[0] == '`' && substr( $name, -1, 1 ) == '`';
        }
 
-       /**
-        * @return bool
-        */
-       function ping() {
-               $ping = $this->mysqlPing();
-               if ( $ping ) {
-                       // Connection was good or lost but reconnected...
-                       // @note: mysqlnd (php 5.6+) does not support this (PHP bug 52561)
-                       return true;
-               }
-
-               // Try a full disconnect/reconnect cycle if ping() failed
+       function reconnect() {
                $this->closeConnection();
                $this->mOpened = false;
                $this->mConn = false;
@@ -626,13 +608,6 @@ abstract class DatabaseMysqlBase extends Database {
                return true;
        }
 
-       /**
-        * Ping a server connection or reconnect if there is no connection
-        *
-        * @return bool
-        */
-       abstract protected function mysqlPing();
-
        function getLag() {
                if ( $this->getLagDetectionMethod() === 'pt-heartbeat' ) {
                        return $this->getLagFromPtHeartbeat();
index d45805a..cb580cc 100644 (file)
@@ -309,12 +309,6 @@ class DatabaseMysqli extends DatabaseMysqlBase {
                return $conn->real_escape_string( $s );
        }
 
-       protected function mysqlPing() {
-               $conn = $this->getBindingHandle();
-
-               return $conn->ping();
-       }
-
        /**
         * Give an id for the connection
         *
index 98d7d12..a132dc5 100644 (file)
@@ -534,7 +534,7 @@ class JobRunner implements LoggerAwareInterface {
                wfGetLBFactory()->forEachLB( function( LoadBalancer $lb ) use ( $fname ) {
                        $lb->forEachOpenConnection( function( IDatabase $conn ) use ( $fname ) {
                                if ( $conn->writesOrCallbacksPending() ) {
-                                       $conn->query( "SELECT 1", $fname );
+                                       $conn->ping();
                                }
                        } );
                } );
index 99b9f33..00d8c4a 100644 (file)
@@ -67,6 +67,8 @@ class SpecialPage {
 
        /**
         * Get a localised Title object for a specified special page name
+        * If you don't need a full Title object, consider using TitleValue through
+        * getTitleValueFor() below.
         *
         * @since 1.9
         * @since 1.21 $fragment parameter added
@@ -78,9 +80,24 @@ class SpecialPage {
         * @throws MWException
         */
        public static function getTitleFor( $name, $subpage = false, $fragment = '' ) {
+               return Title::newFromTitleValue(
+                       self::getTitleValueFor( $name, $subpage, $fragment )
+               );
+       }
+
+       /**
+        * Get a localised TitleValue object for a specified special page name
+        *
+        * @since 1.28
+        * @param string $name
+        * @param string|bool $subpage Subpage string, or false to not use a subpage
+        * @param string $fragment The link fragment (after the "#")
+        * @return TitleValue
+        */
+       public static function getTitleValueFor( $name, $subpage = false, $fragment = '' ) {
                $name = SpecialPageFactory::getLocalNameFor( $name, $subpage );
 
-               return Title::makeTitle( NS_SPECIAL, $name, $fragment );
+               return new TitleValue( NS_SPECIAL, $name, $fragment );
        }
 
        /**
index 030a903..4356bc5 100644 (file)
@@ -699,6 +699,8 @@ class SpecialPageFactory {
                }
 
                if ( $subpage !== false && !is_null( $subpage ) ) {
+                       // Make sure it's in dbkey form
+                       $subpage = str_replace( ' ', '_', $subpage );
                        $name = "$name/$subpage";
                }
 
index 2ebc9ed..395ce37 100644 (file)
@@ -11,7 +11,7 @@
  *     $gen = new AutoloadGenerator( __DIR__ );
  *     $gen->readDir( __DIR__ . '/includes' );
  *     $gen->readFile( __DIR__ . '/foo.php' )
- *     $gen->generateAutoload();
+ *     $gen->getAutoload();
  */
 class AutoloadGenerator {
        const FILETYPE_JSON = 'json';
index bb7eb79..bc7542a 100644 (file)
@@ -76,9 +76,6 @@ class FakeDatabaseMysqlBase extends DatabaseMysqlBase {
        protected function mysqlFetchField( $res, $n ) {
        }
 
-       protected function mysqlPing() {
-       }
-
        protected function mysqlRealEscapeString( $s ) {
 
        }