From: Aaron Schulz Date: Tue, 16 Jul 2019 06:06:13 +0000 (-0700) Subject: rdbms: refactor caching in LoadBalancer::getReadOnlyReason() X-Git-Tag: 1.34.0-rc.0~585^2 X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=c06bda1bed7db0312b71056ec666c00adeb45290;p=lhc%2Fweb%2Fwiklou.git rdbms: refactor caching in LoadBalancer::getReadOnlyReason() Avoid nesting of the same getWithSetCallback() cache updates. Also favor accuracy over initial cache use for the case where there is already a master connection. Add missing "lockTSE" flag to protect against stampedes updating stale values. Change serverIsReadOnly() to use SELECT in mysql instead of SHOW to avoid internal temporary tables. Bug: T227838 Change-Id: I2b0d680c9c3bdc7aaa1d1e1d6beb2dd203a815f1 --- diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index ac8c7c3105..851a178eb4 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php @@ -1056,11 +1056,12 @@ abstract class DatabaseMysqlBase extends Database { } public function serverIsReadOnly() { - $flags = self::QUERY_IGNORE_DBO_TRX; - $res = $this->query( "SHOW GLOBAL VARIABLES LIKE 'read_only'", __METHOD__, $flags ); + // Avoid SHOW to avoid internal temporary tables + $flags = self::QUERY_IGNORE_DBO_TRX | self::QUERY_SILENCE_ERRORS; + $res = $this->query( "SELECT @@GLOBAL.read_only AS Value", __METHOD__, $flags ); $row = $this->fetchObject( $res ); - return $row ? ( strtolower( $row->Value ) === 'on' ) : false; + return $row ? (bool)$row->Value : false; } /** diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index b21da72b49..160d501298 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -95,6 +95,8 @@ interface ILoadBalancer { const CONN_SILENCE_ERRORS = 2; /** @var int Caller is requesting the master DB server for possibly writes */ const CONN_INTENT_WRITABLE = 4; + /** @var int Bypass and update any server-side read-only mode state cache */ + const CONN_REFRESH_READ_ONLY = 8; /** @var string Manager of ILoadBalancer instances is running post-commit callbacks */ const STAGE_POSTCOMMIT_CALLBACKS = 'stage-postcommit-callbacks'; @@ -661,10 +663,9 @@ interface ILoadBalancer { /** * @note This method may trigger a DB connection if not yet done * @param string|bool $domain DB domain ID or false for the local domain - * @param IDatabase|null $conn DB master connection; used to avoid loops [optional] * @return string|bool Reason the master is read-only or false if it is not */ - public function getReadOnlyReason( $domain = false, IDatabase $conn = null ); + public function getReadOnlyReason( $domain = false ); /** * Disables/enables lag checks diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index b890df6c0a..5c172d653d 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -892,7 +892,11 @@ class LoadBalancer implements ILoadBalancer { // Get an open connection to that server (might trigger a new connection) $conn = $this->getServerConnection( $serverIndex, $domain, $flags ); // Set master DB handles as read-only if there is high replication lag - if ( $serverIndex === $this->getWriterIndex() && $this->getLaggedReplicaMode( $domain ) ) { + if ( + $serverIndex === $this->getWriterIndex() && + $this->getLaggedReplicaMode( $domain ) && + !is_string( $conn->getLBInfo( 'readOnlyReason' ) ) + ) { $reason = ( $this->getExistingReaderIndex( self::GROUP_GENERIC ) >= 0 ) ? 'The database is read-only until replication lag decreases.' : 'The database is read-only until replica database servers becomes reachable.'; @@ -948,15 +952,15 @@ class LoadBalancer implements ILoadBalancer { // or the master database server is running in server-side read-only mode. Note that // replica DB handles are always read-only via Database::assertIsWritableMaster(). // Read-only mode due to replication lag is *avoided* here to avoid recursion. - if ( $conn->getLBInfo( 'serverIndex' ) === $this->getWriterIndex() ) { + if ( $i === $this->getWriterIndex() ) { if ( $this->readOnlyReason !== false ) { - $conn->setLBInfo( 'readOnlyReason', $this->readOnlyReason ); - } elseif ( $this->masterRunningReadOnly( $domain, $conn ) ) { - $conn->setLBInfo( - 'readOnlyReason', - 'The master database server is running in read-only mode.' - ); + $readOnlyReason = $this->readOnlyReason; + } elseif ( $this->isMasterConnectionReadOnly( $conn, $flags ) ) { + $readOnlyReason = 'The master database server is running in read-only mode.'; + } else { + $readOnlyReason = false; } + $conn->setLBInfo( 'readOnlyReason', $readOnlyReason ); } return $conn; @@ -1938,10 +1942,12 @@ class LoadBalancer implements ILoadBalancer { return $this->laggedReplicaMode; } - public function getReadOnlyReason( $domain = false, IDatabase $conn = null ) { + public function getReadOnlyReason( $domain = false ) { + $domainInstance = DatabaseDomain::newFromId( $this->resolveDomainID( $domain ) ); + if ( $this->readOnlyReason !== false ) { return $this->readOnlyReason; - } elseif ( $this->masterRunningReadOnly( $domain, $conn ) ) { + } elseif ( $this->isMasterRunningReadOnly( $domainInstance ) ) { return 'The master database server is running in read-only mode.'; } elseif ( $this->getLaggedReplicaMode( $domain ) ) { return ( $this->getExistingReaderIndex( self::GROUP_GENERIC ) >= 0 ) @@ -1953,26 +1959,68 @@ class LoadBalancer implements ILoadBalancer { } /** - * @param string $domain Domain ID, or false for the current domain - * @param IDatabase|null $conn DB master connectionl used to avoid loops [optional] - * @return bool + * @param IDatabase $conn Master connection + * @param int $flags Bitfield of class CONN_* constants + * @return bool Whether the entire server or currently selected DB/schema is read-only */ - private function masterRunningReadOnly( $domain, IDatabase $conn = null ) { - $cache = $this->wanCache; - $masterServer = $this->getServerName( $this->getWriterIndex() ); + private function isMasterConnectionReadOnly( IDatabase $conn, $flags = 0 ) { + // Note that table prefixes are not related to server-side read-only mode + $key = $this->srvCache->makeGlobalKey( + 'rdbms-server-readonly', + $conn->getServer(), + $conn->getDBname(), + $conn->dbSchema() + ); + + if ( ( $flags & self::CONN_REFRESH_READ_ONLY ) == self::CONN_REFRESH_READ_ONLY ) { + try { + $readOnly = (int)$conn->serverIsReadOnly(); + } catch ( DBError $e ) { + $readOnly = 0; + } + $this->srvCache->set( $key, $readOnly, BagOStuff::TTL_PROC_SHORT ); + } else { + $readOnly = $this->srvCache->getWithSetCallback( + $key, + BagOStuff::TTL_PROC_SHORT, + function () use ( $conn ) { + try { + return (int)$conn->serverIsReadOnly(); + } catch ( DBError $e ) { + return 0; + } + } + ); + } - return (bool)$cache->getWithSetCallback( - $cache->makeGlobalKey( __CLASS__, 'server-read-only', $masterServer ), + return (bool)$readOnly; + } + + /** + * @param DatabaseDomain $domain + * @return bool Whether the entire master server or the local domain DB is read-only + */ + private function isMasterRunningReadOnly( DatabaseDomain $domain ) { + // Context will often be HTTP GET/HEAD; heavily cache the results + return (bool)$this->wanCache->getWithSetCallback( + // Note that table prefixes are not related to server-side read-only mode + $this->wanCache->makeGlobalKey( + 'rdbms-server-readonly', + $this->getMasterServerName(), + $domain->getDatabase(), + $domain->getSchema() + ), self::TTL_CACHE_READONLY, - function () use ( $domain, $conn ) { + function () use ( $domain ) { $old = $this->trxProfiler->setSilenced( true ); try { $index = $this->getWriterIndex(); - $dbw = $conn ?: $this->getServerConnection( $index, $domain ); - $readOnly = (int)$dbw->serverIsReadOnly(); - if ( !$conn ) { - $this->reuseConnection( $dbw ); - } + // Reset the cache for isMasterConnectionReadOnly() + $flags = self::CONN_REFRESH_READ_ONLY; + $conn = $this->getServerConnection( $index, $domain->getId(), $flags ); + // Reuse the process cache set above + $readOnly = (int)$this->isMasterConnectionReadOnly( $conn ); + $this->reuseConnection( $conn ); } catch ( DBError $e ) { $readOnly = 0; } @@ -1980,7 +2028,7 @@ class LoadBalancer implements ILoadBalancer { return $readOnly; }, - [ 'pcTTL' => $cache::TTL_PROC_LONG, 'busyValue' => 0 ] + [ 'pcTTL' => WANObjectCache::TTL_PROC_LONG, 'lockTSE' => 10, 'busyValue' => 0 ] ); } @@ -2296,6 +2344,13 @@ class LoadBalancer implements ILoadBalancer { return $this->servers[$i]; } + /** + * @return string + */ + private function getMasterServerName() { + return $this->getServerName( $this->getWriterIndex() ); + } + function __destruct() { // Avoid connection leaks for sanity $this->disable();