jenkins-bot has submitted this change and it was merged.

Change subject: Inject "srvCache" and local DB connections into LockManagerDB
......................................................................


Inject "srvCache" and local DB connections into LockManagerDB

* Also simplified the srvCache variable usage to be unconditional.
* The wfRandomString() call has also been replaced.

Change-Id: I17e83b17ec549906ee200bbe9eb2f0b151423e26
---
M includes/DefaultSettings.php
M includes/filebackend/lockmanager/DBLockManager.php
M includes/filebackend/lockmanager/LockManagerGroup.php
M includes/filebackend/lockmanager/MySqlLockManager.php
4 files changed, 49 insertions(+), 52 deletions(-)

Approvals:
  Legoktm: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 135c3e5..be858c2 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -644,6 +644,10 @@
  * See LockManager::__construct() for more details.
  * Additional parameters are specific to the lock manager class used.
  * These settings should be global to all wikis.
+ *
+ * When using DBLockManager, the 'dbsByBucket' map can reference 
'localDBMaster' as
+ * a peer database in each bucket. This will result in an extra connection to 
the domain
+ * that the LockManager services, which must also be a valid wiki ID.
  */
 $wgLockManagers = [];
 
diff --git a/includes/filebackend/lockmanager/DBLockManager.php 
b/includes/filebackend/lockmanager/DBLockManager.php
index 4667dde..c36ff48 100644
--- a/includes/filebackend/lockmanager/DBLockManager.php
+++ b/includes/filebackend/lockmanager/DBLockManager.php
@@ -36,7 +36,7 @@
  * @since 1.19
  */
 abstract class DBLockManager extends QuorumLockManager {
-       /** @var array[] Map of DB names to server config */
+       /** @var array[]|IDatabase[] Map of (DB names => server config or 
IDatabase) */
        protected $dbServers; // (DB name => server config array)
        /** @var BagOStuff */
        protected $statusCache;
@@ -63,19 +63,15 @@
         *                     - flags       : DB flags (see DatabaseBase)
         *   - dbsByBucket : Array of 1-16 consecutive integer keys, starting 
from 0,
         *                   each having an odd-numbered list of DB names 
(peers) as values.
-        *                   Any DB named 'localDBMaster' will automatically 
use the DB master
-        *                   settings for this wiki (without the need for a 
dbServers entry).
-        *                   Only use 'localDBMaster' if the domain is a valid 
wiki ID.
         *   - lockExpiry  : Lock timeout (seconds) for dropped connections. 
[optional]
         *                   This tells the DB server how long to wait before 
assuming
         *                   connection failure and releasing all the locks for 
a session.
+        *   - srvCache    : A BagOStuff instance using APC or the like.
         */
        public function __construct( array $config ) {
                parent::__construct( $config );
 
-               $this->dbServers = isset( $config['dbServers'] )
-                       ? $config['dbServers']
-                       : []; // likely just using 'localDBMaster'
+               $this->dbServers = $config['dbServers'];
                // Sanitize srvsByBucket config to prevent PHP errors
                $this->srvsByBucket = array_filter( $config['dbsByBucket'], 
'is_array' );
                $this->srvsByBucket = array_values( $this->srvsByBucket ); // 
consecutive
@@ -90,19 +86,25 @@
                        ? 60 // pick a safe-ish number to match DB timeout 
default
                        : $this->lockExpiry; // cover worst case
 
-               foreach ( $this->srvsByBucket as $bucket ) {
-                       if ( count( $bucket ) > 1 ) { // multiple peers
-                               // Tracks peers that couldn't be queried 
recently to avoid lengthy
-                               // connection timeouts. This is useless if each 
bucket has one peer.
-                               $this->statusCache = 
ObjectCache::getLocalServerInstance();
-                               break;
-                       }
-               }
+               // Tracks peers that couldn't be queried recently to avoid 
lengthy
+               // connection timeouts. This is useless if each bucket has one 
peer.
+               $this->statusCache = isset( $config['srvCache'] )
+                       ? $config['srvCache']
+                       : new HashBagOStuff();
 
-               $this->session = wfRandomString( 31 );
+               $random = [];
+               for ( $i = 1; $i <= 5; ++$i ) {
+                       $random[] = mt_rand( 0, 0xFFFFFFF );
+               }
+               $this->session = substr( md5( implode( '-', $random ) ), 0, 31 
);
        }
 
-       // @todo change this code to work in one batch
+       /**
+        * @TODO change this code to work in one batch
+        * @param string $lockSrv
+        * @param array $pathsByType
+        * @return StatusValue
+        */
        protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
                $status = StatusValue::newGood();
                foreach ( $pathsByType as $type => $paths ) {
@@ -148,44 +150,32 @@
         */
        protected function getConnection( $lockDb ) {
                if ( !isset( $this->conns[$lockDb] ) ) {
-                       if ( $lockDb === 'localDBMaster' ) {
-                               $lb = $this->getLocalLB();
-                               $db = $lb->getConnection( DB_MASTER, [], 
$this->domain );
-                               # Do not mess with settings if the LoadBalancer 
is the main singleton
-                               # to avoid clobbering the settings of handles 
from wfGetDB( DB_MASTER ).
-                               $init = ( wfGetLB() !== $lb );
-                       } elseif ( isset( $this->dbServers[$lockDb] ) ) {
+                       if ( $this->dbServers[$lockDb] instanceof IDatabase ) {
+                               // Direct injected connection hande for $lockDB
+                               $db = $this->dbServers[$lockDb];
+                       } elseif ( is_array( $this->dbServers[$lockDb] ) ) {
+                               // Parameters to construct a new database 
connection
                                $config = $this->dbServers[$lockDb];
                                $db = DatabaseBase::factory( $config['type'], 
$config );
-                               $init = true;
                        } else {
                                throw new UnexpectedValueException( "No server 
called '$lockDb'." );
                        }
 
-                       if ( $init ) {
-                               $db->clearFlag( DBO_TRX );
-                               # If the connection drops, try to avoid letting 
the DB rollback
-                               # and release the locks before the file 
operations are finished.
-                               # This won't handle the case of DB server 
restarts however.
-                               $options = [];
-                               if ( $this->lockExpiry > 0 ) {
-                                       $options['connTimeout'] = 
$this->lockExpiry;
-                               }
-                               $db->setSessionOptions( $options );
-                               $this->initConnection( $lockDb, $db );
+                       $db->clearFlag( DBO_TRX );
+                       # If the connection drops, try to avoid letting the DB 
rollback
+                       # and release the locks before the file operations are 
finished.
+                       # This won't handle the case of DB server restarts 
however.
+                       $options = [];
+                       if ( $this->lockExpiry > 0 ) {
+                               $options['connTimeout'] = $this->lockExpiry;
                        }
+                       $db->setSessionOptions( $options );
+                       $this->initConnection( $lockDb, $db );
 
                        $this->conns[$lockDb] = $db;
                }
 
                return $this->conns[$lockDb];
-       }
-
-       /**
-        * @return LoadBalancer
-        */
-       protected function getLocalLB() {
-               return wfGetLBFactory()->getMainLB( $this->domain );
        }
 
        /**
@@ -206,7 +196,7 @@
         * @return bool
         */
        protected function cacheCheckFailures( $lockDb ) {
-               return ( $this->statusCache && $this->safeDelay > 0 )
+               return ( $this->safeDelay > 0 )
                        ? !$this->statusCache->get( $this->getMissKey( $lockDb 
) )
                        : true;
        }
@@ -218,7 +208,7 @@
         * @return bool Success
         */
        protected function cacheRecordFailure( $lockDb ) {
-               return ( $this->statusCache && $this->safeDelay > 0 )
+               return ( $this->safeDelay > 0 )
                        ? $this->statusCache->set( $this->getMissKey( $lockDb 
), 1, $this->safeDelay )
                        : true;
        }
@@ -230,7 +220,6 @@
         * @return string
         */
        protected function getMissKey( $lockDb ) {
-               $lockDb = ( $lockDb === 'localDBMaster' ) ? wfWikiID() : 
$lockDb; // non-relative
                return 'dblockmanager:downservers:' . str_replace( ' ', '_', 
$lockDb );
        }
 
diff --git a/includes/filebackend/lockmanager/LockManagerGroup.php 
b/includes/filebackend/lockmanager/LockManagerGroup.php
index 602b876..9ad2faf 100644
--- a/includes/filebackend/lockmanager/LockManagerGroup.php
+++ b/includes/filebackend/lockmanager/LockManagerGroup.php
@@ -20,6 +20,7 @@
  * @file
  * @ingroup LockManager
  */
+use MediaWiki\MediaWikiServices;
 
 /**
  * Class to handle file lock manager registration
@@ -29,7 +30,7 @@
  * @since 1.19
  */
 class LockManagerGroup {
-       /** @var array (domain => LockManager) */
+       /** @var LockManagerGroup[] (domain => LockManagerGroup) */
        protected static $instances = [];
 
        protected $domain; // string; domain (usually wiki ID)
@@ -115,6 +116,14 @@
                if ( !isset( $this->managers[$name]['instance'] ) ) {
                        $class = $this->managers[$name]['class'];
                        $config = $this->managers[$name]['config'];
+                       if ( $class === 'DBLockManager' ) {
+                               $lbFactory = 
MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+                               $lb = $lbFactory->newMainLB( $config['domain'] 
);
+                               $dbw = $lb->getLazyConnectionRef( DB_MASTER, 
[], $config['domain'] );
+
+                               $config['dbServers']['localDBMaster'] = $dbw;
+                               $config['srvCache'] = 
ObjectCache::getLocalServerInstance( 'hash' );
+                       }
                        $this->managers[$name]['instance'] = new $class( 
$config );
                }
 
diff --git a/includes/filebackend/lockmanager/MySqlLockManager.php 
b/includes/filebackend/lockmanager/MySqlLockManager.php
index 124d410..441ffea 100644
--- a/includes/filebackend/lockmanager/MySqlLockManager.php
+++ b/includes/filebackend/lockmanager/MySqlLockManager.php
@@ -15,11 +15,6 @@
                self::LOCK_EX => self::LOCK_EX
        ];
 
-       protected function getLocalLB() {
-               // Use a separate connection so releaseAllLocks() doesn't 
rollback the main trx
-               return wfGetLBFactory()->newMainLB( $this->domain );
-       }
-
        protected function initConnection( $lockDb, IDatabase $db ) {
                # Let this transaction see lock rows from other transactions
                $db->query( "SET SESSION TRANSACTION ISOLATION LEVEL READ 
UNCOMMITTED;" );

-- 
To view, visit https://gerrit.wikimedia.org/r/311321
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I17e83b17ec549906ee200bbe9eb2f0b151423e26
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to