rdbms: avoid connections on more lazy DBConnRef methods

Also:
* Fixed LoadBalancer::getAnyOpenConnection for both
  DB_MASTER and DB_REPLICA, which are not real indexes.
* Lock down DBConnRef::close since it can only cause trouble.
* Relax DBConnRef restrictions on tablePrefix()/dbSchema()
  for the harmless "getter" mode case.
* Remove redundant DatabasePostgres::getServer definition.

Change-Id: Ia855d901cc3c28147e52284fdabb1645805d4466
This commit is contained in:
Aaron Schulz 2019-03-21 05:51:28 -07:00
parent 4ab8dc680e
commit 4118762bbe
4 changed files with 70 additions and 11 deletions

View file

@ -74,11 +74,27 @@ class DBConnRef implements IDatabase {
}
public function tablePrefix( $prefix = null ) {
if ( $this->conn === null && $prefix === null ) {
$domain = DatabaseDomain::newFromId( $this->params[self::FLD_DOMAIN] );
// Avoid triggering a database connection
return $domain->getTablePrefix();
} elseif ( $this->conn !== null && $prefix === null ) {
// This will just return the prefix
return $this->__call( __FUNCTION__, func_get_args() );
}
// Disallow things that might confuse the LoadBalancer tracking
throw new DBUnexpectedError( $this, "Database selection is disallowed to enable reuse." );
}
public function dbSchema( $schema = null ) {
if ( $this->conn === null && $schema === null ) {
$domain = DatabaseDomain::newFromId( $this->params[self::FLD_DOMAIN] );
// Avoid triggering a database connection
return $domain->getSchema();
} elseif ( $this->conn !== null && $schema === null ) {
// This will just return the schema
return $this->__call( __FUNCTION__, func_get_args() );
}
// Disallow things that might confuse the LoadBalancer tracking
throw new DBUnexpectedError( $this, "Database selection is disallowed to enable reuse." );
}
@ -235,7 +251,7 @@ class DBConnRef implements IDatabase {
}
public function close() {
return $this->__call( __FUNCTION__, func_get_args() );
throw new DBUnexpectedError( $this->conn, 'Cannot close shared connection.' );
}
public function query( $sql, $fname = __METHOD__, $tempIgnore = false ) {
@ -385,6 +401,12 @@ class DBConnRef implements IDatabase {
}
public function getDBname() {
if ( $this->conn === null ) {
$domain = DatabaseDomain::newFromId( $this->params[self::FLD_DOMAIN] );
// Avoid triggering a database connection
return $domain->getDatabase();
}
return $this->__call( __FUNCTION__, func_get_args() );
}

View file

@ -1346,10 +1346,6 @@ SQL;
return [ $startOpts, $useIndex, $preLimitTail, $postLimitTail, $ignoreIndex ];
}
public function getServer() {
return $this->server;
}
public function buildConcat( $stringList ) {
return implode( ' || ', $stringList );
}

View file

@ -566,15 +566,24 @@ class LoadBalancer implements ILoadBalancer {
}
public function getAnyOpenConnection( $i, $flags = 0 ) {
$i = ( $i === self::DB_MASTER ) ? $this->getWriterIndex() : $i;
$autocommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT );
foreach ( $this->conns as $connsByServer ) {
if ( !isset( $connsByServer[$i] ) ) {
continue;
if ( $i === self::DB_REPLICA ) {
$indexes = array_keys( $connsByServer );
} else {
$indexes = isset( $connsByServer[$i] ) ? [ $i ] : [];
}
foreach ( $connsByServer[$i] as $conn ) {
if ( !$autocommit || $conn->getLBInfo( 'autoCommitOnly' ) ) {
return $conn;
foreach ( $indexes as $index ) {
foreach ( $connsByServer[$index] as $conn ) {
if ( !$conn->isOpen() ) {
continue; // some sort of error occured?
}
if ( !$autocommit || $conn->getLBInfo( 'autoCommitOnly' ) ) {
return $conn;
}
}
}
}

View file

@ -44,7 +44,27 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
->disableOriginalConstructor()
->getMock();
$db->method( 'select' )->willReturn( new FakeResultWrapper( [] ) );
$open = true;
$db->method( 'select' )->willReturnCallback( function () use ( &$open ) {
if ( !$open ) {
throw new LogicException( "Not open" );
}
return new FakeResultWrapper( [] );
} );
$db->method( 'close' )->willReturnCallback( function () use ( &$open ) {
$open = false;
return true;
} );
$db->method( 'isOpen' )->willReturnCallback( function () use ( &$open ) {
return $open;
} );
$db->method( 'open' )->willReturnCallback( function () use ( &$open ) {
$open = true;
return $open;
} );
$db->method( '__toString' )->willReturn( 'MOCK_DB' );
return $db;
@ -122,6 +142,9 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
$this->assertSame( 'dummy', $ref->getDomainID() );
}
/**
* @covers Wikimedia\Rdbms\DBConnRef::select
*/
public function testSelect() {
// select should get passed through normally
$ref = $this->getDBConnRef();
@ -137,4 +160,13 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
$this->assertInternalType( 'string', $ref->__toString() );
}
/**
* @covers Wikimedia\Rdbms\DBConnRef::close
* @expectedException \Wikimedia\Rdbms\DBUnexpectedError
*/
public function testClose() {
$lb = $this->getLoadBalancerMock();
$ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ] );
$ref->close();
}
}