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