Merge "(bug 32827) "[Regression] Block log for IP ranges not shown on Special:Block""
authorDemon <chadh@wikimedia.org>
Thu, 22 Mar 2012 12:31:48 +0000 (12:31 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 22 Mar 2012 12:31:48 +0000 (12:31 +0000)
includes/HTMLForm.php
includes/RecentChange.php
includes/db/Database.php
includes/filerepo/backend/FileBackendStore.php
includes/objectcache/MemcachedClient.php
languages/Language.php
languages/classes/LanguageKaa.php
maintenance/deleteDefaultMessages.php
maintenance/dumpTextPass.php
tests/phpunit/languages/LanguageTest.php

index 3b3e1b6..dccf967 100644 (file)
@@ -271,7 +271,7 @@ class HTMLForm extends ContextSource {
 
        /**
         * The here's-one-I-made-earlier option: do the submission if
-        * posted, or display the form with or without funky valiation
+        * posted, or display the form with or without funky validation
         * errors
         * @return Bool or Status whether submission was successful.
         */
@@ -279,7 +279,7 @@ class HTMLForm extends ContextSource {
                $this->prepareForm();
 
                $result = $this->tryAuthorizedSubmit();
-               if ( $result === true || ( $result instanceof Status && $result->isGood() ) ){
+               if ( $result === true || ( $result instanceof Status && $result->isGood() ) ) {
                        return $result;
                }
 
index a1097be..e57efae 100644 (file)
@@ -751,7 +751,7 @@ class RecentChange {
                return ChangesList::showCharacterDifference( $old, $new );
        }
 
-       public static function checkIPAddress( $ip ) {
+       private static function checkIPAddress( $ip ) {
                global $wgRequest;
                if ( $ip ) {
                        if ( !IP::isIPAddress( $ip ) ) {
index 47eb596..5c03617 100644 (file)
@@ -680,7 +680,7 @@ abstract class DatabaseBase implements DatabaseType {
                $dbType = strtolower( $dbType );
                $class = 'Database' . ucfirst( $dbType );
 
-               if( in_array( $dbType, $canonicalDBTypes ) ) {
+               if( in_array( $dbType, $canonicalDBTypes ) || ( class_exists( $class ) && is_subclass_of( $class, 'DatabaseBase' ) ) ) {
                        return new $class(
                                isset( $p['host'] ) ? $p['host'] : false,
                                isset( $p['user'] ) ? $p['user'] : false,
@@ -689,8 +689,6 @@ abstract class DatabaseBase implements DatabaseType {
                                isset( $p['flags'] ) ? $p['flags'] : 0,
                                isset( $p['tablePrefix'] ) ? $p['tablePrefix'] : 'get from global'
                        );
-               } elseif ( class_exists( $class ) && is_subclass_of( $class, 'DatabaseBase' ) ) {
-                       return new $class( $p );
                } else {
                        return null;
                }
index e96f257..403fc9c 100644 (file)
@@ -66,6 +66,7 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function createInternal( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                if ( strlen( $params['content'] ) > $this->maxFileSizeInternal() ) {
                        $status = Status::newFatal( 'backend-fail-maxsize',
                                $params['dst'], $this->maxFileSizeInternal() );
@@ -73,6 +74,7 @@ abstract class FileBackendStore extends FileBackend {
                        $status = $this->doCreateInternal( $params );
                        $this->clearCache( array( $params['dst'] ) );
                }
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -96,12 +98,14 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function storeInternal( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                if ( filesize( $params['src'] ) > $this->maxFileSizeInternal() ) {
                        $status = Status::newFatal( 'backend-fail-store', $params['dst'] );
                } else {
                        $status = $this->doStoreInternal( $params );
                        $this->clearCache( array( $params['dst'] ) );
                }
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -125,8 +129,10 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function copyInternal( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = $this->doCopyInternal( $params );
                $this->clearCache( array( $params['dst'] ) );
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -149,8 +155,10 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function deleteInternal( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = $this->doDeleteInternal( $params );
                $this->clearCache( array( $params['src'] ) );
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -174,8 +182,10 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function moveInternal( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = $this->doMoveInternal( $params );
                $this->clearCache( array( $params['src'], $params['dst'] ) );
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -201,6 +211,7 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function concatenate( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = Status::newGood();
 
                // Try to lock the source files for the scope of this function
@@ -210,6 +221,7 @@ abstract class FileBackendStore extends FileBackend {
                        $status->merge( $this->doConcatenate( $params ) );
                }
 
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -276,11 +288,13 @@ abstract class FileBackendStore extends FileBackend {
         */
        final protected function doPrepare( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
 
                $status = Status::newGood();
                list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] );
                if ( $dir === null ) {
                        $status->fatal( 'backend-fail-invalidpath', $params['dir'] );
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return $status; // invalid storage path
                }
@@ -295,6 +309,7 @@ abstract class FileBackendStore extends FileBackend {
                        }
                }
 
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -313,11 +328,13 @@ abstract class FileBackendStore extends FileBackend {
         */
        final protected function doSecure( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = Status::newGood();
 
                list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] );
                if ( $dir === null ) {
                        $status->fatal( 'backend-fail-invalidpath', $params['dir'] );
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return $status; // invalid storage path
                }
@@ -332,6 +349,7 @@ abstract class FileBackendStore extends FileBackend {
                        }
                }
 
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -350,11 +368,13 @@ abstract class FileBackendStore extends FileBackend {
         */
        final protected function doClean( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = Status::newGood();
 
                list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] );
                if ( $dir === null ) {
                        $status->fatal( 'backend-fail-invalidpath', $params['dir'] );
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return $status; // invalid storage path
                }
@@ -363,6 +383,7 @@ abstract class FileBackendStore extends FileBackend {
                $filesLockEx = array( $params['dir'] );
                $scopedLockE = $this->getScopedFileLocks( $filesLockEx, LockManager::LOCK_EX, $status );
                if ( !$status->isOK() ) {
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return $status; // abort
                }
@@ -377,6 +398,7 @@ abstract class FileBackendStore extends FileBackend {
                        }
                }
 
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -395,7 +417,9 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function fileExists( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $stat = $this->getFileStat( $params );
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return ( $stat === null ) ? null : (bool)$stat; // null => failure
        }
@@ -406,7 +430,9 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function getFileTimestamp( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $stat = $this->getFileStat( $params );
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $stat ? $stat['mtime'] : false;
        }
@@ -417,7 +443,9 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function getFileSize( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $stat = $this->getFileStat( $params );
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $stat ? $stat['size'] : false;
        }
@@ -428,8 +456,10 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function getFileStat( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $path = self::normalizeStoragePath( $params['src'] );
                if ( $path === null ) {
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return false; // invalid storage path
                }
@@ -438,18 +468,22 @@ abstract class FileBackendStore extends FileBackend {
                        // If we want the latest data, check that this cached
                        // value was in fact fetched with the latest available data.
                        if ( !$latest || $this->cache[$path]['stat']['latest'] ) {
+                               wfProfileOut( __METHOD__ . '-' . $this->name );
                                wfProfileOut( __METHOD__ );
                                return $this->cache[$path]['stat'];
                        }
                }
                wfProfileIn( __METHOD__ . '-miss' );
+               wfProfileIn( __METHOD__ . '-miss-' . $this->name );
                $stat = $this->doGetFileStat( $params );
+               wfProfileOut( __METHOD__ . '-miss-' . $this->name );
                wfProfileOut( __METHOD__ . '-miss' );
                if ( is_array( $stat ) ) { // don't cache negatives
                        $this->trimCache(); // limit memory
                        $this->cache[$path]['stat'] = $stat;
                        $this->cache[$path]['stat']['latest'] = $latest;
                }
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $stat;
        }
@@ -465,14 +499,17 @@ abstract class FileBackendStore extends FileBackend {
         */
        public function getFileContents( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $tmpFile = $this->getLocalReference( $params );
                if ( !$tmpFile ) {
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return false;
                }
                wfSuppressWarnings();
                $data = file_get_contents( $tmpFile->getPath() );
                wfRestoreWarnings();
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $data;
        }
@@ -483,18 +520,23 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function getFileSha1Base36( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $path = $params['src'];
                if ( isset( $this->cache[$path]['sha1'] ) ) {
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return $this->cache[$path]['sha1'];
                }
                wfProfileIn( __METHOD__ . '-miss' );
+               wfProfileIn( __METHOD__ . '-miss-' . $this->name );
                $hash = $this->doGetFileSha1Base36( $params );
+               wfProfileOut( __METHOD__ . '-miss-' . $this->name );
                wfProfileOut( __METHOD__ . '-miss' );
                if ( $hash ) { // don't cache negatives
                        $this->trimCache(); // limit memory
                        $this->cache[$path]['sha1'] = $hash;
                }
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $hash;
        }
@@ -518,8 +560,10 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function getFileProps( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $fsFile = $this->getLocalReference( $params );
                $props = $fsFile ? $fsFile->getProps() : FSFile::placeholderProps();
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $props;
        }
@@ -530,8 +574,10 @@ abstract class FileBackendStore extends FileBackend {
         */
        public function getLocalReference( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $path = $params['src'];
                if ( isset( $this->expensiveCache[$path]['localRef'] ) ) {
+                       wfProfileOut( __METHOD__ . '-' . $this->name );
                        wfProfileOut( __METHOD__ );
                        return $this->expensiveCache[$path]['localRef'];
                }
@@ -540,6 +586,7 @@ abstract class FileBackendStore extends FileBackend {
                        $this->trimExpensiveCache(); // limit memory
                        $this->expensiveCache[$path]['localRef'] = $tmpFile;
                }
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $tmpFile;
        }
@@ -550,6 +597,7 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function streamFile( array $params ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = Status::newGood();
 
                $info = $this->getFileStat( $params );
@@ -564,12 +612,15 @@ abstract class FileBackendStore extends FileBackend {
                        // do nothing; client cache is up to date
                } elseif ( $res == StreamFile::READY_STREAM ) {
                        wfProfileIn( __METHOD__ . '-send' );
+                       wfProfileIn( __METHOD__ . '-send-' . $this->name );
                        $status = $this->doStreamFile( $params );
+                       wfProfileOut( __METHOD__ . '-send-' . $this->name );
                        wfProfileOut( __METHOD__ . '-send' );
                } else {
                        $status->fatal( 'backend-fail-stream', $params['src'] );
                }
 
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -678,6 +729,7 @@ abstract class FileBackendStore extends FileBackend {
         */
        protected function doOperationsInternal( array $ops, array $opts ) {
                wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = Status::newGood();
 
                // Build up a list of FileOps...
@@ -699,6 +751,7 @@ abstract class FileBackendStore extends FileBackend {
                        $scopeLockS = $this->getScopedFileLocks( $filesLockSh, LockManager::LOCK_UW, $status );
                        $scopeLockE = $this->getScopedFileLocks( $filesLockEx, LockManager::LOCK_EX, $status );
                        if ( !$status->isOK() ) {
+                               wfProfileOut( __METHOD__ . '-' . $this->name );
                                wfProfileOut( __METHOD__ );
                                return $status; // abort
                        }
@@ -714,6 +767,7 @@ abstract class FileBackendStore extends FileBackend {
                $status->merge( $subStatus );
                $status->success = $subStatus->success; // not done in merge()
 
+               wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
        }
index f8208f6..4f49f7d 100644 (file)
@@ -344,11 +344,20 @@ class MWMemcached {
                return false;
        }
 
+       /**
+        * @param $key
+        * @param $timeout int
+        * @return bool
+        */
        public function lock( $key, $timeout = 0 ) {
                /* stub */
                return true;
        }
 
+       /**
+        * @param $key
+        * @return bool
+        */
        public function unlock( $key ) {
                /* stub */
                return true;
@@ -471,7 +480,7 @@ class MWMemcached {
                        $this->stats['get_multi'] = 1;
                }
                $sock_keys = array();
-
+               $socks = array();
                foreach ( $keys as $key ) {
                        $sock = $this->get_sock( $key );
                        if ( !is_resource( $sock ) ) {
@@ -485,6 +494,7 @@ class MWMemcached {
                        $sock_keys[$sock][] = $key;
                }
 
+               $gather = array();
                // Send out the requests
                foreach ( $socks as $sock ) {
                        $cmd = 'get';
@@ -579,6 +589,7 @@ class MWMemcached {
                        return array();
                }
 
+               $ret = array();
                while ( true ) {
                        $res = fgets( $sock );
                        $ret[] = $res;
@@ -744,6 +755,9 @@ class MWMemcached {
                $this->_dead_host( $host );
        }
 
+       /**
+        * @param $host
+        */
        function _dead_host( $host ) {
                $parts = explode( ':', $host );
                $ip = $parts[0];
@@ -774,8 +788,8 @@ class MWMemcached {
                }
 
                $hv = is_array( $key ) ? intval( $key[0] ) : $this->_hashfunc( $key );
-
                if ( $this->_buckets === null ) {
+                       $bu = array();
                        foreach ( $this->_servers as $v ) {
                                if ( is_array( $v ) ) {
                                        for( $i = 0; $i < $v[1]; $i++ ) {
@@ -851,7 +865,8 @@ class MWMemcached {
                        $this->stats[$cmd] = 1;
                }
                if ( !$this->_safe_fwrite( $sock, "$cmd $key $amt\r\n" ) ) {
-                       return $this->_dead_sock( $sock );
+                       $this->_dead_sock( $sock );
+                       return null;
                }
 
                $line = fgets( $sock );
@@ -998,7 +1013,8 @@ class MWMemcached {
                        }
                }
                if ( !$this->_safe_fwrite( $sock, "$cmd $key $flags $exp $len\r\n$val\r\n" ) ) {
-                       return $this->_dead_sock( $sock );
+                       $this->_dead_sock( $sock );
+                       return false;
                }
 
                $line = trim( fgets( $sock ) );
@@ -1038,7 +1054,8 @@ class MWMemcached {
                }
 
                if ( !$this->_connect_sock( $sock, $host ) ) {
-                       return $this->_dead_host( $host );
+                       $this->_dead_host( $host );
+                       return null;
                }
 
                // Do not buffer writes
@@ -1049,6 +1066,9 @@ class MWMemcached {
                return $this->_cache_sock[$host];
        }
 
+       /**
+        * @param $str string
+        */
        function _debugprint( $str ) {
                print( $str );
        }
@@ -1080,6 +1100,9 @@ class MWMemcached {
 
        /**
         * Original behaviour
+        * @param $f
+        * @param $buf
+        * @param $len bool
         * @return int
         */
        function _safe_fwrite( $f, $buf, $len = false ) {
@@ -1093,6 +1116,7 @@ class MWMemcached {
 
        /**
         * Flush the read buffer of a stream
+        * @param $f Resource
         */
        function _flush_read_buffer( $f ) {
                if ( !is_resource( $f ) ) {
index 854a9f1..d705b49 100644 (file)
@@ -3758,7 +3758,7 @@ class Language {
 
        /**
         * Decode an expiry (block, protection, etc) which has come from the DB
-        * 
+        *
         * @FIXME: why are we returnings DBMS-dependent strings???
         *
         * @param $expiry String: Database expiry String
index a40fb7a..22e8946 100644 (file)
@@ -41,11 +41,11 @@ class LanguageKaa extends Language {
        }
 
        /**
-        * It fixes issue with  lcfirst for transforming 'I' to 'ı'
+        * It fixes issue with lcfirst for transforming 'I' to 'ı'
         *
         * @param $string string
         *
-        * @return string
+        * @return mixed|string
         */
        function lcfirst ( $string ) {
                if ( substr( $string, 0, 1 ) === 'I' ) {
index 989f7bd..4f5889b 100644 (file)
@@ -74,7 +74,7 @@ class DeleteDefaultMessages extends Maintenance {
                        $dbw->commit( __METHOD__ );
                }
 
-               $this->output( 'done!\n', 'msg' );
+               $this->output( "done!\n", 'msg' );
        }
 }
 
index c03f3df..6796c75 100644 (file)
@@ -41,9 +41,7 @@ class TextPassDumper extends BackupDumper {
        var $prefetchCountLast = 0;
        var $fetchCountLast = 0;
 
-       var $failures = 0;
        var $maxFailures = 5;
-       var $failedTextRetrievals = 0;
        var $maxConsecutiveFailedTextRetrievals = 200;
        var $failureTimeout = 5; // Seconds to sleep after db failure
 
@@ -71,6 +69,52 @@ class TextPassDumper extends BackupDumper {
         */
        protected $db;
 
+
+       /**
+        * Drop the database connection $this->db and try to get a new one.
+        *
+        * This function tries to get a /different/ connection if this is
+        * possible. Hence, (if this is possible) it switches to a different
+        * failover upon each call.
+        *
+        * This function resets $this->lb and closes all connections on it.
+        *
+        * @throws MWException
+        */
+       function rotateDb() {
+               // Cleaning up old connections
+               if ( isset( $this->lb ) ) {
+                       $this->lb->closeAll();
+                       unset( $this->lb );
+               }
+
+               if ( isset( $this->db ) && $this->db->isOpen() ) {
+                       throw new MWException( 'DB is set and has not been closed by the Load Balancer' );
+               }
+
+               unset( $this->db );
+
+               // Trying to set up new connection.
+               // We do /not/ retry upon failure, but delegate to encapsulating logic, to avoid
+               // individually retrying at different layers of code.
+
+               // 1. The LoadBalancer.
+               try {
+                       $this->lb = wfGetLBFactory()->newMainLB();
+               } catch ( Exception $e ) {
+                       throw new MWException( __METHOD__ . " rotating DB failed to obtain new load balancer (" . $e->getMessage() . ")" );
+               }
+
+
+               // 2. The Connection, through the load balancer.
+               try {
+                       $this->db = $this->lb->getConnection( DB_SLAVE, 'backup' );
+               } catch ( Exception $e ) {
+                       throw new MWException( __METHOD__ . " rotating DB failed to obtain new database (" . $e->getMessage() . ")" );
+               }
+       }
+
+
        function initProgress( $history ) {
                parent::initProgress();
                $this->timeOfCheckpoint = $this->startTime;
@@ -87,7 +131,19 @@ class TextPassDumper extends BackupDumper {
 
                $this->initProgress( $this->history );
 
-               $this->db = $this->backupDb();
+               // We are trying to get an initial database connection to avoid that the
+               // first try of this request's first call to getText fails. However, if
+               // obtaining a good DB connection fails it's not a serious issue, as
+               // getText does retry upon failure and can start without having a working
+               // DB connection.
+               try {
+                       $this->rotateDb();
+               } catch ( Exception $e ) {
+                       // We do not even count this as failure. Just let eventual
+                       // watchdogs know.
+                       $this->progress( "Getting initial DB connection failed (" .
+                               $e->getMessage() . ")" );
+               }
 
                $this->egress = new ExportProgressFilter( $this->sink, $this );
 
@@ -124,7 +180,7 @@ class TextPassDumper extends BackupDumper {
                        $this->input = $url;
                        break;
                case 'maxtime':
-                       $this->maxTimeAllowed = intval($val)*60;
+                       $this->maxTimeAllowed = intval( $val ) * 60;
                        break;
                case 'checkpointfile':
                        $this->checkpointFiles[] = $val;
@@ -145,7 +201,7 @@ class TextPassDumper extends BackupDumper {
        }
 
        function processFileOpt( $val, $param ) {
-               $fileURIs = explode(';',$param);
+               $fileURIs = explode( ';', $param );
                foreach ( $fileURIs as $URI ) {
                        switch( $val ) {
                                case "file":
@@ -240,19 +296,19 @@ class TextPassDumper extends BackupDumper {
        function finalOptionCheck() {
                if ( ( $this->checkpointFiles && ! $this->maxTimeAllowed ) ||
                        ( $this->maxTimeAllowed && !$this->checkpointFiles ) ) {
-                       throw new MWException("Options checkpointfile and maxtime must be specified together.\n");
+                       throw new MWException( "Options checkpointfile and maxtime must be specified together.\n" );
                }
-               foreach ($this->checkpointFiles as $checkpointFile) {
-                       $count = substr_count ( $checkpointFile,"%s" );
+               foreach ( $this->checkpointFiles as $checkpointFile ) {
+                       $count = substr_count ( $checkpointFile, "%s" );
                        if ( $count != 2 ) {
-                               throw new MWException("Option checkpointfile must contain two '%s' for substitution of first and last pageids, count is $count instead, file is $checkpointFile.\n");
+                               throw new MWException( "Option checkpointfile must contain two '%s' for substitution of first and last pageids, count is $count instead, file is $checkpointFile.\n" );
                        }
                }
 
                if ( $this->checkpointFiles ) {
                        $filenameList = (array)$this->egress->getFilenames();
                        if ( count( $filenameList ) != count( $this->checkpointFiles ) ) {
-                               throw new MWException("One checkpointfile must be specified for each output option, if maxtime is used.\n");
+                               throw new MWException( "One checkpointfile must be specified for each output option, if maxtime is used.\n" );
                        }
                }
        }
@@ -275,7 +331,7 @@ class TextPassDumper extends BackupDumper {
                $offset = 0; // for context extraction on error reporting
                $bufferSize = 512 * 1024;
                do {
-                       if ($this->checkIfTimeExceeded()) {
+                       if ( $this->checkIfTimeExceeded() ) {
                                $this->setTimeExceeded();
                        }
                        $chunk = fread( $input, $bufferSize );
@@ -285,27 +341,27 @@ class TextPassDumper extends BackupDumper {
                        }
                        $offset += strlen( $chunk );
                } while ( $chunk !== false && !feof( $input ) );
-               if ($this->maxTimeAllowed) {
+               if ( $this->maxTimeAllowed ) {
                        $filenameList = (array)$this->egress->getFilenames();
                        // we wrote some stuff after last checkpoint that needs renamed
-                       if (file_exists($filenameList[0])) {
+                       if ( file_exists( $filenameList[0] ) ) {
                                $newFilenames = array();
                                # we might have just written the header and footer and had no
                                # pages or revisions written... perhaps they were all deleted
                                # there's no pageID 0 so we use that. the caller is responsible
                                # for deciding what to do with a file containing only the
                                # siteinfo information and the mw tags.
-                               if (! $this->firstPageWritten) {
-                                       $firstPageID = str_pad(0,9,"0",STR_PAD_LEFT);
-                                       $lastPageID = str_pad(0,9,"0",STR_PAD_LEFT);
+                               if ( ! $this->firstPageWritten ) {
+                                       $firstPageID = str_pad( 0, 9, "0", STR_PAD_LEFT );
+                                       $lastPageID = str_pad( 0, 9, "0", STR_PAD_LEFT );
                                }
                                else {
-                                       $firstPageID = str_pad($this->firstPageWritten,9,"0",STR_PAD_LEFT);
-                                       $lastPageID = str_pad($this->lastPageWritten,9,"0",STR_PAD_LEFT);
+                                       $firstPageID = str_pad( $this->firstPageWritten, 9, "0", STR_PAD_LEFT );
+                                       $lastPageID = str_pad( $this->lastPageWritten, 9, "0", STR_PAD_LEFT );
                                }
                                for ( $i = 0; $i < count( $filenameList ); $i++ ) {
                                        $checkpointNameFilledIn = sprintf( $this->checkpointFiles[$i], $firstPageID, $lastPageID );
-                                       $fileinfo = pathinfo($filenameList[$i]);
+                                       $fileinfo = pathinfo( $filenameList[$i] );
                                        $newFilenames[] = $fileinfo['dirname'] . '/' . $checkpointNameFilledIn;
                                }
                                $this->egress->closeAndRename( $newFilenames );
@@ -316,98 +372,142 @@ class TextPassDumper extends BackupDumper {
                return true;
        }
 
+       /**
+        * Tries to get the revision text for a revision id.
+        *
+        * Upon errors, retries (Up to $this->maxFailures tries each call).
+        * If still no good revision get could be found even after this retrying, "" is returned.
+        * If no good revision text could be returned for
+        * $this->maxConsecutiveFailedTextRetrievals consecutive calls to getText, MWException
+        * is thrown.
+        *
+        * @param $id string The revision id to get the text for
+        *
+        * @return string The revision text for $id, or ""
+        * @throws MWException
+        */
        function getText( $id ) {
+               $prefetchNotTried = true; // Whether or not we already tried to get the text via prefetch.
+               $text = false; // The candidate for a good text. false if no proper value.
+               $failures = 0; // The number of times, this invocation of getText already failed.
+
+               static $consecutiveFailedTextRetrievals = 0; // The number of times getText failed without
+                                                            // yielding a good text in between.
+
                $this->fetchCount++;
-               if ( isset( $this->prefetch ) ) {
-                       $text = $this->prefetch->prefetch( $this->thisPage, $this->thisRev );
-                       if ( $text !== null ) { // Entry missing from prefetch dump
-                               $dbr = wfGetDB( DB_SLAVE );
+
+               // To allow to simply return on success and do not have to worry about book keeping,
+               // we assume, this fetch works (possible after some retries). Nevertheless, we koop
+               // the old value, so we can restore it, if problems occur (See after the while loop).
+               $oldConsecutiveFailedTextRetrievals = $consecutiveFailedTextRetrievals;
+               $consecutiveFailedTextRetrievals = 0;
+
+               while ( $failures < $this->maxFailures ) {
+
+                       // As soon as we found a good text for the $id, we will return immediately.
+                       // Hence, if we make it past the try catch block, we know that we did not
+                       // find a good text.
+
+                       try {
+                               // Step 1: Get some text (or reuse from previous iteratuon if checking
+                               //         for plausibility failed)
+
+                               // Trying to get prefetch, if it has not been tried before
+                               if ( $text === false && isset( $this->prefetch ) && $prefetchNotTried ) {
+                                       $prefetchNotTried = false;
+                                       $tryIsPrefetch = true;
+                                       $text = $this->prefetch->prefetch( $this->thisPage, $this->thisRev );
+                                       if ( $text === null ) {
+                                               $text = false;
+                                       }
+                               }
+
+                               if ( $text === false ) {
+                                       // Fallback to asking the database
+                                       $tryIsPrefetch = false;
+                                       if ( $this->spawn ) {
+                                               $text = $this->getTextSpawned( $id );
+                                       } else {
+                                               $text = $this->getTextDb( $id );
+                                       }
+                               }
+
+                               if ( $text === false ) {
+                                       throw new MWException( "Generic error while obtaining text for id " . $id );
+                               }
+
+                               // We received a good candidate for the text of $id via some method
+
+                               // Step 2: Checking for plausibility and return the text if it is
+                               //         plausible
                                $revID = intval( $this->thisRev );
-                               $revLength = $dbr->selectField( 'revision', 'rev_len', array( 'rev_id' => $revID ) );
-                               // if length of rev text in file doesn't match length in db, we reload
-                               // this avoids carrying forward broken data from previous xml dumps
-                               if( strlen( $text ) == $revLength ) {
-                                       $this->prefetchCount++;
+                               if ( ! isset( $this->db ) ) {
+                                       throw new MWException( "No database available" );
+                               }
+                               $revLength = $this->db->selectField( 'revision', 'rev_len', array( 'rev_id' => $revID ) );
+                               if ( strlen( $text ) == $revLength ) {
+                                       if ( $tryIsPrefetch ) {
+                                               $this->prefetchCount++;
+                                       }
                                        return $text;
                                }
+
+                               $text = false;
+                               throw new MWException( "Received text is unplausible for id " . $id );
+
+                       } catch ( Exception $e ) {
+                               $msg = "getting/checking text " . $id . " failed (" . $e->getMessage() . ")";
+                               if ( $failures + 1 < $this->maxFailures ) {
+                                       $msg .= " (Will retry " . ( $this->maxFailures - $failures - 1 ) . " more times)";
+                               }
+                               $this->progress( $msg );
                        }
-               }
-               return $this->doGetText( $id );
-       }
 
-       private function doGetText( $id ) {
-               $id = intval( $id );
-               $this->failures = 0;
-               $ex = new MWException( "Graceful storage failure" );
-               while (true) {
-                       if ( $this->spawn ) {
-                               if ($this->failures) {
-                                       // we don't know why it failed, could be the child process
-                                       // borked, could be db entry busted, could be db server out to lunch,
-                                       // so cover all bases
+                       // Something went wrong; we did not a text that was plausible :(
+                       $failures++;
+
+
+                       // After backing off for some time, we try to reboot the whole process as
+                       // much as possible to not carry over failures from one part to the other
+                       // parts
+                       sleep( $this->failureTimeout );
+                       try {
+                               $this->rotateDb();
+                               if ( $this->spawn ) {
                                        $this->closeSpawn();
                                        $this->openSpawn();
                                }
-                               $text = $this->getTextSpawned( $id );
-                       } else {
-                               $text = $this->getTextDbSafe( $id );
-                       }
-                       if ( $text === false ) {
-                               $this->failures++;
-                               if ( $this->failures > $this->maxFailures) {
-                                       $this->progress( "Failed to retrieve revision text for text id ".
-                                                                        "$id after $this->maxFailures tries, giving up" );
-                                       // were there so many bad retrievals in a row we want to bail?
-                                       // at some point we have to declare the dump irretrievably broken
-                                       $this->failedTextRetrievals++;
-                                       if ($this->failedTextRetrievals > $this->maxConsecutiveFailedTextRetrievals) {
-                                               throw $ex;
-                                       } else {
-                                               // would be nice to return something better to the caller someday,
-                                               // log what we know about the failure and about the revision
-                                               return "";
-                                       }
-                               } else {
-                                       $this->progress( "Error $this->failures " .
-                                                                "of allowed $this->maxFailures retrieving revision text for text id $id! " .
-                                                                "Pausing $this->failureTimeout seconds before retry..." );
-                                       sleep( $this->failureTimeout );
-                               }
-                       } else {
-                               $this->failedTextRetrievals= 0;
-                               return $text;
+                       } catch ( Exception $e ) {
+                               $this->progress( "Rebooting getText infrastructure failed (" . $e->getMessage() . ")" .
+                                       " Trying to continue anyways" );
                        }
                }
-               return '';
-       }
 
-       /**
-        * Fetch a text revision from the database, retrying in case of failure.
-        * This may survive some transitory errors by reconnecting, but
-        * may not survive a long-term server outage.
-        *
-        * FIXME: WTF? Why is it using a loop and then returning unconditionally?
-        * @param $id int
-        * @return bool|string
-        */
-       private function getTextDbSafe( $id ) {
-               while ( true ) {
-                       try {
-                               $text = $this->getTextDb( $id );
-                       } catch ( DBQueryError $ex ) {
-                               $text = false;
-                       }
-                       return $text;
+               // Retirieving a good text for $id failed (at least) maxFailures times.
+               // We abort for this $id.
+
+               // Restoring the consecutive failures, and maybe aborting, if the dump
+               // is too broken.
+               $consecutiveFailedTextRetrievals = $oldConsecutiveFailedTextRetrievals + 1;
+               if ( $consecutiveFailedTextRetrievals > $this->maxConsecutiveFailedTextRetrievals ) {
+                       throw new MWException( "Graceful storage failure" );
                }
+
+               return "";
        }
 
+
        /**
         * May throw a database error if, say, the server dies during query.
         * @param $id
         * @return bool|string
+        * @throws MWException
         */
        private function getTextDb( $id ) {
                global $wgContLang;
+               if ( ! isset( $this->db ) ) {
+                       throw new MWException( __METHOD__ . "No database available" );
+               }
                $row = $this->db->selectRow( 'text',
                        array( 'old_text', 'old_flags' ),
                        array( 'old_id' => $id ),
@@ -517,7 +617,7 @@ class TextPassDumper extends BackupDumper {
 
                $nbytes = intval( $len );
                // actual error, not zero-length text
-               if ($nbytes < 0 ) return false;
+               if ( $nbytes < 0 ) return false;
 
                $text = "";
 
@@ -584,15 +684,15 @@ class TextPassDumper extends BackupDumper {
                        $this->buffer = "";
                        $this->thisRev = "";
                } elseif ( $name == 'page' ) {
-                       if (! $this->firstPageWritten) {
-                               $this->firstPageWritten = trim($this->thisPage);
+                       if ( ! $this->firstPageWritten ) {
+                               $this->firstPageWritten = trim( $this->thisPage );
                        }
-                       $this->lastPageWritten = trim($this->thisPage);
-                       if ($this->timeExceeded) {
+                       $this->lastPageWritten = trim( $this->thisPage );
+                       if ( $this->timeExceeded ) {
                                $this->egress->writeClosePage( $this->buffer );
                                // nasty hack, we can't just write the chardata after the
                                // page tag, it will include leading blanks from the next line
-                               $this->egress->sink->write("\n");
+                               $this->egress->sink->write( "\n" );
 
                                $this->buffer = $this->xmlwriterobj->closeStream();
                                $this->egress->writeCloseStream( $this->buffer );
@@ -603,11 +703,11 @@ class TextPassDumper extends BackupDumper {
 
                                $filenameList = (array)$this->egress->getFilenames();
                                $newFilenames = array();
-                               $firstPageID = str_pad($this->firstPageWritten,9,"0",STR_PAD_LEFT);
-                               $lastPageID = str_pad($this->lastPageWritten,9,"0",STR_PAD_LEFT);
+                               $firstPageID = str_pad( $this->firstPageWritten, 9, "0", STR_PAD_LEFT );
+                               $lastPageID = str_pad( $this->lastPageWritten, 9, "0", STR_PAD_LEFT );
                                for ( $i = 0; $i < count( $filenameList ); $i++ ) {
                                        $checkpointNameFilledIn = sprintf( $this->checkpointFiles[$i], $firstPageID, $lastPageID );
-                                       $fileinfo = pathinfo($filenameList[$i]);
+                                       $fileinfo = pathinfo( $filenameList[$i] );
                                        $newFilenames[] = $fileinfo['dirname'] . '/' . $checkpointNameFilledIn;
                                }
                                $this->egress->closeRenameAndReopen( $newFilenames );
@@ -640,9 +740,9 @@ class TextPassDumper extends BackupDumper {
                }
                // have to skip the newline left over from closepagetag line of
                // end of checkpoint files. nasty hack!!
-               if ($this->checkpointJustWritten) {
-                       if ($data[0] == "\n") {
-                               $data = substr($data,1);
+               if ( $this->checkpointJustWritten ) {
+                       if ( $data[0] == "\n" ) {
+                               $data = substr( $data, 1 );
                        }
                        $this->checkpointJustWritten = false;
                }
index c83e01e..c089c31 100644 (file)
@@ -23,12 +23,12 @@ class LanguageTest extends MediaWikiTestCase {
                        'convertDoubleWidth() with the full alphabet and digits'
                );
        }
-       
+
        /** @dataProvider provideFormattableTimes */
        function testFormatTimePeriod( $seconds, $format, $expected, $desc ) {
                $this->assertEquals( $expected, $this->lang->formatTimePeriod( $seconds, $format ), $desc );
        }
-       
+
        function provideFormattableTimes() {
                return array(
                        array( 9.45, array(), '9.5s', 'formatTimePeriod() rounding (<10s)' ),
@@ -62,7 +62,7 @@ class LanguageTest extends MediaWikiTestCase {
                        array( 176460.55, array(), '2d 1h 1m 1s', 'formatTimePeriod() rounding, recursion, (>48h)' ),
                        array( 176460.55, array( 'noabbrevs' => true ), '2 days 1 hour 1 minute 1 second', 'formatTimePeriod() rounding, recursion, (>48h)' ),
                );
-               
+
        }
 
        function testTruncate() {
@@ -224,7 +224,7 @@ class LanguageTest extends MediaWikiTestCase {
                        "sprintfDate('$format', '$ts'): $msg"
                );
 
-               date_default_timezone_set( $oldTZ );            
+               date_default_timezone_set( $oldTZ );
        }
 
        function provideSprintfDateSamples() {