Revert "MessageCache invalidation improvements" (temporary)
authorAndrew Green <andrew.green.df@gmail.com>
Tue, 29 Nov 2016 01:06:00 +0000 (19:06 -0600)
committerAndrew Green <andrew.green.df@gmail.com>
Tue, 29 Nov 2016 01:31:42 +0000 (19:31 -0600)
This reverts commit 9339a08b72c918d7743a9cb286161adb9399a77b
(Change-Id: Idc337a787171949c4f70186b13d7b65304c9b57f).

This is a temporary revert to prevent the change's inclusion in
wmf/1.29.0-wmf.4. That branch is scheduled to be deployed to WMF wikis
during the first week of the WMF's 2016 year-end fundraiser. Since
CentralNotice relies on MessageCache to fetch fundraising banners,
it is preferable not to deploy changes of any significant complexity in
that system at this time.

The original change should be re-applied at a later date. Sincere
apologies to the change's authors! :)

Change-Id: I8330838bbe03ce6ed38fa2e755b44519211d9d43

includes/cache/MessageCache.php
includes/page/WikiPage.php

index 3f78d9a..0c2f9de 100644 (file)
@@ -22,7 +22,6 @@
  */
 use MediaWiki\MediaWikiServices;
 use Wikimedia\ScopedCallback;
-use MediaWiki\Logger\LoggerFactory;
 
 /**
  * MediaWiki message cache structure version.
@@ -53,11 +52,6 @@ class MessageCache {
         */
        protected $mCache;
 
-       /**
-        * @var bool[] Map of (language code => boolean)
-        */
-       protected $mCacheVolatile = [];
-
        /**
         * Should mean that database cannot be used, but check
         * @var bool $mDisable
@@ -71,12 +65,10 @@ class MessageCache {
        protected $mExpiry;
 
        /**
-        * Message cache has its own parser which it uses to transform messages
-        * @var ParserOptions
+        * Message cache has its own parser which it uses to transform
+        * messages.
         */
-       protected $mParserOptions;
-       /** @var Parser */
-       protected $mParser;
+       protected $mParserOptions, $mParser;
 
        /**
         * Variable for tracking which variables are already loaded
@@ -137,7 +129,6 @@ class MessageCache {
         */
        public static function normalizeKey( $key ) {
                global $wgContLang;
-
                $lckey = strtr( $key, ' ', '_' );
                if ( ord( $lckey ) < 128 ) {
                        $lckey[0] = strtolower( $lckey[0] );
@@ -267,7 +258,6 @@ class MessageCache {
                # Hash of the contents is stored in memcache, to detect if data-center cache
                # or local cache goes out of date (e.g. due to replace() on some other server)
                list( $hash, $hashVolatile ) = $this->getValidationHash( $code );
-               $this->mCacheVolatile[$code] = $hashVolatile;
 
                # Try the local cache and check against the cluster hash key...
                $cache = $this->getLocalCache( $code );
@@ -483,16 +473,9 @@ class MessageCache {
                $bigConds[] = 'page_len > ' . intval( $wgMaxMsgCacheEntrySize );
 
                # Load titles for all oversized pages in the MediaWiki namespace
-               $res = $dbr->select(
-                       'page',
-                       [ 'page_title', 'page_latest' ],
-                       $bigConds,
-                       __METHOD__ . "($code)-big"
-               );
+               $res = $dbr->select( 'page', 'page_title', $bigConds, __METHOD__ . "($code)-big" );
                foreach ( $res as $row ) {
                        $cache[$row->page_title] = '!TOO BIG';
-                       // At least include revision ID so page changes are reflected in the hash
-                       $cache['EXCESSIVE'][$row->page_title] = $row->page_latest;
                }
 
                # Conditions to load the remaining pages with their contents
@@ -537,7 +520,7 @@ class MessageCache {
         * Updates cache as necessary when message page is changed
         *
         * @param string|bool $title Name of the page changed (false if deleted)
-        * @param string|bool $text New contents of the page (false if deleted)
+        * @param mixed $text New contents of the page.
         */
        public function replace( $title, $text ) {
                global $wgMaxMsgCacheEntrySize, $wgContLang, $wgLanguageCode;
@@ -557,32 +540,31 @@ class MessageCache {
                // a self-deadlock. This is safe as no reads happen *directly* in this
                // method between getReentrantScopedLock() and load() below. There is
                // no risk of data "changing under our feet" for replace().
-               $scopedLock = $this->getReentrantScopedLock( wfMemcKey( 'messages', $code ) );
-               // Load the messages from the master DB to avoid race conditions
+               $cacheKey = wfMemcKey( 'messages', $code );
+               $scopedLock = $this->getReentrantScopedLock( $cacheKey );
                $this->load( $code, self::FOR_UPDATE );
 
-               // Load the new value into the process cache...
+               $titleKey = wfMemcKey( 'messages', 'individual', $title );
                if ( $text === false ) {
+                       // Article was deleted
                        $this->mCache[$code][$title] = '!NONEXISTENT';
+                       $this->wanCache->delete( $titleKey );
                } elseif ( strlen( $text ) > $wgMaxMsgCacheEntrySize ) {
+                       // Check for size
                        $this->mCache[$code][$title] = '!TOO BIG';
-                       // Pre-fill the individual key cache with the known latest message text
-                       $key = $this->wanCache->makeKey( 'messages-big', $this->mCache[$code]['HASH'], $title );
-                       $this->wanCache->set( $key, " $text", $this->mExpiry );
+                       $this->wanCache->set( $titleKey, ' ' . $text, $this->mExpiry );
                } else {
                        $this->mCache[$code][$title] = ' ' . $text;
+                       $this->wanCache->delete( $titleKey );
                }
-               // Mark this cache as definitely being "latest" (non-volatile) so
-               // load() calls do not try to refresh the cache with replica DB data
+
+               // Mark this cache as definitely "latest" (non-volatile) so
+               // load() calls do try to refresh the cache with replica DB data
                $this->mCache[$code]['LATEST'] = time();
 
                // Update caches if the lock was acquired
                if ( $scopedLock ) {
                        $this->saveToCaches( $this->mCache[$code], 'all', $code );
-               } else {
-                       LoggerFactory::getInstance( 'MessageCache' )->error(
-                               __METHOD__ . ': could not acquire lock to update {title} ({code})',
-                               [ 'title' => $title, 'code' => $code ] );
                }
 
                ScopedCallback::consume( $scopedLock );
@@ -662,26 +644,24 @@ class MessageCache {
        protected function getValidationHash( $code ) {
                $curTTL = null;
                $value = $this->wanCache->get(
-                       $this->wanCache->makeKey( 'messages', $code, 'hash', 'v1' ),
+                       wfMemcKey( 'messages', $code, 'hash', 'v1' ),
                        $curTTL,
                        [ wfMemcKey( 'messages', $code ) ]
                );
 
-               if ( $value ) {
+               if ( !$value ) {
+                       // No hash found at all; cache must regenerate to be safe
+                       $hash = false;
+                       $expired = true;
+               } else {
                        $hash = $value['hash'];
-                       if ( ( time() - $value['latest'] ) < WANObjectCache::TTL_MINUTE ) {
-                               // Cache was recently updated via replace() and should be up-to-date.
-                               // That method is only called in the primary datacenter and uses FOR_UPDATE.
-                               // Also, it is unlikely that the current datacenter is *now* secondary one.
+                       if ( ( time() - $value['latest'] ) < WANObjectCache::HOLDOFF_TTL ) {
+                               // Cache was recently updated via replace() and should be up-to-date
                                $expired = false;
                        } else {
                                // See if the "check" key was bumped after the hash was generated
                                $expired = ( $curTTL < 0 );
                        }
-               } else {
-                       // No hash found at all; cache must regenerate to be safe
-                       $hash = false;
-                       $expired = true;
                }
 
                return [ $hash, $expired ];
@@ -691,15 +671,14 @@ class MessageCache {
         * Set the md5 used to validate the local disk cache
         *
         * If $cache has a 'LATEST' UNIX timestamp key, then the hash will not
-        * be treated as "volatile" by getValidationHash() for the next few seconds.
-        * This is triggered when $cache is generated using FOR_UPDATE mode.
+        * be treated as "volatile" by getValidationHash() for the next few seconds
         *
         * @param string $code
         * @param array $cache Cached messages with a version
         */
        protected function setValidationHash( $code, array $cache ) {
                $this->wanCache->set(
-                       $this->wanCache->makeKey( 'messages', $code, 'hash', 'v1' ),
+                       wfMemcKey( 'messages', $code, 'hash', 'v1' ),
                        [
                                'hash' => $cache['HASH'],
                                'latest' => isset( $cache['LATEST'] ) ? $cache['LATEST'] : 0
@@ -862,7 +841,6 @@ class MessageCache {
         */
        private function getMessageForLang( $lang, $lckey, $useDB, &$alreadyTried ) {
                global $wgContLang;
-
                $langcode = $lang->getCode();
 
                // Try checking the database for the requested language
@@ -921,7 +899,6 @@ class MessageCache {
         */
        private function getMessagePageName( $langcode, $uckey ) {
                global $wgLanguageCode;
-
                if ( $langcode === $wgLanguageCode ) {
                        // Messages created in the content language will not have the /lang extension
                        return $uckey;
@@ -947,7 +924,8 @@ class MessageCache {
                if ( isset( $this->mCache[$code][$title] ) ) {
                        $entry = $this->mCache[$code][$title];
                        if ( substr( $entry, 0, 1 ) === ' ' ) {
-                               // The message exists, so make sure a string is returned.
+                               // The message exists, so make sure a string
+                               // is returned.
                                return (string)substr( $entry, 1 );
                        } elseif ( $entry === '!NONEXISTENT' ) {
                                return false;
@@ -966,19 +944,17 @@ class MessageCache {
                }
 
                // Try the individual message cache
-               $titleKey = $this->wanCache->makeKey( 'messages-big', $this->mCache[$code]['HASH'], $title );
-
-               if ( $this->mCacheVolatile[$code] ) {
-                       $entry = false;
-                       // Make sure that individual keys respect the WAN cache holdoff period too
-                       LoggerFactory::getInstance( 'MessageCache' )->debug(
-                               __METHOD__ . ': loading volatile key \'{titleKey}\'',
-                               [ 'titleKey' => $titleKey, 'code' => $code ] );
-               } else {
-                       $entry = $this->wanCache->get( $titleKey );
-               }
+               $titleKey = wfMemcKey( 'messages', 'individual', $title );
 
-               if ( $entry !== false ) {
+               $curTTL = null;
+               $entry = $this->wanCache->get(
+                       $titleKey,
+                       $curTTL,
+                       [ wfMemcKey( 'messages', $code ) ]
+               );
+               $entry = ( $curTTL >= 0 ) ? $entry : false;
+
+               if ( $entry ) {
                        if ( substr( $entry, 0, 1 ) === ' ' ) {
                                $this->mCache[$code][$title] = $entry;
                                // The message exists, so make sure a string is returned
@@ -993,7 +969,7 @@ class MessageCache {
                        }
                }
 
-               // Try loading the message from the database
+               // Try loading it from the database
                $dbr = wfGetDB( DB_REPLICA );
                $cacheOpts = Database::getCacheSetOptions( $dbr );
                // Use newKnownCurrent() to avoid querying revision/user tables
@@ -1010,18 +986,32 @@ class MessageCache {
 
                if ( $revision ) {
                        $content = $revision->getContent();
-                       if ( $content ) {
-                               $message = $this->getMessageTextFromContent( $content );
-                               if ( is_string( $message ) ) {
+                       if ( !$content ) {
+                               // A possibly temporary loading failure.
+                               wfDebugLog(
+                                       'MessageCache',
+                                       __METHOD__ . ": failed to load message page text for {$title} ($code)"
+                               );
+                               $message = null; // no negative caching
+                       } else {
+                               // XXX: Is this the right way to turn a Content object into a message?
+                               // NOTE: $content is typically either WikitextContent, JavaScriptContent or
+                               //       CssContent. MessageContent is *not* used for storing messages, it's
+                               //       only used for wrapping them when needed.
+                               $message = $content->getWikitextForTransclusion();
+
+                               if ( $message === false || $message === null ) {
+                                       wfDebugLog(
+                                               'MessageCache',
+                                               __METHOD__ . ": message content doesn't provide wikitext "
+                                                       . "(content model: " . $content->getModel() . ")"
+                                       );
+
+                                       $message = false; // negative caching
+                               } else {
                                        $this->mCache[$code][$title] = ' ' . $message;
                                        $this->wanCache->set( $titleKey, ' ' . $message, $this->mExpiry, $cacheOpts );
                                }
-                       } else {
-                               // A possibly temporary loading failure
-                               LoggerFactory::getInstance( 'MessageCache' )->warning(
-                                       __METHOD__ . ': failed to load message page text for \'{titleKey}\'',
-                                       [ 'titleKey' => $titleKey, 'code' => $code ] );
-                               $message = null; // no negative caching
                        }
                } else {
                        $message = false; // negative caching
@@ -1073,7 +1063,6 @@ class MessageCache {
         */
        function getParser() {
                global $wgParser, $wgParserConf;
-
                if ( !$this->mParser && isset( $wgParser ) ) {
                        # Do some initialisation so that we don't have to do it twice
                        $wgParser->firstCallInit();
@@ -1101,8 +1090,6 @@ class MessageCache {
        public function parse( $text, $title = null, $linestart = true,
                $interface = false, $language = null
        ) {
-               global $wgTitle;
-
                if ( $this->mInParser ) {
                        return htmlspecialchars( $text );
                }
@@ -1117,6 +1104,7 @@ class MessageCache {
                $popts->setTargetLanguage( $language );
 
                if ( !$title || !$title instanceof Title ) {
+                       global $wgTitle;
                        wfDebugLog( 'GlobalTitleFail', __METHOD__ . ' called by ' .
                                wfGetAllCallers( 6 ) . ' with no title set.' );
                        $title = $wgTitle;
@@ -1204,7 +1192,6 @@ class MessageCache {
         */
        public function getAllMessageKeys( $code ) {
                global $wgContLang;
-
                $this->load( $code );
                if ( !isset( $this->mCache[$code] ) ) {
                        // Apparently load() failed
@@ -1214,61 +1201,10 @@ class MessageCache {
                $cache = $this->mCache[$code];
                unset( $cache['VERSION'] );
                unset( $cache['EXPIRY'] );
-               unset( $cache['EXCESSIVE'] );
                // Remove any !NONEXISTENT keys
                $cache = array_diff( $cache, [ '!NONEXISTENT' ] );
 
                // Keys may appear with a capital first letter. lcfirst them.
                return array_map( [ $wgContLang, 'lcfirst' ], array_keys( $cache ) );
        }
-
-       /**
-        * Purge message caches when a MediaWiki: page is created, updated, or deleted
-        *
-        * @param Title $title Message page title
-        * @param Content|null $content New content for edit/create, null on deletion
-        * @since 1.29
-        */
-       public function updateMessageOverride( Title $title, Content $content = null ) {
-               global $wgContLang;
-
-               $msgText = $this->getMessageTextFromContent( $content );
-               if ( $msgText === null ) {
-                       $msgText = false; // treat as not existing
-               }
-
-               $this->replace( $title->getDBkey(), $msgText );
-
-               if ( $wgContLang->hasVariants() ) {
-                       $wgContLang->updateConversionTable( $title );
-               }
-       }
-
-       /**
-        * @param Content|null $content Content or null if the message page does not exist
-        * @return string|bool|null Returns false if $content is null and null on error
-        */
-       private function getMessageTextFromContent( Content $content = null ) {
-               // @TODO: could skip pseudo-messages like js/css here, based on content model
-               if ( $content ) {
-                       // Message page exists...
-                       // XXX: Is this the right way to turn a Content object into a message?
-                       // NOTE: $content is typically either WikitextContent, JavaScriptContent or
-                       //       CssContent. MessageContent is *not* used for storing messages, it's
-                       //       only used for wrapping them when needed.
-                       $msgText = $content->getWikitextForTransclusion();
-                       if ( $msgText === false || $msgText === null ) {
-                               // This might be due to some kind of misconfiguration...
-                               $msgText = null;
-                               LoggerFactory::getInstance( 'MessageCache' )->warning(
-                                       __METHOD__ . ": message content doesn't provide wikitext "
-                                       . "(content model: " . $content->getModel() . ")" );
-                       }
-               } else {
-                       // Message page does not exist...
-                       $msgText = false;
-               }
-
-               return $msgText;
-       }
 }
index dc65549..f016494 100644 (file)
@@ -1173,8 +1173,22 @@ class WikiPage implements Page, IDBAccessObject {
                }
 
                if ( $this->mTitle->getNamespace() == NS_MEDIAWIKI ) {
-                       $messageCache = MessageCache::singleton();
-                       $messageCache->updateMessageOverride( $this->mTitle, $this->getContent() );
+                       // @todo move this logic to MessageCache
+                       if ( $this->exists() ) {
+                               // NOTE: use transclusion text for messages.
+                               //       This is consistent with  MessageCache::getMsgFromNamespace()
+
+                               $content = $this->getContent();
+                               $text = $content === null ? null : $content->getWikitextForTransclusion();
+
+                               if ( $text === null ) {
+                                       $text = false;
+                               }
+                       } else {
+                               $text = false;
+                       }
+
+                       MessageCache::singleton()->replace( $this->mTitle->getDBkey(), $text );
                }
 
                return true;
@@ -2236,7 +2250,7 @@ class WikiPage implements Page, IDBAccessObject {
         *   - 'no-change': don't update the article count, ever
         */
        public function doEditUpdates( Revision $revision, User $user, array $options = [] ) {
-               global $wgRCWatchCategoryMembership;
+               global $wgRCWatchCategoryMembership, $wgContLang;
 
                $options += [
                        'changed' => true,
@@ -2374,7 +2388,17 @@ class WikiPage implements Page, IDBAccessObject {
                }
 
                if ( $this->mTitle->getNamespace() == NS_MEDIAWIKI ) {
-                       MessageCache::singleton()->updateMessageOverride( $this->mTitle, $content );
+                       // XXX: could skip pseudo-messages like js/css here, based on content model.
+                       $msgtext = $content ? $content->getWikitextForTransclusion() : null;
+                       if ( $msgtext === false || $msgtext === null ) {
+                               $msgtext = '';
+                       }
+
+                       MessageCache::singleton()->replace( $shortTitle, $msgtext );
+
+                       if ( $wgContLang->hasVariants() ) {
+                               $wgContLang->updateConversionTable( $this->mTitle );
+                       }
                }
 
                if ( $options['created'] ) {
@@ -3372,6 +3396,8 @@ class WikiPage implements Page, IDBAccessObject {
         * @param Title $title
         */
        public static function onArticleDelete( Title $title ) {
+               global $wgContLang;
+
                // Update existence markers on article/talk tabs...
                $other = $title->getOtherPage();
 
@@ -3388,7 +3414,11 @@ class WikiPage implements Page, IDBAccessObject {
 
                // Messages
                if ( $title->getNamespace() == NS_MEDIAWIKI ) {
-                       MessageCache::singleton()->updateMessageOverride( $title, null );
+                       MessageCache::singleton()->replace( $title->getDBkey(), false );
+
+                       if ( $wgContLang->hasVariants() ) {
+                               $wgContLang->updateConversionTable( $title );
+                       }
                }
 
                // Images