From: Aaron Schulz Date: Wed, 27 Mar 2019 02:40:40 +0000 (-0700) Subject: objectcache: defined some missing methods in MultiWriteBagOStuff X-Git-Tag: 1.34.0-rc.0~2312^2 X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22auteur_infos%22%2C%22id_auteur=%24connect_id_auteur%22%29%20.%20%22?a=commitdiff_plain;h=25fd7dbc7b7e2ebba5d1a1a23d8530c341110d83;p=lhc%2Fweb%2Fwiklou.git objectcache: defined some missing methods in MultiWriteBagOStuff Also, reorder some method definitions to match the base class. This makes it easier to compare the two. In addition, refactor the doWrite() method to make the calls clearer. Change-Id: I795b395b9c54645f62962461a28067ae38b291ed --- diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php index 2d3eed583a..fb9971734b 100644 --- a/includes/libs/objectcache/MultiWriteBagOStuff.php +++ b/includes/libs/objectcache/MultiWriteBagOStuff.php @@ -27,6 +27,9 @@ use Wikimedia\ObjectFactory; * are implemented by reading from the caches in the order they are given in * the configuration until a cache gives a positive result. * + * Note that cache key construction will use the first cache backend in the list, + * so make sure that the other backends can handle such keys (e.g. via encoding). + * * @ingroup Cache */ class MultiWriteBagOStuff extends BagOStuff { @@ -124,67 +127,72 @@ class MultiWriteBagOStuff extends BagOStuff { ) { // Backfill the value to the higher (and often faster/smaller) cache tiers $this->doWrite( - $missIndexes, $this->asyncWrites, 'set', $key, $value, self::UPGRADE_TTL + $missIndexes, + $this->asyncWrites, + 'set', + [ $key, $value, self::UPGRADE_TTL ] ); } return $value; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { - $asyncWrites = ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC ) - ? false - : $this->asyncWrites; - - return $this->doWrite( $this->cacheIndexes, $asyncWrites, 'set', $key, $value, $exptime ); + return $this->doWrite( + $this->cacheIndexes, + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() + ); } public function delete( $key, $flags = 0 ) { - return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'delete', $key ); + return $this->doWrite( + $this->cacheIndexes, + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() + ); } public function add( $key, $value, $exptime = 0, $flags = 0 ) { // Try the write to the top-tier cache - $ok = $this->doWrite( [ 0 ], $this->asyncWrites, 'add', $key, $value, $exptime ); + $ok = $this->doWrite( + [ 0 ], + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() + ); + if ( $ok ) { // Relay the add() using set() if it succeeded. This is meant to handle certain // migration scenarios where the same store might get written to twice for certain // keys. In that case, it does not make sense to return false due to "self-conflicts". return $this->doWrite( array_slice( $this->cacheIndexes, 1 ), - $this->asyncWrites, + $this->usesAsyncWritesGivenFlags( $flags ), 'set', - $key, - $value, - $exptime + [ $key, $value, $exptime, $flags ] ); } return false; } - public function incr( $key, $value = 1 ) { - return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'incr', $key, $value ); - } - - public function decr( $key, $value = 1 ) { - return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'decr', $key, $value ); - } - public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) { - $asyncWrites = ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC ) - ? false - : $this->asyncWrites; + return $this->doWrite( + $this->cacheIndexes, + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() + ); + } + public function changeTTL( $key, $exptime = 0, $flags = 0 ) { return $this->doWrite( $this->cacheIndexes, - $asyncWrites, - 'merge', - $key, - $callback, - $exptime, - $attempts, - $flags + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() ); } @@ -197,6 +205,82 @@ class MultiWriteBagOStuff extends BagOStuff { // Only the first cache is locked return $this->caches[0]->unlock( $key ); } + /** + * Delete objects expiring before a certain date. + * + * Succeed if any of the child caches succeed. + * @param string $date + * @param bool|callable $progressCallback + * @return bool + */ + public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) { + $ret = false; + foreach ( $this->caches as $cache ) { + if ( $cache->deleteObjectsExpiringBefore( $date, $progressCallback ) ) { + $ret = true; + } + } + + return $ret; + } + + public function getMulti( array $keys, $flags = 0 ) { + // Just iterate over each key in order to handle all the backfill logic + $res = []; + foreach ( $keys as $key ) { + $val = $this->get( $key, $flags ); + if ( $val !== false ) { + $res[$key] = $val; + } + } + + return $res; + } + + public function setMulti( array $data, $exptime = 0, $flags = 0 ) { + return $this->doWrite( + $this->cacheIndexes, + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() + ); + } + + public function deleteMulti( array $data, $flags = 0 ) { + return $this->doWrite( + $this->cacheIndexes, + $this->usesAsyncWritesGivenFlags( $flags ), + __FUNCTION__, + func_get_args() + ); + } + + public function incr( $key, $value = 1 ) { + return $this->doWrite( + $this->cacheIndexes, + $this->asyncWrites, + __FUNCTION__, + func_get_args() + ); + } + + public function decr( $key, $value = 1 ) { + return $this->doWrite( + $this->cacheIndexes, + $this->asyncWrites, + __FUNCTION__, + func_get_args() + ); + } + + public function incrWithInit( $key, $ttl, $value = 1, $init = 1 ) { + return $this->doWrite( + $this->cacheIndexes, + $this->asyncWrites, + __FUNCTION__, + func_get_args() + ); + } public function getLastError() { return $this->caches[0]->getLastError(); @@ -205,19 +289,17 @@ class MultiWriteBagOStuff extends BagOStuff { public function clearLastError() { $this->caches[0]->clearLastError(); } - /** * Apply a write method to the backing caches specified by $indexes (in order) * * @param int[] $indexes List of backing cache indexes * @param bool $asyncWrites - * @param string $method - * @param mixed $args,... + * @param string $method Method name of backing caches + * @param array[] $args Arguments to the method of backing caches * @return bool */ - protected function doWrite( $indexes, $asyncWrites, $method /*, ... */ ) { + protected function doWrite( $indexes, $asyncWrites, $method, array $args ) { $ret = true; - $args = array_slice( func_get_args(), 3 ); if ( array_diff( $indexes, [ 0 ] ) && $asyncWrites && $method !== 'merge' ) { // Deep-clone $args to prevent misbehavior when something writes an @@ -249,22 +331,11 @@ class MultiWriteBagOStuff extends BagOStuff { } /** - * Delete objects expiring before a certain date. - * - * Succeed if any of the child caches succeed. - * @param string $date - * @param bool|callable $progressCallback + * @param int $flags * @return bool */ - public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) { - $ret = false; - foreach ( $this->caches as $cache ) { - if ( $cache->deleteObjectsExpiringBefore( $date, $progressCallback ) ) { - $ret = true; - } - } - - return $ret; + protected function usesAsyncWritesGivenFlags( $flags ) { + return ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC ) ? false : $this->asyncWrites; } public function makeKeyInternal( $keyspace, $args ) { diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php index 3d8c9cbb0b..b68f1509a7 100644 --- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php +++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php @@ -98,7 +98,13 @@ class BagOStuffTest extends MediaWikiTestCase { $this->cache->merge( $key, $callback, 5, 1 ), 'Non-blocking merge (CAS)' ); - $this->assertEquals( 1, $calls ); + if ( $this->cache instanceof MultiWriteBagOStuff ) { + $wrapper = \Wikimedia\TestingAccessWrapper::newFromObject( $this->cache ); + $n = count( $wrapper->caches ); + } else { + $n = 1; + } + $this->assertEquals( $n, $calls ); } /**