Addshore has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/322644

Change subject: Add DB ConnectionManagers
......................................................................

Add DB ConnectionManagers

This moves and refactors the ConsistentReadConnectionManager
from Wikibase into the core rdbms lib.
The refactoring also creates a generic ConnectionManager.

This relates to Iff20a22f9f2bc7ceefd6defc0ed9a494a6fe62c0
which introduced a DB factory / connection manager in
an extension revealing the need for this in multiple places.

Change-Id: I0c58e15aed5bed88323d18cb95e5008f8d3381c5
---
M autoload.php
A includes/libs/rdbms/connectionmanager/ConnectionManager.php
A includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManager.php
A tests/phpunit/includes/libs/rdbms/connectionmanager/ConnectionManagerTest.php
A 
tests/phpunit/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManagerTest.php
5 files changed, 439 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/44/322644/1

diff --git a/autoload.php b/autoload.php
index 30ef985..9d67be7 100644
--- a/autoload.php
+++ b/autoload.php
@@ -592,6 +592,8 @@
        'IEUrlExtension' => __DIR__ . '/includes/libs/IEUrlExtension.php',
        'IExpiringStore' => __DIR__ . 
'/includes/libs/objectcache/IExpiringStore.php',
        'IJobSpecification' => __DIR__ . 
'/includes/jobqueue/JobSpecification.php',
+       'ConnectionManager' => __DIR__ . 
'/includes/libs/rdbms/connectionmanager/ConnectionManager.php',
+       'ConsistentReadConnectionManager' => __DIR__ . 
'/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManager.php',
        'ILBFactory' => __DIR__ . 
'/includes/libs/rdbms/lbfactory/ILBFactory.php',
        'ILoadBalancer' => __DIR__ . 
'/includes/libs/rdbms/loadbalancer/ILoadBalancer.php',
        'ILoadMonitor' => __DIR__ . 
'/includes/libs/rdbms/loadmonitor/ILoadMonitor.php',
diff --git a/includes/libs/rdbms/connectionmanager/ConnectionManager.php 
b/includes/libs/rdbms/connectionmanager/ConnectionManager.php
new file mode 100644
index 0000000..1b0eb76
--- /dev/null
+++ b/includes/libs/rdbms/connectionmanager/ConnectionManager.php
@@ -0,0 +1,126 @@
+<?php
+
+/**
+ * Database connection manager.
+ *
+ * This manages access to master and slave databases.
+ *
+ * @since 1.29
+ *
+ * @license GPL-2.0+
+ * @author Addshore
+ */
+class ConnectionManager {
+
+       /**
+        * @var LoadBalancer
+        */
+       private $loadBalancer;
+
+       /**
+        * The symbolic name of the target database, or false for the local 
wiki's database.
+        *
+        * @var string|false
+        */
+       private $dbName;
+
+       /**
+        * @var int Db to use for write connections (can be overridden in 
subclasses)
+        */
+       protected $writeDbIndex = DB_MASTER;
+
+       /**
+        * @var int Db to use for read connections (can be overridden in 
subclasses)
+        */
+       protected $readDbIndex = DB_REPLICA;
+
+       /**
+        * @param LoadBalancer $loadBalancer
+        * @param string|bool $dbName Optional, defaults to current wiki.
+        *        This follows the convention for database names used by 
$loadBalancer.
+        *
+        * @throws InvalidArgumentException
+        */
+       public function __construct( LoadBalancer $loadBalancer, $dbName = 
false ) {
+               if ( !is_string( $dbName ) && $dbName !== false ) {
+                       throw new InvalidArgumentException( '$dbName must be a 
string, or false.' );
+               }
+
+               $this->loadBalancer = $loadBalancer;
+               $this->dbName = $dbName;
+       }
+
+       /**
+        * @param int $i
+        *
+        * @return Database
+        */
+       private function getConnection( $i ) {
+               return $this->loadBalancer->getConnection( $i, [], 
$this->dbName );
+       }
+
+       /**
+        * @param int $i
+        *
+        * @return DBConnRef
+        */
+       private function getConnectionRef( $i ) {
+               return $this->loadBalancer->getConnectionRef( $i, [], 
$this->dbName );
+       }
+
+       /**
+        * Returns a connection to the master DB, for updating. The connection 
should later be released
+        * by calling releaseConnection().
+        *
+        * @since 1.29
+        *
+        * @return Database
+        */
+       public function getWriteConnection() {
+               return $this->getConnection( $this->writeDbIndex );
+       }
+
+       /**
+        * Returns a database connection for reading. The connection should 
later be released by
+        * calling releaseConnection().
+        *
+        * @since 1.29
+        *
+        * @return Database
+        */
+       public function getReadConnection() {
+               return $this->getConnection( $this->readDbIndex );
+       }
+
+       /**
+        * @since 1.29
+        *
+        * @param IDatabase $db
+        */
+       public function releaseConnection( IDatabase $db ) {
+               $this->loadBalancer->reuseConnection( $db );
+       }
+
+       /**
+        * Returns a connection ref to the master DB, for updating.
+        *
+        * @since 1.29
+        *
+        * @return DBConnRef
+        */
+       public function getWriteConnectionRef() {
+               return $this->getConnectionRef( $this->writeDbIndex );
+       }
+
+       /**
+        * Returns a database connection ref for reading.
+        *
+        * @since 1.29
+        *
+        * @return DBConnRef
+        */
+       public function getReadConnectionRef() {
+               return $this->getConnectionRef( $this->readDbIndex );
+       }
+
+}
diff --git 
a/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManager.php 
b/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManager.php
new file mode 100644
index 0000000..1145c5f
--- /dev/null
+++ b/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManager.php
@@ -0,0 +1,82 @@
+<?php
+
+/**
+ * Database connection manager.
+ *
+ * This manages access to master and slave databases. It also manages state 
that indicates whether
+ * the slave databases are possibly outdated after a write operation, and thus 
the master database
+ * should be used for subsequent read operations.
+ *
+ * @note: Services that access overlapping sets of database tables, or 
interact with logically
+ * related sets of data in the database, should share a 
ConsistentReadConnectionManager.
+ * Services accessing unrelated sets of information may prefer to not share a
+ * ConsistentReadConnectionManager, so they can still perform read operations 
against slave
+ * databases after a (unrelated, per the assumption) write operation to the 
master database.
+ * Generally, sharing a ConsistentReadConnectionManager improves consistency 
(by avoiding race
+ * conditions due to replication lag), but can reduce performance (by 
directing more read
+ * operations to the master database server).
+ *
+ * @since 1.29
+ *
+ * @license GPL-2.0+
+ * @author Daniel Kinzler
+ */
+class ConsistentReadConnectionManager extends ConnectionManager {
+
+       /**
+        * Forces all future calls to getReadConnection() to return a 
connection to the master DB.
+        * Use this before performing read operations that are critical for a 
future update.
+        * Calling beginAtomicSection() implies a call to forceMaster().
+        *
+        * @since 1.29
+        */
+       public function forceMaster() {
+               $this->readDbIndex = $this->writeDbIndex;
+       }
+
+       /**
+        * Begins an atomic section and returns a database connection to the 
master DB, for updating.
+        *
+        * @since 1.29
+        *
+        * @note: This causes all future calls to getReadConnection() to return 
a connection
+        * to the master DB, even after commitAtomicSection() or 
rollbackAtomicSection() have
+        * been called.
+        *
+        * @param string $fname
+        *
+        * @return Database
+        */
+       public function beginAtomicSection( $fname ) {
+               // Once we have written to master, do not read from slave.
+               $this->forceMaster();
+
+               $db = $this->getWriteConnection();
+               $db->startAtomic( $fname );
+               return $db;
+       }
+
+       /**
+        * @since 1.29
+        *
+        * @param IDatabase $db
+        * @param string $fname
+        */
+       public function commitAtomicSection( IDatabase $db, $fname ) {
+               $db->endAtomic( $fname );
+               $this->releaseConnection( $db );
+       }
+
+       /**
+        * @since 1.29
+        *
+        * @param IDatabase $db
+        * @param string $fname
+        */
+       public function rollbackAtomicSection( IDatabase $db, $fname ) {
+               // FIXME: there does not seem to be a clean way to roll back an 
atomic section?!
+               $db->rollback( $fname, 'flush' );
+               $this->releaseConnection( $db );
+       }
+
+}
diff --git 
a/tests/phpunit/includes/libs/rdbms/connectionmanager/ConnectionManagerTest.php 
b/tests/phpunit/includes/libs/rdbms/connectionmanager/ConnectionManagerTest.php
new file mode 100644
index 0000000..4186854
--- /dev/null
+++ 
b/tests/phpunit/includes/libs/rdbms/connectionmanager/ConnectionManagerTest.php
@@ -0,0 +1,72 @@
+<?php
+
+/**
+ * @covers ConnectionManager
+ *
+ * @license GPL-2.0+
+ * @author Daniel Kinzler
+ */
+class ConnectionManagerTest extends \PHPUnit_Framework_TestCase {
+
+       /**
+        * @return IDatabase|PHPUnit_Framework_MockObject_MockObject
+        */
+       private function getIDatabaseMock() {
+               return $this->getMock( IDatabase::class );
+       }
+
+       /**
+        * @return LoadBalancer|PHPUnit_Framework_MockObject_MockObject
+        */
+       private function getLoadBalancerMock() {
+               $lb = $this->getMockBuilder( LoadBalancer::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+
+               return $lb;
+       }
+
+       public function testGetReadConnection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'getConnection' )
+                       ->with( DB_SLAVE )
+                       ->will( $this->returnValue( $database ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $actual = $manager->getReadConnection();
+
+               $this->assertSame( $database, $actual );
+       }
+
+       public function testGetWriteConnection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'getConnection' )
+                       ->with( DB_MASTER )
+                       ->will( $this->returnValue( $database ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $actual = $manager->getWriteConnection();
+
+               $this->assertSame( $database, $actual );
+       }
+
+       public function testReleaseConnection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'reuseConnection' )
+                       ->with( $database )
+                       ->will( $this->returnValue( null ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->releaseConnection( $database );
+       }
+
+}
diff --git 
a/tests/phpunit/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManagerTest.php
 
b/tests/phpunit/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManagerTest.php
new file mode 100644
index 0000000..f8dd86d
--- /dev/null
+++ 
b/tests/phpunit/includes/libs/rdbms/connectionmanager/ConsistentReadConnectionManagerTest.php
@@ -0,0 +1,157 @@
+<?php
+
+/**
+ * @covers ConsistentReadConnectionManager
+ *
+ * @license GPL-2.0+
+ * @author Daniel Kinzler
+ */
+class ConsistentReadConnectionManagerTest extends \PHPUnit_Framework_TestCase {
+
+       /**
+        * @return IDatabase|PHPUnit_Framework_MockObject_MockObject
+        */
+       private function getIDatabaseMock() {
+               return $this->getMock( IDatabase::class );
+       }
+
+       /**
+        * @return LoadBalancer|PHPUnit_Framework_MockObject_MockObject
+        */
+       private function getLoadBalancerMock() {
+               $lb = $this->getMockBuilder( LoadBalancer::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+
+               return $lb;
+       }
+
+       public function testGetReadConnection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'getConnection' )
+                       ->with( DB_SLAVE )
+                       ->will( $this->returnValue( $database ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $actual = $manager->getReadConnection();
+
+               $this->assertSame( $database, $actual );
+       }
+
+       public function testGetReadConnectionReturnsWriteDbOnForceMatser() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'getConnection' )
+                       ->with( DB_MASTER )
+                       ->will( $this->returnValue( $database ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->forceMaster();
+               $actual = $manager->getReadConnection();
+
+               $this->assertSame( $database, $actual );
+       }
+
+       public function testGetWriteConnection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'getConnection' )
+                       ->with( DB_MASTER )
+                       ->will( $this->returnValue( $database ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $actual = $manager->getWriteConnection();
+
+               $this->assertSame( $database, $actual );
+       }
+
+       public function testForceMaster() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'getConnection' )
+                       ->with( DB_MASTER )
+                       ->will( $this->returnValue( $database ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->forceMaster();
+               $manager->getReadConnection();
+       }
+
+       public function testReleaseConnection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'reuseConnection' )
+                       ->with( $database )
+                       ->will( $this->returnValue( null ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->releaseConnection( $database );
+       }
+
+       public function testBeginAtomicSection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->exactly( 2 ) )
+                       ->method( 'getConnection' )
+                       ->with( DB_MASTER )
+                       ->will( $this->returnValue( $database ) );
+
+               $database->expects( $this->once() )
+                       ->method( 'startAtomic' )
+                       ->will( $this->returnValue( null ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->beginAtomicSection( 'TEST' );
+
+               // Should also ask for a DB_MASTER connection.
+               // This is asserted by the $lb mock.
+               $manager->getReadConnection();
+       }
+
+       public function testCommitAtomicSection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'reuseConnection' )
+                       ->with( $database )
+                       ->will( $this->returnValue( null ) );
+
+               $database->expects( $this->once() )
+                       ->method( 'endAtomic' )
+                       ->will( $this->returnValue( null ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->commitAtomicSection( $database, 'TEST' );
+       }
+
+       public function testRollbackAtomicSection() {
+               $database = $this->getIDatabaseMock();
+               $lb = $this->getLoadBalancerMock();
+
+               $lb->expects( $this->once() )
+                       ->method( 'reuseConnection' )
+                       ->with( $database )
+                       ->will( $this->returnValue( null ) );
+
+               $database->expects( $this->once() )
+                       ->method( 'rollback' )
+                       ->will( $this->returnValue( null ) );
+
+               $manager = new ConsistentReadConnectionManager( $lb );
+               $manager->rollbackAtomicSection( $database, 'TEST' );
+       }
+
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/322644
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c58e15aed5bed88323d18cb95e5008f8d3381c5
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to