From b09b3980f991bb02a64cce0e462b97bada3b4776 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 7 Mar 2019 17:53:39 -0800 Subject: [PATCH] objectcache: add object segmentation support to BagOStuff Use it for ApiStashEdit so that large PaserOutput can be stored. Add flag to allow for value segmentation on set() in BagOStuff. Also add a flag for immediate deletion of segments on delete(). BagOStuff now has base serialize()/unserialize() methods. Bug: T204742 Change-Id: I0667a02612526d8ddfd91d5de48b6faa78bd1ab5 --- autoload.php | 1 + includes/Storage/PageEditStash.php | 42 +-- includes/libs/objectcache/APCBagOStuff.php | 13 +- includes/libs/objectcache/APCUBagOStuff.php | 27 +- includes/libs/objectcache/BagOStuff.php | 293 ++++++++++++++++-- includes/libs/objectcache/EmptyBagOStuff.php | 10 +- includes/libs/objectcache/HashBagOStuff.php | 13 +- .../libs/objectcache/MemcachedBagOStuff.php | 55 +--- includes/libs/objectcache/MemcachedClient.php | 24 +- .../objectcache/MemcachedPeclBagOStuff.php | 108 +++++-- .../objectcache/MemcachedPhpBagOStuff.php | 67 +++- .../libs/objectcache/MultiWriteBagOStuff.php | 21 ++ includes/libs/objectcache/RESTBagOStuff.php | 5 +- includes/libs/objectcache/RedisBagOStuff.php | 25 +- .../libs/objectcache/ReplicatedBagOStuff.php | 22 +- .../libs/objectcache/WinCacheBagOStuff.php | 14 +- .../serialized/SerializedValueContainer.php | 66 ++++ includes/objectcache/SqlBagOStuff.php | 20 +- .../phpunit/includes/api/ApiStashEditTest.php | 2 - .../libs/objectcache/BagOStuffTest.php | 86 ++++- .../objectcache/MemcachedBagOStuffTest.php | 2 +- 21 files changed, 684 insertions(+), 232 deletions(-) create mode 100644 includes/libs/objectcache/serialized/SerializedValueContainer.php diff --git a/autoload.php b/autoload.php index 486239d8cd..8162a19ff4 100644 --- a/autoload.php +++ b/autoload.php @@ -1320,6 +1320,7 @@ $wgAutoloadLocalClasses = [ 'SearchUpdate' => __DIR__ . '/includes/deferred/SearchUpdate.php', 'SectionProfileCallback' => __DIR__ . '/includes/profiler/SectionProfileCallback.php', 'SectionProfiler' => __DIR__ . '/includes/profiler/SectionProfiler.php', + 'SerializedValueContainer' => __DIR__ . '/includes/libs/objectcache/serialized/SerializedValueContainer.php', 'SevenZipStream' => __DIR__ . '/maintenance/includes/SevenZipStream.php', 'ShiConverter' => __DIR__ . '/languages/classes/LanguageShi.php', 'ShortPagesPage' => __DIR__ . '/includes/specials/SpecialShortpages.php', diff --git a/includes/Storage/PageEditStash.php b/includes/Storage/PageEditStash.php index 46f957fded..9c2b3e7de4 100644 --- a/includes/Storage/PageEditStash.php +++ b/includes/Storage/PageEditStash.php @@ -338,7 +338,12 @@ class PageEditStash { public function stashInputText( $text, $textHash ) { $textKey = $this->cache->makeKey( 'stashedit', 'text', $textHash ); - return $this->cache->set( $textKey, $text, self::MAX_CACHE_TTL ); + return $this->cache->set( + $textKey, + $text, + self::MAX_CACHE_TTL, + BagOStuff::WRITE_ALLOW_SEGMENTS + ); } /** @@ -388,7 +393,7 @@ class PageEditStash { */ private function getStashKey( Title $title, $contentHash, User $user ) { return $this->cache->makeKey( - 'stashed-edit-info', + 'stashedit-info-v1', md5( $title->getPrefixedDBkey() ), // Account for the edit model/text $contentHash, @@ -397,29 +402,13 @@ class PageEditStash { ); } - /** - * @param string $hash - * @return string - */ - private function getStashParserOutputKey( $hash ) { - return $this->cache->makeKey( 'stashed-edit-output', $hash ); - } - /** * @param string $key * @return stdClass|bool Object map (pstContent,output,outputID,timestamp,edits) or false */ private function getStashValue( $key ) { $stashInfo = $this->cache->get( $key ); - if ( !is_object( $stashInfo ) ) { - return false; - } - - $parserOutputKey = $this->getStashParserOutputKey( $stashInfo->outputID ); - $parserOutput = $this->cache->get( $parserOutputKey ); - if ( $parserOutput instanceof ParserOutput ) { - $stashInfo->output = $parserOutput; - + if ( is_object( $stashInfo ) && $stashInfo->output instanceof ParserOutput ) { return $stashInfo; } @@ -459,23 +448,14 @@ class PageEditStash { } // Store what is actually needed and split the output into another key (T204742) - $parserOutputID = md5( $key ); $stashInfo = (object)[ 'pstContent' => $pstContent, - 'outputID' => $parserOutputID, + 'output' => $parserOutput, 'timestamp' => $timestamp, 'edits' => $user->getEditCount() ]; - $ok = $this->cache->set( $key, $stashInfo, $ttl ); - if ( $ok ) { - $ok = $this->cache->set( - $this->getStashParserOutputKey( $parserOutputID ), - $parserOutput, - $ttl - ); - } - + $ok = $this->cache->set( $key, $stashInfo, $ttl, BagOStuff::WRITE_ALLOW_SEGMENTS ); if ( $ok ) { // These blobs can waste slots in low cardinality memcached slabs $this->pruneExcessStashedEntries( $user, $key ); @@ -494,7 +474,7 @@ class PageEditStash { $keyList = $this->cache->get( $key ) ?: []; if ( count( $keyList ) >= self::MAX_CACHE_RECENT ) { $oldestKey = array_shift( $keyList ); - $this->cache->delete( $oldestKey ); + $this->cache->delete( $oldestKey, BagOStuff::WRITE_PRUNE_SEGMENTS ); } $keyList[] = $newKey; diff --git a/includes/libs/objectcache/APCBagOStuff.php b/includes/libs/objectcache/APCBagOStuff.php index 5a36c651e7..465fe820e0 100644 --- a/includes/libs/objectcache/APCBagOStuff.php +++ b/includes/libs/objectcache/APCBagOStuff.php @@ -45,6 +45,7 @@ class APCBagOStuff extends BagOStuff { const KEY_SUFFIX = ':4'; public function __construct( array $params = [] ) { + $params['segmentationSize'] = $params['segmentationSize'] ?? INF; parent::__construct( $params ); // The extension serializer is still buggy, unlike "php" and "igbinary" $this->nativeSerialize = ( ini_get( 'apc.serializer' ) !== 'default' ); @@ -62,7 +63,7 @@ class APCBagOStuff extends BagOStuff { return $value; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { apc_store( $key . self::KEY_SUFFIX, $this->nativeSerialize ? $value : $this->serialize( $value ), @@ -80,7 +81,7 @@ class APCBagOStuff extends BagOStuff { ); } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { apc_delete( $key . self::KEY_SUFFIX ); return true; @@ -93,12 +94,4 @@ class APCBagOStuff extends BagOStuff { public function decr( $key, $value = 1 ) { return apc_dec( $key . self::KEY_SUFFIX, $value ); } - - protected function serialize( $value ) { - return $this->isInteger( $value ) ? (int)$value : serialize( $value ); - } - - protected function unserialize( $value ) { - return $this->isInteger( $value ) ? (int)$value : unserialize( $value ); - } } diff --git a/includes/libs/objectcache/APCUBagOStuff.php b/includes/libs/objectcache/APCUBagOStuff.php index 0d9822a147..b14ac7c4df 100644 --- a/includes/libs/objectcache/APCUBagOStuff.php +++ b/includes/libs/objectcache/APCUBagOStuff.php @@ -45,6 +45,7 @@ class APCUBagOStuff extends BagOStuff { const KEY_SUFFIX = ':4'; public function __construct( array $params = [] ) { + $params['segmentationSize'] = $params['segmentationSize'] ?? INF; parent::__construct( $params ); // The extension serializer is still buggy, unlike "php" and "igbinary" $this->nativeSerialize = ( ini_get( 'apc.serializer' ) !== 'default' ); @@ -54,7 +55,7 @@ class APCUBagOStuff extends BagOStuff { $casToken = null; $blob = apcu_fetch( $key . self::KEY_SUFFIX ); - $value = $this->unserialize( $blob ); + $value = $this->nativeSerialize ? $blob : $this->unserialize( $blob ); if ( $value !== false ) { $casToken = $blob; // don't bother hashing this } @@ -62,10 +63,10 @@ class APCUBagOStuff extends BagOStuff { return $value; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { return apcu_store( $key . self::KEY_SUFFIX, - $this->serialize( $value ), + $this->nativeSerialize ? $value : $this->serialize( $value ), $exptime ); } @@ -73,12 +74,12 @@ class APCUBagOStuff extends BagOStuff { public function add( $key, $value, $exptime = 0, $flags = 0 ) { return apcu_add( $key . self::KEY_SUFFIX, - $this->serialize( $value ), + $this->nativeSerialize ? $value : $this->serialize( $value ), $exptime ); } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { apcu_delete( $key . self::KEY_SUFFIX ); return true; @@ -101,20 +102,4 @@ class APCUBagOStuff extends BagOStuff { return false; } } - - protected function serialize( $value ) { - if ( $this->nativeSerialize ) { - return $value; - } - - return $this->isInteger( $value ) ? (int)$value : serialize( $value ); - } - - protected function unserialize( $value ) { - if ( $this->nativeSerialize ) { - return $value; - } - - return $this->isInteger( $value ) ? (int)$value : unserialize( $value ); - } } diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php index 0dd7b57c6d..e6f3e558a3 100644 --- a/includes/libs/objectcache/BagOStuff.php +++ b/includes/libs/objectcache/BagOStuff.php @@ -50,8 +50,14 @@ use Wikimedia\WaitConditionLoop; * For any given instance, methods like lock(), unlock(), merge(), and set() with WRITE_SYNC * should semantically operate over its entire access scope; any nodes/threads in that scope * should serialize appropriately when using them. Likewise, a call to get() with READ_LATEST - * from one node in its access scope should reflect the prior changes of any other node its access - * scope. Any get() should reflect the changes of any prior set() with WRITE_SYNC. + * from one node in its access scope should reflect the prior changes of any other node its + * access scope. Any get() should reflect the changes of any prior set() with WRITE_SYNC. + * + * Subclasses should override the default "segmentationSize" field with an appropriate value. + * The value should not be larger than what the storage backend (by default) supports. It also + * should be roughly informed by common performance bottlenecks (e.g. values over a certain size + * having poor scalability). The same goes for the "segmentedValueMaxSize" member, which limits + * the maximum size and chunk count (indirectly) of values. * * @ingroup Cache */ @@ -68,6 +74,10 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { protected $asyncHandler; /** @var int Seconds */ protected $syncTimeout; + /** @var int Bytes; chunk size of segmented cache values */ + protected $segmentationSize; + /** @var int Bytes; maximum total size of a segmented cache value */ + protected $segmentedValueMaxSize; /** @var bool */ private $debugMode = false; @@ -93,6 +103,11 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { /** Bitfield constants for set()/merge() */ const WRITE_SYNC = 4; // synchronously write to all locations for replicated stores const WRITE_CACHE_ONLY = 8; // Only change state of the in-memory cache + const WRITE_ALLOW_SEGMENTS = 16; // Allow partitioning of the value if it is large + const WRITE_PRUNE_SEGMENTS = 32; // Delete all partition segments of the value + + /** @var string Component to use for key construction of blob segment keys */ + const SEGMENT_COMPONENT = 'segment'; /** * $params include: @@ -103,6 +118,12 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * - reportDupes: Whether to emit warning log messages for all keys that were * requested more than once (requires an asyncHandler). * - syncTimeout: How long to wait with WRITE_SYNC in seconds. + * - segmentationSize: The chunk size, in bytes, of segmented values. The value should + * not exceed the maximum size of values in the storage backend, as configured by + * the site administrator. + * - segmentedValueMaxSize: The maximum total size, in bytes, of segmented values. + * This should be configured to a reasonable size give the site traffic and the + * amount of I/O between application and cache servers that the network can handle. * @param array $params */ public function __construct( array $params = [] ) { @@ -119,6 +140,8 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { } $this->syncTimeout = $params['syncTimeout'] ?? 3; + $this->segmentationSize = $params['segmentationSize'] ?? 8388608; // 8MiB + $this->segmentedValueMaxSize = $params['segmentedValueMaxSize'] ?? 67108864; // 64MiB } /** @@ -180,7 +203,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { public function get( $key, $flags = 0 ) { $this->trackDuplicateKeys( $key ); - return $this->doGet( $key, $flags ); + return $this->resolveSegments( $key, $this->doGet( $key, $flags ) ); } /** @@ -233,16 +256,112 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * @param int $flags Bitfield of BagOStuff::WRITE_* constants * @return bool Success */ - abstract public function set( $key, $value, $exptime = 0, $flags = 0 ); + public function set( $key, $value, $exptime = 0, $flags = 0 ) { + if ( + ( $flags & self::WRITE_ALLOW_SEGMENTS ) != self::WRITE_ALLOW_SEGMENTS || + is_infinite( $this->segmentationSize ) + ) { + return $this->doSet( $key, $value, $exptime, $flags ); + } + + $serialized = $this->serialize( $value ); + $segmentSize = $this->getSegmentationSize(); + $maxTotalSize = $this->getSegmentedValueMaxSize(); + + $size = strlen( $serialized ); + if ( $size <= $segmentSize ) { + // Since the work of serializing it was already done, just use it inline + return $this->doSet( + $key, + SerializedValueContainer::newUnified( $serialized ), + $exptime, + $flags + ); + } elseif ( $size > $maxTotalSize ) { + $this->setLastError( "Key $key exceeded $maxTotalSize bytes." ); + + return false; + } + + $chunksByKey = []; + $segmentHashes = []; + $count = intdiv( $size, $segmentSize ) + ( ( $size % $segmentSize ) ? 1 : 0 ); + for ( $i = 0; $i < $count; ++$i ) { + $segment = substr( $serialized, $i * $segmentSize, $segmentSize ); + $hash = sha1( $segment ); + $chunkKey = $this->makeGlobalKey( self::SEGMENT_COMPONENT, $key, $hash ); + $chunksByKey[$chunkKey] = $segment; + $segmentHashes[] = $hash; + } + + $ok = $this->setMulti( $chunksByKey, $exptime, $flags ); + if ( $ok ) { + // Only when all segments are stored should the main key be changed + $ok = $this->doSet( + $key, + SerializedValueContainer::newSegmented( $segmentHashes ), + $exptime, + $flags + ); + } + + return $ok; + } + + /** + * Set an item + * + * @param string $key + * @param mixed $value + * @param int $exptime Either an interval in seconds or a unix timestamp for expiry + * @param int $flags Bitfield of BagOStuff::WRITE_* constants + * @return bool Success + */ + abstract protected function doSet( $key, $value, $exptime = 0, $flags = 0 ); /** * Delete an item * + * For large values written using WRITE_ALLOW_SEGMENTS, this only deletes the main + * segment list key unless WRITE_PRUNE_SEGMENTS is in the flags. While deleting the segment + * list key has the effect of functionally deleting the key, it leaves unused blobs in cache. + * * @param string $key * @return bool True if the item was deleted or not found, false on failure * @param int $flags Bitfield of BagOStuff::WRITE_* constants */ - abstract public function delete( $key, $flags = 0 ); + public function delete( $key, $flags = 0 ) { + if ( ( $flags & self::WRITE_PRUNE_SEGMENTS ) != self::WRITE_PRUNE_SEGMENTS ) { + return $this->doDelete( $key, $flags ); + } + + $mainValue = $this->doGet( $key, self::READ_LATEST ); + if ( !$this->doDelete( $key, $flags ) ) { + return false; + } + + if ( !SerializedValueContainer::isSegmented( $mainValue ) ) { + return true; // no segments to delete + } + + $orderedKeys = array_map( + function ( $segmentHash ) use ( $key ) { + return $this->makeGlobalKey( self::SEGMENT_COMPONENT, $key, $segmentHash ); + }, + $mainValue->{SerializedValueContainer::SEGMENTED_HASHES} + ); + + return $this->deleteMulti( $orderedKeys, $flags ); + } + + /** + * Delete an item + * + * @param string $key + * @return bool True if the item was deleted or not found, false on failure + * @param int $flags Bitfield of BagOStuff::WRITE_* constants + */ + abstract protected function doDelete( $key, $flags = 0 ); /** * Insert an item if it does not already exist @@ -291,7 +410,10 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { $casToken = null; // passed by reference // Get the old value and CAS token from cache $this->clearLastError(); - $currentValue = $this->doGet( $key, self::READ_LATEST, $casToken ); + $currentValue = $this->resolveSegments( + $key, + $this->doGet( $key, self::READ_LATEST, $casToken ) + ); if ( $this->getLastError() ) { $this->logger->warning( __METHOD__ . ' failed due to I/O error on get() for {key}.', @@ -324,6 +446,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { return false; // IO error; don't spam retries } + } while ( !$success && --$attempts ); return $success; @@ -338,7 +461,6 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * @param int $exptime Either an interval in seconds or a unix timestamp for expiry * @param int $flags Bitfield of BagOStuff::WRITE_* constants * @return bool Success - * @throws Exception */ protected function cas( $casToken, $key, $value, $exptime = 0, $flags = 0 ) { if ( !$this->lock( $key, 0 ) ) { @@ -368,28 +490,40 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * * If an expiry in the past is given then the key will immediately be expired * + * For large values written using WRITE_ALLOW_SEGMENTS, this only changes the TTL of the + * main segment list key. While lowering the TTL of the segment list key has the effect of + * functionally lowering the TTL of the key, it might leave unused blobs in cache for longer. + * Raising the TTL of such keys is not effective, since the expiration of a single segment + * key effectively expires the entire value. + * * @param string $key - * @param int $expiry TTL or UNIX timestamp + * @param int $exptime TTL or UNIX timestamp * @param int $flags Bitfield of BagOStuff::WRITE_* constants (since 1.33) * @return bool Success Returns false on failure or if the item does not exist * @since 1.28 */ - public function changeTTL( $key, $expiry = 0, $flags = 0 ) { - $found = false; + public function changeTTL( $key, $exptime = 0, $flags = 0 ) { + $expiry = $this->convertToExpiry( $exptime ); + $delete = ( $expiry != 0 && $expiry < $this->getCurrentTime() ); - $ok = $this->merge( - $key, - function ( $cache, $ttl, $currentValue ) use ( &$found ) { - $found = ( $currentValue !== false ); + if ( !$this->lock( $key, 0 ) ) { + return false; + } + // Use doGet() to avoid having to trigger resolveSegments() + $blob = $this->doGet( $key, self::READ_LATEST ); + if ( $blob ) { + if ( $delete ) { + $ok = $this->doDelete( $key, $flags ); + } else { + $ok = $this->doSet( $key, $blob, $exptime, $flags ); + } + } else { + $ok = false; + } - return $currentValue; // nothing is written if this is false - }, - $expiry, - 1, // 1 attempt - $flags - ); + $this->unlock( $key ); - return ( $ok && $found ); + return $ok; } /** @@ -459,7 +593,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { if ( isset( $this->locks[$key] ) && --$this->locks[$key]['depth'] <= 0 ) { unset( $this->locks[$key] ); - $ok = $this->delete( "{$key}:lock" ); + $ok = $this->doDelete( "{$key}:lock" ); if ( !$ok ) { $this->logger->warning( __METHOD__ . ' failed to release lock for {key}.', @@ -533,9 +667,25 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * @return array */ public function getMulti( array $keys, $flags = 0 ) { + $valuesBykey = $this->doGetMulti( $keys, $flags ); + foreach ( $valuesBykey as $key => $value ) { + // Resolve one blob at a time (avoids too much I/O at once) + $valuesBykey[$key] = $this->resolveSegments( $key, $value ); + } + + return $valuesBykey; + } + + /** + * Get an associative array containing the item for each of the keys that have items. + * @param string[] $keys List of keys + * @param int $flags Bitfield; supports READ_LATEST [optional] + * @return array + */ + protected function doGetMulti( array $keys, $flags = 0 ) { $res = []; foreach ( $keys as $key ) { - $val = $this->get( $key, $flags ); + $val = $this->doGet( $key, $flags ); if ( $val !== false ) { $res[$key] = $val; } @@ -546,6 +696,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { /** * Batch insertion/replace + * + * This does not support WRITE_ALLOW_SEGMENTS to avoid excessive read I/O + * * @param mixed[] $data Map of (key => value) * @param int $exptime Either an interval in seconds or a unix timestamp for expiry * @param int $flags Bitfield of BagOStuff::WRITE_* constants (since 1.33) @@ -553,11 +706,13 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * @since 1.24 */ public function setMulti( array $data, $exptime = 0, $flags = 0 ) { + if ( ( $flags & self::WRITE_ALLOW_SEGMENTS ) === self::WRITE_ALLOW_SEGMENTS ) { + throw new InvalidArgumentException( __METHOD__ . ' got WRITE_ALLOW_SEGMENTS' ); + } + $res = true; foreach ( $data as $key => $value ) { - if ( !$this->set( $key, $value, $exptime, $flags ) ) { - $res = false; - } + $res = $this->doSet( $key, $value, $exptime, $flags ) && $res; } return $res; @@ -565,6 +720,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { /** * Batch deletion + * + * This does not support WRITE_ALLOW_SEGMENTS to avoid excessive read I/O + * * @param string[] $keys List of keys * @param int $flags Bitfield of BagOStuff::WRITE_* constants * @return bool Success @@ -573,7 +731,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { public function deleteMulti( array $keys, $flags = 0 ) { $res = true; foreach ( $keys as $key ) { - $res = $this->delete( $key, $flags ) && $res; + $res = $this->doDelete( $key, $flags ) && $res; } return $res; @@ -624,6 +782,43 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { return $newValue; } + /** + * Get and reassemble the chunks of blob at the given key + * + * @param string $key + * @param mixed $mainValue + * @return string|null|bool The combined string, false if missing, null on error + */ + protected function resolveSegments( $key, $mainValue ) { + if ( SerializedValueContainer::isUnified( $mainValue ) ) { + return $this->unserialize( $mainValue->{SerializedValueContainer::UNIFIED_DATA} ); + } + + if ( SerializedValueContainer::isSegmented( $mainValue ) ) { + $orderedKeys = array_map( + function ( $segmentHash ) use ( $key ) { + return $this->makeGlobalKey( self::SEGMENT_COMPONENT, $key, $segmentHash ); + }, + $mainValue->{SerializedValueContainer::SEGMENTED_HASHES} + ); + + $segmentsByKey = $this->doGetMulti( $orderedKeys ); + + $parts = []; + foreach ( $orderedKeys as $segmentKey ) { + if ( isset( $segmentsByKey[$segmentKey] ) ) { + $parts[] = $segmentsByKey[$segmentKey]; + } else { + return false; // missing segment + } + } + + return $this->unserialize( implode( '', $parts ) ); + } + + return $mainValue; + } + /** * Get the "last error" registered; clearLastError() should be called manually * @return int ERR_* constant for the "last error" registry @@ -732,7 +927,15 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { * @return bool */ protected function isInteger( $value ) { - return ( is_int( $value ) || ctype_digit( $value ) ); + if ( is_int( $value ) ) { + return true; + } elseif ( !is_string( $value ) ) { + return false; + } + + $integer = (int)$value; + + return ( $value === (string)$integer ); } /** @@ -784,6 +987,22 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { return $this->attrMap[$flag] ?? self::QOS_UNKNOWN; } + /** + * @return int|float The chunk size, in bytes, of segmented objects (INF for no limit) + * @since 1.34 + */ + public function getSegmentationSize() { + return $this->segmentationSize; + } + + /** + * @return int|float Maximum total segmented object size in bytes (INF for no limit) + * @since 1.34 + */ + public function getSegmentedValueMaxSize() { + return $this->segmentedValueMaxSize; + } + /** * Merge the flag maps of one or more BagOStuff objects into a "lowest common denominator" map * @@ -820,4 +1039,22 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { public function setMockTime( &$time ) { $this->wallClockOverride =& $time; } + + /** + * @param mixed $value + * @return string|int String/integer representation + * @note Special handling is usually needed for integers so incr()/decr() work + */ + protected function serialize( $value ) { + return is_int( $value ) ? $value : serialize( $value ); + } + + /** + * @param string|int $value + * @return mixed Original value or false on error + * @note Special handling is usually needed for integers so incr()/decr() work + */ + protected function unserialize( $value ) { + return $this->isInteger( $value ) ? (int)$value : unserialize( $value ); + } } diff --git a/includes/libs/objectcache/EmptyBagOStuff.php b/includes/libs/objectcache/EmptyBagOStuff.php index ffe3a4c53e..575bc58746 100644 --- a/includes/libs/objectcache/EmptyBagOStuff.php +++ b/includes/libs/objectcache/EmptyBagOStuff.php @@ -33,15 +33,15 @@ class EmptyBagOStuff extends BagOStuff { return false; } - public function add( $key, $value, $exp = 0, $flags = 0 ) { + protected function doSet( $key, $value, $exp = 0, $flags = 0 ) { return true; } - public function set( $key, $value, $exp = 0, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { return true; } - public function delete( $key, $flags = 0 ) { + public function add( $key, $value, $exptime = 0, $flags = 0 ) { return true; } @@ -49,6 +49,10 @@ class EmptyBagOStuff extends BagOStuff { return false; } + public function incrWithInit( $key, $ttl, $value = 1, $init = 1 ) { + return false; // faster + } + public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) { return true; // faster } diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php index d24f40854e..016bdfe36e 100644 --- a/includes/libs/objectcache/HashBagOStuff.php +++ b/includes/libs/objectcache/HashBagOStuff.php @@ -49,6 +49,7 @@ class HashBagOStuff extends BagOStuff { * - maxKeys : only allow this many keys (using oldest-first eviction) */ function __construct( $params = [] ) { + $params['segmentationSize'] = $params['segmentationSize'] ?? INF; parent::__construct( $params ); $this->token = microtime( true ) . ':' . mt_rand(); @@ -75,7 +76,7 @@ class HashBagOStuff extends BagOStuff { return $this->bag[$key][self::KEY_VAL]; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { // Refresh key position for maxCacheKeys eviction unset( $this->bag[$key] ); $this->bag[$key] = [ @@ -94,14 +95,14 @@ class HashBagOStuff extends BagOStuff { } public function add( $key, $value, $exptime = 0, $flags = 0 ) { - if ( $this->get( $key ) === false ) { - return $this->set( $key, $value, $exptime, $flags ); + if ( $this->hasKey( $key ) && !$this->expire( $key ) ) { + return false; // key already set } - return false; // key already set + return $this->doSet( $key, $value, $exptime, $flags ); } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { unset( $this->bag[$key] ); return true; @@ -136,7 +137,7 @@ class HashBagOStuff extends BagOStuff { return false; } - $this->delete( $key ); + $this->doDelete( $key ); return true; } diff --git a/includes/libs/objectcache/MemcachedBagOStuff.php b/includes/libs/objectcache/MemcachedBagOStuff.php index 3d6bd16129..cfbf2b3e80 100644 --- a/includes/libs/objectcache/MemcachedBagOStuff.php +++ b/includes/libs/objectcache/MemcachedBagOStuff.php @@ -26,14 +26,12 @@ * * @ingroup Cache */ -class MemcachedBagOStuff extends BagOStuff { - /** @var MemcachedClient|Memcached */ - protected $client; - +abstract class MemcachedBagOStuff extends BagOStuff { function __construct( array $params ) { parent::__construct( $params ); $this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_BE; // unreliable + $this->segmentationSize = $params['maxPreferedKeySize'] ?? 917504; // < 1MiB } /** @@ -50,55 +48,6 @@ class MemcachedBagOStuff extends BagOStuff { ]; } - protected function doGet( $key, $flags = 0, &$casToken = null ) { - return $this->client->get( $this->validateKeyEncoding( $key ), $casToken ); - } - - public function set( $key, $value, $exptime = 0, $flags = 0 ) { - return $this->client->set( $this->validateKeyEncoding( $key ), $value, - $this->fixExpiry( $exptime ) ); - } - - protected function cas( $casToken, $key, $value, $exptime = 0, $flags = 0 ) { - return $this->client->cas( $casToken, $this->validateKeyEncoding( $key ), - $value, $this->fixExpiry( $exptime ) ); - } - - public function delete( $key, $flags = 0 ) { - return $this->client->delete( $this->validateKeyEncoding( $key ) ); - } - - public function add( $key, $value, $exptime = 0, $flags = 0 ) { - return $this->client->add( $this->validateKeyEncoding( $key ), $value, - $this->fixExpiry( $exptime ) ); - } - - public function incr( $key, $value = 1 ) { - $n = $this->client->incr( $this->validateKeyEncoding( $key ), $value ); - - return ( $n !== false && $n !== null ) ? $n : false; - } - - public function decr( $key, $value = 1 ) { - $n = $this->client->decr( $this->validateKeyEncoding( $key ), $value ); - - return ( $n !== false && $n !== null ) ? $n : false; - } - - public function changeTTL( $key, $exptime = 0, $flags = 0 ) { - return $this->client->touch( $this->validateKeyEncoding( $key ), - $this->fixExpiry( $exptime ) ); - } - - /** - * Get the underlying client object. This is provided for debugging - * purposes. - * @return MemcachedClient|Memcached - */ - public function getClient() { - return $this->client; - } - /** * Construct a cache key. * diff --git a/includes/libs/objectcache/MemcachedClient.php b/includes/libs/objectcache/MemcachedClient.php index 937ca5546d..eecf7ec799 100644 --- a/includes/libs/objectcache/MemcachedClient.php +++ b/includes/libs/objectcache/MemcachedClient.php @@ -278,6 +278,23 @@ class MemcachedClient { } // }}} + + /** + * @param mixed $value + * @return string|integer + */ + public function serialize( $value ) { + return serialize( $value ); + } + + /** + * @param string $value + * @return mixed + */ + public function unserialize( $value ) { + return unserialize( $value ); + } + // {{{ add() /** @@ -503,7 +520,8 @@ class MemcachedClient { if ( $this->_debug ) { foreach ( $val as $k => $v ) { - $this->_debugprint( sprintf( "MemCache: sock %s got %s", serialize( $sock ), $k ) ); + $this->_debugprint( + sprintf( "MemCache: sock %s got %s", $this->serialize( $sock ), $k ) ); } } @@ -1018,7 +1036,7 @@ class MemcachedClient { * yet read "END"), these 2 calls would collide. */ if ( $flags & self::SERIALIZED ) { - $ret[$rkey] = unserialize( $ret[$rkey] ); + $ret[$rkey] = $this->unserialize( $ret[$rkey] ); } elseif ( $flags & self::INTVAL ) { $ret[$rkey] = intval( $ret[$rkey] ); } @@ -1072,7 +1090,7 @@ class MemcachedClient { if ( is_int( $val ) ) { $flags |= self::INTVAL; } elseif ( !is_scalar( $val ) ) { - $val = serialize( $val ); + $val = $this->serialize( $val ); $flags |= self::SERIALIZED; if ( $this->_debug ) { $this->_debugprint( sprintf( "client: serializing data as it is not scalar" ) ); diff --git a/includes/libs/objectcache/MemcachedPeclBagOStuff.php b/includes/libs/objectcache/MemcachedPeclBagOStuff.php index db9450387f..43cebd3255 100644 --- a/includes/libs/objectcache/MemcachedPeclBagOStuff.php +++ b/includes/libs/objectcache/MemcachedPeclBagOStuff.php @@ -27,6 +27,8 @@ * @ingroup Cache */ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { + /** @var Memcached */ + protected $client; /** * Available parameters are: @@ -93,24 +95,22 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { $this->client->setOption( Memcached::OPT_LIBKETAMA_COMPATIBLE, true ); // Set the serializer - switch ( $params['serializer'] ) { - case 'php': - $this->client->setOption( Memcached::OPT_SERIALIZER, Memcached::SERIALIZER_PHP ); - break; - case 'igbinary': - if ( !Memcached::HAVE_IGBINARY ) { - throw new InvalidArgumentException( - __CLASS__ . ': the igbinary extension is not available ' . - 'but igbinary serialization was requested.' - ); - } - $this->client->setOption( Memcached::OPT_SERIALIZER, Memcached::SERIALIZER_IGBINARY ); - break; - default: + $ok = false; + if ( $params['serializer'] === 'php' ) { + $ok = $this->client->setOption( Memcached::OPT_SERIALIZER, Memcached::SERIALIZER_PHP ); + } elseif ( $params['serializer'] === 'igbinary' ) { + if ( !Memcached::HAVE_IGBINARY ) { throw new InvalidArgumentException( - __CLASS__ . ': invalid value for serializer parameter' + __CLASS__ . ': the igbinary extension is not available ' . + 'but igbinary serialization was requested.' ); + } + $ok = $this->client->setOption( Memcached::OPT_SERIALIZER, Memcached::SERIALIZER_IGBINARY ); + } + if ( !$ok ) { + throw new InvalidArgumentException( __CLASS__ . ': invalid serializer parameter' ); } + $servers = []; foreach ( $params['servers'] as $host ) { if ( preg_match( '/^\[(.+)\]:(\d+)$/', $host, $m ) ) { @@ -138,9 +138,6 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { return $params; } - /** - * @suppress PhanTypeNonVarPassByRef - */ protected function doGet( $key, $flags = 0, &$casToken = null ) { $this->debug( "get($key)" ); if ( defined( Memcached::class . '::GET_EXTENDED' ) ) { // v3.0.0 @@ -160,9 +157,13 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { return $result; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { $this->debug( "set($key)" ); - $result = parent::set( $key, $value, $exptime, $flags = 0 ); + $result = $this->client->set( + $this->validateKeyEncoding( $key ), + $value, + $this->fixExpiry( $exptime ) + ); if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTSTORED ) { // "Not stored" is always used as the mcrouter response with AllAsyncRoute return true; @@ -172,12 +173,14 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { protected function cas( $casToken, $key, $value, $exptime = 0, $flags = 0 ) { $this->debug( "cas($key)" ); - return $this->checkResult( $key, parent::cas( $casToken, $key, $value, $exptime, $flags ) ); + $result = $this->client->cas( $casToken, $this->validateKeyEncoding( $key ), + $value, $this->fixExpiry( $exptime ) ); + return $this->checkResult( $key, $result ); } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { $this->debug( "delete($key)" ); - $result = parent::delete( $key ); + $result = $this->client->delete( $this->validateKeyEncoding( $key ) ); if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTFOUND ) { // "Not found" is counted as success in our interface return true; @@ -187,7 +190,12 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { public function add( $key, $value, $exptime = 0, $flags = 0 ) { $this->debug( "add($key)" ); - return $this->checkResult( $key, parent::add( $key, $value, $exptime ) ); + $result = $this->client->add( + $this->validateKeyEncoding( $key ), + $value, + $this->fixExpiry( $exptime ) + ); + return $this->checkResult( $key, $result ); } public function incr( $key, $value = 1 ) { @@ -242,7 +250,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { return $result; } - public function getMulti( array $keys, $flags = 0 ) { + public function doGetMulti( array $keys, $flags = 0 ) { $this->debug( 'getMulti(' . implode( ', ', $keys ) . ')' ); foreach ( $keys as $key ) { $this->validateKeyEncoding( $key ); @@ -260,9 +268,55 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { return $this->checkResult( false, $result ); } - public function changeTTL( $key, $expiry = 0, $flags = 0 ) { + public function deleteMulti( array $keys, $flags = 0 ) { + $this->debug( 'deleteMulti(' . implode( ', ', $keys ) . ')' ); + foreach ( $keys as $key ) { + $this->validateKeyEncoding( $key ); + } + $result = $this->client->deleteMulti( $keys ) ?: []; + $ok = true; + foreach ( $result as $code ) { + if ( !in_array( $code, [ true, Memcached::RES_NOTFOUND ], true ) ) { + // "Not found" is counted as success in our interface + $ok = false; + } + } + return $this->checkResult( false, $ok ); + } + + public function changeTTL( $key, $exptime = 0, $flags = 0 ) { $this->debug( "touch($key)" ); - $result = $this->client->touch( $key, $expiry ); + $result = $this->client->touch( $key, $exptime ); return $this->checkResult( $key, $result ); } + + protected function serialize( $value ) { + if ( is_int( $value ) ) { + return $value; + } + + $serializer = $this->client->getOption( Memcached::OPT_SERIALIZER ); + if ( $serializer === Memcached::SERIALIZER_PHP ) { + return serialize( $value ); + } elseif ( $serializer === Memcached::SERIALIZER_IGBINARY ) { + return igbinary_serialize( $value ); + } + + throw new UnexpectedValueException( __METHOD__ . ": got serializer '$serializer'." ); + } + + protected function unserialize( $value ) { + if ( $this->isInteger( $value ) ) { + return (int)$value; + } + + $serializer = $this->client->getOption( Memcached::OPT_SERIALIZER ); + if ( $serializer === Memcached::SERIALIZER_PHP ) { + return unserialize( $value ); + } elseif ( $serializer === Memcached::SERIALIZER_IGBINARY ) { + return igbinary_unserialize( $value ); + } + + throw new UnexpectedValueException( __METHOD__ . ": got serializer '$serializer'." ); + } } diff --git a/includes/libs/objectcache/MemcachedPhpBagOStuff.php b/includes/libs/objectcache/MemcachedPhpBagOStuff.php index 8f190c3b76..ea73cbaee7 100644 --- a/includes/libs/objectcache/MemcachedPhpBagOStuff.php +++ b/includes/libs/objectcache/MemcachedPhpBagOStuff.php @@ -27,6 +27,9 @@ * @ingroup Cache */ class MemcachedPhpBagOStuff extends MemcachedBagOStuff { + /** @var MemcachedClient */ + protected $client; + /** * Available parameters are: * - servers: The list of IP:port combinations holding the memcached servers. @@ -51,11 +54,73 @@ class MemcachedPhpBagOStuff extends MemcachedBagOStuff { $this->client->set_debug( $debug ); } - public function getMulti( array $keys, $flags = 0 ) { + protected function doGet( $key, $flags = 0, &$casToken = null ) { + $casToken = null; + + return $this->client->get( $this->validateKeyEncoding( $key ), $casToken ); + } + + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { + return $this->client->set( + $this->validateKeyEncoding( $key ), + $value, + $this->fixExpiry( $exptime ) + ); + } + + protected function doDelete( $key, $flags = 0 ) { + return $this->client->delete( $this->validateKeyEncoding( $key ) ); + } + + public function add( $key, $value, $exptime = 0, $flags = 0 ) { + return $this->client->add( + $this->validateKeyEncoding( $key ), + $value, + $this->fixExpiry( $exptime ) + ); + } + + protected function cas( $casToken, $key, $value, $exptime = 0, $flags = 0 ) { + return $this->client->cas( + $casToken, + $this->validateKeyEncoding( $key ), + $value, + $this->fixExpiry( $exptime ) + ); + } + + public function incr( $key, $value = 1 ) { + $n = $this->client->incr( $this->validateKeyEncoding( $key ), $value ); + + return ( $n !== false && $n !== null ) ? $n : false; + } + + public function decr( $key, $value = 1 ) { + $n = $this->client->decr( $this->validateKeyEncoding( $key ), $value ); + + return ( $n !== false && $n !== null ) ? $n : false; + } + + public function changeTTL( $key, $exptime = 0, $flags = 0 ) { + return $this->client->touch( + $this->validateKeyEncoding( $key ), + $this->fixExpiry( $exptime ) + ); + } + + public function doGetMulti( array $keys, $flags = 0 ) { foreach ( $keys as $key ) { $this->validateKeyEncoding( $key ); } return $this->client->get_multi( $keys ); } + + protected function serialize( $value ) { + return is_int( $value ) ? $value : $this->client->serialize( $value ); + } + + protected function unserialize( $value ) { + return $this->isInteger( $value ) ? (int)$value : $this->client->unserialize( $value ); + } } diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php index 1ed91ea298..050338237d 100644 --- a/includes/libs/objectcache/MultiWriteBagOStuff.php +++ b/includes/libs/objectcache/MultiWriteBagOStuff.php @@ -130,6 +130,7 @@ class MultiWriteBagOStuff extends BagOStuff { $missIndexes, $this->asyncWrites, 'set', + // @TODO: consider using self::WRITE_ALLOW_SEGMENTS here? [ $key, $value, self::UPGRADE_TTL ] ); } @@ -356,4 +357,24 @@ class MultiWriteBagOStuff extends BagOStuff { protected function doGet( $key, $flags = 0, &$casToken = null ) { throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); } + + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function doDelete( $key, $flags = 0 ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function doGetMulti( array $keys, $flags = 0 ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function serialize( $value ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function unserialize( $value ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } } diff --git a/includes/libs/objectcache/RESTBagOStuff.php b/includes/libs/objectcache/RESTBagOStuff.php index a10d1a47b2..2a126891a6 100644 --- a/includes/libs/objectcache/RESTBagOStuff.php +++ b/includes/libs/objectcache/RESTBagOStuff.php @@ -79,6 +79,7 @@ class RESTBagOStuff extends BagOStuff { private $extendedErrorBodyFields; public function __construct( $params ) { + $params['segmentationSize'] = $params['segmentationSize'] ?? INF; if ( empty( $params['url'] ) ) { throw new InvalidArgumentException( 'URL parameter is required' ); } @@ -146,7 +147,7 @@ class RESTBagOStuff extends BagOStuff { return false; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM) // @TODO: respect $exptime $req = [ @@ -172,7 +173,7 @@ class RESTBagOStuff extends BagOStuff { return false; // key already set } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM) $req = [ 'method' => 'DELETE', diff --git a/includes/libs/objectcache/RedisBagOStuff.php b/includes/libs/objectcache/RedisBagOStuff.php index 2c74d45916..0ba9c3f448 100644 --- a/includes/libs/objectcache/RedisBagOStuff.php +++ b/includes/libs/objectcache/RedisBagOStuff.php @@ -106,7 +106,7 @@ class RedisBagOStuff extends BagOStuff { return $result; } - public function set( $key, $value, $expiry = 0, $flags = 0 ) { + protected function doSet( $key, $value, $expiry = 0, $flags = 0 ) { list( $server, $conn ) = $this->getConnection( $key ); if ( !$conn ) { return false; @@ -128,7 +128,7 @@ class RedisBagOStuff extends BagOStuff { return $result; } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { list( $server, $conn ) = $this->getConnection( $key ); if ( !$conn ) { return false; @@ -146,7 +146,7 @@ class RedisBagOStuff extends BagOStuff { return $result; } - public function getMulti( array $keys, $flags = 0 ) { + public function doGetMulti( array $keys, $flags = 0 ) { $batches = []; $conns = []; foreach ( $keys as $key ) { @@ -351,25 +351,6 @@ class RedisBagOStuff extends BagOStuff { return $result; } - /** - * @param mixed $data - * @return string - */ - protected function serialize( $data ) { - // Serialize anything but integers so INCR/DECR work - // Do not store integer-like strings as integers to avoid type confusion (T62563) - return is_int( $data ) ? $data : serialize( $data ); - } - - /** - * @param string $data - * @return mixed - */ - protected function unserialize( $data ) { - $int = intval( $data ); - return $data === (string)$int ? $int : unserialize( $data ); - } - /** * Get a Redis object with a connection suitable for fetching the specified key * @param string $key diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php index 70f9096001..f79c1ff65f 100644 --- a/includes/libs/objectcache/ReplicatedBagOStuff.php +++ b/includes/libs/objectcache/ReplicatedBagOStuff.php @@ -75,7 +75,7 @@ class ReplicatedBagOStuff extends BagOStuff { } public function get( $key, $flags = 0 ) { - return ( $flags & self::READ_LATEST ) + return ( ( $flags & self::READ_LATEST ) == self::READ_LATEST ) ? $this->writeStore->get( $key, $flags ) : $this->readStore->get( $key, $flags ); } @@ -164,4 +164,24 @@ class ReplicatedBagOStuff extends BagOStuff { protected function doGet( $key, $flags = 0, &$casToken = null ) { throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); } + + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function doDelete( $key, $flags = 0 ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function doGetMulti( array $keys, $flags = 0 ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function serialize( $value ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } + + protected function unserialize( $blob ) { + throw new LogicException( __METHOD__ . ': proxy class does not need this method.' ); + } } diff --git a/includes/libs/objectcache/WinCacheBagOStuff.php b/includes/libs/objectcache/WinCacheBagOStuff.php index 8c419b22cd..9d7e143682 100644 --- a/includes/libs/objectcache/WinCacheBagOStuff.php +++ b/includes/libs/objectcache/WinCacheBagOStuff.php @@ -36,7 +36,7 @@ class WinCacheBagOStuff extends BagOStuff { return false; } - $value = unserialize( $blob ); + $value = $this->unserialize( $blob ); if ( $value !== false ) { $casToken = (string)$blob; // don't bother hashing this } @@ -67,8 +67,8 @@ class WinCacheBagOStuff extends BagOStuff { return $success; } - public function set( $key, $value, $expire = 0, $flags = 0 ) { - $result = wincache_ucache_set( $key, serialize( $value ), $expire ); + protected function doSet( $key, $value, $expire = 0, $flags = 0 ) { + $result = wincache_ucache_set( $key, $this->serialize( $value ), $expire ); // false positive, wincache_ucache_set returns an empty array // in some circumstances. @@ -77,7 +77,11 @@ class WinCacheBagOStuff extends BagOStuff { } public function add( $key, $value, $exptime = 0, $flags = 0 ) { - $result = wincache_ucache_add( $key, serialize( $value ), $exptime ); + if ( wincache_ucache_exists( $key ) ) { + return false; // avoid warnings + } + + $result = wincache_ucache_add( $key, $this->serialize( $value ), $exptime ); // false positive, wincache_ucache_add returns an empty array // in some circumstances. @@ -85,7 +89,7 @@ class WinCacheBagOStuff extends BagOStuff { return ( $result === [] || $result === true ); } - public function delete( $key, $flags = 0 ) { + protected function doDelete( $key, $flags = 0 ) { wincache_ucache_delete( $key ); return true; diff --git a/includes/libs/objectcache/serialized/SerializedValueContainer.php b/includes/libs/objectcache/serialized/SerializedValueContainer.php new file mode 100644 index 0000000000..7c7d8aa153 --- /dev/null +++ b/includes/libs/objectcache/serialized/SerializedValueContainer.php @@ -0,0 +1,66 @@ + self::SCHEMA_UNIFIED, + self::UNIFIED_DATA => $serialized + ]; + } + + /** + * @param string[] $segmentHashList Ordered list of hashes for each segment + * @return stdClass + */ + public static function newSegmented( array $segmentHashList ) { + return (object)[ + self::SCHEMA => self::SCHEMA_SEGMENTED, + self::SEGMENTED_HASHES => $segmentHashList + ]; + } + + /** + * @param mixed $value + * @return bool + */ + public static function isUnified( $value ) { + return self::instanceOf( $value, self::SCHEMA_UNIFIED ); + } + + /** + * @param mixed $value + * @return bool + */ + public static function isSegmented( $value ) { + return self::instanceOf( $value, self::SCHEMA_SEGMENTED ); + } + + /** + * @param mixed $value + * @param string $schema SCHEMA_* class constant + * @return bool + */ + private static function instanceOf( $value, $schema ) { + return ( + $value instanceof stdClass && + property_exists( $value, self::SCHEMA ) && + $value->{self::SCHEMA} === $schema + ); + } +} diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php index 088f94e37c..14f596a780 100644 --- a/includes/objectcache/SqlBagOStuff.php +++ b/includes/objectcache/SqlBagOStuff.php @@ -250,7 +250,7 @@ class SqlBagOStuff extends BagOStuff { return false; } - public function getMulti( array $keys, $flags = 0 ) { + protected function doGetMulti( array $keys, $flags = 0 ) { $values = []; $blobs = $this->fetchBlobMulti( $keys ); @@ -261,7 +261,7 @@ class SqlBagOStuff extends BagOStuff { return $values; } - public function fetchBlobMulti( array $keys, $flags = 0 ) { + protected function fetchBlobMulti( array $keys, $flags = 0 ) { $values = []; // array of (key => value) $keysByTable = []; @@ -391,8 +391,8 @@ class SqlBagOStuff extends BagOStuff { return $result; } - public function set( $key, $value, $exptime = 0, $flags = 0 ) { - $ok = $this->setMulti( [ $key => $value ], $exptime ); + protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) { + $ok = $this->insertMulti( [ $key => $value ], $exptime, $flags, true ); return $ok; } @@ -446,6 +446,10 @@ class SqlBagOStuff extends BagOStuff { } public function deleteMulti( array $keys, $flags = 0 ) { + return $this->purgeMulti( $keys, $flags ); + } + + public function purgeMulti( array $keys, $flags = 0 ) { $keysByTable = []; foreach ( $keys as $key ) { list( $serverIndex, $tableName ) = $this->getTableByKey( $key ); @@ -482,8 +486,8 @@ class SqlBagOStuff extends BagOStuff { return $result; } - public function delete( $key, $flags = 0 ) { - $ok = $this->deleteMulti( [ $key ], $flags ); + protected function doDelete( $key, $flags = 0 ) { + $ok = $this->purgeMulti( [ $key ], $flags ); return $ok; } @@ -730,10 +734,10 @@ class SqlBagOStuff extends BagOStuff { * On typical message and page data, this can provide a 3X decrease * in storage requirements. * - * @param mixed &$data + * @param mixed $data * @return string */ - protected function serialize( &$data ) { + protected function serialize( $data ) { $serial = serialize( $data ); if ( function_exists( 'gzdeflate' ) ) { diff --git a/tests/phpunit/includes/api/ApiStashEditTest.php b/tests/phpunit/includes/api/ApiStashEditTest.php index c6ed8a7879..47a6d81d7a 100644 --- a/tests/phpunit/includes/api/ApiStashEditTest.php +++ b/tests/phpunit/includes/api/ApiStashEditTest.php @@ -347,8 +347,6 @@ class ApiStashEditTest extends ApiTestCase { $cache = $editStash->cache; $editInfo = $cache->get( $key ); - $outputKey = $cache->makeKey( 'stashed-edit-output', $editInfo->outputID ); - $editInfo->output = $cache->get( $outputKey ); $editInfo->output->setCacheTime( wfTimestamp( TS_MW, wfTimestamp( TS_UNIX, $editInfo->output->getCacheTime() ) - $howOld - 1 ) ); diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php index 4a09a2e00d..1d11fd8ef2 100644 --- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php +++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php @@ -1,6 +1,7 @@ @@ -98,13 +99,13 @@ class BagOStuffTest extends MediaWikiTestCase { $this->cache->merge( $key, $callback, 5, 1 ), 'Non-blocking merge (CAS)' ); + if ( $this->cache instanceof MultiWriteBagOStuff ) { - $wrapper = \Wikimedia\TestingAccessWrapper::newFromObject( $this->cache ); - $n = count( $wrapper->caches ); + $wrapper = TestingAccessWrapper::newFromObject( $this->cache ); + $this->assertEquals( count( $wrapper->caches ), $calls ); } else { - $n = 1; + $this->assertEquals( 1, $calls ); } - $this->assertEquals( $n, $calls ); } /** @@ -115,10 +116,17 @@ class BagOStuffTest extends MediaWikiTestCase { $value = 'meow'; $this->cache->add( $key, $value, 5 ); - $this->assertTrue( $this->cache->changeTTL( $key, 5 ) ); + $this->assertEquals( $value, $this->cache->get( $key ) ); + $this->assertTrue( $this->cache->changeTTL( $key, 10 ) ); + $this->assertTrue( $this->cache->changeTTL( $key, 10 ) ); + $this->assertTrue( $this->cache->changeTTL( $key, 0 ) ); $this->assertEquals( $this->cache->get( $key ), $value ); $this->cache->delete( $key ); - $this->assertFalse( $this->cache->changeTTL( $key, 5 ) ); + $this->assertFalse( $this->cache->changeTTL( $key, 15 ) ); + + $this->cache->add( $key, $value, 5 ); + $this->assertTrue( $this->cache->changeTTL( $key, time() - 3600 ) ); + $this->assertFalse( $this->cache->get( $key ) ); } /** @@ -126,7 +134,9 @@ class BagOStuffTest extends MediaWikiTestCase { */ public function testAdd() { $key = $this->cache->makeKey( self::TEST_KEY ); + $this->assertFalse( $this->cache->get( $key ) ); $this->assertTrue( $this->cache->add( $key, 'test', 5 ) ); + $this->assertFalse( $this->cache->add( $key, 'test', 5 ) ); } /** @@ -237,20 +247,73 @@ class BagOStuffTest extends MediaWikiTestCase { $this->cache->makeKey( 'test-6' ) => 'ever' ]; - $this->cache->setMulti( $map, 5 ); + $this->assertTrue( $this->cache->setMulti( $map ) ); $this->assertEquals( $map, $this->cache->getMulti( array_keys( $map ) ) ); - $this->assertTrue( $this->cache->deleteMulti( array_keys( $map ), 5 ) ); + $this->assertTrue( $this->cache->deleteMulti( array_keys( $map ) ) ); + $this->assertEquals( + [], + $this->cache->getMulti( array_keys( $map ), BagOStuff::READ_LATEST ) + ); $this->assertEquals( [], $this->cache->getMulti( array_keys( $map ) ) ); } + /** + * @covers BagOStuff::get + * @covers BagOStuff::getMulti + * @covers BagOStuff::merge + * @covers BagOStuff::delete + */ + public function testSetSegmentable() { + $key = $this->cache->makeKey( self::TEST_KEY ); + $tiny = 418; + $small = wfRandomString( 32 ); + // 64 * 8 * 32768 = 16777216 bytes + $big = str_repeat( wfRandomString( 32 ) . '-' . wfRandomString( 32 ), 32768 ); + + $callback = function ( $cache, $key, $oldValue ) { + return $oldValue . '!'; + }; + + foreach ( [ $tiny, $small, $big ] as $value ) { + $this->cache->set( $key, $value, 10, BagOStuff::WRITE_ALLOW_SEGMENTS ); + $this->assertEquals( $value, $this->cache->get( $key ) ); + $this->assertEquals( $value, $this->cache->getMulti( [ $key ] )[$key] ); + + $this->assertTrue( $this->cache->merge( $key, $callback, 5 ) ); + $this->assertEquals( "$value!", $this->cache->get( $key ) ); + $this->assertEquals( "$value!", $this->cache->getMulti( [ $key ] )[$key] ); + + $this->assertTrue( $this->cache->deleteMulti( [ $key ] ) ); + $this->assertFalse( $this->cache->get( $key ) ); + $this->assertEquals( [], $this->cache->getMulti( [ $key ] ) ); + + $this->cache->set( $key, "@$value", 10, BagOStuff::WRITE_ALLOW_SEGMENTS ); + $this->assertEquals( "@$value", $this->cache->get( $key ) ); + $this->assertTrue( $this->cache->delete( $key, BagOStuff::WRITE_PRUNE_SEGMENTS ) ); + $this->assertFalse( $this->cache->get( $key ) ); + $this->assertEquals( [], $this->cache->getMulti( [ $key ] ) ); + } + + $this->cache->set( $key, 666, 10, BagOStuff::WRITE_ALLOW_SEGMENTS ); + + $this->assertEquals( 667, $this->cache->incr( $key ) ); + $this->assertEquals( 667, $this->cache->get( $key ) ); + + $this->assertEquals( 664, $this->cache->decr( $key, 3 ) ); + $this->assertEquals( 664, $this->cache->get( $key ) ); + + $this->assertTrue( $this->cache->delete( $key ) ); + $this->assertFalse( $this->cache->get( $key ) ); + } + /** * @covers BagOStuff::getScopedLock */ @@ -316,4 +379,11 @@ class BagOStuffTest extends MediaWikiTestCase { $this->assertTrue( $this->cache->unlock( $key2 ) ); $this->assertTrue( $this->cache->unlock( $key2 ) ); } + + public function tearDown() { + $this->cache->delete( $this->cache->makeKey( self::TEST_KEY ) ); + $this->cache->delete( $this->cache->makeKey( self::TEST_KEY ) . ':lock' ); + + parent::tearDown(); + } } diff --git a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php index 432754b602..45971daced 100644 --- a/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php +++ b/tests/phpunit/includes/objectcache/MemcachedBagOStuffTest.php @@ -8,7 +8,7 @@ class MemcachedBagOStuffTest extends MediaWikiTestCase { protected function setUp() { parent::setUp(); - $this->cache = new MemcachedBagOStuff( [ 'keyspace' => 'test' ] ); + $this->cache = new MemcachedPhpBagOStuff( [ 'keyspace' => 'test', 'servers' => [] ] ); } /** -- 2.20.1