objectcache: only use memcached "gets" when tokens are needed

This saves a few bytes in the response size and make it easy
for memcached proxies to distinguish key fetches that are part
of check-and-set cycles from those that are not.

MediumSpecificBagOStuff now requires PASS_BY_REF to fetch CAS
tokens. BagOStuff::merge() and WinCacheBagOStuff::doCas() are
the only callers that need this mode.

Bug: T257003
Change-Id: If91963f58adc4cda94f6d634ee0252a479a0fc5e
This commit is contained in:
Aaron Schulz 2020-07-02 14:32:37 -07:00 committed by Krinkle
parent 6939a6c263
commit 7523716ebd
11 changed files with 51 additions and 23 deletions

View file

@ -62,11 +62,12 @@ class APCUBagOStuff extends MediumSpecificBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
$blob = apcu_fetch( $key . self::KEY_SUFFIX );
$value = $this->nativeSerialize ? $blob : $this->unserialize( $blob );
if ( $value !== false ) {
if ( $getToken && $value !== false ) {
$casToken = $blob; // don't bother hashing this
}

View file

@ -65,6 +65,7 @@ class HashBagOStuff extends MediumSpecificBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
if ( !$this->hasKey( $key ) || $this->expire( $key ) ) {
@ -76,7 +77,9 @@ class HashBagOStuff extends MediumSpecificBagOStuff {
unset( $this->bag[$key] );
$this->bag[$key] = $temp;
$casToken = $this->bag[$key][self::KEY_CAS];
if ( $getToken ) {
$casToken = $this->bag[$key][self::KEY_CAS];
}
return $this->bag[$key][self::KEY_VAL];
}

View file

@ -61,6 +61,9 @@ abstract class MediumSpecificBagOStuff extends BagOStuff {
/** @var string Component to use for key construction of blob segment keys */
private const SEGMENT_COMPONENT = 'segment';
/** @var int Idiom for doGet() to return extra information by reference */
protected const PASS_BY_REF = -1;
/**
* @see BagOStuff::__construct()
* Additional $params options include:
@ -151,7 +154,7 @@ abstract class MediumSpecificBagOStuff extends BagOStuff {
/**
* @param string $key
* @param int $flags Bitfield of BagOStuff::READ_* constants [optional]
* @param mixed|null &$casToken Token to use for check-and-set comparisons
* @param mixed|null &$casToken cas() token if MediumSpecificBagOStuff::PASS_BY_REF [returned]
* @return mixed Returns false on failure or if the item does not exist
*/
abstract protected function doGet( $key, $flags = 0, &$casToken = null );
@ -275,7 +278,7 @@ abstract class MediumSpecificBagOStuff extends BagOStuff {
final protected function mergeViaCas( $key, callable $callback, $exptime, $attempts, $flags ) {
$attemptsLeft = $attempts;
do {
$token = null; // passed by reference
$token = self::PASS_BY_REF; // passed by reference
// Get the old value and CAS token from cache
$this->clearLastError();
$currentValue = $this->resolveSegments(
@ -366,7 +369,7 @@ abstract class MediumSpecificBagOStuff extends BagOStuff {
return false; // non-blocking
}
$curCasToken = null; // passed by reference
$curCasToken = self::PASS_BY_REF; // passed by reference
$this->clearLastError();
$this->doGet( $key, self::READ_LATEST, $curCasToken );
if ( is_object( $curCasToken ) ) {

View file

@ -178,10 +178,14 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
$this->debug( "get($key)" );
$client = $this->acquireSyncClient();
if ( defined( Memcached::class . '::GET_EXTENDED' ) ) { // v3.0.0
// T257003: only require "gets" (instead of "get") when a CAS token is needed
if ( $getToken ) {
/** @noinspection PhpUndefinedClassConstantInspection */
$flags = Memcached::GET_EXTENDED;
$res = $client->get( $this->validateKeyEncoding( $key ), null, $flags );
@ -193,7 +197,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
$casToken = null;
}
} else {
$result = $client->get( $this->validateKeyEncoding( $key ), null, $casToken );
$result = $client->get( $this->validateKeyEncoding( $key ) );
}
return $this->checkResult( $key, $result );
@ -327,7 +331,9 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
$this->validateKeyEncoding( $key );
}
// The PECL implementation uses "gets" which works as well as a pipeline
// The PECL implementation uses multi-key "get"/"gets"; no need to pipeline.
// T257003: avoid Memcached::GET_EXTENDED; no tokens are needed and that requires "gets"
// https://github.com/libmemcached/libmemcached/blob/eda2becbec24363f56115fa5d16d38a2d1f54775/libmemcached/get.cc#L272
$result = $this->acquireSyncClient()->getMulti( $keys ) ?: [];
return $this->checkResult( false, $result );

View file

@ -59,9 +59,12 @@ class MemcachedPhpBagOStuff extends MemcachedBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
return $this->client->get( $this->validateKeyEncoding( $key ), $casToken );
// T257003: only require "gets" (instead of "get") when a CAS token is needed
return $getToken
? $this->client->get( $this->validateKeyEncoding( $key ), $casToken )
: $this->client->get( $this->validateKeyEncoding( $key ) );
}
protected function doSet( $key, $value, $exptime = 0, $flags = 0 ) {

View file

@ -137,6 +137,7 @@ class RESTBagOStuff extends MediumSpecificBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
$req = [
@ -150,7 +151,9 @@ class RESTBagOStuff extends MediumSpecificBagOStuff {
if ( is_string( $rbody ) ) {
$value = $this->decodeBody( $rbody );
/// @FIXME: use some kind of hash or UUID header as CAS token
$casToken = ( $value !== false ) ? $rbody : null;
if ( $getToken && $value !== false ) {
$casToken = $rbody;
}
return $value;
}

View file

@ -89,6 +89,7 @@ class RedisBagOStuff extends MediumSpecificBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
$conn = $this->getConnection( $key );
@ -99,7 +100,9 @@ class RedisBagOStuff extends MediumSpecificBagOStuff {
$e = null;
try {
$value = $conn->get( $key );
$casToken = $value;
if ( $getToken && $value !== false ) {
$casToken = $value;
}
$result = $this->unserialize( $value );
} catch ( RedisException $e ) {
$result = false;

View file

@ -34,6 +34,7 @@ class WinCacheBagOStuff extends MediumSpecificBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
$blob = wincache_ucache_get( $key );
@ -42,7 +43,7 @@ class WinCacheBagOStuff extends MediumSpecificBagOStuff {
}
$value = $this->unserialize( $blob );
if ( $value !== false ) {
if ( $getToken && $value !== false ) {
$casToken = (string)$blob; // don't bother hashing this
}
@ -54,7 +55,7 @@ class WinCacheBagOStuff extends MediumSpecificBagOStuff {
return false;
}
$curCasToken = null; // passed by reference
$curCasToken = self::PASS_BY_REF; // passed by reference
$this->doGet( $key, self::READ_LATEST, $curCasToken );
if ( $casToken === $curCasToken ) {
$success = $this->set( $key, $value, $exptime, $flags );

View file

@ -465,6 +465,8 @@ class MemcachedClient {
* @return mixed
*/
public function get( $key, &$casToken = null ) {
$getToken = ( func_num_args() >= 2 );
if ( $this->_debug ) {
$this->_debugprint( "get($key)" );
}
@ -491,7 +493,8 @@ class MemcachedClient {
$this->stats['get'] = 1;
}
$cmd = "gets $key\r\n";
$cmd = $getToken ? "gets" : "get";
$cmd .= " $key\r\n";
if ( !$this->_fwrite( $sock, $cmd ) ) {
return false;
}
@ -551,7 +554,7 @@ class MemcachedClient {
$gather = array();
// Send out the requests
foreach ( $socks as $sock ) {
$cmd = 'gets';
$cmd = 'get';
foreach ( $sock_keys[intval( $sock )] as $key ) {
$cmd .= ' ' . $key;
}
@ -565,7 +568,7 @@ class MemcachedClient {
// Parse responses
$val = array();
foreach ( $gather as $sock ) {
$this->_load_items( $sock, $val, $casToken );
$this->_load_items( $sock, $val );
}
if ( $this->_debug ) {
@ -968,7 +971,7 @@ class MemcachedClient {
* to stop reading (right after "END") and we return right after that.
*/
return false;
} elseif ( preg_match( '/^VALUE (\S+) (\d+) (\d+) (\d+)$/', $decl, $match ) ) {
} elseif ( preg_match( '/^VALUE (\S+) (\d+) (\d+)(?: (\d+))?$/', $decl, $match ) ) {
/*
* Read all data returned. This can be either one or multiple values.
* Save all that data (in an array) to be processed later: we'll first
@ -980,7 +983,7 @@ class MemcachedClient {
$match[1], // rkey
$match[2], // flags
$match[3], // len
$match[4], // casToken
$match[4] ?? null, // casToken (appears with "gets" but not "get")
$this->_fread( $sock, $match[3] + 2 ), // data
);
} elseif ( $decl == "END" ) {
@ -993,7 +996,7 @@ class MemcachedClient {
* meaningful return values.
*/
foreach ( $results as $vars ) {
list( $rkey, $flags, $len, $casToken, $data ) = $vars;
list( $rkey, $flags, /* length */, $casToken, $data ) = $vars;
if ( $data === false || substr( $data, -2 ) !== "\r\n" ) {
$this->_handle_error( $sock,

View file

@ -201,7 +201,7 @@ class WANObjectCache implements
/** @var string Default process cache name and max key count */
private const PC_PRIMARY = 'primary:1000';
/** @var int Idion for get()/getMulti() to return extra information by reference */
/** @var int Idiom for get()/getMulti() to return extra information by reference */
public const PASS_BY_REF = -1;
/** @var int Use twemproxy-style Hash Tag key scheme (e.g. "{...}") */

View file

@ -259,14 +259,16 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$getToken = ( $casToken === self::PASS_BY_REF );
$casToken = null;
$blobs = $this->fetchBlobMulti( [ $key ] );
if ( array_key_exists( $key, $blobs ) ) {
$blob = $blobs[$key];
$value = $this->unserialize( $blob );
$casToken = ( $value !== false ) ? $blob : null;
if ( $getToken && $value !== false ) {
$casToken = $blob;
}
return $value;
}