Merge "rdbms: rename CONN_TRX_AUTO constant to CONN_TRX_AUTOCOMMIT"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 12 Apr 2018 20:38:58 +0000 (20:38 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 12 Apr 2018 20:38:58 +0000 (20:38 +0000)
1  2 
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
tests/phpunit/includes/libs/rdbms/database/DBConnRefTest.php

@@@ -85,6 -85,8 +85,8 @@@ interface ILoadBalancer 
        const DOMAIN_ANY = '';
  
        /** @var int DB handle should have DBO_TRX disabled and the caller will leave it as such */
+       const CONN_TRX_AUTOCOMMIT = 1;
+       /** @var int Alias for CONN_TRX_AUTOCOMMIT for b/c; deprecated since 1.31 */
        const CONN_TRX_AUTO = 1;
  
        /**
         *  - queryLogger: PSR-3 logger instance. [optional]
         *  - perfLogger: PSR-3 logger instance. [optional]
         *  - errorLogger : Callback that takes an Exception and logs it. [optional]
 +       *  - deprecationLogger: Callback to log a deprecation warning. [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $params );
        /**
         * Get a connection handle by server index
         *
-        * The CONN_TRX_AUTO flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
+        * The CONN_TRX_AUTOCOMMIT flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
         * (e.g. sqlite) in order to avoid deadlocks. ILoadBalancer::getServerAttributes()
         * can be used to check such flags beforehand.
         *
-        * If the caller uses $domain or sets CONN_TRX_AUTO in $flags, then it must also
+        * If the caller uses $domain or sets CONN_TRX_AUTOCOMMIT in $flags, then it must also
         * call ILoadBalancer::reuseConnection() on the handle when finished using it.
         * In all other cases, this is not necessary, though not harmful either.
         *
         *
         * The handle's methods simply wrap those of a Database handle
         *
-        * The CONN_TRX_AUTO flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
+        * The CONN_TRX_AUTOCOMMIT flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
         * (e.g. sqlite) in order to avoid deadlocks. ILoadBalancer::getServerAttributes()
         * can be used to check such flags beforehand.
         *
         * @param int $i Server index or DB_MASTER/DB_REPLICA
         * @param array|string|bool $groups Query group(s), or false for the generic reader
         * @param string|bool $domain Domain ID, or false for the current domain
-        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO)
+        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTOCOMMIT)
         * @return DBConnRef
         */
        public function getConnectionRef( $i, $groups = [], $domain = false, $flags = 0 );
         *
         * The handle's methods simply wrap those of a Database handle
         *
-        * The CONN_TRX_AUTO flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
+        * The CONN_TRX_AUTOCOMMIT flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
         * (e.g. sqlite) in order to avoid deadlocks. ILoadBalancer::getServerAttributes()
         * can be used to check such flags beforehand.
         *
         * @param int $i Server index or DB_MASTER/DB_REPLICA
         * @param array|string|bool $groups Query group(s), or false for the generic reader
         * @param string|bool $domain Domain ID, or false for the current domain
-        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO)
+        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTOCOMMIT)
         * @return DBConnRef
         */
        public function getLazyConnectionRef( $i, $groups = [], $domain = false, $flags = 0 );
         *
         * The handle's methods simply wrap those of a Database handle
         *
-        * The CONN_TRX_AUTO flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
+        * The CONN_TRX_AUTOCOMMIT flag is ignored for databases with ATTR_DB_LEVEL_LOCKING
         * (e.g. sqlite) in order to avoid deadlocks. ILoadBalancer::getServerAttributes()
         * can be used to check such flags beforehand.
         *
         * @param int $db Server index or DB_MASTER/DB_REPLICA
         * @param array|string|bool $groups Query group(s), or false for the generic reader
         * @param string|bool $domain Domain ID, or false for the current domain
-        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO)
+        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTOCOMMIT)
         * @return MaintainableDBConnRef
         */
        public function getMaintenanceConnectionRef( $db, $groups = [], $domain = false, $flags = 0 );
         * The index must be an actual index into the array. If a connection to the server is
         * already open and not considered an "in use" foreign connection, this simply returns it.
         *
-        * Avoid using CONN_TRX_AUTO for databases with ATTR_DB_LEVEL_LOCKING (e.g. sqlite) in
+        * Avoid using CONN_TRX_AUTOCOMMIT for databases with ATTR_DB_LEVEL_LOCKING (e.g. sqlite) in
         * order to avoid deadlocks. ILoadBalancer::getServerAttributes() can be used to check
         * such flags beforehand.
         *
-        * If the caller uses $domain or sets CONN_TRX_AUTO in $flags, then it must also
+        * If the caller uses $domain or sets CONN_TRX_AUTOCOMMIT in $flags, then it must also
         * call ILoadBalancer::reuseConnection() on the handle when finished using it.
         * In all other cases, this is not necessary, though not harmful either.
         *
         *
         * @param int $i Server index (does not support DB_MASTER/DB_REPLICA)
         * @param string|bool $domain Domain ID, or false for the current domain
-        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO)
+        * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTOCOMMIT)
         * @return Database|bool Returns false on errors
         * @throws DBAccessError
         */
@@@ -111,8 -111,6 +111,8 @@@ class LoadBalancer implements ILoadBala
  
        /** @var callable Exception logger */
        private $errorLogger;
 +      /** @var callable Deprecation logger */
 +      private $deprecationLogger;
  
        /** @var bool */
        private $disabled = false;
                        : function ( Exception $e ) {
                                trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
                        };
 +              $this->deprecationLogger = isset( $params['deprecationLogger'] )
 +                      ? $params['deprecationLogger']
 +                      : function ( $msg ) {
 +                              trigger_error( $msg, E_USER_DEPRECATED );
 +                      };
  
                foreach ( [ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ] as $key ) {
                        $this->$key = isset( $params[$key] ) ? $params[$key] : new NullLogger();
                                if ( $this->loads[$i] > 0 ) {
                                        $start = microtime( true );
                                        $ok = $this->doWait( $i, true, $timeout ) && $ok;
 -                                      $timeout -= ( microtime( true ) - $start );
 +                                      $timeout -= intval( microtime( true ) - $start );
                                        if ( $timeout <= 0 ) {
                                                break; // timeout reached
                                        }
                        if ( !empty( $connsByServer[$i] ) ) {
                                /** @var IDatabase[] $serverConns */
                                $serverConns = $connsByServer[$i];
                                return reset( $serverConns );
                        }
                }
                        $domain = false; // local connection requested
                }
  
-               if ( ( $flags & self::CONN_TRX_AUTO ) === self::CONN_TRX_AUTO ) {
+               if ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) === self::CONN_TRX_AUTOCOMMIT ) {
                        // Assuming all servers are of the same type (or similar), which is overwhelmingly
                        // the case, use the master server information to get the attributes. The information
                        // for $i cannot be used since it might be DB_REPLICA, which might require connection
                                // rows (e.g. FOR UPDATE) or (b) make small commits during a larger transactions
                                // to reduce lock contention. None of these apply for sqlite and using separate
                                // connections just causes self-deadlocks.
-                               $flags &= ~self::CONN_TRX_AUTO;
-                               $this->connLogger->info( __METHOD__ . ': ignoring CONN_TRX_AUTO to avoid deadlocks.' );
+                               $flags &= ~self::CONN_TRX_AUTOCOMMIT;
+                               $this->connLogger->info( __METHOD__ .
+                                       ': ignoring CONN_TRX_AUTOCOMMIT to avoid deadlocks.' );
                        }
                }
  
                // main set of DB connections but rather its own pool since:
                // a) those are usually set to implicitly use transaction rounds via DBO_TRX
                // b) those must support the use of explicit transaction rounds via beginMasterChanges()
-               $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO );
+               $autoCommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT );
  
                if ( $domain !== false ) {
                        // Connection is to a foreign domain
                $domainInstance = DatabaseDomain::newFromId( $domain );
                $dbName = $domainInstance->getDatabase();
                $prefix = $domainInstance->getTablePrefix();
-               $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO );
+               $autoCommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT );
  
                if ( $autoCommit ) {
                        $connFreeKey = self::KEY_FOREIGN_FREE_NOROUND;
                $server['connLogger'] = $this->connLogger;
                $server['queryLogger'] = $this->queryLogger;
                $server['errorLogger'] = $this->errorLogger;
 +              $server['deprecationLogger'] = $this->deprecationLogger;
                $server['profiler'] = $this->profiler;
                $server['trxProfiler'] = $this->trxProfiler;
                // Use the same agent and PHP mode for all DB handles
        }
  
        public function closeConnection( IDatabase $conn ) {
-               $serverIndex = $conn->getLBInfo( 'serverIndex' ); // second index level of mConns
+               $serverIndex = $conn->getLBInfo( 'serverIndex' );
                foreach ( $this->conns as $type => $connsByServer ) {
                        if ( !isset( $connsByServer[$serverIndex] ) ) {
                                continue;
@@@ -12,8 -12,6 +12,8 @@@ use Wikimedia\Rdbms\ResultWrapper
   */
  class DBConnRefTest extends PHPUnit\Framework\TestCase {
  
 +      use PHPUnit4And6Compat;
 +
        /**
         * @return ILoadBalancer
         */
@@@ -69,7 -67,7 +69,7 @@@
  
                $lb->expects( $this->once() )
                        ->method( 'getConnection' )
-                       ->with( DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTO )
+                       ->with( DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTOCOMMIT )
                        ->willReturnCallback(
                                function () {
                                        return $this->getDatabaseMock();
@@@ -78,7 -76,7 +78,7 @@@
  
                $ref = new DBConnRef(
                        $lb,
-                       [ DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTO ]
+                       [ DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTOCOMMIT ]
                );
  
                $this->assertInstanceOf( ResultWrapper::class, $ref->select( 'whatever', '*' ) );