jenkins-bot has submitted this change and it was merged.
Change subject: Support async writes to secondary MultiWriteBagOStuff stores
......................................................................
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
---
M includes/objectcache/MultiWriteBagOStuff.php
M includes/parser/ParserCache.php
A tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php
3 files changed, 107 insertions(+), 15 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/objectcache/MultiWriteBagOStuff.php
b/includes/objectcache/MultiWriteBagOStuff.php
index bbfaa5e..b390659 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 @@
$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 @@
$ret = true;
}
}
+
return $ret;
}
}
diff --git a/includes/parser/ParserCache.php b/includes/parser/ParserCache.php
index 47fcd30..abff543 100644
--- a/includes/parser/ParserCache.php
+++ b/includes/parser/ParserCache.php
@@ -44,15 +44,14 @@
/**
* 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 0000000..2b66181
--- /dev/null
+++ b/tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php
@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * @group Database
+ */
+class MultiWriteBagOStuffTest extends MediaWikiTestCase {
+ /** @var HashBagOStuff */
+ private $cache1;
+ /** @var HashBagOStuff */
+ private $cache2;
+ /** @var MultiWriteBagOStuff */
+ private $cache;
+
+ protected function setUp() {
+ parent::setUp();
+
+ $this->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' );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/232899
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia64eb44a9b52a988fde27b468d604d9163bed4b4
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: Jackmcbarn <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
