A few more DBLockManager fixes and cleanups

* Do not do the connection init step if the same DB handle as
  wfGetDB( DB_MASTER ) is being used to avoid clobbering it.
* Remove begin(), since only one of the subclasses wants
  transactions. That one now uses startAtomic() now.
* Make getConnection() throw an error for bad config instead
  of return null, which was not documented or expected.

Change-Id: Ib09a7972d6569c29e83e329a8f7f9f47a393b896

diff --git a/includes/filebackend/lockmanager/DBLockManager.php b/includes/filebackend/lockmanager/DBLockManager.php
index 0aefbfa..cccf71a 100644 (file)
--- a/includes/filebackend/lockmanager/DBLockManager.php
+++ b/includes/filebackend/lockmanager/DBLockManager.php
@@ -144,33 +144,38 @@ abstract class DBLockManager extends QuorumLockManager {
         * @param string $lockDb
         * @return IDatabase
         * @throws DBError
+        * @throws UnexpectedValueException
         */
        protected function getConnection( $lockDb ) {
                if ( !isset( $this->conns[$lockDb] ) ) {
-                       $db = null;
                        if ( $lockDb === 'localDBMaster' ) {
-                               $db = $this->getLocalLB()->getConnection( DB_MASTER, [], $this->domain );
+                               $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] ) ) {
                                $config = $this->dbServers[$lockDb];
                                $db = DatabaseBase::factory( $config['type'], $config );
+                               $init = true;
+                       } else {
+                               throw new UnexpectedValueException( "No server called '$lockDb'." );
                        }
-                       if ( !$db ) {
-                               return null; // config error?
+
+                       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 );
                        }
+
                        $this->conns[$lockDb] = $db;
-                       $this->conns[$lockDb]->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;
-                       }
-                       $this->conns[$lockDb]->setSessionOptions( $options );
-                       $this->initConnection( $lockDb, $this->conns[$lockDb] );
-               }
-               if ( !$this->conns[$lockDb]->trxLevel() ) {
-                       $this->conns[$lockDb]->begin( __METHOD__ ); // start transaction
                }
 
                return $this->conns[$lockDb];

diff --git a/includes/filebackend/lockmanager/MySqlLockManager.php b/includes/filebackend/lockmanager/MySqlLockManager.php
index c51df16..0536091 100644 (file)
--- a/includes/filebackend/lockmanager/MySqlLockManager.php
+++ b/includes/filebackend/lockmanager/MySqlLockManager.php
@@ -23,6 +23,8 @@ class MySqlLockManager extends DBLockManager {
        protected function initConnection( $lockDb, IDatabase $db ) {
                # Let this transaction see lock rows from other transactions
                $db->query( "SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;" );
+               # Do everything in a transaction as it all gets rolled back eventually
+               $db->startAtomic( __CLASS__ );
        }
 
        /**