Aaron Schulz has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/396546 )

Change subject: [WIP] Replace cpPosTime cookie/parameter with cpPosIndex
......................................................................

[WIP] Replace cpPosTime cookie/parameter with cpPosIndex

This handles multi-DB transactions properly, instead of causing wait
timeouts in the WaitConditionLoop. It also is more correct in using
a counter instead of relying on wall clocks.

In addition:
* Refactor related logic in MediaWiki.php to be comprehensible.
* Renamed ChronologyProtector field and constant to avoid any
  confusion of "wait for server X to reach Y" with "wait for Y
  to show up in position store".
* Add an "asOfTime" field to the position keys for debugging.

Change-Id: I5c73cd07eaf664f02ba00c38fab9f49b609f4284
---
M includes/MediaWiki.php
M includes/Setup.php
M includes/libs/rdbms/ChronologyProtector.php
M includes/libs/rdbms/lbfactory/ILBFactory.php
M includes/libs/rdbms/lbfactory/LBFactory.php
M tests/phpunit/includes/db/LBFactoryTest.php
6 files changed, 201 insertions(+), 131 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/46/396546/1

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 9e3bc10..edc714f 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -602,50 +602,62 @@
                DeferredUpdates::doUpdates( 'enqueue', DeferredUpdates::PRESEND 
);
                wfDebug( __METHOD__ . ': pre-send deferred updates completed' );
 
-               // Decide when clients block on ChronologyProtector DB position 
writes
-               $urlDomainDistance = (
-                       $request->wasPosted() &&
-                       $output->getRedirect() &&
-                       $lbFactory->hasOrMadeRecentMasterChanges( INF )
-               ) ? self::getUrlDomainDistance( $output->getRedirect() ) : 
false;
+               if ( $lbFactory->hasOrMadeRecentMasterChanges( INF ) ) {
+                       $redirectDomainDistance = $output->getRedirect()
+                               ? self::getUrlDomainDistance( 
$output->getRedirect() )
+                               : false;
 
-               $allowHeaders = !( $output->isDisabled() || headers_sent() );
-               if ( $urlDomainDistance === 'local' || $urlDomainDistance === 
'remote' ) {
-                       // OutputPage::output() will be fast; $postCommitWork 
will not be useful for
-                       // masking the latency of syncing DB positions accross 
all datacenters synchronously.
-                       // Instead, make use of the RTT time of the client 
follow redirects.
-                       $flags = $lbFactory::SHUTDOWN_CHRONPROT_ASYNC;
-                       $cpPosTime = microtime( true );
-                       // Client's next request should see 1+ positions with 
this DBMasterPos::asOf() time
-                       if ( $urlDomainDistance === 'local' && $allowHeaders ) {
-                               // Client will stay on this domain, so set an 
unobtrusive cookie
+                       // Decide when clients block on ChronologyProtector DB 
position writes
+                       $allowHeaders = !( $output->isDisabled() || 
headers_sent() );
+                       if ( in_array( $redirectDomainDistance, [ 'local', 
'remote' ] ) ) {
+                               // OutputPage::output() will be fast; 
$postCommitWork will be useless for masking
+                               // the latency of syncing DB positions accross 
all datacenters synchronously.
+                               // Instead, make use of the RTT time of the 
client follow redirects.
+                               $flags = $lbFactory::SHUTDOWN_CHRONPROT_ASYNC;
+                               // Client's next request should see these new 
DB positions to wait on
+                               if ( $redirectDomainDistance === 'local' && 
$allowHeaders ) {
+                                       // Client will stay on this domain, so 
set an unobtrusive cookie
+                                       $headerStrategy = 'cookie';
+                               } else {
+                                       // Cookies may not work across wiki 
domains, so use a URL parameter
+                                       $headerStrategy = 'redirect';
+                               }
+                       } else {
+                               // OutputPage::output() is fairly slow; run it 
in $postCommitWork to mask
+                               // the latency of syncing DB positions accross 
all datacenters synchronously
+                               $flags = $lbFactory::SHUTDOWN_CHRONPROT_SYNC;
+                               if ( $allowHeaders ) {
+                                       // Set a cookie in case the DB position 
store cannot sync accross datacenters.
+                                       // This will at least cover the common 
case of the user staying on the domain.
+                                       $headerStrategy = 'cookie';
+                               } else {
+                                       $headerStrategy = null;
+                               }
+                       }
+
+                       // Record ChronologyProtector positions for DBs 
affected in this request at this point
+                       $cpIndex = null;
+                       $lbFactory->shutdown( $flags, $postCommitWork, $cpIndex 
);
+                       wfDebug( __METHOD__ . ': LBFactory shutdown completed; 
writes performed' );
+                       if ( $cpIndex === null ) {
+                               // No DB positions updated (e.g. no replication 
or if save failed)
+                               wfDebug( __METHOD__ . ': LBFactory saved no DB 
positions' );
+                       } elseif ( $headerStrategy === 'cookie' ) {
                                $expires = time() + 
ChronologyProtector::POSITION_TTL;
                                $options = [ 'prefix' => '' ];
-                               $request->response()->setCookie( 'cpPosTime', 
$cpPosTime, $expires, $options );
-                       } else {
-                               // Cookies may not work across wiki domains, so 
use a URL parameter
+                               $request->response()->setCookie( 'cpPosIndex', 
$cpIndex, $expires, $options );
+                       } elseif ( $headerStrategy === 'redirect' ) {
                                $safeUrl = 
$lbFactory->appendPreShutdownTimeAsQuery(
                                        $output->getRedirect(),
-                                       $cpPosTime
+                                       $cpIndex
                                );
                                $output->redirect( $safeUrl );
                        }
                } else {
-                       // OutputPage::output() is fairly slow; run it in 
$postCommitWork to mask
-                       // the latency of syncing DB positions accross all 
datacenters synchronously
-                       $flags = $lbFactory::SHUTDOWN_CHRONPROT_SYNC;
-                       if ( $lbFactory->hasOrMadeRecentMasterChanges( INF ) && 
$allowHeaders ) {
-                               $cpPosTime = microtime( true );
-                               // Set a cookie in case the DB position store 
cannot sync accross datacenters.
-                               // This will at least cover the common case of 
the user staying on the domain.
-                               $expires = time() + 
ChronologyProtector::POSITION_TTL;
-                               $options = [ 'prefix' => '' ];
-                               $request->response()->setCookie( 'cpPosTime', 
$cpPosTime, $expires, $options );
-                       }
+                       // No writes happened, but go through the normal 
shutdown anyway
+                       $lbFactory->shutdown( 
$lbFactory::SHUTDOWN_CHRONPROT_SYNC, $postCommitWork );
+                       wfDebug( __METHOD__ . ': LBFactory shutdown completed; 
no writes performed' );
                }
-               // Record ChronologyProtector positions for DBs affected in 
this request at this point
-               $lbFactory->shutdown( $flags, $postCommitWork );
-               wfDebug( __METHOD__ . ': LBFactory shutdown completed' );
 
                // Set a cookie to tell all CDN edge nodes to "stick" the user 
to the DC that handles this
                // POST request (e.g. the "master" data center). Also have the 
user briefly bypass CDN so
diff --git a/includes/Setup.php b/includes/Setup.php
index d6f4b2f..a77aff4 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -737,19 +737,19 @@
 // Initialize the request object in $wgRequest
 $wgRequest = RequestContext::getMain()->getRequest(); // BackCompat
 // Set user IP/agent information for causal consistency purposes.
-// The cpPosTime cookie has no prefix and is set by 
MediaWiki::preOutputCommit().
-$cpPosTime = $wgRequest->getFloat( 'cpPosTime', $wgRequest->getCookie( 
'cpPosTime', '' ) );
+// The cpPosIndex cookie has no prefix and is set by 
MediaWiki::preOutputCommit().
+$cpPosIndex = $wgRequest->getFloat( 'cpPosIndex', $wgRequest->getCookie( 
'cpPosIndex', '' ) );
 MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->setRequestInfo( [
        'IPAddress' => $wgRequest->getIP(),
        'UserAgent' => $wgRequest->getHeader( 'User-Agent' ),
        'ChronologyProtection' => $wgRequest->getHeader( 'ChronologyProtection' 
),
-       'ChronologyPositionTime' => $cpPosTime
+       'ChronologyPositionIndex' => $cpPosIndex
 ] );
 // Make sure that caching does not compromise the consistency improvements
-if ( $cpPosTime ) {
+if ( $cpPosIndex ) {
        
MediaWikiServices::getInstance()->getMainWANObjectCache()->useInterimHoldOffCaching(
 false );
 }
-unset( $cpPosTime );
+unset( $cpPosIndex );
 
 // Useful debug output
 if ( $wgCommandLineMode ) {
diff --git a/includes/libs/rdbms/ChronologyProtector.php 
b/includes/libs/rdbms/ChronologyProtector.php
index 8121654..3c859a6 100644
--- a/includes/libs/rdbms/ChronologyProtector.php
+++ b/includes/libs/rdbms/ChronologyProtector.php
@@ -43,10 +43,10 @@
        protected $key;
        /** @var string Hash of client parameters */
        protected $clientId;
-       /** @var float|null Minimum UNIX timestamp of 1+ expected startup 
positions */
-       protected $waitForPosTime;
+       /** @var float|integer Expected minimum index of the last write to the 
position store */
+       protected $waitForPosIndex;
        /** @var int Max seconds to wait on positions to appear */
-       protected $waitForPosTimeout = self::POS_WAIT_TIMEOUT;
+       protected $waitForPosStoreTimeout = self::POS_STORE_WAIT_TIMEOUT;
        /** @var bool Whether to no-op all method calls */
        protected $enabled = true;
        /** @var bool Whether to check and wait on positions */
@@ -64,19 +64,19 @@
        /** @var int Seconds to store positions */
        const POSITION_TTL = 60;
        /** @var int Max time to wait for positions to appear */
-       const POS_WAIT_TIMEOUT = 5;
+       const POS_STORE_WAIT_TIMEOUT = 5;
 
        /**
         * @param BagOStuff $store
-        * @param array $client Map of (ip: <IP>, agent: <user-agent>)
-        * @param float $posTime UNIX timestamp
+        * @param array[] $client Map of (ip: <IP>, agent: <user-agent>)
+        * @param int $posIndex Write counter index
         * @since 1.27
         */
-       public function __construct( BagOStuff $store, array $client, $posTime 
= null ) {
+       public function __construct( BagOStuff $store, array $client, $posIndex 
= 0 ) {
                $this->store = $store;
                $this->clientId = md5( $client['ip'] . "\n" . $client['agent'] 
);
                $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 
'v1' );
-               $this->waitForPosTime = $posTime;
+               $this->waitForPosIndex = $posIndex;
                $this->logger = new NullLogger();
        }
 
@@ -161,9 +161,10 @@
         *
         * @param callable|null $workCallback Work to do instead of waiting on 
syncing positions
         * @param string $mode One of (sync, async); whether to wait on remote 
datacenters
+        * @param int|null &$cpIndex DB position key write counter; incremented 
on update
         * @return DBMasterPos[] Empty on success; returns the (db name => 
position) map on failure
         */
-       public function shutdown( callable $workCallback = null, $mode = 'sync' 
) {
+       public function shutdown( callable $workCallback = null, $mode = 
'sync', &$cpIndex = null ) {
                if ( !$this->enabled ) {
                        return [];
                }
@@ -198,13 +199,18 @@
                        }
                        $ok = $store->set(
                                $this->key,
-                               self::mergePositions( $store->get( $this->key 
), $this->shutdownPositions ),
+                               $this->mergePositions(
+                                       $store->get( $this->key ),
+                                       $this->shutdownPositions,
+                                       $cpIndex
+                               ),
                                self::POSITION_TTL,
                                ( $mode === 'sync' ) ? $store::WRITE_SYNC : 0
                        );
                        $store->unlock( $this->key );
                } else {
                        $ok = false;
+                       $cpIndex = null; // nothing saved
                }
 
                if ( !$ok ) {
@@ -256,26 +262,29 @@
                if ( $this->wait ) {
                        // If there is an expectation to see master positions 
with a certain min
                        // timestamp, then block until they appear, or until a 
timeout is reached.
-                       if ( $this->waitForPosTime > 0.0 ) {
+                       if ( $this->waitForPosIndex > 0 ) {
                                $data = null;
                                $loop = new WaitConditionLoop(
                                        function () use ( &$data ) {
                                                $data = $this->store->get( 
$this->key );
+                                               if ( !isset( 
$data['writeIndex'] ) ) {
+                                                       return 
WaitConditionLoop::CONDITION_REACHED; // b/c
+                                               }
 
-                                               return ( self::minPosTime( 
$data ) >= $this->waitForPosTime )
+                                               return ( $data['writeIndex'] >= 
$this->waitForPosIndex )
                                                        ? 
WaitConditionLoop::CONDITION_REACHED
                                                        : 
WaitConditionLoop::CONDITION_CONTINUE;
                                        },
-                                       $this->waitForPosTimeout
+                                       $this->waitForPosStoreTimeout
                                );
                                $result = $loop->invoke();
                                $waitedMs = $loop->getLastWaitTime() * 1e3;
 
                                if ( $result == $loop::CONDITION_REACHED ) {
-                                       $msg = "expected and found pos time 
{$this->waitForPosTime} ({$waitedMs}ms)";
+                                       $msg = "expected and found pos time 
{$this->waitForPosIndex} ({$waitedMs}ms)";
                                        $this->logger->debug( $msg );
                                } else {
-                                       $msg = "expected but missed pos time 
{$this->waitForPosTime} ({$waitedMs}ms)";
+                                       $msg = "expected but missed pos time 
{$this->waitForPosIndex} ({$waitedMs}ms)";
                                        $this->logger->info( $msg );
                                }
                        } else {
@@ -291,47 +300,38 @@
        }
 
        /**
-        * @param array|bool $data
-        * @return float|null
+        * @param array|bool $curValue
+        * @param DBMasterPos[] $shutdownPositions
+        * @param int|null &$cpIndex
+        * @return array
         */
-       private static function minPosTime( $data ) {
-               if ( !isset( $data['positions'] ) ) {
-                       return null;
-               }
-
-               $min = null;
-               foreach ( $data['positions'] as $pos ) {
-                       if ( $pos instanceof DBMasterPos ) {
-                               $min = $min ? min( $pos->asOfTime(), $min ) : 
$pos->asOfTime();
+       protected function mergePositions( $curValue, array $shutdownPositions, 
&$cpIndex = null ) {
+               /** @var DBMasterPos[] $curPositions */
+               $curPositions = isset( $curValue['positions'] ) ? 
$curValue['positions'] : [];
+               // Use the newest positions for each DB master
+               foreach ( $shutdownPositions as $db => $pos ) {
+                       if (
+                               !isset( $curPositions[$db] ) ||
+                               !( $curPositions[$db] instanceof DBMasterPos ) 
||
+                               $pos->asOfTime() > 
$curPositions[$db]->asOfTime()
+                       ) {
+                               $curPositions[$db] = $pos;
                        }
                }
 
-               return $min;
+               $cpIndex = isset( $curValue['writeIndex'] ) ? 
$curValue['writeIndex'] : 0;
+
+               return [
+                       'positions' => $curPositions,
+                       'writeIndex' => ++$cpIndex,
+                       'asOfTime' => $this->getCurrentTime()
+               ];
        }
 
        /**
-        * @param array|bool $curValue
-        * @param DBMasterPos[] $shutdownPositions
-        * @return array
+        * @return float UNIX timestamp
         */
-       private static function mergePositions( $curValue, array 
$shutdownPositions ) {
-               /** @var DBMasterPos[] $curPositions */
-               if ( $curValue === false ) {
-                       $curPositions = $shutdownPositions;
-               } else {
-                       $curPositions = $curValue['positions'];
-                       // Use the newest positions for each DB master
-                       foreach ( $shutdownPositions as $db => $pos ) {
-                               if (
-                                       !isset( $curPositions[$db] ) ||
-                                       !( $curPositions[$db] instanceof 
DBMasterPos ) ||
-                                       $pos->asOfTime() > 
$curPositions[$db]->asOfTime()
-                               ) {
-                                       $curPositions[$db] = $pos;
-                               }
-                       }
-               }
-
-               return [ 'positions' => $curPositions ];
+       protected function getCurrentTime() {
+               return microtime( true );
        }
 }
diff --git a/includes/libs/rdbms/lbfactory/ILBFactory.php 
b/includes/libs/rdbms/lbfactory/ILBFactory.php
index 697af0e..d355e03 100644
--- a/includes/libs/rdbms/lbfactory/ILBFactory.php
+++ b/includes/libs/rdbms/lbfactory/ILBFactory.php
@@ -140,9 +140,10 @@
         * Prepare all tracked load balancers for shutdown
         * @param int $mode One of the class SHUTDOWN_* constants
         * @param callable|null $workCallback Work to mask ChronologyProtector 
writes
+        * @param int|null &$cpIndex Position key write counter for 
ChronologyProtector
         */
        public function shutdown(
-               $mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = 
null
+               $mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = 
null, &$cpIndex = null
        );
 
        /**
@@ -304,7 +305,7 @@
        public function setAgentName( $agent );
 
        /**
-        * Append ?cpPosTime parameter to a URL for ChronologyProtector 
purposes if needed
+        * Append ?cpPosIndex parameter to a URL for ChronologyProtector 
purposes if needed
         *
         * Note that unlike cookies, this works accross domains
         *
@@ -319,7 +320,7 @@
         *   - IPAddress : IP address
         *   - UserAgent : User-Agent HTTP header
         *   - ChronologyProtection : cookie/header value specifying 
ChronologyProtector usage
-        *   - ChronologyPositionTime: timestamp used to get up-to-date DB 
positions for the agent
+        *   - ChronologyPositionIndex: timestamp used to get up-to-date DB 
positions for the agent
         */
        public function setRequestInfo( array $info );
 }
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php 
b/includes/libs/rdbms/lbfactory/LBFactory.php
index ef716b6..46032e8 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -116,7 +116,7 @@
                        'IPAddress' => isset( $_SERVER[ 'REMOTE_ADDR' ] ) ? 
$_SERVER[ 'REMOTE_ADDR' ] : '',
                        'UserAgent' => isset( $_SERVER['HTTP_USER_AGENT'] ) ? 
$_SERVER['HTTP_USER_AGENT'] : '',
                        'ChronologyProtection' => 'true',
-                       'ChronologyPositionTime' => isset( $_GET['cpPosTime'] ) 
? $_GET['cpPosTime'] : null
+                       'ChronologyPositionIndex' => isset( $_GET['cpPosIndex'] 
) ? $_GET['cpPosIndex'] : null
                ];
 
                $this->cliMode = isset( $conf['cliMode'] ) ? $conf['cliMode'] : 
PHP_SAPI === 'cli';
@@ -132,13 +132,13 @@
        }
 
        public function shutdown(
-               $mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = 
null
+               $mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = 
null, &$cpIndex = null
        ) {
                $chronProt = $this->getChronologyProtector();
                if ( $mode === self::SHUTDOWN_CHRONPROT_SYNC ) {
-                       $this->shutdownChronologyProtector( $chronProt, 
$workCallback, 'sync' );
+                       $this->shutdownChronologyProtector( $chronProt, 
$workCallback, 'sync', $cpIndex );
                } elseif ( $mode === self::SHUTDOWN_CHRONPROT_ASYNC ) {
-                       $this->shutdownChronologyProtector( $chronProt, null, 
'async' );
+                       $this->shutdownChronologyProtector( $chronProt, null, 
'async', $cpIndex );
                }
 
                $this->commitMasterChanges( __METHOD__ ); // sanity
@@ -441,7 +441,7 @@
                                'ip' => $this->requestInfo['IPAddress'],
                                'agent' => $this->requestInfo['UserAgent'],
                        ],
-                       $this->requestInfo['ChronologyPositionTime']
+                       $this->requestInfo['ChronologyPositionIndex']
                );
                $this->chronProt->setLogger( $this->replLogger );
 
@@ -465,9 +465,10 @@
         * @param ChronologyProtector $cp
         * @param callable|null $workCallback Work to do instead of waiting on 
syncing positions
         * @param string $mode One of (sync, async); whether to wait on remote 
datacenters
+        * @param int|null &$cpIndex DB position key write counter; incremented 
on update
         */
        protected function shutdownChronologyProtector(
-               ChronologyProtector $cp, $workCallback, $mode
+               ChronologyProtector $cp, $workCallback, $mode, &$cpIndex = null
        ) {
                // Record all the master positions needed
                $this->forEachLB( function ( ILoadBalancer $lb ) use ( $cp ) {
@@ -475,7 +476,7 @@
                } );
                // Write them to the persistent stash. Try to do something 
useful by running $work
                // while ChronologyProtector waits for the stash write to 
replicate to all DCs.
-               $unsavedPositions = $cp->shutdown( $workCallback, $mode );
+               $unsavedPositions = $cp->shutdown( $workCallback, $mode, 
$cpIndex );
                if ( $unsavedPositions && $workCallback ) {
                        // Invoke callback in case it did not cache the result 
yet
                        $workCallback(); // work now to block for less time in 
waitForAll()
@@ -554,7 +555,7 @@
                        return $url; // no master/replica clusters touched
                }
 
-               return strpos( $url, '?' ) === false ? "$url?cpPosTime=$time" : 
"$url&cpPosTime=$time";
+               return strpos( $url, '?' ) === false ? "$url?cpPosIndex=$time" 
: "$url&cpPosIndex=$time";
        }
 
        public function setRequestInfo( array $info ) {
diff --git a/tests/phpunit/includes/db/LBFactoryTest.php 
b/tests/phpunit/includes/db/LBFactoryTest.php
index 1efeeeb..564927fb 100644
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -184,33 +184,61 @@
 
        public function testChronologyProtector() {
                // (a) First HTTP request
-               $mPos = new MySQLMasterPos( 'db1034-bin.000976', '843431247' );
+               $m1Pos = new MySQLMasterPos( 'db1034-bin.000976', '843431247' );
+               $m2Pos = new MySQLMasterPos( 'db1064-bin.002400', '794074907' );
 
                $now = microtime( true );
-               $mockDB = $this->getMockBuilder( 'DatabaseMysqli' )
-                       ->disableOriginalConstructor()
-                       ->getMock();
-               $mockDB->method( 'writesOrCallbacksPending' )->willReturn( true 
);
-               $mockDB->method( 'lastDoneWrites' )->willReturn( $now );
-               $mockDB->method( 'getMasterPos' )->willReturn( $mPos );
 
-               $lb = $this->getMockBuilder( 'LoadBalancer' )
+               // Master DB 1
+               $mockDB1 = $this->getMockBuilder( 'DatabaseMysqli' )
                        ->disableOriginalConstructor()
                        ->getMock();
-               $lb->method( 'getConnection' )->willReturn( $mockDB );
-               $lb->method( 'getServerCount' )->willReturn( 2 );
-               $lb->method( 'parentInfo' )->willReturn( [ 'id' => 
"main-DEFAULT" ] );
-               $lb->method( 'getAnyOpenConnection' )->willReturn( $mockDB );
-               $lb->method( 'hasOrMadeRecentMasterChanges' )->will( 
$this->returnCallback(
-                               function () use ( $mockDB ) {
+               $mockDB1->method( 'writesOrCallbacksPending' )->willReturn( 
true );
+               $mockDB1->method( 'lastDoneWrites' )->willReturn( $now );
+               $mockDB1->method( 'getMasterPos' )->willReturn( $m1Pos );
+               // Load balancer for master DB 1
+               $lb1 = $this->getMockBuilder( 'LoadBalancer' )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $lb1->method( 'getConnection' )->willReturn( $mockDB1 );
+               $lb1->method( 'getServerCount' )->willReturn( 2 );
+               $lb1->method( 'getAnyOpenConnection' )->willReturn( $mockDB1 );
+               $lb1->method( 'hasOrMadeRecentMasterChanges' )->will( 
$this->returnCallback(
+                               function () use ( $mockDB1 ) {
                                        $p = 0;
-                                       $p |= call_user_func( [ $mockDB, 
'writesOrCallbacksPending' ] );
-                                       $p |= call_user_func( [ $mockDB, 
'lastDoneWrites' ] );
+                                       $p |= call_user_func( [ $mockDB1, 
'writesOrCallbacksPending' ] );
+                                       $p |= call_user_func( [ $mockDB1, 
'lastDoneWrites' ] );
 
                                        return (bool)$p;
                                }
                        ) );
-               $lb->method( 'getMasterPos' )->willReturn( $mPos );
+               $lb1->method( 'getMasterPos' )->willReturn( $m1Pos );
+               $lb1->method( 'getServerName' )->with( 0 )->willReturn( 
'master1' );
+               // Master DB 2
+               $mockDB2 = $this->getMockBuilder( 'DatabaseMysqli' )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $mockDB2->method( 'writesOrCallbacksPending' )->willReturn( 
true );
+               $mockDB2->method( 'lastDoneWrites' )->willReturn( $now );
+               $mockDB2->method( 'getMasterPos' )->willReturn( $m2Pos );
+               // Load balancer for master DB 2
+               $lb2 = $this->getMockBuilder( 'LoadBalancer' )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $lb2->method( 'getConnection' )->willReturn( $mockDB2 );
+               $lb2->method( 'getServerCount' )->willReturn( 2 );
+               $lb2->method( 'getAnyOpenConnection' )->willReturn( $mockDB2 );
+               $lb2->method( 'hasOrMadeRecentMasterChanges' )->will( 
$this->returnCallback(
+                       function () use ( $mockDB2 ) {
+                               $p = 0;
+                               $p |= call_user_func( [ $mockDB2, 
'writesOrCallbacksPending' ] );
+                               $p |= call_user_func( [ $mockDB2, 
'lastDoneWrites' ] );
+
+                               return (bool)$p;
+                       }
+               ) );
+               $lb2->method( 'getMasterPos' )->willReturn( $m2Pos );
+               $lb2->method( 'getServerName' )->with( 0 )->willReturn( 
'master2' );
 
                $bag = new HashBagOStuff();
                $cp = new ChronologyProtector(
@@ -221,32 +249,60 @@
                        ]
                );
 
-               $mockDB->expects( $this->exactly( 2 ) )->method( 
'writesOrCallbacksPending' );
-               $mockDB->expects( $this->exactly( 2 ) )->method( 
'lastDoneWrites' );
+               $mockDB1->expects( $this->exactly( 1 ) )->method( 
'writesOrCallbacksPending' );
+               $mockDB1->expects( $this->exactly( 1 ) )->method( 
'lastDoneWrites' );
+               $mockDB2->expects( $this->exactly( 1 ) )->method( 
'writesOrCallbacksPending' );
+               $mockDB2->expects( $this->exactly( 1 ) )->method( 
'lastDoneWrites' );
 
-               // Nothing to wait for
-               $cp->initLB( $lb );
-               // Record in stash
-               $cp->shutdownLB( $lb );
-               $cp->shutdown();
+               // Nothing to wait for on first HTTP request start
+               $cp->initLB( $lb1 );
+               $cp->initLB( $lb2 );
+               // Record positions in stash on first HTTP request end
+               $cp->shutdownLB( $lb1 );
+               $cp->shutdownLB( $lb2 );
+               $cpIndex = null;
+               $cp->shutdown( null, 'sync', $cpIndex );
+
+               $this->assertEquals( 1, $cpIndex, "CP write index set" );
 
                // (b) Second HTTP request
+
+               // Load balancer for master DB 1
+               $lb1 = $this->getMockBuilder( 'LoadBalancer' )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $lb1->method( 'getServerCount' )->willReturn( 2 );
+               $lb1->method( 'getServerName' )->with( 0 )->willReturn( 
'master1' );
+               $lb1->expects( $this->once() )
+                       ->method( 'waitFor' )->with( $this->equalTo( $m1Pos ) );
+               // Load balancer for master DB 2
+               $lb2 = $this->getMockBuilder( 'LoadBalancer' )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $lb2->method( 'getServerCount' )->willReturn( 2 );
+               $lb2->method( 'getServerName' )->with( 0 )->willReturn( 
'master2' );
+               $lb2->expects( $this->once() )
+                       ->method( 'waitFor' )->with( $this->equalTo( $m2Pos ) );
+
                $cp = new ChronologyProtector(
                        $bag,
                        [
                                'ip' => '127.0.0.1',
                                'agent' => "Totally-Not-FireFox"
-                       ]
+                       ],
+                       $cpIndex
                );
 
-               $lb->expects( $this->once() )
-                       ->method( 'waitFor' )->with( $this->equalTo( $mPos ) );
+               // Wait for last positions to be reached on second HTTP request 
start
+               $cp->initLB( $lb1 );
+               $cp->initLB( $lb2 );
+               // Shutdown (nothing to record)
+               $cp->shutdownLB( $lb1 );
+               $cp->shutdownLB( $lb2 );
+               $cpIndex = null;
+               $cp->shutdown( null, 'sync', $cpIndex );
 
-               // Wait
-               $cp->initLB( $lb );
-               // Record in stash
-               $cp->shutdownLB( $lb );
-               $cp->shutdown();
+               $this->assertEquals( null, $cpIndex, "CP write index retained" 
);
        }
 
        private function newLBFactoryMulti( array $baseOverride = [], array 
$serverOverride = [] ) {

-- 
To view, visit https://gerrit.wikimedia.org/r/396546
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5c73cd07eaf664f02ba00c38fab9f49b609f4284
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to