Defer calling ChronologyProtector::initLB() until connecting
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 24 Mar 2017 00:30:29 +0000 (17:30 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 28 Mar 2017 23:57:55 +0000 (16:57 -0700)
Bug: T160678
Change-Id: Id51c2854f38d9e4697e52776168b16996e9152e4

includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/lbfactory/LBFactoryMulti.php
includes/libs/rdbms/lbfactory/LBFactorySimple.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php

index f05dabc..beeb3b9 100644 (file)
@@ -508,7 +508,8 @@ abstract class LBFactory implements ILBFactory {
                        'errorLogger' => $this->errorLogger,
                        'hostname' => $this->hostname,
                        'cliMode' => $this->cliMode,
-                       'agent' => $this->agent
+                       'agent' => $this->agent,
+                       'chronologyProtector' => $this->getChronologyProtector()
                ];
        }
 
index 447b96f..8b6b354 100644 (file)
@@ -252,9 +252,7 @@ class LBFactoryMulti extends LBFactory {
        public function getMainLB( $domain = false ) {
                $section = $this->getSectionForDomain( $domain );
                if ( !isset( $this->mainLBs[$section] ) ) {
-                       $lb = $this->newMainLB( $domain );
-                       $this->getChronologyProtector()->initLB( $lb );
-                       $this->mainLBs[$section] = $lb;
+                       $this->mainLBs[$section] = $this->newMainLB( $domain );
                }
 
                return $this->mainLBs[$section];
@@ -283,7 +281,6 @@ class LBFactoryMulti extends LBFactory {
        public function getExternalLB( $cluster ) {
                if ( !isset( $this->extLBs[$cluster] ) ) {
                        $this->extLBs[$cluster] = $this->newExternalLB( $cluster );
-                       $this->getChronologyProtector()->initLB( $this->extLBs[$cluster] );
                }
 
                return $this->extLBs[$cluster];
index 15cd508..df0a806 100644 (file)
@@ -89,7 +89,6 @@ class LBFactorySimple extends LBFactory {
        public function getMainLB( $domain = false ) {
                if ( !isset( $this->mainLB ) ) {
                        $this->mainLB = $this->newMainLB( $domain );
-                       $this->getChronologyProtector()->initLB( $this->mainLB );
                }
 
                return $this->mainLB;
@@ -106,7 +105,6 @@ class LBFactorySimple extends LBFactory {
        public function getExternalLB( $cluster ) {
                if ( !isset( $this->extLBs[$cluster] ) ) {
                        $this->extLBs[$cluster] = $this->newExternalLB( $cluster );
-                       $this->getChronologyProtector()->initLB( $this->extLBs[$cluster] );
                }
 
                return $this->extLBs[$cluster];
index 4c277ff..a256221 100644 (file)
@@ -105,6 +105,7 @@ interface ILoadBalancer {
         *  - srvCache : BagOStuff object for server cache [optional]
         *  - memCache : BagOStuff object for cluster memory cache [optional]
         *  - wanCache : WANObjectCache object [optional]
+        *  - chronologyProtector: ChronologyProtector object [optional]
         *  - hostname : The name of the current server [optional]
         *  - cliMode: Whether the execution context is a CLI script. [optional]
         *  - profiler : Class name or instance with profileIn()/profileOut() methods. [optional]
@@ -138,7 +139,7 @@ interface ILoadBalancer {
         * If a DB_REPLICA connection has been opened already, then wait immediately.
         * Otherwise sets a variable telling it to wait if such a connection is opened.
         *
-        * @param DBMasterPos $pos
+        * @param DBMasterPos|bool $pos Master position or false
         */
        public function waitFor( $pos );
 
@@ -147,7 +148,7 @@ interface ILoadBalancer {
         *
         * This can be used a faster proxy for waitForAll()
         *
-        * @param DBMasterPos $pos
+        * @param DBMasterPos|bool $pos Master position or false
         * @param int $timeout Max seconds to wait; default is mWaitTimeout
         * @return bool Success (able to connect and no timeouts reached)
         */
@@ -156,7 +157,7 @@ interface ILoadBalancer {
        /**
         * Set the master wait position and wait for ALL replica DBs to catch up to it
         *
-        * @param DBMasterPos $pos
+        * @param DBMasterPos|bool $pos Master position or false
         * @param int $timeout Max seconds to wait; default is mWaitTimeout
         * @return bool Success (able to connect and no timeouts reached)
         */
index 6878712..baedecf 100644 (file)
@@ -69,6 +69,8 @@ class LoadBalancer implements ILoadBalancer {
 
        /** @var ILoadMonitor */
        private $loadMonitor;
+       /** @var ChronologyProtector|null */
+       private $chronProt;
        /** @var BagOStuff */
        private $srvCache;
        /** @var BagOStuff */
@@ -124,6 +126,8 @@ class LoadBalancer implements ILoadBalancer {
 
        /** @var boolean */
        private $disabled = false;
+       /** @var boolean */
+       private $chronProtInitialized = false;
 
        /** @var integer Warn when this many connection are held */
        const CONN_HELD_WARN_THRESHOLD = 10;
@@ -222,6 +226,10 @@ class LoadBalancer implements ILoadBalancer {
                        : ( gethostname() ?: 'unknown' );
                $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : PHP_SAPI === 'cli';
                $this->agent = isset( $params['agent'] ) ? $params['agent'] : '';
+
+               if ( isset( $params['chronologyProtector'] ) ) {
+                       $this->chronProt = $params['chronologyProtector'];
+               }
        }
 
        /**
@@ -424,21 +432,24 @@ class LoadBalancer implements ILoadBalancer {
                return $i;
        }
 
-       /**
-        * @param DBMasterPos|false $pos
-        */
        public function waitFor( $pos ) {
+               $oldPos = $this->mWaitForPos;
                $this->mWaitForPos = $pos;
-               $i = $this->mReadIndex;
 
+               // If a generic reader connection was already established, then wait now
+               $i = $this->mReadIndex;
                if ( $i > 0 ) {
                        if ( !$this->doWait( $i ) ) {
                                $this->laggedReplicaMode = true;
                        }
                }
+
+               // Restore the older position if it was higher
+               $this->setWaitForPositionIfHigher( $oldPos );
        }
 
        public function waitForOne( $pos, $timeout = null ) {
+               $oldPos = $this->mWaitForPos;
                $this->mWaitForPos = $pos;
 
                $i = $this->mReadIndex;
@@ -456,10 +467,14 @@ class LoadBalancer implements ILoadBalancer {
                        $ok = true; // no applicable loads
                }
 
+               // Restore the older position if it was higher
+               $this->setWaitForPositionIfHigher( $oldPos );
+
                return $ok;
        }
 
        public function waitForAll( $pos, $timeout = null ) {
+               $oldPos = $this->mWaitForPos;
                $this->mWaitForPos = $pos;
                $serverCount = count( $this->mServers );
 
@@ -470,9 +485,25 @@ class LoadBalancer implements ILoadBalancer {
                        }
                }
 
+               // Restore the older position if it was higher
+               $this->setWaitForPositionIfHigher( $oldPos );
+
                return $ok;
        }
 
+       /**
+        * @param DBMasterPos|bool $pos
+        */
+       private function setWaitForPositionIfHigher( $pos ) {
+               if ( !$pos ) {
+                       return;
+               }
+
+               if ( !$this->mWaitForPos || $pos->hasReached( $this->mWaitForPos ) ) {
+                       $this->mWaitForPos = $pos;
+               }
+       }
+
        /**
         * @param int $i
         * @return IDatabase|bool
@@ -718,6 +749,13 @@ class LoadBalancer implements ILoadBalancer {
                        $domain = false; // local connection requested
                }
 
+               if ( !$this->chronProtInitialized && $this->chronProt ) {
+                       $this->connLogger->debug( __METHOD__ . ': calling initLB() before first connection.' );
+                       // Load CP positions before connecting so that doWait() triggers later if needed
+                       $this->chronProtInitialized = true;
+                       $this->chronProt->initLB( $this );
+               }
+
                if ( $domain !== false ) {
                        $conn = $this->openForeignConnection( $i, $domain );
                } elseif ( isset( $this->mConns['local'][$i][0] ) ) {