From f1a352ccf9424f4d5cba65f49903c799aa36d08a Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Sun, 28 Sep 2008 01:42:55 +0000 Subject: [PATCH] * Revert revert r41234 of ES-related changes. The site_stats complaint should be fixed by the transaction added in r41287. * 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 | 3 -- includes/Revision.php | 7 ++-- includes/db/Database.php | 45 +++++++++++++++---------- includes/db/LoadBalancer.php | 65 +++++++++++++++++++----------------- 4 files changed, 67 insertions(+), 53 deletions(-) diff --git a/includes/ExternalStoreDB.php b/includes/ExternalStoreDB.php index f2e80c98fe..9fa7d1b170 100644 --- a/includes/ExternalStoreDB.php +++ b/includes/ExternalStoreDB.php @@ -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 ), diff --git a/includes/Revision.php b/includes/Revision.php index aa77bf963d..857c50f6e9 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -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'; diff --git a/includes/db/Database.php b/includes/db/Database.php index 272dd9c26b..1fa170752d 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -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\"> $text = str_replace( '$1', $this->error, $noconnect ); + /* + if ( $GLOBALS['wgShowExceptionDetails'] ) { + $text .= '

Backtrace:

' . + nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . + "

\n"; + }*/ + if($wgUseFileCache) { if($wgTitle) { $t =& $wgTitle; diff --git a/includes/db/LoadBalancer.php b/includes/db/LoadBalancer.php index 93fedf70d1..16c4b316fb 100644 --- a/includes/db/LoadBalancer.php +++ b/includes/db/LoadBalancer.php @@ -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__ ); } -- 2.20.1