From c4bbaf94c0d9a406770c5a2b5099b0de9635e4d0 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 24 Sep 2008 18:09:22 +0000 Subject: [PATCH] Revert some recent ES-related changes -- they made behavior much worse when we encountered problems with site_stats updates hanging and stacking up extra open ES connections. r41230 r41229 r41093 r41091 r41092 r41086 r41063 r40696 Also reverted r41231 which no longer applies --- includes/ExternalStore.php | 47 ++---------------------------------- includes/ExternalStoreDB.php | 16 ++++++------ includes/Revision.php | 17 +++++++++---- includes/db/Database.php | 45 ++++++++++++++-------------------- includes/db/LoadBalancer.php | 25 ++++++------------- 5 files changed, 46 insertions(+), 104 deletions(-) diff --git a/includes/ExternalStore.php b/includes/ExternalStore.php index 272bcfc000..e2b7856673 100644 --- a/includes/ExternalStore.php +++ b/includes/ExternalStore.php @@ -14,7 +14,7 @@ */ class ExternalStore { /* Fetch data from given URL */ - static function fetchFromURL( $url ) { + static function fetchFromURL($url) { global $wgExternalStores; if( !$wgExternalStores ) @@ -44,7 +44,7 @@ class ExternalStore { $class = 'ExternalStore' . ucfirst( $proto ); /* Any custom modules should be added to $wgAutoLoadClasses for on-demand loading */ - if( !class_exists( $class ) ) { + if( !class_exists( $class ) ){ return false; } @@ -66,47 +66,4 @@ class ExternalStore { return $store->store( $params, $data ); } } - - /** - * Like insert() above, but does more of the work for us. - * This function does not need a url param, it builds it by - * itself. It also fails-over to the next possible clusters. - * - * @param string $data - * Returns the URL of the stored data item, or false on error - */ - public static function randomInsert( $data ) { - global $wgDefaultExternalStore; - $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" ); - } - try { - $url = $store->store( $params, $data ); // Try to save the object - } catch ( DBConnectionError $error ) { - $url = false; - } - if ( $url ) { - return $url; // Done! - } else { - unset( $tryStores[$index] ); // Don't try this one again! - $tryStores = array_values( $tryStores ); // Must have consecutive keys - wfDebugLog( 'ExternalStorage', "Unable to store text to external storage $storeUrl" ); - } - } - // 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 1cd54650ae..549412d18b 100644 --- a/includes/ExternalStoreDB.php +++ b/includes/ExternalStoreDB.php @@ -40,7 +40,7 @@ class ExternalStoreDB { /** @todo Document.*/ function &getMaster( $cluster ) { $lb =& $this->getLoadBalancer( $cluster ); - return $lb->getConnection( DB_MASTER, array() ); + return $lb->getConnection( DB_MASTER ); } /** @todo Document.*/ @@ -56,7 +56,7 @@ class ExternalStoreDB { * Fetch data from given URL * @param string $url An url of the form DB://cluster/id or DB://cluster/id/itemid for concatened storage. */ - function fetchFromURL( $url ) { + function fetchFromURL($url) { $path = explode( '/', $url ); $cluster = $path[2]; $id = $path[3]; @@ -122,14 +122,12 @@ class ExternalStoreDB { * @return string URL */ function store( $cluster, $data ) { - $dbw = $this->getMaster( $cluster ); - if( !$dbw ) { - return false; - } + $fname = 'ExternalStoreDB::store'; + + $dbw =& $this->getMaster( $cluster ); + $id = $dbw->nextSequenceValue( 'blob_blob_id_seq' ); - $dbw->insert( $this->getTable( $dbw ), - array( 'blob_id' => $id, 'blob_text' => $data ), - __METHOD__ ); + $dbw->insert( $this->getTable( $dbw ), array( 'blob_id' => $id, 'blob_text' => $data ), $fname ); $id = $dbw->insertId(); if ( $dbw->getFlag( DBO_TRX ) ) { $dbw->immediateCommit(); diff --git a/includes/Revision.php b/includes/Revision.php index 2661149bbf..b1d293ee4a 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -761,13 +761,20 @@ class Revision { $flags = Revision::compressRevisionText( $data ); # Write to external storage if required - if( $wgDefaultExternalStore ) { + if ( $wgDefaultExternalStore ) { + if ( is_array( $wgDefaultExternalStore ) ) { + // Distribute storage across multiple clusters + $store = $wgDefaultExternalStore[mt_rand(0, count( $wgDefaultExternalStore ) - 1)]; + } else { + $store = $wgDefaultExternalStore; + } // Store and get the URL - $data = ExternalStore::randomInsert( $data ); - if( !$data ) { - throw new MWException( "Unable to store text to external storage" ); + $data = ExternalStore::insert( $store, $data ); + if ( !$data ) { + # This should only happen in the case of a configuration error, where the external store is not valid + throw new MWException( "Unable to store text to external storage $store" ); } - if( $flags ) { + if ( $flags ) { $flags .= ','; } $flags .= 'external'; diff --git a/includes/db/Database.php b/includes/db/Database.php index 1fa170752d..272dd9c26b 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -343,17 +343,13 @@ 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. Retrying means that a small - # but finite rate of SYN packet loss won't cause user-visible errors. + # so we use a short timeout plus a manual retry. $this->mConn = false; - if ( ini_get( 'mysql.connect_timeout' ) <= 3 ) { - $numAttempts = 2; - } else { - $numAttempts = 1; - } + $max = 3; $this->installErrorHandler(); - for ( $i = 0; $i < $numAttempts && !$this->mConn; $i++ ) { + for ( $i = 0; $i < $max && !$this->mConn; $i++ ) { if ( $i > 1 ) { usleep( 1000 ); } @@ -369,28 +365,30 @@ 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 != '' && $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 ); + 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; } } else { # Delay USE query @@ -2507,13 +2505,6 @@ 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 d92ded838d..93fedf70d1 100644 --- a/includes/db/LoadBalancer.php +++ b/includes/db/LoadBalancer.php @@ -398,20 +398,11 @@ 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; } @@ -441,12 +432,12 @@ class LoadBalancer { # Operation-based index if ( $i == DB_SLAVE ) { $i = $this->getReaderIndex( false, $wiki ); - # 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; - } + } 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 ); } # Now we have an explicit index into the servers array @@ -527,7 +518,7 @@ class LoadBalancer { } else { $server = $this->mServers[$i]; $server['serverIndex'] = $i; - $conn = $this->reallyOpenConnection( $server, false ); + $conn = $this->reallyOpenConnection( $server ); if ( $conn->isOpen() ) { $this->mConns['local'][$i][0] = $conn; } else { @@ -669,7 +660,6 @@ class LoadBalancer { $reporting = true; 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 ); @@ -685,7 +675,6 @@ class LoadBalancer { $conn->failFunction( false ); } $server = $conn->getProperty( 'mServer' ); - wfLogDBError( "Connection error: {$this->mLastError} ({$server})\n" ); $conn->reportConnectionError( "{$this->mLastError} ({$server})" ); } $reporting = false; -- 2.20.1