From 4154c688e898360fa2f024da5452d29db1241848 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 5 Oct 2016 12:47:10 -0700 Subject: [PATCH] Make LoadMonitor use $serverIndexes in the cache key * This avoids collisions of generic and custom LB group server list keys, which could cause warnings and errors. * Remove $group param from scaleLoads(), which was unused and less robust for making keys anyway. * Remove clearCaches() method, which only had one caller in a script that printed lag times in a loop. Its not worth keeping and having to pass in the server index list. * Also guard scaleLoads() against recent server additions. Bug: T147359 Change-Id: Idd15f0bebb68782fda36f483880cf7fe9673b940 --- .../libs/rdbms/loadbalancer/ILoadBalancer.php | 7 ---- .../libs/rdbms/loadbalancer/LoadBalancer.php | 6 +--- .../libs/rdbms/loadmonitor/ILoadMonitor.php | 17 +++------- .../libs/rdbms/loadmonitor/LoadMonitor.php | 32 ++++++++++--------- .../rdbms/loadmonitor/LoadMonitorNull.php | 2 +- maintenance/lag.php | 1 - 6 files changed, 24 insertions(+), 41 deletions(-) diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index aa7d1b4a9d..e5ed2f1e84 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -517,13 +517,6 @@ interface ILoadBalancer { */ public function safeWaitForMasterPos( IDatabase $conn, $pos = false, $timeout = null ); - /** - * Clear the cache for slag lag delay times - * - * This is only used for testing - */ - public function clearLagTimeCache(); - /** * Set a callback via IDatabase::setTransactionListener() on * all current and future master connections of this load balancer diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index 31c022c7f9..9f1021d3c5 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -301,7 +301,7 @@ class LoadBalancer implements ILoadBalancer { } # Scale the configured load ratios according to the dynamic load if supported - $this->getLoadMonitor()->scaleLoads( $nonErrorLoads, $group, $domain ); + $this->getLoadMonitor()->scaleLoads( $nonErrorLoads, $domain ); $laggedReplicaMode = false; @@ -1471,10 +1471,6 @@ class LoadBalancer implements ILoadBalancer { return $ok; } - public function clearLagTimeCache() { - $this->getLoadMonitor()->clearCaches(); - } - public function setTransactionListener( $name, callable $callback = null ) { if ( $callback ) { $this->trxRecurringCallbacks[$name] = $callback; diff --git a/includes/libs/rdbms/loadmonitor/ILoadMonitor.php b/includes/libs/rdbms/loadmonitor/ILoadMonitor.php index 72a8785901..14a52c5a8c 100644 --- a/includes/libs/rdbms/loadmonitor/ILoadMonitor.php +++ b/includes/libs/rdbms/loadmonitor/ILoadMonitor.php @@ -41,12 +41,12 @@ interface ILoadMonitor extends LoggerAwareInterface { ); /** - * Perform pre-connection load ratio adjustment. - * @param int[] &$weightByServer Map of (server index => integer weight) - * @param string|bool $group The selected query group. Default: false - * @param string|bool $domain Default: false + * Perform load ratio adjustment before deciding which server to use + * + * @param int[] &$weightByServer Map of (server index => float weight) + * @param string|bool $domain */ - public function scaleLoads( array &$weightByServer, $group = false, $domain = false ); + public function scaleLoads( array &$weightByServer, $domain ); /** * Get an estimate of replication lag (in seconds) for each server @@ -55,14 +55,7 @@ interface ILoadMonitor extends LoggerAwareInterface { * * @param integer[] $serverIndexes * @param string $domain - * * @return array Map of (server index => float|int|bool) */ public function getLagTimes( array $serverIndexes, $domain ); - - /** - * Clear any process and persistent cache of lag times - * @since 1.27 - */ - public function clearCaches(); } diff --git a/includes/libs/rdbms/loadmonitor/LoadMonitor.php b/includes/libs/rdbms/loadmonitor/LoadMonitor.php index 60400e3de2..499542dd4a 100644 --- a/includes/libs/rdbms/loadmonitor/LoadMonitor.php +++ b/includes/libs/rdbms/loadmonitor/LoadMonitor.php @@ -40,7 +40,7 @@ class LoadMonitor implements ILoadMonitor { /** @var float Moving average ratio (e.g. 0.1 for 10% weight to new weight) */ private $movingAveRatio; - const VERSION = 1; + const VERSION = 1; // cache key version public function __construct( ILoadBalancer $lb, BagOStuff $srvCache, BagOStuff $cache, array $options = [] @@ -59,12 +59,17 @@ class LoadMonitor implements ILoadMonitor { $this->replLogger = $logger; } - public function scaleLoads( array &$weightByServer, $group = false, $domain = false ) { + public function scaleLoads( array &$weightByServer, $domain ) { $serverIndexes = array_keys( $weightByServer ); $states = $this->getServerStates( $serverIndexes, $domain ); $coefficientsByServer = $states['weightScales']; foreach ( $weightByServer as $i => $weight ) { - $weightByServer[$i] = $weight * $coefficientsByServer[$i]; + if ( isset( $coefficientsByServer[$i] ) ) { + $weightByServer[$i] = $weight * $coefficientsByServer[$i]; + } else { // server recently added to config? + $host = $this->parent->getServerName( $i ); + $this->replLogger->error( __METHOD__ . ": host $host not in cache" ); + } } } @@ -75,15 +80,16 @@ class LoadMonitor implements ILoadMonitor { } protected function getServerStates( array $serverIndexes, $domain ) { - if ( count( $serverIndexes ) == 1 && reset( $serverIndexes ) == 0 ) { + $writerIndex = $this->parent->getWriterIndex(); + if ( count( $serverIndexes ) == 1 && reset( $serverIndexes ) == $writerIndex ) { # Single server only, just return zero without caching return [ - 'lagTimes' => [ $this->parent->getWriterIndex() => 0 ], - 'weightScales' => [ $this->parent->getWriterIndex() => 1 ] + 'lagTimes' => [ $writerIndex => 0 ], + 'weightScales' => [ $writerIndex => 1.0 ] ]; } - $key = $this->getCacheKey(); + $key = $this->getCacheKey( $serverIndexes ); # Randomize TTLs to reduce stampedes (4.0 - 5.0 sec) $ttl = mt_rand( 4e6, 5e6 ) / 1e6; # Keep keys around longer as fallbacks @@ -191,18 +197,14 @@ class LoadMonitor implements ILoadMonitor { return $conn ? 1.0 : 0.0; } - public function clearCaches() { - $key = $this->getCacheKey(); - $this->srvCache->delete( $key ); - $this->mainCache->delete( $key ); - } - - private function getCacheKey() { + private function getCacheKey( array $serverIndexes ) { + sort( $serverIndexes ); // Lag is per-server, not per-DB, so key on the master DB name return $this->srvCache->makeGlobalKey( 'lag-times', self::VERSION, - $this->parent->getServerName( $this->parent->getWriterIndex() ) + $this->parent->getServerName( $this->parent->getWriterIndex() ), + implode( '-', $serverIndexes ) ); } } diff --git a/includes/libs/rdbms/loadmonitor/LoadMonitorNull.php b/includes/libs/rdbms/loadmonitor/LoadMonitorNull.php index 67bac2bb35..c4e25dc24a 100644 --- a/includes/libs/rdbms/loadmonitor/LoadMonitorNull.php +++ b/includes/libs/rdbms/loadmonitor/LoadMonitorNull.php @@ -30,7 +30,7 @@ class LoadMonitorNull implements ILoadMonitor { public function setLogger( LoggerInterface $logger ) { } - public function scaleLoads( array &$loads, $group = false, $domain = false ) { + public function scaleLoads( array &$loads, $domain ) { } diff --git a/maintenance/lag.php b/maintenance/lag.php index 9d92794129..fa2bd54e03 100644 --- a/maintenance/lag.php +++ b/maintenance/lag.php @@ -48,7 +48,6 @@ class DatabaseLag extends Maintenance { echo "\n"; while ( 1 ) { - $lb->clearLagTimeCache(); $lags = $lb->getLagTimes(); unset( $lags[0] ); echo gmdate( 'H:i:s' ) . ' '; -- 2.20.1