From cd8378a5a6b90305c485d0d9d1b668c2db63632a Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Sun, 21 Sep 2008 06:42:46 +0000 Subject: [PATCH] Revert/rewrite of r40696. * We used to have parameters to ignore errors, but they're obsolete now that we have exceptions. Implemented ES master failover using exceptions instead. * Changing the number of DB connection attempts from 3 to 2 for some random getConnection() calls is almost pointless, adds lots of ugly formal parameters all of the place, and misses the big picture. It should be 2 by default, based on the original rationale. Any reasonable implementation of failover should have zero timeouts per request, by storing state. Changed the default to 2, or 1 if a long timeout is set. --- includes/ExternalStore.php | 42 ++++++++++++++++++++---------------- includes/ExternalStoreDB.php | 15 ++++++------- includes/db/Database.php | 28 ++++++++++++++++-------- includes/db/LoadBalancer.php | 28 +++++++++--------------- 4 files changed, 59 insertions(+), 54 deletions(-) diff --git a/includes/ExternalStore.php b/includes/ExternalStore.php index e73b9a1bd6..34570e2df2 100644 --- a/includes/ExternalStore.php +++ b/includes/ExternalStore.php @@ -77,30 +77,36 @@ class ExternalStore { */ public static function randomInsert( $data ) { global $wgDefaultExternalStore; - $tryStorages = (array)$wgDefaultExternalStore; - // Do not wait and do second retry per master if we - // have other active cluster masters to try instead. - $retry = count($tryStorages) > 1 ? false : true; - while ( count($tryStorages) > 0 ) { - $index = mt_rand(0, count( $tryStorages ) - 1); - $storeUrl = $tryStorages[$index]; + $tryStores = (array)$wgDefaultExternalStore; + $error = false; + while ( count( $tryStores ) > 0 ) { + $index = mt_rand(0, count( $tryStores ) - 1); + $storeUrl = $tryStores[$index]; + wfDebug( __METHOD__.": trying $storeUrl\n" ); list( $proto, $params ) = explode( '://', $storeUrl, 2 ); $store = self::getStoreObject( $proto ); if ( $store === false ) { throw new MWException( "Invalid external storage protocol - $storeUrl" ); - return false; + } + try { + $url = $store->store( $params, $data ); // Try to save the object + } catch ( DBConnectionError $error ) { + $url = false; + unset( $tryStores[$index] ); // Don't try this one again! + $tryStores = array_values( $tryStores ); // Must have consecutive keys + } + if ( $url ) { + return $url; // Done! } else { - $url = $store->store( $params, $data, $retry ); // Try to save the object - if ( $url ) { - return $url; // Done! - } else { - unset( $tryStorages[$index] ); // Don't try this one again! - sort( $tryStorages ); // Must have consecutive keys - wfDebugLog( 'ExternalStorage', "Unable to store text to external storage $storeUrl" ); - } + wfDebugLog( 'ExternalStorage', "Unable to store text to external storage $storeUrl" ); } } - throw new MWException( "Unable to store text to external storage" ); - return false; // All cluster masters dead :( + // All stores failed + if ( $error ) { + // Rethrow the last connection error + throw $error; + } else { + throw new MWException( "Unable to store text to external storage" ); + } } } diff --git a/includes/ExternalStoreDB.php b/includes/ExternalStoreDB.php index fb3853ae3b..caac3b1485 100644 --- a/includes/ExternalStoreDB.php +++ b/includes/ExternalStoreDB.php @@ -34,14 +34,13 @@ class ExternalStoreDB { /** @todo Document.*/ function &getSlave( $cluster ) { $lb =& $this->getLoadBalancer( $cluster ); - // Make only two connection attempts, since we still have the master to try - return $lb->getConnection( DB_SLAVE, array(), false, 2 ); + return $lb->getConnection( DB_SLAVE ); } /** @todo Document.*/ - function &getMaster( $cluster, $retry = true ) { + function &getMaster( $cluster ) { $lb =& $this->getLoadBalancer( $cluster ); - return $lb->getConnection( DB_MASTER, array(), false, false, LoadBalancer::GRACEFUL ); + return $lb->getConnection( DB_MASTER, array() ); } /** @todo Document.*/ @@ -120,13 +119,11 @@ class ExternalStoreDB { * * @param $cluster String: the cluster name * @param $data String: the data item - * @param $retry bool: allows an extra DB connection retry after 1 second * @return string URL */ - function store( $cluster, $data, $retry = true ) { - if( !$dbw = $this->getMaster( $cluster, $retry ) ) { - return false; // failed, maybe another cluster is up... - } + function store( $cluster, $data ) { + $dbw = $this->getMaster( $cluster ); + $id = $dbw->nextSequenceValue( 'blob_blob_id_seq' ); $dbw->insert( $this->getTable( $dbw ), array( 'blob_id' => $id, 'blob_text' => $data ), diff --git a/includes/db/Database.php b/includes/db/Database.php index 476260dc32..83cf368fa6 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -257,11 +257,9 @@ class Database { * @param failFunction * @param $flags * @param $tablePrefix String: database table prefixes. By default use the prefix gave in LocalSettings.php - * @param int $max, max connection attempts - ** After the first retry (second attempt), each retry waits 1 second */ function __construct( $server = false, $user = false, $password = false, $dbName = false, - $failFunction = false, $flags = 0, $tablePrefix = 'get from global', $max = false ) { + $failFunction = false, $flags = 0, $tablePrefix = 'get from global' ) { global $wgOut, $wgDBprefix, $wgCommandLineMode; # Can't get a reference if it hasn't been set yet @@ -295,7 +293,7 @@ class Database { } if ( $server ) { - $this->open( $server, $user, $password, $dbName, $max ); + $this->open( $server, $user, $password, $dbName ); } } @@ -313,7 +311,7 @@ class Database { * Usually aborts on failure * If the failFunction is set to a non-zero integer, returns success */ - function open( $server, $user, $password, $dbName, $max = false ) { + function open( $server, $user, $password, $dbName ) { global $wguname, $wgAllDBsAreLocalhost; wfProfileIn( __METHOD__ ); @@ -345,13 +343,17 @@ class Database { wfProfileIn("dbconnect-$server"); - if( !$max ) { $max = 3; } - # Try to connect up to three times (by default) # 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; + 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 ); } @@ -367,6 +369,7 @@ class Database { } } $phpError = $this->restoreErrorHandler(); + # Always log connection errors if ( !$this->mConn ) { $error = $this->lastError(); if ( !$error ) { @@ -2507,6 +2510,13 @@ border=\"0\" ALT=\"Google\"> $text = str_replace( '$1', $this->error, $noconnect ); + global $wgShowExceptionDetails; + 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 733ee5ee38..cfea8b3597 100644 --- a/includes/db/LoadBalancer.php +++ b/includes/db/LoadBalancer.php @@ -11,8 +11,6 @@ * @ingroup Database */ class LoadBalancer { - const GRACEFUL = 1; - /* private */ var $mServers, $mConns, $mLoads, $mGroupLoads; /* private */ var $mFailFunction, $mErrorConnection; /* private */ var $mReadIndex, $mAllowLagged; @@ -172,7 +170,7 @@ class LoadBalancer { * * Side effect: opens connections to databases */ - function getReaderIndex( $group = false, $wiki = false, $attempts = false ) { + function getReaderIndex( $group = false, $wiki = false ) { global $wgReadOnly, $wgDBClusterTimeout, $wgDBAvgStatusPoll, $wgDBtype; # FIXME: For now, only go through all this for mysql databases @@ -249,7 +247,7 @@ class LoadBalancer { } wfDebugLog( 'connect', __METHOD__.": Using reader #$i: {$this->mServers[$i]['host']}...\n" ); - $conn = $this->openConnection( $i, $wiki, $attempts ); + $conn = $this->openConnection( $i, $wiki ); if ( !$conn ) { wfDebugLog( 'connect', __METHOD__.": Failed connecting to $i/$wiki\n" ); @@ -403,10 +401,8 @@ class LoadBalancer { * @param int $i Database * @param array $groups Query groups * @param string $wiki Wiki ID - * @param int $attempts Max DB connect attempts - * @param int $flags */ - public function &getConnection( $i, $groups = array(), $wiki = false, $attempts = false, $flags = 0 ) { + public function &getConnection( $i, $groups = array(), $wiki = false ) { global $wgDBtype; wfProfileIn( __METHOD__ ); @@ -438,21 +434,18 @@ class LoadBalancer { # Operation-based index if ( $i == DB_SLAVE ) { - $i = $this->getReaderIndex( false, $wiki, $attempts ); + $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 ) { - if( $flags && self::GRACEFUL ) { - return false; - } $this->reportConnectionError( $this->mErrorConnection ); } # Now we have an explicit index into the servers array - $conn = $this->openConnection( $i, $wiki, $attempts ); - if ( !$conn && !($flags & self::GRACEFUL) ) { + $conn = $this->openConnection( $i, $wiki ); + if ( !$conn ) { $this->reportConnectionError( $this->mErrorConnection ); } @@ -512,12 +505,11 @@ class LoadBalancer { * * @param integer $i Server index * @param string $wiki Wiki ID to open - * @param int $attempts Maximum connection attempts * @return Database * * @access private */ - function openConnection( $i, $wiki = false, $attempts = false ) { + function openConnection( $i, $wiki = false ) { wfProfileIn( __METHOD__ ); if ( $wiki !== false ) { $conn = $this->openForeignConnection( $i, $wiki ); @@ -529,7 +521,7 @@ class LoadBalancer { } else { $server = $this->mServers[$i]; $server['serverIndex'] = $i; - $conn = $this->reallyOpenConnection( $server, false, $attempts ); + $conn = $this->reallyOpenConnection( $server, false ); if ( $conn->isOpen() ) { $this->mConns['local'][$i][0] = $conn; } else { @@ -631,7 +623,7 @@ class LoadBalancer { * Returns a Database object whether or not the connection was successful. * @access private */ - function reallyOpenConnection( $server, $dbNameOverride = false, $attempts = false ) { + function reallyOpenConnection( $server, $dbNameOverride = false ) { if( !is_array( $server ) ) { throw new MWException( 'You must update your load-balancing configuration. See DefaultSettings.php entry for $wgDBservers.' ); } @@ -646,7 +638,7 @@ class LoadBalancer { # Create object wfDebug( "Connecting to $host $dbname...\n" ); - $db = new $class( $host, $user, $password, $dbname, 1, $flags, 'get from global', $attempts ); + $db = new $class( $host, $user, $password, $dbname, 1, $flags ); if ( $db->isOpen() ) { wfDebug( "Connected\n" ); } else { -- 2.20.1