From fd8e85fc2d48442f137fd25baa12bf47cce3851f Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 20 Aug 2015 23:53:52 -0700 Subject: [PATCH] Support async writes to secondary MultiWriteBagOStuff stores * This is useful for ParserCache, as it tries to focus on memcached and use other caches (e.g. mariadb) for the long-tail of less used content, as setup on WMF. The class uses BagOStuff in a way that is compatible with this approach. Bug: T109751 Change-Id: Ia64eb44a9b52a988fde27b468d604d9163bed4b4 --- includes/objectcache/MultiWriteBagOStuff.php | 58 +++++++++++++++---- includes/parser/ParserCache.php | 9 ++- .../objectcache/MultiWriteBagOStuffTest.php | 55 ++++++++++++++++++ 3 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php diff --git a/includes/objectcache/MultiWriteBagOStuff.php b/includes/objectcache/MultiWriteBagOStuff.php index bbfaa5e1c9..b3906596be 100644 --- a/includes/objectcache/MultiWriteBagOStuff.php +++ b/includes/objectcache/MultiWriteBagOStuff.php @@ -31,26 +31,49 @@ class MultiWriteBagOStuff extends BagOStuff { /** @var BagOStuff[] */ protected $caches; + /** @var bool Use async secondary writes */ + protected $asyncReplication = false; + /** @var array[] */ + protected $asyncWrites = array(); /** - * Constructor. Parameters are: - * - * - caches: This should have a numbered array of cache parameter - * structures, in the style required by $wgObjectCaches. See - * the documentation of $wgObjectCaches for more detail. + * $params include: + * - caches: This should have a numbered array of cache parameter + * structures, in the style required by $wgObjectCaches. See + * the documentation of $wgObjectCaches for more detail. + * BagOStuff objects can also be used as values. + * The first cache is the primary one, being the first to + * be read in the fallback chain. Writes happen to all stores + * in the order they are defined. However, lock()/unlock() calls + * only use the primary store. + * - replication: Either 'sync' or 'async'. This controls whether writes to + * secondary stores are deferred when possible. Async writes + * require the HHVM register_postsend_function() function. + * Async writes can increase the chance of some race conditions + * or cause keys to expire seconds later than expected. It is + * safe to use for modules when cached values: are immutable, + * invalidation uses logical TTLs, invalidation uses etag/timestamp + * validation against the DB, or merge() is used to handle races. * * @param array $params * @throws InvalidArgumentException */ public function __construct( $params ) { parent::__construct( $params ); + if ( !isset( $params['caches'] ) ) { - throw new InvalidArgumentException( __METHOD__ . ': the caches parameter is required' ); + throw new InvalidArgumentException( __METHOD__ . ': "caches" parameter required' ); } $this->caches = array(); foreach ( $params['caches'] as $cacheInfo ) { - $this->caches[] = ObjectCache::newFromParams( $cacheInfo ); + $this->caches[] = ( $cacheInfo instanceof BagOStuff ) + ? $cacheInfo + : ObjectCache::newFromParams( $cacheInfo ); + } + + if ( isset( $params['replication'] ) && $params['replication'] === 'async' ) { + $this->asyncReplication = true; } } @@ -175,11 +198,25 @@ class MultiWriteBagOStuff extends BagOStuff { $args = func_get_args(); array_shift( $args ); - foreach ( $this->caches as $cache ) { - if ( !call_user_func_array( array( $cache, $method ), $args ) ) { - $ret = false; + foreach ( $this->caches as $i => $cache ) { + if ( $i == 0 || !$this->asyncReplication ) { + // First store or in sync mode: write now and get result + if ( !call_user_func_array( array( $cache, $method ), $args ) ) { + $ret = false; + } + } else { + // Secondary write in async mode: do not block this HTTP request + $logger = $this->logger; + DeferredUpdates::addCallableUpdate( + function() use ( $cache, $method, $args, $logger ) { + if ( !call_user_func_array( array( $cache, $method ), $args ) ) { + $logger->warning( "Async $method op failed" ); + } + } + ); } } + return $ret; } @@ -198,6 +235,7 @@ class MultiWriteBagOStuff extends BagOStuff { $ret = true; } } + return $ret; } } diff --git a/includes/parser/ParserCache.php b/includes/parser/ParserCache.php index 47fcd30a64..abff543510 100644 --- a/includes/parser/ParserCache.php +++ b/includes/parser/ParserCache.php @@ -44,15 +44,14 @@ class ParserCache { /** * Setup a cache pathway with a given back-end storage mechanism. - * May be a memcached client or a BagOStuff derivative. + * + * This class use an invalidation strategy that is compatible with + * MultiWriteBagOStuff in async replication mode. * * @param BagOStuff $memCached * @throws MWException */ - protected function __construct( $memCached ) { - if ( !$memCached ) { - throw new MWException( "Tried to create a ParserCache with an invalid memcached" ); - } + protected function __construct( BagOStuff $memCached ) { $this->mMemc = $memCached; } diff --git a/tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php b/tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php new file mode 100644 index 0000000000..2b66181c9f --- /dev/null +++ b/tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php @@ -0,0 +1,55 @@ +cache1 = new HashBagOStuff(); + $this->cache2 = new HashBagOStuff(); + $this->cache = new MultiWriteBagOStuff( array( + 'caches' => array( $this->cache1, $this->cache2 ), + 'replication' => 'async' + ) ); + } + + public function testSetImmediate() { + $key = wfRandomString(); + $value = wfRandomString(); + $this->cache->set( $key, $value ); + + // Set in tier 1 + $this->assertEquals( $value, $this->cache1->get( $key ), 'Written to tier 1' ); + // Set in tier 2 + $this->assertEquals( $value, $this->cache2->get( $key ), 'Written to tier 2' ); + } + + public function testSetDelayed() { + $key = wfRandomString(); + $value = wfRandomString(); + + // XXX: DeferredUpdates bound to transactions in CLI mode + $dbw = wfGetDB( DB_MASTER ); + $dbw->begin(); + $this->cache->set( $key, $value ); + + // Set in tier 1 + $this->assertEquals( $value, $this->cache1->get( $key ), 'Written to tier 1' ); + // Not yet set in tier 2 + $this->assertEquals( false, $this->cache2->get( $key ), 'Not written to tier 2' ); + + $dbw->commit(); + + // Set in tier 2 + $this->assertEquals( $value, $this->cache2->get( $key ), 'Written to tier 2' ); + } +} -- 2.20.1