Merge "Support async writes to secondary MultiWriteBagOStuff stores"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sat, 19 Sep 2015 18:23:06 +0000 (18:23 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sat, 19 Sep 2015 18:23:06 +0000 (18:23 +0000)
includes/objectcache/MultiWriteBagOStuff.php
includes/parser/ParserCache.php
tests/phpunit/includes/objectcache/MultiWriteBagOStuffTest.php [new file with mode: 0644]

index bbfaa5e..b390659 100644 (file)
 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;
        }
 }
index 47fcd30..abff543 100644 (file)
@@ -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 (file)
index 0000000..2b66181
--- /dev/null
@@ -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' );
+       }
+}