Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Allow resources to be salvaged across service resets.

Browse source 
NOTE: This also changes the semantics of MediaWikiServices::resetGlobalInstance
to only reset services instances, not service wiring. The wiring will be copied
from the old global MediaWikiServices instance to the new one.

Bug: T132707
Change-Id: Ie2ca3ff99aa74fffa9eb6c8faccab857dc0874f7
This commit is contained in:
daniel 2016-04-14 21:02:31 +02:00
commit bca436db92
10 changed files with 382 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
autoload.php
View file

@ -800,6 +800,7 @@ $wgAutoloadLocalClasses = [
'MediaWiki\\Services\\CannotReplaceActiveServiceException' => __DIR__ . '/includes/Services/CannotReplaceActiveServiceException.php',
'MediaWiki\\Services\\ContainerDisabledException' => __DIR__ . '/includes/Services/ContainerDisabledException.php',
'MediaWiki\\Services\\DestructibleService' => __DIR__ . '/includes/Services/DestructibleService.php',
'MediaWiki\\Services\\SalvageableService' => __DIR__ . '/includes/Services/SalvageableService.php',
'MediaWiki\\Services\\NoSuchServiceException' => __DIR__ . '/includes/Services/NoSuchServiceException.php',
'MediaWiki\\Services\\ServiceAlreadyDefinedException' => __DIR__ . '/includes/Services/ServiceAlreadyDefinedException.php',
'MediaWiki\\Services\\ServiceContainer' => __DIR__ . '/includes/Services/ServiceContainer.php',

61
includes/MediaWikiServices.php
View file

@ -10,6 +10,7 @@ use Hooks;
use LBFactory;
use Liuggio\StatsdClient\Factory\StatsdDataFactory;
use LoadBalancer;
use MediaWiki\Services\SalvageableService;
use MediaWiki\Services\ServiceContainer;
use MWException;
use ResourceLoader;
@ -87,7 +88,7 @@ class MediaWikiServices extends ServiceContainer {
// even if it's just a file name or database credentials to load
// configuration from.
$bootstrapConfig = new GlobalVarConfig();
self::$instance = self::newInstance( $bootstrapConfig );
self::$instance = self::newInstance( $bootstrapConfig, 'load' );
}
return self::$instance;
@ -120,7 +121,7 @@ class MediaWikiServices extends ServiceContainer {
/**
* Creates a new instance of MediaWikiServices and sets it as the global default
* instance. getInstance() will return a different MediaWikiServices object
* after every call to resetGlobalServiceLocator().
* after every call to resetGlobalInstance().
*
* @since 1.28
*
@ -128,7 +129,7 @@ class MediaWikiServices extends ServiceContainer {
* when the configuration has changed significantly since bootstrap time, e.g.
* during the installation process or during testing.
*
* @warning Calling resetGlobalServiceLocator() may leave the application in an inconsistent
* @warning Calling resetGlobalInstance() may leave the application in an inconsistent
* state. Calling this is only safe under the ASSUMPTION that NO REFERENCE to
* any of the services managed by MediaWikiServices exist. If any service objects
* managed by the old MediaWikiServices instance remain in use, they may INTERFERE
@ -149,11 +150,14 @@ class MediaWikiServices extends ServiceContainer {
* was no previous instance, a new GlobalVarConfig object will be used to
* bootstrap the services.
*
* @param string $quick Set this to "quick" to allow expensive resources to be re-used.
* See SalvageableService for details.
*
* @throws MWException If called after MW_SERVICE_BOOTSTRAP_COMPLETE has been defined in
* Setup.php (unless MW_PHPUNIT_TEST or MEDIAWIKI_INSTALL or RUN_MAINTENANCE_IF_MAIN
* is defined).
*/
public static function resetGlobalInstance( Config $bootstrapConfig = null ) {
public static function resetGlobalInstance( Config $bootstrapConfig = null, $quick = '' ) {
if ( self::$instance === null ) {
// no global instance yet, nothing to reset
return;
@ -165,9 +169,38 @@ class MediaWikiServices extends ServiceContainer {
$bootstrapConfig = self::$instance->getBootstrapConfig();
}
self::$instance->destroy();
$oldInstance = self::$instance;
self::$instance = self::newInstance( $bootstrapConfig );
self::$instance->importWiring( $oldInstance, [ 'BootstrapConfig' ] );
if ( $quick === 'quick' ) {
self::$instance->salvage( $oldInstance );
} else {
$oldInstance->destroy();
}
}
/**
* Salvages the state of any salvageable service instances in $other.
*
* @note $other will have been destroyed when salvage() returns.
*
* @param MediaWikiServices $other
*/
private function salvage( self $other ) {
foreach ( $this->getServiceNames() as $name ) {
$oldService = $other->peekService( $name );
if ( $oldService instanceof SalvageableService ) {
/** @var SalvageableService $newService */
$newService = $this->getService( $name );
$newService->salvage( $oldService );
}
}
$other->destroy();
}
/**
@ -176,21 +209,23 @@ class MediaWikiServices extends ServiceContainer {
* ServiceWiringFiles setting are loaded, and the MediaWikiServices hook is called.
*
* @param Config|null $bootstrapConfig The Config object to be registered as the
* 'BootstrapConfig' service. This has to contain at least the information
* needed to set up the 'ConfigFactory' service. If not provided, any call
* to getBootstrapConfig(), getConfigFactory, or getMainConfig will fail.
* A MediaWikiServices instance without access to configuration is called
* "primordial".
* 'BootstrapConfig' service.
*
* @param string $loadWiring set this to 'load' to load the wiring files specified
* in the 'ServiceWiringFiles' setting in $bootstrapConfig.
*
* @return MediaWikiServices
* @throws MWException
* @throws \FatalError
*/
private static function newInstance( Config $bootstrapConfig ) {
private static function newInstance( Config $bootstrapConfig, $loadWiring = '' ) {
$instance = new self( $bootstrapConfig );
// Load the default wiring from the specified files.
$wiringFiles = $bootstrapConfig->get( 'ServiceWiringFiles' );
$instance->loadWiringFiles( $wiringFiles );
if ( $loadWiring === 'load' ) {
$wiringFiles = $bootstrapConfig->get( 'ServiceWiringFiles' );
$instance->loadWiringFiles( $wiringFiles );
}
// Provide a traditional hook point to allow extensions to configure services.
Hooks::run( 'MediaWikiServices', [ $instance ] );

58
includes/Services/SalvageableService.php Normal file
View file

@ -0,0 +1,58 @@
<?php
namespace MediaWiki\Services;
/**
* Interface for salvageable services.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
*
* @since 1.28
*/
/**
* SalvageableService defines an interface for services that are able to salvage state from a
* previous instance of the same class. The intent is to allow new service instances to re-use
* resources that would be expensive to re-create, such as cached data or network connections.
*
* @note There is no expectation that services will be destroyed when the process (or web request)
* terminates.
*/
interface SalvageableService {
/**
* Re-uses state from $other. $other must not be used after being passed to salvage(),
* and should be considered to be destroyed.
*
* @note Implementations are responsible for determining what parts of $other can be re-used
* safely. In particular, implementations should check that the relevant configuration of
* $other is the same as in $this before re-using resources from $other.
*
* @note Implementations must take care to detach any re-used resources from the original
* service instance. If $other is destroyed later, resources that are now used by the
* new service instance must not be affected.
*
* @note If $other is a DestructibleService, implementations should make sure that $other
* is in destroyed state after salvage finished. This may be done by calling $other->destroy()
* after carefully detaching all relevant resources.
*
* @param SalvageableService $other The object to salvage state from. $other must have the
* exact same type as $this.
*/
public function salvage( SalvageableService $other );
}

41
includes/Services/ServiceContainer.php
View file

@ -55,6 +55,11 @@ class ServiceContainer implements DestructibleService {
*/
private $serviceInstantiators = [];
/**
* @var boolean[] disabled status, per service name
*/
private $disabled = [];
/**
* @var array
*/
@ -126,6 +131,28 @@ class ServiceContainer implements DestructibleService {
}
}
/**
* Imports all wiring defined in $container. Wiring defined in $container
* will override any wiring already defined locally. However, already
* existing service instances will be preserved.
*
* @since 1.28
*
* @param ServiceContainer $container
* @param string[] $skip A list of service names to skip during import
*/
public function importWiring( ServiceContainer $container, $skip = [] ) {
$newInstantiators = array_diff_key(
$container->serviceInstantiators,
array_flip( $skip )
);
$this->serviceInstantiators = array_merge(
$this->serviceInstantiators,
$newInstantiators
);
}
/**
* Returns true if a service is defined for $name, that is, if a call to getService( $name )
* would return a service instance.
@ -220,6 +247,7 @@ class ServiceContainer implements DestructibleService {
}
$this->serviceInstantiators[$name] = $instantiator;
unset( $this->disabled[$name] );
}
/**
@ -244,9 +272,7 @@ class ServiceContainer implements DestructibleService {
public function disableService( $name ) {
$this->resetService( $name );
$this->redefineService( $name, function() use ( $name ) {
throw new ServiceDisabledException( $name );
} );
$this->disabled[$name] = true;
}
/**
@ -282,6 +308,7 @@ class ServiceContainer implements DestructibleService {
}
unset( $this->services[$name] );
unset( $this->disabled[$name] );
}
/**
@ -299,7 +326,8 @@ class ServiceContainer implements DestructibleService {
* @param string $name The service name
*
* @throws NoSuchServiceException if $name is not a known service.
* @throws ServiceDisabledException if this container has already been destroyed.
* @throws ContainerDisabledException if this container has already been destroyed.
* @throws ServiceDisabledException if the requested service has been disabled.
*
* @return object The service instance
*/
@ -308,6 +336,10 @@ class ServiceContainer implements DestructibleService {
throw new ContainerDisabledException();
}
if ( isset( $this->disabled[$name] ) ) {
throw new ServiceDisabledException( $name );
}
if ( !isset( $this->services[$name] ) ) {
$this->services[$name] = $this->createService( $name );
}
@ -327,6 +359,7 @@ class ServiceContainer implements DestructibleService {
$this->serviceInstantiators[$name],
array_merge( [ $this ], $this->extraInstantiationParams )
);
// NOTE: when adding more wiring logic here, make sure copyWiring() is kept in sync!
} else {
throw new NoSuchServiceException( $name );
}

2
includes/Setup.php
View file

@ -501,7 +501,7 @@ if ( !class_exists( 'AutoLoader' ) ) {
// Reset the global service locator, so any services that have already been created will be
// re-created while taking into account any custom settings and extensions.
MediaWikiServices::resetGlobalInstance( new GlobalVarConfig() );
MediaWikiServices::resetGlobalInstance( new GlobalVarConfig(), 'quick' );
// Define a constant that indicates that the bootstrapping of the service locator
// is complete.

63
includes/config/ConfigFactory.php
View file

@ -20,13 +20,15 @@
*
* @file
*/
use MediaWiki\Services\SalvageableService;
use Wikimedia\Assert\Assert;
/**
* Factory class to create Config objects
*
* @since 1.23
*/
class ConfigFactory {
class ConfigFactory implements SalvageableService {
/**
* Map of config name => callback
@ -50,6 +52,41 @@ class ConfigFactory {
return \MediaWiki\MediaWikiServices::getInstance()->getConfigFactory();
}
/**
* Re-uses existing Cache objects from $other. Cache objects are only re-used if the
* registered factory function for both is the same. Cache config is not copied,
* and only instances of caches defined on this instance with the same config
* are copied.
*
* @see SalvageableService::salvage()
*
* @param SalvageableService $other The object to salvage state from. $other must have the
* exact same type as $this.
*/
public function salvage( SalvageableService $other ) {
Assert::parameterType( self::class, $other, '$other' );
/** @var ConfigFactory $other */
foreach ( $other->factoryFunctions as $name => $otherFunc ) {
if ( !isset( $this->factoryFunctions[$name] ) ) {
continue;
}
// if the callback function is the same, salvage the Cache object
// XXX: Closures are never equal!
if ( isset( $other->configs[$name] )
&& $this->factoryFunctions[$name] == $otherFunc
) {
$this->configs[$name] = $other->configs[$name];
unset( $other->configs[$name] );
}
}
// disable $other
$other->factoryFunctions = [];
$other->configs = [];
}
/**
* @return string[]
*/
@ -67,23 +104,11 @@ class ConfigFactory {
* @throws InvalidArgumentException If an invalid callback is provided
*/
public function register( $name, $callback ) {
if ( $callback instanceof Config ) {
$instance = $callback;
// Register a callback anyway, for consistency. Note that getConfigNames()
// relies on $factoryFunctions to have all config names.
$callback = function() use ( $instance ) {
return $instance;
};
} else {
$instance = null;
}
if ( !is_callable( $callback ) ) {
if ( !is_callable( $callback ) && !( $callback instanceof Config ) ) {
throw new InvalidArgumentException( 'Invalid callback provided' );
}
$this->configs[$name] = $instance;
unset( $this->configs[$name] );
$this->factoryFunctions[$name] = $callback;
}
@ -105,7 +130,13 @@ class ConfigFactory {
if ( !isset( $this->factoryFunctions[$key] ) ) {
throw new ConfigException( "No registered builder available for $name." );
}
$conf = call_user_func( $this->factoryFunctions[$key], $this );
if ( $this->factoryFunctions[$key] instanceof Config ) {
$conf = $this->factoryFunctions[$key];
} else {
$conf = call_user_func( $this->factoryFunctions[$key], $this );
}
if ( $conf instanceof Config ) {
$this->configs[$name] = $conf;
} else {

2
maintenance/updateCollation.php
View file

@ -34,7 +34,7 @@ require_once __DIR__ . '/Maintenance.php';
*/
class UpdateCollation extends Maintenance {
const BATCH_SIZE = 100; // Number of rows to process in one batch
const SYNC_INTERVAL = 20; // Wait for slaves after this many batches
const SYNC_INTERVAL = 5; // Wait for slaves after this many batches
public $sizeHistogram = [];

97
tests/phpunit/includes/MediaWikiServicesTest.php
View file

@ -1,6 +1,8 @@
<?php
use Liuggio\StatsdClient\Factory\StatsdDataFactory;
use MediaWiki\MediaWikiServices;
use MediaWiki\Services\DestructibleService;
use MediaWiki\Services\SalvageableService;
use MediaWiki\Services\ServiceDisabledException;
/**
@ -8,7 +10,7 @@ use MediaWiki\Services\ServiceDisabledException;
*
* @group MediaWiki
*/
class MediaWikiServicesTest extends PHPUnit_Framework_TestCase {
class MediaWikiServicesTest extends MediaWikiTestCase {
/**
* @return Config
@ -65,9 +67,69 @@ class MediaWikiServicesTest extends PHPUnit_Framework_TestCase {
$newServices = $this->newMediaWikiServices();
$oldServices = MediaWikiServices::forceGlobalInstance( $newServices );
$service1 = $this->getMock( SalvageableService::class );
$service1->expects( $this->never() )
->method( 'salvage' );
$newServices->defineService(
'Test',
function() use ( $service1 ) {
return $service1;
}
);
// force instantiation
$newServices->getService( 'Test' );
MediaWikiServices::resetGlobalInstance( $this->newTestConfig() );
$theServices = MediaWikiServices::getInstance();
$this->assertSame(
$service1,
$theServices->getService( 'Test' ),
'service definition should survive reset'
);
$this->assertNotSame( $theServices, $newServices );
$this->assertNotSame( $theServices, $oldServices );
MediaWikiServices::forceGlobalInstance( $oldServices );
}
public function testResetGlobalInstance_quick() {
$newServices = $this->newMediaWikiServices();
$oldServices = MediaWikiServices::forceGlobalInstance( $newServices );
$service1 = $this->getMock( SalvageableService::class );
$service1->expects( $this->never() )
->method( 'salvage' );
$service2 = $this->getMock( SalvageableService::class );
$service2->expects( $this->once() )
->method( 'salvage' )
->with( $service1 );
// sequence of values the instantiator will return
$instantiatorReturnValues = [
$service1,
$service2,
];
$newServices->defineService(
'Test',
function() use ( &$instantiatorReturnValues ) {
return array_shift( $instantiatorReturnValues );
}
);
// force instantiation
$newServices->getService( 'Test' );
MediaWikiServices::resetGlobalInstance( $this->newTestConfig(), 'quick' );
$theServices = MediaWikiServices::getInstance();
$this->assertSame( $service2, $theServices->getService( 'Test' ) );
$this->assertNotSame( $theServices, $newServices );
$this->assertNotSame( $theServices, $oldServices );
@ -109,35 +171,42 @@ class MediaWikiServicesTest extends PHPUnit_Framework_TestCase {
}
MediaWikiServices::forceGlobalInstance( $oldServices );
$newServices->destroy();
}
public function testResetChildProcessServices() {
$newServices = $this->newMediaWikiServices();
$oldServices = MediaWikiServices::forceGlobalInstance( $newServices );
$lbFactory = $this->getMockBuilder( 'LBFactorySimple' )
->disableOriginalConstructor()
->getMock();
$lbFactory->expects( $this->once() )
$service1 = $this->getMock( DestructibleService::class );
$service1->expects( $this->once() )
->method( 'destroy' );
$newServices->redefineService(
'DBLoadBalancerFactory',
function() use ( $lbFactory ) {
return $lbFactory;
$service2 = $this->getMock( DestructibleService::class );
$service2->expects( $this->never() )
->method( 'destroy' );
// sequence of values the instantiator will return
$instantiatorReturnValues = [
$service1,
$service2,
];
$newServices->defineService(
'Test',
function() use ( &$instantiatorReturnValues ) {
return array_shift( $instantiatorReturnValues );
}
);
// force the service to become active, so we can check that it does get destroyed
$oldLBFactory = $newServices->getService( 'DBLoadBalancerFactory' );
$oldTestService = $newServices->getService( 'Test' );
MediaWikiServices::resetChildProcessServices();
$finalServices = MediaWikiServices::getInstance();
$newLBFactory = $finalServices->getService( 'DBLoadBalancerFactory' );
$this->assertNotSame( $oldLBFactory, $newLBFactory );
$newTestService = $finalServices->getService( 'Test' );
$this->assertNotSame( $oldTestService, $newTestService );
MediaWikiServices::forceGlobalInstance( $oldServices );
}

77
tests/phpunit/includes/Services/ServiceContainerTest.php
View file

@ -166,6 +166,55 @@ class ServiceContainerTest extends PHPUnit_Framework_TestCase {
$this->assertSame( 'Bar!', $services->getService( 'Bar' ) );
}
public function testImportWiring() {
$services = $this->newServiceContainer();
$wiring = [
'Foo' => function() {
return 'Foo!';
},
'Bar' => function() {
return 'Bar!';
},
'Car' => function() {
return 'FUBAR!';
},
];
$services->applyWiring( $wiring );
$newServices = $this->newServiceContainer();
// define a service before importing, so we can later check that
// existing service instances survive importWiring()
$newServices->defineService( 'Car', function() {
return 'Car!';
} );
// force instantiation
$newServices->getService( 'Car' );
// Define another service, so we can later check that extra wiring
// is not lost.
$newServices->defineService( 'Xar', function() {
return 'Xar!';
} );
// import wiring, but skip `Bar`
$newServices->importWiring( $services, [ 'Bar' ] );
$this->assertNotContains( 'Bar', $newServices->getServiceNames(), 'Skip `Bar` service' );
$this->assertSame( 'Foo!', $newServices->getService( 'Foo' ) );
// import all wiring, but preserve existing service instance
$newServices->importWiring( $services );
$this->assertContains( 'Bar', $newServices->getServiceNames(), 'Import all services' );
$this->assertSame( 'Bar!', $newServices->getService( 'Bar' ) );
$this->assertSame( 'Car!', $newServices->getService( 'Car' ), 'Use existing service instance' );
$this->assertSame( 'Xar!', $newServices->getService( 'Xar' ), 'Predefined services are kept' );
}
public function testLoadWiringFiles() {
$services = $this->newServiceContainer();
@ -220,6 +269,27 @@ class ServiceContainerTest extends PHPUnit_Framework_TestCase {
$this->assertSame( $theService1, $services->getService( $name ) );
}
public function testRedefineService_disabled() {
$services = $this->newServiceContainer( [ 'Foo' ] );
$theService1 = new stdClass();
$name = 'TestService92834576';
$services->defineService( $name, function() {
return 'Foo';
} );
// disable the service. we should be able to redefine it anyway.
$services->disableService( $name );
$services->redefineService( $name, function() use ( $theService1 ) {
return $theService1;
} );
// force instantiation, check result
$this->assertSame( $theService1, $services->getService( $name ) );
}
public function testRedefineService_fail_undefined() {
$services = $this->newServiceContainer();
@ -294,13 +364,6 @@ class ServiceContainerTest extends PHPUnit_Framework_TestCase {
$this->assertContains( 'Bar', $services->getServiceNames() );
$this->assertContains( 'Qux', $services->getServiceNames() );
// re-enable Bar service
$services->redefineService( 'Bar', function() {
return new stdClass();
} );
$services->getService( 'Bar' );
$this->setExpectedException( 'MediaWiki\Services\ServiceDisabledException' );
$services->getService( 'Qux' );
}

36
tests/phpunit/includes/config/ConfigFactoryTest.php
View file

@ -44,6 +44,42 @@ class ConfigFactoryTest extends MediaWikiTestCase {
$this->assertNotSame( $config1, $config2 );
}
/**
* @covers ConfigFactory::register
*/
public function testSalvage() {
$oldFactory = new ConfigFactory();
$oldFactory->register( 'foo', 'GlobalVarConfig::newInstance' );
$oldFactory->register( 'bar', 'GlobalVarConfig::newInstance' );
$oldFactory->register( 'quux', 'GlobalVarConfig::newInstance' );
// instantiate two of the three defined configurations
$foo = $oldFactory->makeConfig( 'foo' );
$bar = $oldFactory->makeConfig( 'bar' );
$quux = $oldFactory->makeConfig( 'quux' );
// define new config instance
$newFactory = new ConfigFactory();
$newFactory->register( 'foo', 'GlobalVarConfig::newInstance' );
$newFactory->register( 'bar', function() {
return new HashConfig();
} );
// "foo" and "quux" are defined in the old and the new factory.
// The old factory has instances for "foo" and "bar", but not "quux".
$newFactory->salvage( $oldFactory );
$newFoo = $newFactory->makeConfig( 'foo' );
$this->assertSame( $foo, $newFoo, 'existing instance should be salvaged' );
$newBar = $newFactory->makeConfig( 'bar' );
$this->assertNotSame( $bar, $newBar, 'don\'t salvage if callbacks differ' );
// the new factory doesn't have quux defined, so the quux instance should not be salvaged
$this->setExpectedException( 'ConfigException' );
$newFactory->makeConfig( 'quux' );
}
/**
* @covers ConfigFactory::register
*/