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

diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index aa7d1b4..e5ed2f1 100644 (file)
--- 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 31c022c..9f1021d 100644 (file)
--- 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 72a8785..14a52c5 100644 (file)
--- 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 60400e3..499542d 100644 (file)
--- 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 67bac2b..c4e25dc 100644 (file)
--- 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 9d92794..fa2bd54 100644 (file)
--- 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' ) . ' ';