Prevent write operations to database replicas.

Bug: T183265
Change-Id: I8e17644d1b447416adee18e42cf0122b52a80b22
This commit is contained in:
daniel 2017-12-19 17:11:08 +01:00
parent d6b0b92535
commit 36f4daf32c
3 changed files with 60 additions and 0 deletions

View file

@ -236,6 +236,12 @@ 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
*
@ -277,6 +283,7 @@ 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();
@ -908,6 +915,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
if ( $isWrite ) {
if ( !$this->allowWrite ) {
throw new DBError(
$this,
'Write operations are not allowed on this database connection!'
);
}
# 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();
@ -2951,6 +2965,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
final public function begin( $fname = __METHOD__, $mode = self::TRANSACTION_EXPLICIT ) {
if ( !$this->allowWrite ) {
throw new DBError(
$this,
'Write operations are not allowed on this database connection!'
);
}
// Protect against mismatched atomic section, transaction nesting, and snapshot loss
if ( $this->mTrxLevel ) {
if ( $this->mTrxAtomicLevels ) {
@ -3012,6 +3033,14 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
final public function commit( $fname = __METHOD__, $flush = '' ) {
if ( !$this->allowWrite ) {
// we should never get here, since begin() would already throw
throw new DBError(
$this,
'Write operations are not allowed on this database connection!'
);
}
if ( $this->mTrxLevel && $this->mTrxAtomicLevels ) {
// There are still atomic sections open. This cannot be ignored
$levels = implode( ', ', $this->mTrxAtomicLevels );

View file

@ -87,6 +87,9 @@ 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
*

View file

@ -645,6 +645,12 @@ 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)
@ -655,6 +661,9 @@ class LoadBalancer implements ILoadBalancer {
break;
}
}
// Request no-write connection, even if $i == $this->getWriterIndex().
$flags |= self::CONN_NO_WRITE;
}
# Operation-based index
@ -671,6 +680,9 @@ 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
@ -791,6 +803,13 @@ 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
@ -807,6 +826,7 @@ 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 );
@ -863,6 +883,13 @@ 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;
@ -910,6 +937,7 @@ 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" );