From c954a4efe8cd121a8f298f0d80011f9edd25d9fd Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 13 Sep 2016 22:49:10 -0700 Subject: [PATCH] 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 --- RELEASE-NOTES-1.28 | 7 ++--- includes/MediaWiki.php | 36 ++++++++++++++++---------- includes/db/ChronologyProtector.php | 2 +- includes/db/loadbalancer/LBFactory.php | 2 +- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/RELEASE-NOTES-1.28 b/RELEASE-NOTES-1.28 index 116dc2ad88..dfa482a16b 100644 --- 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 0bd183f1d0..5ea8c31b15 100644 --- 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 9b01adcdf4..4d03bc6057 100644 --- 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 27bef0a27a..22a6597c2d 100644 --- 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 ); -- 2.20.1