Merge "Revert "Make LocalisationCache a service""
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Mon, 26 Aug 2019 22:00:46 +0000 (22:00 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 26 Aug 2019 22:00:46 +0000 (22:00 +0000)
16 files changed:
includes/DefaultSettings.php
includes/Linker.php
includes/OutputPage.php
includes/Setup.php
includes/Title.php
includes/actions/InfoAction.php
includes/api/ApiPageSet.php
includes/api/ApiQueryAllDeletedRevisions.php
includes/block/BlockManager.php
includes/filebackend/FileBackendGroup.php
includes/installer/SqliteInstaller.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
includes/objectcache/SqlBagOStuff.php
includes/specialpage/ChangesListSpecialPage.php
includes/specials/SpecialRecentChanges.php
includes/specials/pagers/ImageListPager.php

index 0b14e56..7e13fc2 100644 (file)
@@ -6842,6 +6842,8 @@ $wgRCLinkLimits = [ 50, 100, 250, 500 ];
 /**
  * List of Days options to list in the Special:Recentchanges and
  * Special:Recentchangeslinked pages.
+ *
+ * @see ChangesListSpecialPage::getLinkDays
  */
 $wgRCLinkDays = [ 1, 3, 7, 14, 30 ];
 
index 47be8a2..a79ec3a 100644 (file)
@@ -1322,7 +1322,7 @@ class Linker {
                                        $services->getNamespaceInfo()->getCanonicalName( NS_MEDIA ), '/' );
                                $medians .= '|';
                                $medians .= preg_quote(
-                                       MediaWikiServices::getInstance()->getContentLanguage()->getNsText( NS_MEDIA ),
+                                       $services->getContentLanguage()->getNsText( NS_MEDIA ),
                                        '/'
                                ) . '):';
 
@@ -1359,7 +1359,7 @@ class Linker {
                                        }
                                        if ( $match[1] !== false && $match[1] !== '' ) {
                                                if ( preg_match(
-                                                       MediaWikiServices::getInstance()->getContentLanguage()->linkTrail(),
+                                                       $services->getContentLanguage()->linkTrail(),
                                                        $match[3],
                                                        $submatch
                                                ) ) {
@@ -1375,7 +1375,7 @@ class Linker {
 
                                                Title::newFromText( $linkTarget );
                                                try {
-                                                       $target = MediaWikiServices::getInstance()->getTitleParser()->
+                                                       $target = $services->getTitleParser()->
                                                                parseTitle( $linkTarget );
 
                                                        if ( $target->getText() == '' && !$target->isExternal()
index 3f2dcf7..b2ca53a 100644 (file)
@@ -3227,7 +3227,7 @@ class OutputPage extends ContextSource {
 
                $title = $this->getTitle();
                $ns = $title->getNamespace();
-               $nsInfo = MediaWikiServices::getInstance()->getNamespaceInfo();
+               $nsInfo = $services->getNamespaceInfo();
                $canonicalNamespace = $nsInfo->exists( $ns )
                        ? $nsInfo->getCanonicalName( $ns )
                        : $title->getNsText();
index 201e1a9..4717d1a 100644 (file)
@@ -368,19 +368,6 @@ foreach ( $wgForeignFileRepos as &$repo ) {
 unset( $repo ); // no global pollution; destroy reference
 
 $rcMaxAgeDays = $wgRCMaxAge / ( 3600 * 24 );
-if ( $wgRCFilterByAge ) {
-       // Trim down $wgRCLinkDays so that it only lists links which are valid
-       // as determined by $wgRCMaxAge.
-       // Note that we allow 1 link higher than the max for things like 56 days but a 60 day link.
-       sort( $wgRCLinkDays );
-
-       foreach ( $wgRCLinkDays as $i => $days ) {
-               if ( $days >= $rcMaxAgeDays ) {
-                       array_splice( $wgRCLinkDays, $i + 1 );
-                       break;
-               }
-       }
-}
 // Ensure that default user options are not invalid, since that breaks Special:Preferences
 $wgDefaultUserOptions['rcdays'] = min(
        $wgDefaultUserOptions['rcdays'],
index 75ddea8..eb19076 100644 (file)
@@ -3184,7 +3184,7 @@ class Title implements LinkTarget, IDBAccessObject {
        public static function capitalize( $text, $ns = NS_MAIN ) {
                $services = MediaWikiServices::getInstance();
                if ( $services->getNamespaceInfo()->isCapitalized( $ns ) ) {
-                       return MediaWikiServices::getInstance()->getContentLanguage()->ucfirst( $text );
+                       return $services->getContentLanguage()->ucfirst( $text );
                } else {
                        return $text;
                }
index 15cee94..7bcfc88 100644 (file)
@@ -280,7 +280,7 @@ class InfoAction extends FormlessAction {
                // Language in which the page content is (supposed to be) written
                $pageLang = $title->getPageLanguage()->getCode();
 
-               $permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
+               $permissionManager = $services->getPermissionManager();
 
                $pageLangHtml = $pageLang . ' - ' .
                        Language::fetchLanguageName( $pageLang, $lang->getCode() );
index 1b58865..c604322 100644 (file)
@@ -1253,7 +1253,7 @@ class ApiPageSet extends ApiBase {
 
                        // Need gender information
                        if (
-                               MediaWikiServices::getInstance()->getNamespaceInfo()->
+                               $services->getNamespaceInfo()->
                                        hasGenderDistinction( $titleObj->getNamespace() )
                        ) {
                                $usernames[] = $titleObj->getText();
index d713b3a..7d6d342 100644 (file)
@@ -134,7 +134,7 @@ class ApiQueryAllDeletedRevisions extends ApiQueryRevisionsBase {
                        $this->addJoinConds(
                                [ 'change_tag' => [ 'JOIN', [ 'ar_rev_id=ct_rev_id' ] ] ]
                        );
-                       $changeTagDefStore = MediaWikiServices::getInstance()->getChangeTagDefStore();
+                       $changeTagDefStore = $services->getChangeTagDefStore();
                        try {
                                $this->addWhereFld( 'ct_tag_id', $changeTagDefStore->getId( $params['tag'] ) );
                        } catch ( NameTableAccessException $exception ) {
index 03043e1..948ed93 100644 (file)
@@ -104,6 +104,7 @@ class BlockManager {
         */
        public function getUserBlock( User $user, $fromReplica ) {
                $isAnon = $user->getId() === 0;
+               $fromMaster = !$fromReplica;
 
                // TODO: If $user is the current user, we should use the current request. Otherwise,
                // we should not look for XFF or cookie blocks.
@@ -127,7 +128,7 @@ class BlockManager {
                // User/IP blocking
                // After this, $blocks is an array of blocks or an empty array
                // TODO: remove dependency on DatabaseBlock
-               $blocks = DatabaseBlock::newListFromTarget( $user, $ip, !$fromReplica );
+               $blocks = DatabaseBlock::newListFromTarget( $user, $ip, $fromMaster );
 
                // Cookie blocking
                $cookieBlock = $this->getBlockFromCookieValue( $user, $request );
@@ -164,7 +165,7 @@ class BlockManager {
                        $xff = array_map( 'trim', explode( ',', $xff ) );
                        $xff = array_diff( $xff, [ $ip ] );
                        // TODO: remove dependency on DatabaseBlock
-                       $xffblocks = DatabaseBlock::getBlocksForIPList( $xff, $isAnon, !$fromReplica );
+                       $xffblocks = DatabaseBlock::getBlocksForIPList( $xff, $isAnon, $fromMaster );
                        $blocks = array_merge( $blocks, $xffblocks );
                }
 
index db7141f..8b31f05 100644 (file)
@@ -186,7 +186,7 @@ class FileBackendGroup {
                                'mimeCallback' => [ $this, 'guessMimeInternal' ],
                                'obResetFunc' => 'wfResetOutputBuffers',
                                'streamMimeFunc' => [ StreamFile::class, 'contentTypeFromPath' ],
-                               'tmpFileFactory' => MediaWikiServices::getInstance()->getTempFSFileFactory(),
+                               'tmpFileFactory' => $services->getTempFSFileFactory(),
                                'statusWrapper' => [ Status::class, 'wrap' ],
                                'wanCache' => $services->getMainWANObjectCache(),
                                'srvCache' => ObjectCache::getLocalServerInstance( 'hash' ),
index 7c39ded..01bb30e 100644 (file)
@@ -242,27 +242,6 @@ class SqliteInstaller extends DatabaseInstaller {
                $this->setVar( 'wgDBpassword', '' );
                $this->setupSchemaVars();
 
-               # Create the global cache DB
-               try {
-                       $conn = Database::factory(
-                               'sqlite', [ 'dbname' => 'wikicache', 'dbDirectory' => $dir ] );
-                       # @todo: don't duplicate objectcache definition, though it's very simple
-                       $sql =
-<<<EOT
-       CREATE TABLE IF NOT EXISTS objectcache (
-               keyname BLOB NOT NULL default '' PRIMARY KEY,
-               value BLOB,
-               exptime TEXT
-       )
-EOT;
-                       $conn->query( $sql );
-                       $conn->query( "CREATE INDEX IF NOT EXISTS exptime ON objectcache (exptime)" );
-                       $conn->query( "PRAGMA journal_mode=WAL" ); // this is permanent
-                       $conn->close();
-               } catch ( DBConnectionError $e ) {
-                       return Status::newFatal( 'config-sqlite-connection-error', $e->getMessage() );
-               }
-
                # Create the l10n cache DB
                try {
                        $conn = Database::factory(
index 5c172d6..066d4b4 100644 (file)
@@ -970,17 +970,7 @@ class LoadBalancer implements ILoadBalancer {
                $serverIndex = $conn->getLBInfo( 'serverIndex' );
                $refCount = $conn->getLBInfo( 'foreignPoolRefCount' );
                if ( $serverIndex === null || $refCount === null ) {
-                       /**
-                        * This can happen in code like:
-                        *   foreach ( $dbs as $db ) {
-                        *     $conn = $lb->getConnection( $lb::DB_REPLICA, [], $db );
-                        *     ...
-                        *     $lb->reuseConnection( $conn );
-                        *   }
-                        * When a connection to the local DB is opened in this way, reuseConnection()
-                        * should be ignored
-                        */
-                       return;
+                       return; // non-foreign connection; no domain-use tracking to update
                } elseif ( $conn instanceof DBConnRef ) {
                        // DBConnRef already handles calling reuseConnection() and only passes the live
                        // Database instance to this method. Any caller passing in a DBConnRef is broken.
@@ -1312,7 +1302,7 @@ class LoadBalancer implements ILoadBalancer {
 
                // Create a live connection object
                try {
-                       $db = Database::factory( $server['type'], $server );
+                       $conn = Database::factory( $server['type'], $server );
                        // Log when many connection are made on requests
                        ++$this->connectionCounter;
                        $currentConnCount = $this->getCurrentConnectionCount();
@@ -1325,28 +1315,28 @@ class LoadBalancer implements ILoadBalancer {
                } catch ( DBConnectionError $e ) {
                        // FIXME: This is probably the ugliest thing I have ever done to
                        // PHP. I'm half-expecting it to segfault, just out of disgust. -- TS
-                       $db = $e->db;
+                       $conn = $e->db;
                }
 
-               $db->setLBInfo( $server );
-               $db->setLazyMasterHandle(
-                       $this->getLazyConnectionRef( self::DB_MASTER, [], $db->getDomainID() )
+               $conn->setLBInfo( $server );
+               $conn->setLazyMasterHandle(
+                       $this->getLazyConnectionRef( self::DB_MASTER, [], $conn->getDomainID() )
                );
-               $db->setTableAliases( $this->tableAliases );
-               $db->setIndexAliases( $this->indexAliases );
+               $conn->setTableAliases( $this->tableAliases );
+               $conn->setIndexAliases( $this->indexAliases );
 
                if ( $server['serverIndex'] === $this->getWriterIndex() ) {
                        if ( $this->trxRoundId !== false ) {
-                               $this->applyTransactionRoundFlags( $db );
+                               $this->applyTransactionRoundFlags( $conn );
                        }
                        foreach ( $this->trxRecurringCallbacks as $name => $callback ) {
-                               $db->setTransactionListener( $name, $callback );
+                               $conn->setTransactionListener( $name, $callback );
                        }
                }
 
                $this->lazyLoadReplicationPositions(); // session consistency
 
-               return $db;
+               return $conn;
        }
 
        /**
@@ -2283,9 +2273,9 @@ class LoadBalancer implements ILoadBalancer {
                ) );
 
                // Update the prefix for all local connections...
-               $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) {
-                       if ( !$db->getLBInfo( 'foreign' ) ) {
-                               $db->tablePrefix( $prefix );
+               $this->forEachOpenConnection( function ( IDatabase $conn ) use ( $prefix ) {
+                       if ( !$conn->getLBInfo( 'foreign' ) ) {
+                               $conn->tablePrefix( $prefix );
                        }
                } );
        }
index db4838f..e634edc 100644 (file)
@@ -31,6 +31,7 @@ use Wikimedia\Rdbms\DBConnectionError;
 use Wikimedia\Rdbms\IMaintainableDatabase;
 use Wikimedia\Rdbms\LoadBalancer;
 use Wikimedia\ScopedCallback;
+use Wikimedia\Timestamp\ConvertibleTimestamp;
 use Wikimedia\WaitConditionLoop;
 
 /**
@@ -190,28 +191,30 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
                                $type = $info['type'] ?? 'mysql';
                                $host = $info['host'] ?? '[unknown]';
                                $this->logger->debug( __CLASS__ . ": connecting to $host" );
-                               $db = Database::factory( $type, $info );
-                               $db->clearFlag( DBO_TRX ); // auto-commit mode
-                               $this->conns[$shardIndex] = $db;
+                               $conn = Database::factory( $type, $info );
+                               $conn->clearFlag( DBO_TRX ); // auto-commit mode
+                               $this->conns[$shardIndex] = $conn;
                        }
-                       $db = $this->conns[$shardIndex];
+                       $conn = $this->conns[$shardIndex];
                } else {
                        // Use the main LB database
                        $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
                        $index = $this->replicaOnly ? DB_REPLICA : DB_MASTER;
-                       if ( $lb->getServerType( $lb->getWriterIndex() ) !== 'sqlite' ) {
-                               // Keep a separate connection to avoid contention and deadlocks
-                               $db = $lb->getConnectionRef( $index, [], false, $lb::CONN_TRX_AUTOCOMMIT );
-                       } else {
-                               // However, SQLite has the opposite behavior due to DB-level locking.
-                               // Stock sqlite MediaWiki installs use a separate sqlite cache DB instead.
-                               $db = $lb->getConnectionRef( $index );
+                       // If the RDBMS has row-level locking, use the autocommit connection to avoid
+                       // contention and deadlocks. Do not do this if it only has DB-level locking since
+                       // that would just cause deadlocks.
+                       $attribs = $lb->getServerAttributes( $lb->getWriterIndex() );
+                       $flags = $attribs[Database::ATTR_DB_LEVEL_LOCKING] ? 0 : $lb::CONN_TRX_AUTOCOMMIT;
+                       $conn = $lb->getMaintenanceConnectionRef( $index, [], false, $flags );
+                       // Automatically create the objectcache table for sqlite as needed
+                       if ( $conn->getType() === 'sqlite' ) {
+                               $this->initSqliteDatabase( $conn );
                        }
                }
 
-               $this->logger->debug( sprintf( "Connection %s will be used for SqlBagOStuff", $db ) );
+               $this->logger->debug( sprintf( "Connection %s will be used for SqlBagOStuff", $conn ) );
 
-               return $db;
+               return $conn;
        }
 
        /**
@@ -582,10 +585,10 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
         * @param string $exptime
         * @return bool
         */
-       private function isExpired( $db, $exptime ) {
+       private function isExpired( IDatabase $db, $exptime ) {
                return (
                        $exptime != $this->getMaxDateTime( $db ) &&
-                       wfTimestamp( TS_UNIX, $exptime ) < $this->getCurrentTime()
+                       ConvertibleTimestamp::convert( TS_UNIX, $exptime ) < $this->getCurrentTime()
                );
        }
 
@@ -687,7 +690,7 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
                $serversDoneCount = 0,
                &$keysDeletedCount = 0
        ) {
-               $cutoffUnix = wfTimestamp( TS_UNIX, $timestamp );
+               $cutoffUnix = ConvertibleTimestamp::convert( TS_UNIX, $timestamp );
                $shardIndexes = range( 0, $this->numTableShards - 1 );
                shuffle( $shardIndexes );
 
@@ -709,7 +712,8 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
                                if ( $res->numRows() ) {
                                        $row = $res->current();
                                        if ( $lag === null ) {
-                                               $lag = max( $cutoffUnix - wfTimestamp( TS_UNIX, $row->exptime ), 1 );
+                                               $rowExpUnix = ConvertibleTimestamp::convert( TS_UNIX, $row->exptime );
+                                               $lag = max( $cutoffUnix - $rowExpUnix, 1 );
                                        }
 
                                        $keys = [];
@@ -731,7 +735,8 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
 
                                if ( is_callable( $progressCallback ) ) {
                                        if ( $lag ) {
-                                               $remainingLag = $cutoffUnix - wfTimestamp( TS_UNIX, $continue );
+                                               $continueUnix = ConvertibleTimestamp::convert( TS_UNIX, $continue );
+                                               $remainingLag = $cutoffUnix - $continueUnix;
                                                $processedLag = max( $lag - $remainingLag, 0 );
                                                $doneRatio = ( $numShardsDone + $processedLag / $lag ) / $this->numTableShards;
                                        } else {
@@ -948,20 +953,47 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
        }
 
        /**
-        * Create shard tables. For use from eval.php.
+        * @param IMaintainableDatabase $db
+        * @throws DBError
+        */
+       private function initSqliteDatabase( IMaintainableDatabase $db ) {
+               if ( $db->tableExists( 'objectcache' ) ) {
+                       return;
+               }
+               // Use one table for SQLite; sharding does not seem to have much benefit
+               $db->query( "PRAGMA journal_mode=WAL" ); // this is permanent
+               $db->startAtomic( __METHOD__ ); // atomic DDL
+               try {
+                       $encTable = $db->tableName( 'objectcache' );
+                       $encExptimeIndex = $db->addIdentifierQuotes( $db->tablePrefix() . 'exptime' );
+                       $db->query(
+                               "CREATE TABLE $encTable (\n" .
+                               "       keyname BLOB NOT NULL default '' PRIMARY KEY,\n" .
+                               "       value BLOB,\n" .
+                               "       exptime TEXT\n" .
+                               ")",
+                               __METHOD__
+                       );
+                       $db->query( "CREATE INDEX $encExptimeIndex ON $encTable (exptime)" );
+                       $db->endAtomic( __METHOD__ );
+               } catch ( DBError $e ) {
+                       $db->rollback( __METHOD__ );
+                       throw $e;
+               }
+       }
+
+       /**
+        * Create the shard tables on all databases (e.g. via eval.php/shell.php)
         */
        public function createTables() {
                for ( $shardIndex = 0; $shardIndex < $this->numServerShards; $shardIndex++ ) {
                        $db = $this->getConnection( $shardIndex );
-                       if ( $db->getType() !== 'mysql' ) {
-                               throw new MWException( __METHOD__ . ' is not supported on this DB server' );
-                       }
-
-                       for ( $i = 0; $i < $this->numTableShards; $i++ ) {
-                               $db->query(
-                                       'CREATE TABLE ' . $db->tableName( $this->getTableNameByShard( $i ) ) .
-                                       ' LIKE ' . $db->tableName( 'objectcache' ),
-                                       __METHOD__ );
+                       if ( in_array( $db->getType(), [ 'mysql', 'postgres' ], true ) ) {
+                               for ( $i = 0; $i < $this->numTableShards; $i++ ) {
+                                       $encBaseTable = $db->tableName( 'objectcache' );
+                                       $encShardTable = $db->tableName( $this->getTableNameByShard( $i ) );
+                                       $db->query( "CREATE TABLE $encShardTable LIKE $encBaseTable" );
+                               }
                        }
                }
        }
index 2fa8fab..3893e92 100644 (file)
@@ -766,6 +766,33 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                }
        }
 
+       /**
+        * @see $wgRCLinkDays in DefaultSettings.php.
+        * @see $wgRCFilterByAge in DefaultSettings.php.
+        * @return int[]
+        */
+       protected function getLinkDays() {
+               $linkDays = $this->getConfig()->get( 'RCLinkDays' );
+               $filterByAge = $this->getConfig()->get( 'RCFilterByAge' );
+               $maxAge = $this->getConfig()->get( 'RCMaxAge' );
+               if ( $filterByAge ) {
+                       // Trim it to only links which are within $wgRCMaxAge.
+                       // Note that we allow one link higher than the max for things like
+                       // "age 56 days" being accessible through the "60 days" link.
+                       sort( $linkDays );
+
+                       $maxAgeDays = $maxAge / ( 3600 * 24 );
+                       foreach ( $linkDays as $i => $days ) {
+                               if ( $days >= $maxAgeDays ) {
+                                       array_splice( $linkDays, $i + 1 );
+                                       break;
+                               }
+                       }
+               }
+
+               return $linkDays;
+       }
+
        /**
         * Include the modules and configuration for the RCFilters app.
         * Conditional on the user having the feature enabled.
@@ -798,7 +825,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                                        'maxDays' => (int)$this->getConfig()->get( 'RCMaxAge' ) / ( 24 * 3600 ), // Translate to days
                                        'limitArray' => $this->getConfig()->get( 'RCLinkLimits' ),
                                        'limitDefault' => $this->getDefaultLimit(),
-                                       'daysArray' => $this->getConfig()->get( 'RCLinkDays' ),
+                                       'daysArray' => $this->getLinkDays(),
                                        'daysDefault' => $this->getDefaultDays(),
                                ]
                        );
index 6949c61..30f4655 100644 (file)
@@ -847,7 +847,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
                sort( $linkLimits );
                $linkLimits = array_unique( $linkLimits );
 
-               $linkDays = $config->get( 'RCLinkDays' );
+               $linkDays = $this->getLinkDays();
                $linkDays[] = $options['days'];
                sort( $linkDays );
                $linkDays = array_unique( $linkDays );
index 2d3b6b2..d05694a 100644 (file)
@@ -478,7 +478,7 @@ class ImageListPager extends TablePager {
 
                                        // Add delete links if allowed
                                        // From https://github.com/Wikia/app/pull/3859
-                                       $permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
+                                       $permissionManager = $services->getPermissionManager();
 
                                        if ( $permissionManager->userCan( 'delete', $this->getUser(), $filePage ) ) {
                                                $deleteMsg = $this->msg( 'listfiles-delete' )->text();