Merge "rdbms: make LoadBalancer::waitForAll() better respect the timeout"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 23 Jan 2018 17:26:12 +0000 (17:26 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 23 Jan 2018 17:26:12 +0000 (17:26 +0000)
1  2 
includes/libs/rdbms/loadbalancer/LoadBalancer.php

@@@ -146,10 -146,17 +146,10 @@@ class LoadBalancer implements ILoadBala
                        }
                }
  
 -              $this->localDomain = isset( $params['localDomain'] )
 +              $localDomain = isset( $params['localDomain'] )
                        ? DatabaseDomain::newFromId( $params['localDomain'] )
                        : DatabaseDomain::newUnspecified();
 -              // In case a caller assumes that the domain ID is simply <db>-<prefix>, which is almost
 -              // always true, gracefully handle the case when they fail to account for escaping.
 -              if ( $this->localDomain->getTablePrefix() != '' ) {
 -                      $this->localDomainIdAlias =
 -                              $this->localDomain->getDatabase() . '-' . $this->localDomain->getTablePrefix();
 -              } else {
 -                      $this->localDomainIdAlias = $this->localDomain->getDatabase();
 -              }
 +              $this->setLocalDomain( $localDomain );
  
                $this->mWaitTimeout = isset( $params['waitTimeout'] ) ? $params['waitTimeout'] : 10;
  
        }
  
        public function waitForAll( $pos, $timeout = null ) {
+               $timeout = $timeout ?: $this->mWaitTimeout;
                $oldPos = $this->mWaitForPos;
                try {
                        $this->mWaitForPos = $pos;
                        $ok = true;
                        for ( $i = 1; $i < $serverCount; $i++ ) {
                                if ( $this->mLoads[$i] > 0 ) {
-                                       $ok = $this->doWait( $i, true, $timeout ) && $ok;
+                                       $start = microtime( true );
+                                       $ok = $this->doWait( $i, true, max( 1, (int)$timeout ) ) && $ok;
+                                       $timeout -= ( microtime( true ) - $start );
+                                       if ( $timeout <= 0 ) {
+                                               break; // timeout reached
+                                       }
                                }
                        }
                } finally {
                                $server = $this->mServers[$i];
                                $server['serverIndex'] = $i;
                                $server['autoCommitOnly'] = $autoCommit;
 -                              $conn = $this->reallyOpenConnection( $server, false );
 +                              if ( $this->localDomain->getDatabase() !== null ) {
 +                                      // Use the local domain table prefix if the local domain is specified
 +                                      $server['tablePrefix'] = $this->localDomain->getTablePrefix();
 +                              }
 +                              $conn = $this->reallyOpenConnection( $server, $this->localDomain->getDatabase() );
                                $host = $this->getServerName( $i );
                                if ( $conn->isOpen() ) {
                                        $this->connLogger->debug( "Connected to database $i at '$host'." );
                        // Reuse a free connection from another domain
                        $conn = reset( $this->mConns[$connFreeKey][$i] );
                        $oldDomain = key( $this->mConns[$connFreeKey][$i] );
 -                      // The empty string as a DB name means "don't care".
 -                      // DatabaseMysqlBase::open() already handle this on connection.
                        if ( strlen( $dbName ) && !$conn->selectDB( $dbName ) ) {
                                $this->mLastError = "Error selecting database '$dbName' on server " .
                                        $conn->getServer() . " from client host {$this->host}";
                        } else {
                                $conn->tablePrefix( $prefix );
                                unset( $this->mConns[$connFreeKey][$i][$oldDomain] );
 -                              $this->mConns[$connInUseKey][$i][$domain] = $conn;
 +                              // Note that if $domain is an empty string, getDomainID() might not match it
 +                              $this->mConns[$connInUseKey][$i][$conn->getDomainId()] = $conn;
                                $this->connLogger->debug( __METHOD__ .
                                        ": reusing free connection from $oldDomain for $domain" );
                        }
                                $this->errorConnection = $conn;
                                $conn = false;
                        } else {
 -                              $conn->tablePrefix( $prefix );
 -                              $this->mConns[$connInUseKey][$i][$domain] = $conn;
 +                              $conn->tablePrefix( $prefix ); // as specified
 +                              // Note that if $domain is an empty string, getDomainID() might not match it
 +                              $this->mConns[$connInUseKey][$i][$conn->getDomainID()] = $conn;
                                $this->connLogger->debug( __METHOD__ . ": opened new connection for $i/$domain" );
                        }
                }
        }
  
        /**
 -       * Really opens a connection. Uncached.
 +       * Open a new network connection to a server (uncached)
 +       *
         * Returns a Database object whether or not the connection was successful.
 -       * @access private
         *
         * @param array $server
 -       * @param string|bool $dbNameOverride Use "" to not select any database
 +       * @param string|null $dbNameOverride Use "" to not select any database
         * @return Database
         * @throws DBAccessError
         * @throws InvalidArgumentException
         */
 -      protected function reallyOpenConnection( array $server, $dbNameOverride = false ) {
 +      protected function reallyOpenConnection( array $server, $dbNameOverride ) {
                if ( $this->disabled ) {
                        throw new DBAccessError();
                }
  
 -              if ( $dbNameOverride !== false ) {
 +              if ( $dbNameOverride !== null ) {
                        $server['dbname'] = $dbNameOverride;
                }
  
                                "Foreign domain connections are still in use ($domains)." );
                }
  
 -              $this->localDomain = new DatabaseDomain(
 +              $oldDomain = $this->localDomain->getId();
 +              $this->setLocalDomain( new DatabaseDomain(
                        $this->localDomain->getDatabase(),
                        null,
                        $prefix
 -              );
 +              ) );
  
 -              $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) {
 -                      $db->tablePrefix( $prefix );
 +              $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix, $oldDomain ) {
 +                      if ( !$db->getLBInfo( 'foreign' ) ) {
 +                              $db->tablePrefix( $prefix );
 +                      }
                } );
        }
  
 +      /**
 +       * @param DatabaseDomain $domain
 +       */
 +      private function setLocalDomain( DatabaseDomain $domain ) {
 +              $this->localDomain = $domain;
 +              // In case a caller assumes that the domain ID is simply <db>-<prefix>, which is almost
 +              // always true, gracefully handle the case when they fail to account for escaping.
 +              if ( $this->localDomain->getTablePrefix() != '' ) {
 +                      $this->localDomainIdAlias =
 +                              $this->localDomain->getDatabase() . '-' . $this->localDomain->getTablePrefix();
 +              } else {
 +                      $this->localDomainIdAlias = $this->localDomain->getDatabase();
 +              }
 +      }
 +
        /**
         * Make PHP ignore user aborts/disconnects until the returned
         * value leaves scope. This returns null and does nothing in CLI mode.