In ChronologyProtector fix confusion between DB name and master name
The one caller of LBFactory::getChronologyProtectorTouched() was calling it with a domain ID instead of a master server name. Using the master server name to identify replication position makes sense for ChronologyProtector, since the replication position may be reset when the master changes, but it is an odd convention for LBFactory. So: * Rename all $dbName variables in ChronologyProtector to $masterName, for clarity. * Interpret the first parameter to ILBFactory::getChronologyProtectorTouched() as a database domain, to make its only existing caller work. * Change the first parameter to ChronologyProtector::getTouched() from a string to a strongly typed ILoadBalancer, by analogy with applySessionReplicationPosition(). This removes the master name concept from the public interface. * Mark ChronologyProtector @internal. The accessor in LBFactory is protected, so extensions can't use it anyway. This is just to clarify why I think changing the parameter to getTouched() without b/c is OK. * Add a simple test which mostly just checks that ChronologyProtector gets called with the correct parameters. It's an LBFactory/ChronologyProtector integration test. Change-Id: I3b4832b5a4d7410e94b9c51577b30b31d49bc63d
This commit is contained in:
parent
dad17c82da
commit
fbf4fe4357
5 changed files with 35 additions and 11 deletions
|
|
@ -644,6 +644,9 @@ because of Phabricator reports.
|
|||
- AuthenticationProvider
|
||||
- ResourceLoaderModule
|
||||
- SearchEngine
|
||||
* The parameters to ChronologyProtector::getTouched() and
|
||||
ILBFactory::getChronologyProtectorTouched() were changed without backwards
|
||||
compatibility.
|
||||
* …
|
||||
|
||||
=== Deprecations in 1.35 ===
|
||||
|
|
|
|||
|
|
@ -35,6 +35,8 @@ use Wikimedia\WaitConditionLoop;
|
|||
* This helps to ensure a consistent ordering of events as seen by an client
|
||||
*
|
||||
* Kind of like Hawking's [[Chronology Protection Agency]].
|
||||
*
|
||||
* @internal
|
||||
*/
|
||||
class ChronologyProtector implements LoggerAwareInterface {
|
||||
/** @var BagOStuff */
|
||||
|
|
@ -269,21 +271,23 @@ class ChronologyProtector implements LoggerAwareInterface {
|
|||
}
|
||||
|
||||
/**
|
||||
* @param string $dbName DB master name (e.g. "db1052")
|
||||
* @param ILoadBalancer $lb The load balancer. Prior to 1.35, the first parameter was the
|
||||
* master name.
|
||||
* @return float|bool UNIX timestamp when client last touched the DB; false if not on record
|
||||
* @since 1.28
|
||||
* @since 1.35
|
||||
*/
|
||||
public function getTouched( $dbName ) {
|
||||
return $this->store->get( $this->getTouchedKey( $this->store, $dbName ) );
|
||||
public function getTouched( ILoadBalancer $lb ) {
|
||||
$masterName = $lb->getServerName( $lb->getWriterIndex() );
|
||||
return $this->store->get( $this->getTouchedKey( $this->store, $masterName ) );
|
||||
}
|
||||
|
||||
/**
|
||||
* @param BagOStuff $store
|
||||
* @param string $dbName
|
||||
* @param string $masterName
|
||||
* @return string
|
||||
*/
|
||||
private function getTouchedKey( BagOStuff $store, $dbName ) {
|
||||
return $store->makeGlobalKey( __CLASS__, 'mtime', $this->clientId, $dbName );
|
||||
private function getTouchedKey( BagOStuff $store, $masterName ) {
|
||||
return $store->makeGlobalKey( __CLASS__, 'mtime', $this->clientId, $masterName );
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -330,10 +330,10 @@ interface ILBFactory {
|
|||
public function commitAndWaitForReplication( $fname, $ticket, array $opts = [] );
|
||||
|
||||
/**
|
||||
* @param string $dbName DB master name (e.g. "db1052")
|
||||
* @param DatabaseDomain|string|bool $domain Domain ID, or false for the current domain
|
||||
* @return float|bool UNIX timestamp when client last touched the DB or false if not recent
|
||||
*/
|
||||
public function getChronologyProtectorTouched( $dbName );
|
||||
public function getChronologyProtectorTouched( $domain = false );
|
||||
|
||||
/**
|
||||
* Disable the ChronologyProtector for all load balancers
|
||||
|
|
|
|||
|
|
@ -530,8 +530,8 @@ abstract class LBFactory implements ILBFactory {
|
|||
return $waitSucceeded;
|
||||
}
|
||||
|
||||
public function getChronologyProtectorTouched( $dbName ) {
|
||||
return $this->getChronologyProtector()->getTouched( $dbName );
|
||||
public function getChronologyProtectorTouched( $domain = false ) {
|
||||
return $this->getChronologyProtector()->getTouched( $this->getMainLB( $domain ) );
|
||||
}
|
||||
|
||||
public function disableChronologyProtection() {
|
||||
|
|
|
|||
|
|
@ -799,4 +799,21 @@ class LBFactoryTest extends MediaWikiTestCase {
|
|||
$this->assertEquals( 'realdb', $lb->resolveDomainID( 'alias-db' ) );
|
||||
$this->assertEquals( "realdb-realprefix_", $lb->resolveDomainID( "alias-db-prefix_" ) );
|
||||
}
|
||||
|
||||
/**
|
||||
* @covers \Wikimedia\Rdbms\ChronologyProtector
|
||||
* @covers \Wikimedia\Rdbms\LBFactory
|
||||
*/
|
||||
public function testGetChronologyProtectorTouched() {
|
||||
$store = new HashBagOStuff;
|
||||
$lbFactory = $this->newLBFactoryMulti( [
|
||||
'memStash' => $store
|
||||
] );
|
||||
$lbFactory->setRequestInfo( [ 'ChronologyClientId' => 'ii' ] );
|
||||
$key = $store->makeGlobalKey( ChronologyProtector::class,
|
||||
'mtime', 'ii', 'test-db1' );
|
||||
$store->set( $key, 2 );
|
||||
$touched = $lbFactory->getChronologyProtectorTouched();
|
||||
$this->assertEquals( 2, $touched );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue