* Revert revert r41234 of ES-related changes. The site_stats complaint should be...
authorTim Starling <tstarling@users.mediawiki.org>
Sun, 28 Sep 2008 01:42:55 +0000 (01:42 +0000)
committerTim Starling <tstarling@users.mediawiki.org>
Sun, 28 Sep 2008 01:42:55 +0000 (01:42 +0000)
* Fix broken recursion guard in LoadBalancer::reportConnectionError(), which was causing getConnection() to return false on the second and subsequent errors, instead of throwing an exception. Revert incorrect fix r41229/r41230.

includes/ExternalStoreDB.php
includes/Revision.php
includes/db/Database.php
includes/db/LoadBalancer.php

index f2e80c9..9fa7d1b 100644 (file)
@@ -123,9 +123,6 @@ class ExternalStoreDB {
         */
        function store( $cluster, $data ) {
                $dbw = $this->getMaster( $cluster );
-               if( !$dbw ) {
-                       return false;
-               }
                $id = $dbw->nextSequenceValue( 'blob_blob_id_seq' );
                $dbw->insert( $this->getTable( $dbw ), 
                        array( 'blob_id' => $id, 'blob_text' => $data ), 
index aa77bf9..857c50f 100644 (file)
@@ -769,10 +769,13 @@ class Revision {
                $flags = Revision::compressRevisionText( $data );
 
                # Write to external storage if required
-               if ( $wgDefaultExternalStore ) {
+               if( $wgDefaultExternalStore ) {
                        // Store and get the URL
                        $data = ExternalStore::insertToDefault( $data );
-                       if ( $flags ) {
+                       if( !$data ) {
+                               throw new MWException( "Unable to store text to external storage" );
+                       }
+                       if( $flags ) {
                                $flags .= ',';
                        }
                        $flags .= 'external';
index 272dd9c..1fa1707 100644 (file)
@@ -343,13 +343,17 @@ class Database {
 
                wfProfileIn("dbconnect-$server");
 
-               # Try to connect up to three times
                # The kernel's default SYN retransmission period is far too slow for us,
-               # so we use a short timeout plus a manual retry.
+               # so we use a short timeout plus a manual retry. Retrying means that a small
+               # but finite rate of SYN packet loss won't cause user-visible errors.
                $this->mConn = false;
-               $max = 3;
+               if ( ini_get( 'mysql.connect_timeout' ) <= 3 ) {
+                       $numAttempts = 2;
+               } else {
+                       $numAttempts = 1;
+               }
                $this->installErrorHandler();
-               for ( $i = 0; $i < $max && !$this->mConn; $i++ ) {
+               for ( $i = 0; $i < $numAttempts && !$this->mConn; $i++ ) {
                        if ( $i > 1 ) {
                                usleep( 1000 );
                        }
@@ -365,30 +369,28 @@ class Database {
                        }
                }
                $phpError = $this->restoreErrorHandler();
+               # Always log connection errors
                if ( !$this->mConn ) {
                        $error = $this->lastError();
                        if ( !$error ) {
                                $error = $phpError;
                        }
                        wfLogDBError( "Error connecting to {$this->mServer}: $error\n" );
+                       wfDebug( "DB connection error\n" );
+                       wfDebug( "Server: $server, User: $user, Password: " .
+                               substr( $password, 0, 3 ) . "..., error: " . mysql_error() . "\n" );
+                       $success = false;
                }
                
                wfProfileOut("dbconnect-$server");
 
-               if ( $dbName != '' ) {
-                       if ( $this->mConn !== false ) {
-                               $success = @/**/mysql_select_db( $dbName, $this->mConn );
-                               if ( !$success ) {
-                                       $error = "Error selecting database $dbName on server {$this->mServer} " .
-                                               "from client host {$wguname['nodename']}\n";
-                                       wfLogDBError(" Error selecting database $dbName on server {$this->mServer} \n");
-                                       wfDebug( $error );
-                               }
-                       } else {
-                               wfDebug( "DB connection error\n" );
-                               wfDebug( "Server: $server, User: $user, Password: " .
-                                       substr( $password, 0, 3 ) . "..., error: " . mysql_error() . "\n" );
-                               $success = false;
+               if ( $dbName != '' && $this->mConn !== false ) {
+                       $success = @/**/mysql_select_db( $dbName, $this->mConn );
+                       if ( !$success ) {
+                               $error = "Error selecting database $dbName on server {$this->mServer} " .
+                                       "from client host {$wguname['nodename']}\n";
+                               wfLogDBError(" Error selecting database $dbName on server {$this->mServer} \n");
+                               wfDebug( $error );
                        }
                } else {
                        # Delay USE query
@@ -2505,6 +2507,13 @@ border=\"0\" ALT=\"Google\"></A>
 
                $text = str_replace( '$1', $this->error, $noconnect );
 
+               /*
+               if ( $GLOBALS['wgShowExceptionDetails'] ) {
+                       $text .= '</p><p>Backtrace:</p><p>' . 
+                               nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . 
+                               "</p>\n";
+               }*/
+
                if($wgUseFileCache) {
                        if($wgTitle) {
                                $t =& $wgTitle;
index 93fedf7..16c4b31 100644 (file)
@@ -398,11 +398,20 @@ class LoadBalancer {
        /**
         * Get a connection by index
         * This is the main entry point for this class.
+        * @param int $i Database
+        * @param array $groups Query groups
+        * @param string $wiki Wiki ID
         */
        public function &getConnection( $i, $groups = array(), $wiki = false ) {
                global $wgDBtype;
                wfProfileIn( __METHOD__ );
 
+               if ( $i == DB_LAST ) {
+                       throw new MWException( 'Attempt to call ' . __METHOD__ . ' with deprecated server index DB_LAST' );
+               } elseif ( $i === null || $i === false ) {
+                       throw new MWException( 'Attempt to call ' . __METHOD__ . ' with invalid server index' );
+               }
+
                if ( $wiki === wfWikiID() ) {
                        $wiki = false;
                }
@@ -432,12 +441,12 @@ class LoadBalancer {
                # Operation-based index
                if ( $i == DB_SLAVE ) {
                        $i = $this->getReaderIndex( false, $wiki );
-               } elseif ( $i == DB_LAST ) {
-                       throw new MWException( 'Attempt to call ' . __METHOD__ . ' with deprecated server index DB_LAST' );
-               }
-               # Couldn't find a working server in getReaderIndex()?
-               if ( $i === false ) {
-                       $this->reportConnectionError( $this->mErrorConnection );
+                       # Couldn't find a working server in getReaderIndex()?
+                       if ( $i === false ) {
+                               $this->mLastError = 'No working slave server: ' . $this->mLastError;
+                               $this->reportConnectionError( $this->mErrorConnection );
+                               return false;
+                       }
                }
 
                # Now we have an explicit index into the servers array
@@ -518,7 +527,7 @@ class LoadBalancer {
                } else {
                        $server = $this->mServers[$i];
                        $server['serverIndex'] = $i;
-                       $conn = $this->reallyOpenConnection( $server );
+                       $conn = $this->reallyOpenConnection( $server, false );
                        if ( $conn->isOpen() ) {
                                $this->mConns['local'][$i][0] = $conn;
                        } else {
@@ -653,31 +662,27 @@ class LoadBalancer {
 
        function reportConnectionError( &$conn ) {
                wfProfileIn( __METHOD__ );
-               # Prevent infinite recursion
-
-               static $reporting = false;
-               if ( !$reporting ) {
-                       $reporting = true;
-                       if ( !is_object( $conn ) ) {
-                               // No last connection, probably due to all servers being too busy
-                               $conn = new Database;
-                               if ( $this->mFailFunction ) {
-                                       $conn->failFunction( $this->mFailFunction );
-                                       $conn->reportConnectionError( $this->mLastError );
-                               } else {
-                                       // If all servers were busy, mLastError will contain something sensible
-                                       throw new DBConnectionError( $conn, $this->mLastError );
-                               }
+
+               if ( !is_object( $conn ) ) {
+                       // No last connection, probably due to all servers being too busy
+                       wfLogDBError( "LB failure with no last connection\n" );
+                       $conn = new Database;
+                       if ( $this->mFailFunction ) {
+                               $conn->failFunction( $this->mFailFunction );
+                               $conn->reportConnectionError( $this->mLastError );
                        } else {
-                               if ( $this->mFailFunction ) {
-                                       $conn->failFunction( $this->mFailFunction );
-                               } else {
-                                       $conn->failFunction( false );
-                               }
-                               $server = $conn->getProperty( 'mServer' );
-                               $conn->reportConnectionError( "{$this->mLastError} ({$server})" );
+                               // If all servers were busy, mLastError will contain something sensible
+                               throw new DBConnectionError( $conn, $this->mLastError );
+                       }
+               } else {
+                       if ( $this->mFailFunction ) {
+                               $conn->failFunction( $this->mFailFunction );
+                       } else {
+                               $conn->failFunction( false );
                        }
-                       $reporting = false;
+                       $server = $conn->getProperty( 'mServer' );
+                       wfLogDBError( "Connection error: {$this->mLastError} ({$server})\n" );
+                       $conn->reportConnectionError( "{$this->mLastError} ({$server})" );
                }
                wfProfileOut( __METHOD__ );
        }