rdbms: add $flags argument to ILoadBalancer::getAnyOpenConnection

Make LoadMonitor use this flag for getting a connection.

Change-Id: I32ea9aadc0223c86db5979d6579d781a6af0ff53

diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index a699b23..2f493c7 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -167,10 +167,13 @@ interface ILoadBalancer {
        /**
         * Get any open connection to a given server index, local or foreign
         *
        /**
         * Get any open connection to a given server index, local or foreign
         *

+        * Use CONN_TRX_AUTOCOMMIT to only look for connections opened with that flag
+        *

         * @param int $i Server index or DB_MASTER/DB_REPLICA
         * @param int $i Server index or DB_MASTER/DB_REPLICA

+        * @param int $flags Bitfield of CONN_* class constants

         * @return Database|bool False if no such connection is open
         */
         * @return Database|bool False if no such connection is open
         */

-       public function getAnyOpenConnection( $i );
+       public function getAnyOpenConnection( $i, $flags = 0 );

 
        /**
         * Get a connection handle by server index
 
        /**
         * Get a connection handle by server index

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 96ea949..be7d021 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -566,16 +566,17 @@ class LoadBalancer implements ILoadBalancer {
                }
        }
 
                }
        }
 

-       /**
-        * @param int $i
-        * @return IDatabase|bool
-        */
-       public function getAnyOpenConnection( $i ) {
+       public function getAnyOpenConnection( $i, $flags = 0 ) {
+               $autocommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT );

                foreach ( $this->conns as $connsByServer ) {
                foreach ( $this->conns as $connsByServer ) {

-                       if ( !empty( $connsByServer[$i] ) ) {
-                               /** @var IDatabase[] $serverConns */
-                               $serverConns = $connsByServer[$i];
-                               return reset( $serverConns );
+                       if ( !isset( $connsByServer[$i] ) ) {
+                               continue;
+                       }
+
+                       foreach ( $connsByServer[$i] as $conn ) {
+                               if ( !$autocommit || $conn->getLBInfo( 'autoCommitOnly' ) ) {
+                                       return $conn;
+                               }

                        }
                }
 
                        }
                }
 

diff --git a/includes/libs/rdbms/loadmonitor/LoadMonitor.php b/includes/libs/rdbms/loadmonitor/LoadMonitor.php
index 3372839..c7f807a 100644 (file)
--- a/includes/libs/rdbms/loadmonitor/LoadMonitor.php
+++ b/includes/libs/rdbms/loadmonitor/LoadMonitor.php
@@ -156,13 +156,14 @@ class LoadMonitor implements ILoadMonitor {
                                continue;
                        }
 
                                continue;
                        }
 

-                       $conn = $this->parent->getAnyOpenConnection( $i );
-                       if ( $conn && !$conn->trxLevel() ) {
-                               # Handles with open transactions are avoided since they might be subject
-                               # to REPEATABLE-READ snapshots, which could affect the lag estimate query.
+                       # Handles with open transactions are avoided since they might be subject
+                       # to REPEATABLE-READ snapshots, which could affect the lag estimate query.
+                       $flags = ILoadBalancer::CONN_TRX_AUTOCOMMIT;
+                       $conn = $this->parent->getAnyOpenConnection( $i, $flags );
+                       if ( $conn ) {

                                $close = false; // already open
                        } else {
                                $close = false; // already open
                        } else {

-                               $conn = $this->parent->openConnection( $i, '' );
+                               $conn = $this->parent->openConnection( $i, ILoadBalancer::DOMAIN_ANY, $flags );

                                $close = true; // new connection
                        }
 
                                $close = true; // new connection
                        }
 

diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php
index 88cf0e0..7e58555 100644 (file)
--- a/tests/phpunit/includes/db/LoadBalancerTest.php
+++ b/tests/phpunit/includes/db/LoadBalancerTest.php
@@ -32,25 +32,27 @@ use Wikimedia\Rdbms\LoadMonitorNull;
  * @covers \Wikimedia\Rdbms\LoadBalancer
  */
 class LoadBalancerTest extends MediaWikiTestCase {
  * @covers \Wikimedia\Rdbms\LoadBalancer
  */
 class LoadBalancerTest extends MediaWikiTestCase {

-       public function testWithoutReplica() {
+       private function makeServerConfig() {

                global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir;
 
                global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir;
 

-               $servers = [
-                       [
-                               'host'        => $wgDBserver,
-                               'dbname'      => $wgDBname,
-                               'tablePrefix' => $this->dbPrefix(),
-                               'user'        => $wgDBuser,
-                               'password'    => $wgDBpassword,
-                               'type'        => $wgDBtype,
-                               'dbDirectory' => $wgSQLiteDataDir,
-                               'load'        => 0,
-                               'flags'       => DBO_TRX // REPEATABLE-READ for consistency
-                       ],
+               return [
+                       'host' => $wgDBserver,
+                       'dbname' => $wgDBname,
+                       'tablePrefix' => $this->dbPrefix(),
+                       'user' => $wgDBuser,
+                       'password' => $wgDBpassword,
+                       'type' => $wgDBtype,
+                       'dbDirectory' => $wgSQLiteDataDir,
+                       'load' => 0,
+                       'flags' => DBO_TRX // REPEATABLE-READ for consistency

                ];
                ];

+       }
+
+       public function testWithoutReplica() {
+               global $wgDBname;

 
                $lb = new LoadBalancer( [
 
                $lb = new LoadBalancer( [

-                       'servers' => $servers,
+                       'servers' => [ $this->makeServerConfig() ],

                        'queryLogger' => MediaWiki\Logger\LoggerFactory::getInstance( 'DBQuery' ),
                        'localDomain' => new DatabaseDomain( $wgDBname, null, $this->dbPrefix() )
                ] );
                        'queryLogger' => MediaWiki\Logger\LoggerFactory::getInstance( 'DBQuery' ),
                        'localDomain' => new DatabaseDomain( $wgDBname, null, $this->dbPrefix() )
                ] );


@@ -259,4 +261,39 @@ class LoadBalancerTest extends MediaWikiTestCase {
 
                $this->assertFalse( $lb->getServerAttributes( 1 )[Database::ATTR_DB_LEVEL_LOCKING] );
        }
 
                $this->assertFalse( $lb->getServerAttributes( 1 )[Database::ATTR_DB_LEVEL_LOCKING] );
        }

+
+       /**
+        * @covers LoadBalancer::openConnection()
+        * @covers LoadBalancer::getAnyOpenConnection()
+        */
+       function testOpenConnection() {
+               global $wgDBname;
+
+               $lb = new LoadBalancer( [
+                       'servers' => [ $this->makeServerConfig() ],
+                       'localDomain' => new DatabaseDomain( $wgDBname, null, $this->dbPrefix() )
+               ] );
+
+               $i = $lb->getWriterIndex();
+               $this->assertEquals( null, $lb->getAnyOpenConnection( $i ) );
+               $conn1 = $lb->getConnection( $i );
+               $this->assertNotEquals( null, $conn1 );
+               $this->assertEquals( $conn1, $lb->getAnyOpenConnection( $i ) );
+               $conn2 = $lb->getConnection( $i, [], false, $lb::CONN_TRX_AUTOCOMMIT );
+               $this->assertNotEquals( null, $conn2 );
+               if ( $lb->getServerAttributes( $i )[Database::ATTR_DB_LEVEL_LOCKING] ) {
+                       $this->assertEquals( null,
+                               $lb->getAnyOpenConnection( $i, $lb::CONN_TRX_AUTOCOMMIT ) );
+                       $this->assertEquals( $conn1,
+                               $lb->getConnection(
+                                       $i, [], false, $lb::CONN_TRX_AUTOCOMMIT ), $lb::CONN_TRX_AUTOCOMMIT );
+               } else {
+                       $this->assertEquals( $conn2,
+                               $lb->getAnyOpenConnection( $i, $lb::CONN_TRX_AUTOCOMMIT ) );
+                       $this->assertEquals( $conn2,
+                               $lb->getConnection( $i, [], false, $lb::CONN_TRX_AUTOCOMMIT ) );
+               }
+
+               $lb->closeAll();
+       }

 }
 }