From 60e8a35c2d344cc22d5dc149aed10cf71f21b4c5 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 20 Dec 2017 04:31:07 -0800 Subject: [PATCH] Simplify logic to prevent writes on replica DB connections This reverts most of 36f4daf32c591d6b7e2435629fc6e431398b641a. Change-Id: Ie8205749b14be186e80296b168c32310c10ce875 --- includes/libs/rdbms/database/Database.php | 12 ++------ .../libs/rdbms/loadbalancer/ILoadBalancer.php | 3 -- .../libs/rdbms/loadbalancer/LoadBalancer.php | 28 ------------------- 3 files changed, 2 insertions(+), 41 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 3d40417d65..15e02ad085 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -236,12 +236,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware /** @var TransactionProfiler */ protected $trxProfiler; - /** - * @var bool Whether writing is allowed on this connection. - * Should be false for connections to replicas. - */ - protected $allowWrite = true; - /** * Constructor and database handle and attempt to connect to the DB server * @@ -283,7 +277,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $this->connLogger = $params['connLogger']; $this->queryLogger = $params['queryLogger']; $this->errorLogger = $params['errorLogger']; - $this->allowWrite = empty( $params['noWrite'] ); // Set initial dummy domain until open() sets the final DB/prefix $this->currentDomain = DatabaseDomain::newUnspecified(); @@ -915,13 +908,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } if ( $isWrite ) { - if ( !$this->allowWrite ) { + if ( $this->getLBInfo( 'replica' ) === true ) { throw new DBError( $this, - 'Write operations are not allowed on this database connection!' + 'Write operations are not allowed on replica database connections.' ); } - # In theory, non-persistent writes are allowed in read-only mode, but due to things # like https://bugs.mysql.com/bug.php?id=33669 that might not work anyway... $reason = $this->getReadOnlyReason(); diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index b565b3b0ef..86c43350a0 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -87,9 +87,6 @@ interface ILoadBalancer { /** @var int DB handle should have DBO_TRX disabled and the caller will leave it as such */ const CONN_TRX_AUTO = 1; - /** Disable writing for the given connection. Used internally. Do not use with DB_MASTER! */ - const CONN_NO_WRITE = 2; - /** * Construct a manager of IDatabase connection objects * diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index eb288dd63d..a9eaa9974e 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -645,12 +645,6 @@ class LoadBalancer implements ILoadBalancer { $oldConnsOpened = $this->connsOpened; // connections open now if ( $i == self::DB_MASTER ) { - if ( $flags & self::CONN_NO_WRITE ) { - throw new InvalidArgumentException( - 'Cannot set CONN_NO_WRITE flag on master connection!' - ); - } - $i = $this->getWriterIndex(); } else { # Try to find an available server in any the query groups (in order) @@ -661,9 +655,6 @@ class LoadBalancer implements ILoadBalancer { break; } } - - // Request no-write connection, even if $i == $this->getWriterIndex(). - $flags |= self::CONN_NO_WRITE; } # Operation-based index @@ -680,9 +671,6 @@ class LoadBalancer implements ILoadBalancer { $this->reportConnectionError(); return null; // not reached } - - // Request no-write connection, even if $i == $this->getWriterIndex(). - $flags |= self::CONN_NO_WRITE; } # Now we have an explicit index into the servers array @@ -803,13 +791,6 @@ class LoadBalancer implements ILoadBalancer { // 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 ); - $noWrite = ( ( $flags & self::CONN_NO_WRITE ) == self::CONN_NO_WRITE ); - - if ( $noWrite && $i === $this->getWriterIndex() ) { - // We can't disable writes on the master connection! - // TODO: Wrap the master connection, so write operations fail! - $noWrite = false; - } if ( $domain !== false ) { // Connection is to a foreign domain @@ -826,7 +807,6 @@ class LoadBalancer implements ILoadBalancer { // Open a new connection $server = $this->mServers[$i]; $server['serverIndex'] = $i; - $server['noWrite'] = $noWrite; $server['autoCommitOnly'] = $autoCommit; $conn = $this->reallyOpenConnection( $server, false ); $host = $this->getServerName( $i ); @@ -883,13 +863,6 @@ class LoadBalancer implements ILoadBalancer { $dbName = $domainInstance->getDatabase(); $prefix = $domainInstance->getTablePrefix(); $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO ); - $noWrite = ( ( $flags & self::CONN_NO_WRITE ) == self::CONN_NO_WRITE ); - - if ( $noWrite && $i === $this->getWriterIndex() ) { - // We can't disable writes on the master connection! - // TODO: Wrap the master connection, so write operations fail! - $noWrite = false; - } if ( $autoCommit ) { $connFreeKey = self::KEY_FOREIGN_FREE_NOROUND; @@ -937,7 +910,6 @@ class LoadBalancer implements ILoadBalancer { $server['foreignPoolRefCount'] = 0; $server['foreign'] = true; $server['autoCommitOnly'] = $autoCommit; - $server['noWrite'] = $noWrite; $conn = $this->reallyOpenConnection( $server, $dbName ); if ( !$conn->isOpen() ) { $this->connLogger->warning( __METHOD__ . ": connection error for $i/$domain" ); -- 2.20.1