Use cpPosTime cookie for same-domain redirects on DB change

This follows-up a3dacac90f to show the URL parameter in less cases.

Change-Id: Ibe015352962fb3ee14d5aa322f0e748ef4cba5c1

diff --git a/RELEASE-NOTES-1.28 b/RELEASE-NOTES-1.28
index 116dc2a..dfa482a 100644 (file)
--- a/RELEASE-NOTES-1.28
+++ b/RELEASE-NOTES-1.28
@@ -56,9 +56,10 @@ production.
   the mw.Api instance seems to be for the local wiki.
 * After a client performs an action which alters a database that has replica databases,
   MediaWiki will wait for the replica databases to synchronize with the master database
-  while it renders the HTML output. However, if the output is a redirect, it will instead
-  alter the redirect URL to include a ?cpPosTime parameter that triggers the database
-  synchronization when the URL is followed by the client.
+  while it renders the HTML output. However, if the output is a redirect to another wiki
+  on the wiki farm with a different domain, MediaWiki will instead alter the redirect
+  URL to include a ?cpPosTime parameter that triggers the database synchronization when
+  the URL is followed by the client. The same-domain case uses a new cpPosTime cookie.
 
 === External library changes in 1.28 ===
 

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 0bd183f..5ea8c31 100644 (file)
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -574,22 +574,32 @@ class MediaWiki {
                wfDebug( __METHOD__ . ': pre-send deferred updates completed' );
 
                // Decide when clients block on ChronologyProtector DB position writes
-               if (
+               $urlDomainDistance = (
                        $request->wasPosted() &&
                        $output->getRedirect() &&
-                       $lbFactory->hasOrMadeRecentMasterChanges( INF ) &&
-                       self::isWikiClusterURL( $output->getRedirect(), $context )
-               ) {
+                       $lbFactory->hasOrMadeRecentMasterChanges( INF )
+               ) ? self::getUrlDomainDistance( $output->getRedirect(), $context ) : false;
+
+               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
-                       $safeUrl = $lbFactory->appendPreShutdownTimeAsQuery(
-                               $output->getRedirect(),
-                               microtime( true )
-                       );
-                       $output->redirect( $safeUrl );
+                       if ( $urlDomainDistance === 'local' ) {
+                               // Client will stay on this domain, so set an unobtrusive 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
+                               $safeUrl = $lbFactory->appendPreShutdownTimeAsQuery(
+                                       $output->getRedirect(),
+                                       $cpPosTime
+                               );
+                               $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
@@ -629,9 +639,9 @@ class MediaWiki {
        /**
         * @param string $url
         * @param IContextSource $context
-        * @return bool Whether $url is to something on this wiki farm
+        * @return string|bool Either "local" or "remote" if in the farm, false otherwise
         */
-       private function isWikiClusterURL( $url, IContextSource $context ) {
+       private function getUrlDomainDistance( $url, IContextSource $context ) {
                static $relevantKeys = [ 'host' => true, 'port' => true ];
 
                $infoCandidate = wfParseUrl( $url );
@@ -647,14 +657,14 @@ class MediaWiki {
                        $context->getConfig()->get( 'LocalVirtualHosts' )
                );
 
-               foreach ( $clusterHosts as $clusterHost ) {
+               foreach ( $clusterHosts as $i => $clusterHost ) {
                        $parseUrl = wfParseUrl( $clusterHost );
                        if ( !$parseUrl ) {
                                continue;
                        }
                        $infoHost = array_intersect_key( $parseUrl, $relevantKeys );
                        if ( $infoCandidate === $infoHost ) {
-                               return true;
+                               return ( $i === 0 ) ? 'local' : 'remote';
                        }
                }
 

diff --git a/includes/db/ChronologyProtector.php b/includes/db/ChronologyProtector.php
index 9b01adc..4d03bc6 100644 (file)
--- a/includes/db/ChronologyProtector.php
+++ b/includes/db/ChronologyProtector.php
@@ -248,7 +248,7 @@ class ChronologyProtector implements LoggerAwareInterface{
                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 ) {
+                       if ( $this->waitForPosTime > 0.0 ) {
                                $data = null;
                                $loop = new WaitConditionLoop(
                                        function () use ( &$data ) {

diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php
index 27bef0a..22a6597 100644 (file)
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -600,7 +600,7 @@ abstract class LBFactory implements DestructibleService {
                                'ip' => $request->getIP(),
                                'agent' => $request->getHeader( 'User-Agent' ),
                        ],
-                       $request->getFloat( 'cpPosTime', null )
+                       $request->getFloat( 'cpPosTime', $request->getCookie( 'cpPosTime', '' ) )
                );
                if ( PHP_SAPI === 'cli' ) {
                        $chronProt->setEnabled( false );