Merge "Make LBFactory::waitForReplication() mask wait latency with callbacks"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 13 Sep 2016 01:31:28 +0000 (01:31 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 13 Sep 2016 01:31:28 +0000 (01:31 +0000)
22 files changed:
RELEASE-NOTES-1.28
includes/EditPage.php
includes/MediaWiki.php
includes/OutputPage.php
includes/WebRequest.php
includes/api/ApiLogin.php
includes/db/ChronologyProtector.php
includes/db/loadbalancer/LBFactory.php
includes/jobqueue/JobQueueGroup.php
includes/libs/objectcache/IExpiringStore.php
includes/libs/objectcache/MemcachedBagOStuff.php
includes/libs/objectcache/RESTBagOStuff.php
includes/objectcache/RedisBagOStuff.php
includes/objectcache/SqlBagOStuff.php
includes/specials/SpecialBotPasswords.php
includes/user/BotPassword.php
includes/user/User.php
languages/i18n/en.json
languages/i18n/qqq.json
maintenance/doMaintenance.php
tests/phpunit/includes/user/BotPasswordTest.php
tests/phpunit/structure/ContentHandlerSanityTest.php [new file with mode: 0644]

index 979da5c..1f4b8dc 100644 (file)
@@ -54,6 +54,11 @@ production.
 * mw.Api has a new option, useUS, to use U+001F (Unit Separator) when
   appropriate for sending multi-valued parameters. This defaults to true when
   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.
 
 === External library changes in 1.28 ===
 
index c5330ee..140cd72 100644 (file)
@@ -1008,9 +1008,17 @@ class EditPage {
                // May be overridden by revision.
                $this->contentFormat = $request->getText( 'format', $this->contentFormat );
 
-               if ( !ContentHandler::getForModelID( $this->contentModel )
-                       ->isSupportedFormat( $this->contentFormat )
-               ) {
+               try {
+                       $handler = ContentHandler::getForModelID( $this->contentModel );
+               } catch ( MWUnknownContentModelException $e ) {
+                       throw new ErrorPageError(
+                               'editpage-invalidcontentmodel-title',
+                               'editpage-invalidcontentmodel-text',
+                               [ $this->contentModel ]
+                       );
+               }
+
+               if ( !$handler->isSupportedFormat( $this->contentFormat ) ) {
                        throw new ErrorPageError(
                                'editpage-notsupportedcontentformat-title',
                                'editpage-notsupportedcontentformat-text',
index bca7a21..7f20de1 100644 (file)
@@ -535,10 +535,11 @@ class MediaWiki {
 
        /**
         * @see MediaWiki::preOutputCommit()
+        * @param callable $postCommitWork [default: null]
         * @since 1.26
         */
-       public function doPreOutputCommit() {
-               self::preOutputCommit( $this->context );
+       public function doPreOutputCommit( callable $postCommitWork = null ) {
+               self::preOutputCommit( $this->context, $postCommitWork );
        }
 
        /**
@@ -546,33 +547,61 @@ class MediaWiki {
         * the user can receive a response (in case commit fails)
         *
         * @param IContextSource $context
+        * @param callable $postCommitWork [default: null]
         * @since 1.27
         */
-       public static function preOutputCommit( IContextSource $context ) {
+       public static function preOutputCommit(
+               IContextSource $context, callable $postCommitWork = null
+       ) {
                // Either all DBs should commit or none
                ignore_user_abort( true );
 
                $config = $context->getConfig();
-
+               $request = $context->getRequest();
+               $output = $context->getOutput();
                $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+
                // Commit all changes
                $lbFactory->commitMasterChanges(
                        __METHOD__,
                        // Abort if any transaction was too big
                        [ 'maxWriteDuration' => $config->get( 'MaxUserDBWriteDuration' ) ]
                );
+               wfDebug( __METHOD__ . ': primary transaction round committed' );
 
+               // Run updates that need to block the user or affect output (this is the last chance)
                DeferredUpdates::doUpdates( 'enqueue', DeferredUpdates::PRESEND );
                wfDebug( __METHOD__ . ': pre-send deferred updates completed' );
 
-               // Record ChronologyProtector positions
-               $lbFactory->shutdown();
-               wfDebug( __METHOD__ . ': all transactions committed' );
+               // Decide when clients block on ChronologyProtector DB position writes
+               if (
+                       $request->wasPosted() &&
+                       $output->getRedirect() &&
+                       $lbFactory->hasOrMadeRecentMasterChanges( INF ) &&
+                       self::isWikiClusterURL( $output->getRedirect(), $context )
+               ) {
+                       // 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;
+                       // Client's next request should see 1+ positions with this DBMasterPos::asOf() time
+                       $safeUrl = $lbFactory->appendPreShutdownTimeAsQuery(
+                               $output->getRedirect(),
+                               microtime( true )
+                       );
+                       $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;
+               }
+               // 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
                // ChronologyProtector works for cacheable URLs.
-               $request = $context->getRequest();
                if ( $request->wasPosted() && $lbFactory->hasOrMadeRecentMasterChanges() ) {
                        $expires = time() + $config->get( 'DataCenterUpdateStickTTL' );
                        $options = [ 'prefix' => '' ];
@@ -584,7 +613,7 @@ class MediaWiki {
                // also intimately related to the value of $wgCdnReboundPurgeDelay.
                if ( $lbFactory->laggedReplicaUsed() ) {
                        $maxAge = $config->get( 'CdnMaxageLagged' );
-                       $context->getOutput()->lowerCdnMaxage( $maxAge );
+                       $output->lowerCdnMaxage( $maxAge );
                        $request->response()->header( "X-Database-Lagged: true" );
                        wfDebugLog( 'replication', "Lagged DB used; CDN cache TTL limited to $maxAge seconds" );
                }
@@ -592,11 +621,42 @@ class MediaWiki {
                // Avoid long-term cache pollution due to message cache rebuild timeouts (T133069)
                if ( MessageCache::singleton()->isDisabled() ) {
                        $maxAge = $config->get( 'CdnMaxageSubstitute' );
-                       $context->getOutput()->lowerCdnMaxage( $maxAge );
+                       $output->lowerCdnMaxage( $maxAge );
                        $request->response()->header( "X-Response-Substitute: true" );
                }
        }
 
+       /**
+        * @param string $url
+        * @param IContextSource $context
+        * @return bool Whether $url is to something on this wiki farm
+        */
+       private function isWikiClusterURL( $url, IContextSource $context ) {
+               static $relevantKeys = [ 'host' => true, 'port' => true ];
+
+               $infoCandidate = wfParseUrl( $url );
+               if ( $infoCandidate === false ) {
+                       return false;
+               }
+
+               $infoCandidate = array_intersect_key( $infoCandidate, $relevantKeys );
+               $clusterHosts = array_merge(
+                       // Local wiki host (the most common case)
+                       [ $context->getConfig()->get( 'CanonicalServer' ) ],
+                       // Any local/remote wiki virtual hosts for this wiki farm
+                       $context->getConfig()->get( 'LocalVirtualHosts' )
+               );
+
+               foreach ( $clusterHosts as $clusterHost ) {
+                       $infoHost = array_intersect_key( wfParseUrl( $clusterHost ), $relevantKeys );
+                       if ( $infoCandidate === $infoHost ) {
+                               return true;
+                       }
+               }
+
+               return false;
+       }
+
        /**
         * This function does work that can be done *after* the
         * user gets the HTTP response so they don't block on it
@@ -614,10 +674,9 @@ class MediaWiki {
                // Show visible profiling data if enabled (which cannot be post-send)
                Profiler::instance()->logDataPageOutputOnly();
 
-               $that = $this;
-               $callback = function () use ( $that, $mode ) {
+               $callback = function () use ( $mode ) {
                        try {
-                               $that->restInPeace( $mode );
+                               $this->restInPeace( $mode );
                        } catch ( Exception $e ) {
                                MWExceptionHandler::handleException( $e );
                        }
@@ -643,6 +702,7 @@ class MediaWiki {
        private function main() {
                global $wgTitle;
 
+               $output = $this->context->getOutput();
                $request = $this->context->getRequest();
 
                // Send Ajax requests to the Ajax dispatcher.
@@ -656,6 +716,7 @@ class MediaWiki {
 
                        $dispatcher = new AjaxDispatcher( $this->config );
                        $dispatcher->performAction( $this->context->getUser() );
+
                        return;
                }
 
@@ -717,11 +778,11 @@ class MediaWiki {
                                // Setup dummy Title, otherwise OutputPage::redirect will fail
                                $title = Title::newFromText( 'REDIR', NS_MAIN );
                                $this->context->setTitle( $title );
-                               $output = $this->context->getOutput();
                                // Since we only do this redir to change proto, always send a vary header
                                $output->addVaryHeader( 'X-Forwarded-Proto' );
                                $output->redirect( $redirUrl );
                                $output->output();
+
                                return;
                        }
                }
@@ -733,14 +794,15 @@ class MediaWiki {
                                if ( $cache->isCacheGood( /* Assume up to date */ ) ) {
                                        // Check incoming headers to see if client has this cached
                                        $timestamp = $cache->cacheTimestamp();
-                                       if ( !$this->context->getOutput()->checkLastModified( $timestamp ) ) {
+                                       if ( !$output->checkLastModified( $timestamp ) ) {
                                                $cache->loadFromFileCache( $this->context );
                                        }
                                        // Do any stats increment/watchlist stuff
                                        // Assume we're viewing the latest revision (this should always be the case with file cache)
                                        $this->context->getWikiPage()->doViewUpdates( $this->context->getUser() );
                                        // Tell OutputPage that output is taken care of
-                                       $this->context->getOutput()->disable();
+                                       $output->disable();
+
                                        return;
                                }
                        }
@@ -749,13 +811,24 @@ class MediaWiki {
                // Actually do the work of the request and build up any output
                $this->performRequest();
 
+               // GUI-ify and stash the page output in MediaWiki::doPreOutputCommit() while
+               // ChronologyProtector synchronizes DB positions or slaves accross all datacenters.
+               $buffer = null;
+               $outputWork = function () use ( $output, &$buffer ) {
+                       if ( $buffer === null ) {
+                               $buffer = $output->output( true );
+                       }
+
+                       return $buffer;
+               };
+
                // Now commit any transactions, so that unreported errors after
                // output() don't roll back the whole DB transaction and so that
                // we avoid having both success and error text in the response
-               $this->doPreOutputCommit();
+               $this->doPreOutputCommit( $outputWork );
 
-               // Output everything!
-               $this->context->getOutput()->output();
+               // Now send the actual output
+               print $outputWork();
        }
 
        /**
index 9b2d8da..d9230b0 100644 (file)
@@ -2214,10 +2214,16 @@ class OutputPage extends ContextSource {
        /**
         * Finally, all the text has been munged and accumulated into
         * the object, let's actually output it:
+        *
+        * @param bool $return Set to true to get the result as a string rather than sending it
+        * @return string|null
+        * @throws Exception
+        * @throws FatalError
+        * @throws MWException
         */
-       public function output() {
+       public function output( $return = false ) {
                if ( $this->mDoNothing ) {
-                       return;
+                       return $return ? '' : null;
                }
 
                $response = $this->getRequest()->response();
@@ -2253,7 +2259,7 @@ class OutputPage extends ContextSource {
                                }
                        }
 
-                       return;
+                       return $return ? '' : null;
                } elseif ( $this->mStatusCode ) {
                        $response->statusHeader( $this->mStatusCode );
                }
@@ -2322,8 +2328,12 @@ class OutputPage extends ContextSource {
 
                $this->sendCacheControl();
 
-               ob_end_flush();
-
+               if ( $return ) {
+                       return ob_get_clean();
+               } else {
+                       ob_end_flush();
+                       return null;
+               }
        }
 
        /**
index 8f78164..a5ae461 100644 (file)
@@ -520,7 +520,7 @@ class WebRequest {
         * @return int
         */
        public function getInt( $name, $default = 0 ) {
-               return intval( $this->getVal( $name, $default ) );
+               return intval( $this->getRawVal( $name, $default ) );
        }
 
        /**
@@ -532,7 +532,7 @@ class WebRequest {
         * @return int|null
         */
        public function getIntOrNull( $name ) {
-               $val = $this->getVal( $name );
+               $val = $this->getRawVal( $name );
                return is_numeric( $val )
                        ? intval( $val )
                        : null;
@@ -549,7 +549,7 @@ class WebRequest {
         * @return float
         */
        public function getFloat( $name, $default = 0.0 ) {
-               return floatval( $this->getVal( $name, $default ) );
+               return floatval( $this->getRawVal( $name, $default ) );
        }
 
        /**
@@ -562,7 +562,7 @@ class WebRequest {
         * @return bool
         */
        public function getBool( $name, $default = false ) {
-               return (bool)$this->getVal( $name, $default );
+               return (bool)$this->getRawVal( $name, $default );
        }
 
        /**
@@ -575,7 +575,8 @@ class WebRequest {
         * @return bool
         */
        public function getFuzzyBool( $name, $default = false ) {
-               return $this->getBool( $name, $default ) && strcasecmp( $this->getVal( $name ), 'false' ) !== 0;
+               return $this->getBool( $name, $default )
+                       && strcasecmp( $this->getRawVal( $name ), 'false' ) !== 0;
        }
 
        /**
@@ -589,7 +590,7 @@ class WebRequest {
        public function getCheck( $name ) {
                # Checkboxes and buttons are only present when clicked
                # Presence connotes truth, absence false
-               return $this->getVal( $name, null ) !== null;
+               return $this->getRawVal( $name, null ) !== null;
        }
 
        /**
index 9bc0b3a..55edd99 100644 (file)
@@ -110,17 +110,18 @@ class ApiLogin extends ApiBase {
                }
 
                // Try bot passwords
-               if ( $authRes === false && $this->getConfig()->get( 'EnableBotPasswords' ) &&
-                       strpos( $params['name'], BotPassword::getSeparator() ) !== false
+               if (
+                       $authRes === false && $this->getConfig()->get( 'EnableBotPasswords' ) &&
+                       ( $botLoginData = BotPassword::canonicalizeLoginData( $params['name'], $params['password'] ) )
                ) {
                        $status = BotPassword::login(
-                               $params['name'], $params['password'], $this->getRequest()
+                               $botLoginData[0], $botLoginData[1], $this->getRequest()
                        );
                        if ( $status->isOK() ) {
                                $session = $status->getValue();
                                $authRes = 'Success';
                                $loginType = 'BotPassword';
-                       } else {
+                       } elseif ( !$botLoginData[2] ) {
                                $authRes = 'Failed';
                                $message = $status->getMessage();
                                LoggerFactory::getInstance( 'authentication' )->info(
index 39c9957..1cdb49f 100644 (file)
@@ -33,6 +33,10 @@ class ChronologyProtector {
        protected $key;
        /** @var string Hash of client parameters */
        protected $clientId;
+       /** @var float|null Minimum UNIX timestamp of 1+ expected startup positions */
+       protected $waitForPosTime;
+       /** @var int Max seconds to wait on positions to appear */
+       protected $waitForPosTimeout = self::POS_WAIT_TIMEOUT;
        /** @var bool Whether to no-op all method calls */
        protected $enabled = true;
        /** @var bool Whether to check and wait on positions */
@@ -47,15 +51,22 @@ class ChronologyProtector {
        /** @var float[] Map of (DB master name => 1) */
        protected $shutdownTouchDBs = [];
 
+       /** @var integer Seconds to store positions */
+       const POSITION_TTL = 60;
+       /** @var integer Max time to wait for positions to appear */
+       const POS_WAIT_TIMEOUT = 5;
+
        /**
         * @param BagOStuff $store
         * @param array $client Map of (ip: <IP>, agent: <user-agent>)
+        * @param float $posTime UNIX timestamp
         * @since 1.27
         */
-       public function __construct( BagOStuff $store, array $client ) {
+       public function __construct( BagOStuff $store, array $client, $posTime = null ) {
                $this->store = $store;
                $this->clientId = md5( $client['ip'] . "\n" . $client['agent'] );
                $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId );
+               $this->waitForPosTime = $posTime;
        }
 
        /**
@@ -130,20 +141,23 @@ class ChronologyProtector {
         * Notify the ChronologyProtector that the LBFactory is done calling shutdownLB() for now.
         * May commit chronology data to persistent storage.
         *
+        * @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
         * @return DBMasterPos[] Empty on success; returns the (db name => position) map on failure
         */
-       public function shutdown() {
+       public function shutdown( callable $workCallback = null, $mode = 'sync' ) {
                if ( !$this->enabled ) {
                        return [];
                }
 
+               $store = $this->store;
                // Some callers might want to know if a user recently touched a DB.
                // These writes do not need to block on all datacenters receiving them.
                foreach ( $this->shutdownTouchDBs as $dbName => $unused ) {
-                       $this->store->set(
+                       $store->set(
                                $this->getTouchedKey( $this->store, $dbName ),
                                microtime( true ),
-                               BagOStuff::TTL_DAY
+                               $store::TTL_DAY
                        );
                }
 
@@ -159,29 +173,42 @@ class ChronologyProtector {
                // CP-protected writes should overwhemingly go to the master datacenter, so get DC-local
                // lock to merge the values. Use a DC-local get() and a synchronous all-DC set(). This
                // makes it possible for the BagOStuff class to write in parallel to all DCs with one RTT.
-               if ( $this->store->lock( $this->key, 3 ) ) {
-                       $ok = $this->store->set(
+               if ( $store->lock( $this->key, 3 ) ) {
+                       if ( $workCallback ) {
+                               // Let the store run the work before blocking on a replication sync barrier. By the
+                               // time it's done with the work, the barrier should be fast if replication caught up.
+                               $store->addBusyCallback( $workCallback );
+                       }
+                       $ok = $store->set(
                                $this->key,
-                               self::mergePositions( $this->store->get( $this->key ), $this->shutdownPositions ),
-                               BagOStuff::TTL_MINUTE,
-                               BagOStuff::WRITE_SYNC
+                               self::mergePositions( $store->get( $this->key ), $this->shutdownPositions ),
+                               self::POSITION_TTL,
+                               ( $mode === 'sync' ) ? $store::WRITE_SYNC : 0
                        );
-                       $this->store->unlock( $this->key );
+                       $store->unlock( $this->key );
                } else {
                        $ok = false;
                }
 
                if ( !$ok ) {
+                       $bouncedPositions = $this->shutdownPositions;
                        // Raced out too many times or stash is down
                        wfDebugLog( 'replication',
                                __METHOD__ . ": failed to save master pos for " .
                                implode( ', ', array_keys( $this->shutdownPositions ) ) . "\n"
                        );
-
-                       return $this->shutdownPositions;
+               } elseif ( $mode === 'sync' &&
+                       $store->getQoS( $store::ATTR_SYNCWRITES ) < $store::QOS_SYNCWRITES_BE
+               ) {
+                       // Positions may not be in all datacenters, force LBFactory to play it safe
+                       wfDebugLog( 'replication',
+                               __METHOD__ . ": store does not report ability to sync replicas. " );
+                       $bouncedPositions = $this->shutdownPositions;
+               } else {
+                       $bouncedPositions = [];
                }
 
-               return [];
+               return $bouncedPositions;
        }
 
        /**
@@ -212,7 +239,33 @@ class ChronologyProtector {
 
                $this->initialized = true;
                if ( $this->wait ) {
-                       $data = $this->store->get( $this->key );
+                       // 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 ) {
+                               $data = null;
+                               $loop = new WaitConditionLoop(
+                                       function () use ( &$data ) {
+                                               $data = $this->store->get( $this->key );
+
+                                               return ( self::minPosTime( $data ) >= $this->waitForPosTime )
+                                                       ? WaitConditionLoop::CONDITION_REACHED
+                                                       : WaitConditionLoop::CONDITION_CONTINUE;
+                                       },
+                                       $this->waitForPosTimeout
+                               );
+                               $result = $loop->invoke();
+                               $waitedMs = $loop->getLastWaitTime() * 1e3;
+
+                               if ( $result == $loop::CONDITION_REACHED ) {
+                                       $msg = "expected and found pos time {$this->waitForPosTime} ({$waitedMs}ms)";
+                               } else {
+                                       $msg = "expected but missed pos time {$this->waitForPosTime} ({$waitedMs}ms)";
+                               }
+                               wfDebugLog( 'replication', $msg );
+                       } else {
+                               $data = $this->store->get( $this->key );
+                       }
+
                        $this->startupPositions = $data ? $data['positions'] : [];
                        wfDebugLog( 'replication', __METHOD__ . ": key is {$this->key} (read)\n" );
                } else {
@@ -221,6 +274,24 @@ class ChronologyProtector {
                }
        }
 
+       /**
+        * @param array|bool $data
+        * @return float|null
+        */
+       private static function minPosTime( $data ) {
+               if ( !isset( $data['positions'] ) ) {
+                       return null;
+               }
+
+               $min = null;
+               foreach ( $data['positions'] as $pos ) {
+                       /** @var DBMasterPos $pos */
+                       $min = $min ? min( $pos->asOfTime(), $min ) : $pos->asOfTime();
+               }
+
+               return $min;
+       }
+
        /**
         * @param array|bool $curValue
         * @param DBMasterPos[] $shutdownPositions
index 4758cc7..74353b4 100644 (file)
@@ -51,7 +51,9 @@ abstract class LBFactory implements DestructibleService {
        /** @var callable[] */
        protected $replicationWaitCallbacks = [];
 
-       const SHUTDOWN_NO_CHRONPROT = 1; // don't save ChronologyProtector positions (for async code)
+       const SHUTDOWN_NO_CHRONPROT = 0; // don't save DB positions at all
+       const SHUTDOWN_CHRONPROT_ASYNC = 1; // save DB positions, but don't wait on remote DCs
+       const SHUTDOWN_CHRONPROT_SYNC = 2; // save DB positions, waiting on all DCs
 
        /**
         * Construct a factory based on a configuration array (typically from $wgLBFactoryConf)
@@ -87,7 +89,7 @@ abstract class LBFactory implements DestructibleService {
         * @see LoadBalancer::disable()
         */
        public function destroy() {
-               $this->shutdown();
+               $this->shutdown( self::SHUTDOWN_NO_CHRONPROT );
                $this->forEachLBCallMethod( 'disable' );
        }
 
@@ -199,12 +201,18 @@ abstract class LBFactory implements DestructibleService {
 
        /**
         * Prepare all tracked load balancers for shutdown
-        * @param integer $flags Supports SHUTDOWN_* flags
-        */
-       public function shutdown( $flags = 0 ) {
-               if ( !( $flags & self::SHUTDOWN_NO_CHRONPROT ) ) {
-                       $this->shutdownChronologyProtector( $this->chronProt );
+        * @param integer $mode One of the class SHUTDOWN_* constants
+        * @param callable|null $workCallback Work to mask ChronologyProtector writes
+        */
+       public function shutdown(
+               $mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = null
+       ) {
+               if ( $mode === self::SHUTDOWN_CHRONPROT_SYNC ) {
+                       $this->shutdownChronologyProtector( $this->chronProt, $workCallback, 'sync' );
+               } elseif ( $mode === self::SHUTDOWN_CHRONPROT_ASYNC ) {
+                       $this->shutdownChronologyProtector( $this->chronProt, null, 'async' );
                }
+
                $this->commitMasterChanges( __METHOD__ ); // sanity
        }
 
@@ -387,13 +395,14 @@ abstract class LBFactory implements DestructibleService {
 
        /**
         * Determine if any master connection has pending/written changes from this request
+        * @param float $age How many seconds ago is "recent" [defaults to LB lag wait timeout]
         * @return bool
         * @since 1.27
         */
-       public function hasOrMadeRecentMasterChanges() {
+       public function hasOrMadeRecentMasterChanges( $age = null ) {
                $ret = false;
-               $this->forEachLB( function ( LoadBalancer $lb ) use ( &$ret ) {
-                       $ret = $ret || $lb->hasOrMadeRecentMasterChanges();
+               $this->forEachLB( function ( LoadBalancer $lb ) use ( $age, &$ret ) {
+                       $ret = $ret || $lb->hasOrMadeRecentMasterChanges( $age );
                } );
                return $ret;
        }
@@ -586,8 +595,9 @@ abstract class LBFactory implements DestructibleService {
                        ObjectCache::getMainStashInstance(),
                        [
                                'ip' => $request->getIP(),
-                               'agent' => $request->getHeader( 'User-Agent' )
-                       ]
+                               'agent' => $request->getHeader( 'User-Agent' ),
+                       ],
+                       $request->getFloat( 'cpPosTime', null )
                );
                if ( PHP_SAPI === 'cli' ) {
                        $chronProt->setEnabled( false );
@@ -601,15 +611,26 @@ abstract class LBFactory implements DestructibleService {
        }
 
        /**
+        * Get and record all of the staged DB positions into persistent memory storage
+        *
         * @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
         */
-       protected function shutdownChronologyProtector( ChronologyProtector $cp ) {
-               // Get all the master positions needed
+       protected function shutdownChronologyProtector(
+               ChronologyProtector $cp, $workCallback, $mode
+       ) {
+               // Record all the master positions needed
                $this->forEachLB( function ( LoadBalancer $lb ) use ( $cp ) {
                        $cp->shutdownLB( $lb );
                } );
-               // Write them to the stash
-               $unsavedPositions = $cp->shutdown();
+               // 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 );
+               if ( $unsavedPositions && $workCallback ) {
+                       // Invoke callback in case it did not cache the result yet
+                       $workCallback(); // work now to block for less time in waitForAll()
+               }
                // If the positions failed to write to the stash, at least wait on local datacenter
                // replica DBs to catch up before responding. Even if there are several DCs, this increases
                // the chance that the user will see their own changes immediately afterwards. As long
@@ -631,6 +652,29 @@ abstract class LBFactory implements DestructibleService {
                }
        }
 
+       /**
+        * Append ?cpPosTime parameter to a URL for ChronologyProtector purposes if needed
+        *
+        * Note that unlike cookies, this works accross domains
+        *
+        * @param string $url
+        * @param float $time UNIX timestamp just before shutdown() was called
+        * @return string
+        * @since 1.28
+        */
+       public function appendPreShutdownTimeAsQuery( $url, $time ) {
+               $usedCluster = 0;
+               $this->forEachLB( function ( LoadBalancer $lb ) use ( &$usedCluster ) {
+                       $usedCluster |= ( $lb->getServerCount() > 1 );
+               } );
+
+               if ( !$usedCluster ) {
+                       return $url; // no master/replica clusters touched
+               }
+
+               return wfAppendQuery( $url, [ 'cpPosTime' => $time ] );
+       }
+
        /**
         * Close all open database connections on all open load balancers.
         * @since 1.28
@@ -638,5 +682,4 @@ abstract class LBFactory implements DestructibleService {
        public function closeAll() {
                $this->forEachLBCallMethod( 'closeAll', [] );
        }
-
 }
index 8d57562..de5f410 100644 (file)
@@ -142,6 +142,20 @@ class JobQueueGroup {
                                $this->cache->clear( 'queues-ready' );
                        }
                }
+
+               $cache = ObjectCache::getLocalClusterInstance();
+               $cache->set(
+                       $cache->makeGlobalKey( 'jobqueue', $this->wiki, 'hasjobs', self::TYPE_ANY ),
+                       'true',
+                       15
+               );
+               if ( array_intersect( array_keys( $jobsByType ), $this->getDefaultQueueTypes() ) ) {
+                       $cache->set(
+                               $cache->makeGlobalKey( 'jobqueue', $this->wiki, 'hasjobs', self::TYPE_DEFAULT ),
+                               'true',
+                               15
+                       );
+               }
        }
 
        /**
@@ -298,8 +312,8 @@ class JobQueueGroup {
         * @since 1.23
         */
        public function queuesHaveJobs( $type = self::TYPE_ANY ) {
-               $key = wfMemcKey( 'jobqueue', 'queueshavejobs', $type );
                $cache = ObjectCache::getLocalClusterInstance();
+               $key = $cache->makeGlobalKey( 'jobqueue', $this->wiki, 'hasjobs', $type );
 
                $value = $cache->get( $key );
                if ( $value === false ) {
index 62c4fa5..0e09f16 100644 (file)
@@ -47,6 +47,12 @@ interface IExpiringStore {
        // Medium attributes constants related to emulation or media type
        const ATTR_EMULATION = 1;
        const QOS_EMULATION_SQL = 1;
+       // Medium attributes constants related to replica consistency
+       const ATTR_SYNCWRITES = 2; // SYNC_WRITES flag support
+       const QOS_SYNCWRITES_NONE = 1; // replication only supports eventual consistency or less
+       const QOS_SYNCWRITES_BE = 2; // best effort synchronous with limited retries
+       const QOS_SYNCWRITES_QC = 3; // write quorum applied directly to state machines where R+W > N
+       const QOS_SYNCWRITES_SS = 4; // strict-serializable, nodes refuse reads if possible stale
        // Generic "unknown" value that is useful for comparisons (e.g. always good enough)
        const QOS_UNKNOWN = INF;
 }
index 5967441..6973392 100644 (file)
@@ -30,6 +30,12 @@ class MemcachedBagOStuff extends BagOStuff {
        /** @var MemcachedClient|Memcached */
        protected $client;
 
+       function __construct( array $params ) {
+               parent::__construct( $params );
+
+               $this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_BE; // unreliable
+       }
+
        /**
         * Fill in some defaults for missing keys in $params.
         *
index 9fc3fe1..ae91be5 100644 (file)
@@ -51,6 +51,8 @@ class RESTBagOStuff extends BagOStuff {
                }
                // Make sure URL ends with /
                $this->url = rtrim( $params['url'], '/' ) . '/';
+               // Default config, R+W > N; no locks on reads though; writes go straight to state-machine
+               $this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_QC;
        }
 
        /**
index f9d201f..64cd686 100644 (file)
@@ -83,6 +83,8 @@ class RedisBagOStuff extends BagOStuff {
                } else {
                        $this->automaticFailover = true;
                }
+
+               $this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_NONE;
        }
 
        protected function doGet( $key, $flags = 0 ) {
index 3baae50..d06213f 100644 (file)
@@ -97,6 +97,7 @@ class SqlBagOStuff extends BagOStuff {
                parent::__construct( $params );
 
                $this->attrMap[self::ATTR_EMULATION] = self::QOS_EMULATION_SQL;
+               $this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_NONE;
 
                if ( isset( $params['servers'] ) ) {
                        $this->serverInfos = [];
@@ -119,6 +120,7 @@ class SqlBagOStuff extends BagOStuff {
                        // Default to using the main wiki's database servers
                        $this->serverInfos = false;
                        $this->numServers = 1;
+                       $this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_BE;
                }
                if ( isset( $params['purgePeriod'] ) ) {
                        $this->purgePeriod = intval( $params['purgePeriod'] );
index 61ab642..9975e41 100644 (file)
@@ -290,9 +290,7 @@ class SpecialBotPasswords extends FormSpecialPage {
                ] );
 
                if ( $this->operation === 'insert' || !empty( $data['resetPassword'] ) ) {
-                       $this->password = PasswordFactory::generateRandomPasswordString(
-                               max( 32, $this->getConfig()->get( 'MinimalPasswordLength' ) )
-                       );
+                       $this->password = BotPassword::generatePassword( $this->getConfig() );
                        $passwordFactory = new PasswordFactory();
                        $passwordFactory->init( RequestContext::getMain()->getConfig() );
                        $password = $passwordFactory->newFromPlaintext( $this->password );
@@ -335,7 +333,9 @@ class SpecialBotPasswords extends FormSpecialPage {
                        $out->addWikiMsg(
                                'botpasswords-newpassword',
                                htmlspecialchars( $username . $sep . $this->par ),
-                               htmlspecialchars( $this->password )
+                               htmlspecialchars( $this->password ),
+                               htmlspecialchars( $username ),
+                               htmlspecialchars( $this->par . $sep . $this->password )
                        );
                        $this->password = null;
                }
index 4ce3cde..0bbe12e 100644 (file)
@@ -388,6 +388,44 @@ class BotPassword implements IDBAccessObject {
                return (bool)$dbw->affectedRows();
        }
 
+       /**
+        * Returns a (raw, unhashed) random password string.
+        * @param Config $config
+        * @return string
+        */
+       public static function generatePassword( $config ) {
+               return PasswordFactory::generateRandomPasswordString(
+                       max( 32, $config->get( 'MinimalPasswordLength' ) ) );
+       }
+
+       /**
+        * There are two ways to login with a bot password: "username@appId", "password" and
+        * "username", "appId@password". Transform it so it is always in the first form.
+        * Returns [bot username, bot password, could be normal password?] where the last one is a flag
+        * meaning this could either be a bot password or a normal password, it cannot be decided for
+        * certain (although in such cases it almost always will be a bot password).
+        * If this cannot be a bot password login just return false.
+        * @param string $username
+        * @param string $password
+        * @return array|false
+        */
+       public static function canonicalizeLoginData( $username, $password ) {
+               $sep = BotPassword::getSeparator();
+               if ( strpos( $username, $sep ) !== false ) {
+                       // the separator is not valid in usernames so this must be a bot login
+                       return [ $username, $password, false ];
+               } elseif ( strlen( $password ) > 32 && strpos( $password, $sep ) !== false ) {
+                       // the strlen check helps minimize the password information obtainable from timing
+                       $segments = explode( $sep, $password );
+                       $password = array_pop( $segments );
+                       $appId = implode( $sep, $segments );
+                       if ( preg_match( '/^[0-9a-w]{32,}$/', $password ) ) {
+                               return [ $username . $sep . $appId, $password, true ];
+                       }
+               }
+               return false;
+       }
+
        /**
         * Try to log the user in
         * @param string $username Combined user name and app ID
index 7109a4a..2af0324 100644 (file)
@@ -3613,31 +3613,69 @@ class User implements IDBAccessObject {
         * @note If the user doesn't have 'editmywatchlist', this will do nothing.
         */
        public function clearAllNotifications() {
-               if ( wfReadOnly() ) {
-                       return;
-               }
-
+               global $wgUseEnotif, $wgShowUpdatedMarker;
                // Do nothing if not allowed to edit the watchlist
-               if ( !$this->isAllowed( 'editmywatchlist' ) ) {
+               if ( wfReadOnly() || !$this->isAllowed( 'editmywatchlist' ) ) {
                        return;
                }
 
-               global $wgUseEnotif, $wgShowUpdatedMarker;
                if ( !$wgUseEnotif && !$wgShowUpdatedMarker ) {
                        $this->setNewtalk( false );
                        return;
                }
+
                $id = $this->getId();
-               if ( $id != 0 ) {
-                       $dbw = wfGetDB( DB_MASTER );
-                       $dbw->update( 'watchlist',
-                               [ /* SET */ 'wl_notificationtimestamp' => null ],
-                               [ /* WHERE */ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ],
-                               __METHOD__
-                       );
-                       // We also need to clear here the "you have new message" notification for the own user_talk page;
-                       // it's cleared one page view later in WikiPage::doViewUpdates().
+               if ( !$id ) {
+                       return;
                }
+
+               $dbw = wfGetDB( DB_MASTER );
+               $asOfTimes = array_unique( $dbw->selectFieldValues(
+                       'watchlist',
+                       'wl_notificationtimestamp',
+                       [ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ],
+                       __METHOD__,
+                       [ 'ORDER BY' => 'wl_notificationtimestamp DESC', 'LIMIT' => 500 ]
+               ) );
+               if ( !$asOfTimes ) {
+                       return;
+               }
+               // Immediately update the most recent touched rows, which hopefully covers what
+               // the user sees on the watchlist page before pressing "mark all pages visited"....
+               $dbw->update(
+                       'watchlist',
+                       [ 'wl_notificationtimestamp' => null ],
+                       [ 'wl_user' => $id, 'wl_notificationtimestamp' => $asOfTimes ],
+                       __METHOD__
+               );
+               // ...and finish the older ones in a post-send update with lag checks...
+               DeferredUpdates::addUpdate( new AutoCommitUpdate(
+                       $dbw,
+                       __METHOD__,
+                       function () use ( $dbw, $id ) {
+                               global $wgUpdateRowsPerQuery;
+
+                               $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+                               $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
+                               $asOfTimes = array_unique( $dbw->selectFieldValues(
+                                       'watchlist',
+                                       'wl_notificationtimestamp',
+                                       [ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ],
+                                       __METHOD__
+                               ) );
+                               foreach ( array_chunk( $asOfTimes, $wgUpdateRowsPerQuery ) as $asOfTimeBatch ) {
+                                       $dbw->update(
+                                               'watchlist',
+                                               [ 'wl_notificationtimestamp' => null ],
+                                               [ 'wl_user' => $id, 'wl_notificationtimestamp' => $asOfTimeBatch ],
+                                               __METHOD__
+                                       );
+                                       $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket );
+                               }
+                       }
+               ) );
+               // We also need to clear here the "you have new message" notification for the own
+               // user_talk page; it's cleared one page view later in WikiPage::doViewUpdates().
        }
 
        /**
index cc7466b..558a452 100644 (file)
        "botpasswords-updated-body": "The bot password for bot name \"$1\" of user \"$2\" was updated.",
        "botpasswords-deleted-title": "Bot password deleted",
        "botpasswords-deleted-body": "The bot password for bot name \"$1\" of user \"$2\" was deleted.",
-       "botpasswords-newpassword": "The new password to log in with <strong>$1</strong> is <strong>$2</strong>. <em>Please record this for future reference.</em>",
+       "botpasswords-newpassword": "The new password to log in with <strong>$1</strong> is <strong>$2</strong>. <em>Please record this for future reference.</em> <br> (For old bots which require the login name to be the same as the eventual username, you can also use <strong>$3</strong> as username and <strong>$4</strong> as password.)",
        "botpasswords-no-provider": "BotPasswordsSessionProvider is not available.",
        "botpasswords-restriction-failed": "Bot password restrictions prevent this login.",
        "botpasswords-invalid-name": "The username specified does not contain the bot password separator (\"$1\").",
        "invalid-content-data": "Invalid content data",
        "content-not-allowed-here": "\"$1\" content is not allowed on page [[$2]]",
        "editwarning-warning": "Leaving this page may cause you to lose any changes you have made.\nIf you are logged in, you can disable this warning in the \"{{int:prefs-editing}}\" section of your preferences.",
+       "editpage-invalidcontentmodel-title": "Content model not supported",
+       "editpage-invalidcontentmodel-text": "The content model \"$1\" is not a supported.",
        "editpage-notsupportedcontentformat-title": "Content format not supported",
        "editpage-notsupportedcontentformat-text": "The content format $1 is not supported by the content model $2.",
        "content-model-wikitext": "wikitext",
index 924d25f..bd8a904 100644 (file)
        "botpasswords-updated-body": "Success message when a bot password is updated. Parameters:\n* $1 - Bot name\n* $2 - User name",
        "botpasswords-deleted-title": "Title of the success page when a bot password is deleted.",
        "botpasswords-deleted-body": "Success message when a bot password is deleted. Parameters:\n* $1 - Bot name\n* $2 - User name",
-       "botpasswords-newpassword": "Success message to display the new password when a bot password is created or updated. Parameters:\n* $1 - User name to be used for login.\n* $2 - Password to be used for login.",
+       "botpasswords-newpassword": "Success message to display the new password when a bot password is created or updated. Parameters:\n* $1 - User name to be used for login.\n* $2 - Password to be used for login.\n* $3, $4 - an alternative version of the user name and password, respectively, which is less preferred, but more compatible with old bots.",
        "botpasswords-no-provider": "Error message when login is attempted but the BotPasswordsSessionProvider is not included in <code>$wgSessionProviders</code>.",
        "botpasswords-restriction-failed": "Error message when login is rejected because the configured restrictions were not satisfied.",
        "botpasswords-invalid-name": "Error message when a username lacking the separator character is passed to BotPassword. Parameters:\n* $1 - The separator character.",
        "invalid-content-data": "Error message indicating that the page's content can not be saved because it is invalid. This may occurr for content types with internal consistency constraints.",
        "content-not-allowed-here": "Error message indicating that the desired content model is not supported in given localtion.\n* $1 - the human readable name of the content model: {{msg-mw|Content-model-wikitext}}, {{msg-mw|Content-model-javascript}}, {{msg-mw|Content-model-css}} or {{msg-mw|Content-model-text}}\n* $2 - the title of the page in question",
        "editwarning-warning": "Uses {{msg-mw|Prefs-editing}}",
+       "editpage-invalidcontentmodel-title": "Title of error page shown when using an unrecognized content model on EditPage",
+       "editpage-invalidcontentmodel-text": "Error message shown when using an unrecognized content model on EditPage. $1 is the user's invalid input",
        "editpage-notsupportedcontentformat-title": "Title of error page shown when using an incompatible format on EditPage.\n\nUsed as title for the following error message:\n* {{msg-mw|Editpage-notsupportedcontentformat-text}}.",
        "editpage-notsupportedcontentformat-text": "Error message shown when using an incompatible format on EditPage.\n\nThe title for this error is {{msg-mw|Editpage-notsupportedcontentformat-title}}.\n\nParameters:\n* $1 - the format id\n* $2 - the content model name",
        "content-model-wikitext": "Name for the wikitext content model, used when decribing what type of content a page contains.\n\nThis message is substituted in:\n*{{msg-mw|Bad-target-model}}\n*{{msg-mw|Content-not-allowed-here}}",
index 95bd089..890fe45 100644 (file)
@@ -121,4 +121,4 @@ wfLogProfilingData();
 // Commit and close up!
 $factory = wfGetLBFactory();
 $factory->commitMasterChanges( 'doMaintenance' );
-$factory->shutdown();
+$factory->shutdown( $factory::SHUTDOWN_NO_CHRONPROT );
index cfd5f78..d637704 100644 (file)
@@ -228,6 +228,33 @@ class BotPasswordTest extends MediaWikiTestCase {
                $this->assertNotNull( BotPassword::newFromCentralId( 43, 'BotPassword' ) );
        }
 
+       /**
+        * @dataProvider provideCanonicalizeLoginData
+        */
+       public function testCanonicalizeLoginData( $username, $password, $expectedResult ) {
+               $result = BotPassword::canonicalizeLoginData( $username, $password );
+               if ( is_array( $expectedResult ) ) {
+                       $this->assertArrayEquals( $expectedResult, $result, true, true );
+               } else {
+                       $this->assertSame( $expectedResult, $result );
+               }
+       }
+
+       public function provideCanonicalizeLoginData() {
+               return [
+                       [ 'user', 'pass', false ],
+                       [ 'user', 'abc@def', false ],
+                       [ 'user@bot', '12345678901234567890123456789012',
+                               [ 'user@bot', '12345678901234567890123456789012', false ] ],
+                       [ 'user', 'bot@12345678901234567890123456789012',
+                               [ 'user@bot', '12345678901234567890123456789012', true ] ],
+                       [ 'user', 'bot@12345678901234567890123456789012345',
+                               [ 'user@bot', '12345678901234567890123456789012345', true ] ],
+                       [ 'user', 'bot@x@12345678901234567890123456789012',
+                               [ 'user@bot@x', '12345678901234567890123456789012', true ] ],
+               ];
+       }
+
        public function testLogin() {
                // Test failure when bot passwords aren't enabled
                $this->setMwGlobals( 'wgEnableBotPasswords', false );
diff --git a/tests/phpunit/structure/ContentHandlerSanityTest.php b/tests/phpunit/structure/ContentHandlerSanityTest.php
new file mode 100644 (file)
index 0000000..98a0fbb
--- /dev/null
@@ -0,0 +1,53 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+class ContentHandlerSanityTest extends MediaWikiTestCase {
+
+       public static function provideHandlers() {
+               $models = ContentHandler::getContentModels();
+               $handlers = [];
+               foreach ( $models as $model ) {
+                       $handlers[] = [ ContentHandler::getForModelID( $model ) ];
+               }
+
+               return $handlers;
+       }
+
+       /**
+        * @dataProvider provideHandlers
+        * @param ContentHandler $handler
+        */
+       public function testMakeEmptyContent( ContentHandler $handler ) {
+               $content = $handler->makeEmptyContent();
+               $this->assertInstanceOf( Content::class, $content );
+               if ( $handler instanceof TextContentHandler ) {
+                       // TextContentHandler::getContentClass() is protected, so bypass
+                       // that restriction
+                       $testingWrapper = TestingAccessWrapper::newFromObject( $handler );
+                       $this->assertInstanceOf( $testingWrapper->getContentClass(), $content );
+               }
+
+               $handlerClass = get_class( $handler );
+               $contentClass = get_class( $content );
+
+               $this->assertTrue(
+                       $content->isValid(),
+                       "$handlerClass::makeEmptyContent() did not return a valid content ($contentClass::isValid())"
+               );
+       }
+}