Daniel Kinzler has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/399447 )
Change subject: Set CONN_NO_WRITE on all relevant code pathes in LoadBalancer.
......................................................................
Set CONN_NO_WRITE on all relevant code pathes in LoadBalancer.
This also adds a test that verifies that the replica connection
cannot be written to.
Bug: T183265
Change-Id: I32602876905d40197f109fe26adf9cb0d2a1aa30
---
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
M tests/phpunit/includes/db/LoadBalancerTest.php
2 files changed, 39 insertions(+), 8 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/47/399447/1
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index eb288dd..1f0ee01 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -434,7 +434,7 @@
$serverName = $this->getServerName( $i );
$this->connLogger->debug( __METHOD__ . ": Using reader
#$i: $serverName..." );
- $conn = $this->openConnection( $i, $domain );
+ $conn = $this->openConnection( $i, $domain,
self::CONN_NO_WRITE );
if ( !$conn ) {
$this->connLogger->warning( __METHOD__ . ":
Failed connecting to $i/$domain" );
unset( $currentLoads[$i] ); // avoid this
server next iteration
@@ -588,7 +588,7 @@
return false;
} else {
- $conn = $this->openConnection( $index,
self::DOMAIN_ANY );
+ $conn = $this->openConnection( $index,
self::DOMAIN_ANY, self::CONN_NO_WRITE );
if ( !$conn ) {
$this->replLogger->warning( __METHOD__
. ': failed to connect to {dbserver}',
[ 'dbserver' => $server ] );
diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php
b/tests/phpunit/includes/db/LoadBalancerTest.php
index f8ab7f4..a7b0975 100644
--- a/tests/phpunit/includes/db/LoadBalancerTest.php
+++ b/tests/phpunit/includes/db/LoadBalancerTest.php
@@ -1,5 +1,7 @@
<?php
+use Wikimedia\Rdbms\DBError;
+use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\LoadBalancer;
/**
@@ -24,7 +26,7 @@
* @file
*/
class LoadBalancerTest extends MediaWikiTestCase {
- public function testLBSimpleServer() {
+ public function testWithoutReplica() {
global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword,
$wgDBtype, $wgSQLiteDataDir;
$servers = [
@@ -48,10 +50,11 @@
$dbw = $lb->getConnection( DB_MASTER );
$this->assertTrue( $dbw->getLBInfo( 'master' ), 'master shows
as master' );
$this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set
on master" );
+ $this->assertWriteAllowed( $dbw );
$dbr = $lb->getConnection( DB_REPLICA );
$this->assertTrue( $dbr->getLBInfo( 'master' ), 'DB_REPLICA
also gets the master' );
- $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set
on replica" );
+ $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX set
on replica" );
$dbwAuto = $lb->getConnection( DB_MASTER, [], false,
$lb::CONN_TRX_AUTO );
$this->assertFalse( $dbwAuto->getFlag( $dbw::DBO_TRX ), "No
DBO_TRX with CONN_TRX_AUTO" );
@@ -69,7 +72,7 @@
$lb->closeAll();
}
- public function testLBSimpleServers() {
+ public function testWithReplica() {
global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword,
$wgDBtype, $wgSQLiteDataDir;
$servers = [
@@ -83,7 +86,7 @@
'load' => 0,
'flags' => DBO_TRX // REPEATABLE-READ for
consistency
],
- [ // emulated slave
+ [ // emulated replica
'host' => $wgDBserver,
'dbname' => $wgDBname,
'user' => $wgDBuser,
@@ -108,14 +111,16 @@
$dbw->getLBInfo( 'clusterMasterHost' ),
'cluster master set' );
$this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set
on master" );
+ $this->assertWriteAllowed( $dbw );
$dbr = $lb->getConnection( DB_REPLICA );
- $this->assertTrue( $dbr->getLBInfo( 'replica' ), 'slave shows
as slave' );
+ $this->assertTrue( $dbr->getLBInfo( 'replica' ), 'replica shows
as replica' );
$this->assertEquals(
( $wgDBserver != '' ) ? $wgDBserver : 'localhost',
$dbr->getLBInfo( 'clusterMasterHost' ),
'cluster master set' );
- $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set
on replica" );
+ $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX set
on replica" );
+ $this->assertWriteForbidden( $dbr );
$dbwAuto = $lb->getConnection( DB_MASTER, [], false,
$lb::CONN_TRX_AUTO );
$this->assertFalse( $dbwAuto->getFlag( $dbw::DBO_TRX ), "No
DBO_TRX with CONN_TRX_AUTO" );
@@ -132,4 +137,30 @@
$lb->closeAll();
}
+
+ private function assertWriteForbidden( IDatabase $db ) {
+ try {
+ $db->delete( 'user', [ 'user_id' => 57634126 ], 'TEST'
);
+ $this->fail( 'Write operation should have failed!' );
+ } catch ( DBError $ex ) {
+ // check that the exception message contains "Write
operation"
+ $constriant = new
PHPUnit_Framework_Constraint_StringContains( 'Write operation' );
+
+ if ( !$constriant->evaluate( $ex->getMessage(), '',
true ) ) {
+ // re-throw original error, to preserve stack
trace
+ throw $ex;
+ }
+ } finally {
+ $db->rollback( __METHOD__, 'flush' );
+ }
+ }
+
+ private function assertWriteAllowed( IDatabase $db ) {
+ try {
+ $this->assertNotSame( false, $db->delete( 'user', [
'user_id' => 57634126 ] ) );
+ } finally {
+ $db->rollback( __METHOD__, 'flush' );
+ }
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/399447
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32602876905d40197f109fe26adf9cb0d2a1aa30
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
