From 8bb4f561499ba8ce546df16013b3c38c260ea6f2 Mon Sep 17 00:00:00 2001 From: Ralf Becker Date: Thu, 27 May 2021 12:29:51 +0200 Subject: [PATCH] implement (increment|decrement)Cache to avoid race-conditions if multiple processes update a value implemented in memcached and APCu backends, default implementation using get&set in base-class --- api/src/Cache.php | 76 ++++++++++++++++++++ api/src/Cache/Apcu.php | 47 ++++++++++++ api/src/Cache/Base.php | 138 ++++++++++++++++++++++++++++++++++-- api/src/Cache/Memcached.php | 46 +++++++++++- api/src/Cache/Provider.php | 24 +++++++ 5 files changed, 325 insertions(+), 6 deletions(-) diff --git a/api/src/Cache.php b/api/src/Cache.php index 73bb00f1ca..426bf8f385 100644 --- a/api/src/Cache.php +++ b/api/src/Cache.php @@ -179,6 +179,82 @@ class Cache throw new Exception\WrongParameter(__METHOD__."() unknown level '$level'!"); } + /** + * Increment some value in the cache + * + * @param string $level use Cache::(TREE|INSTANCE), Cache::(SESSION|REQUEST) are NOT supported! + * @param string $app application storing data + * @param string $location location name for data + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + public static function incrementCache(string $level, string $app, string $location, int $offset=1, int $intial_value=0, int $expiration=0) + { + //error_log(__METHOD__."('$level', '$app', '$location', $offset, $inital_value, $expiration)"); + switch($level) + { + case self::SESSION: + case self::REQUEST: + break; + + case self::INSTANCE: + case self::TREE: + default: + if (!($provider = self::get_provider($level))) + { + return false; + } + // limit expiration to configured maximum time + if (isset(self::$max_expiration) && (!$expiration || $expiration > self::$max_expiration)) + { + $expiration = self::$max_expiration; + } + return $provider->increment(self::keys($level, $app, $location), $offset, $intial_value, $expiration); + } + throw new Exception\WrongParameter(__METHOD__."() unsupported level '$level'!"); + } + + /** + * Decrements value in cache, but never below 0 + * + * If new value would be below 0, 0 will be set as new value! + * + * @param string $level use Cache::(TREE|INSTANCE), Cache::(SESSION|REQUEST) are NOT supported! + * @param string $app application storing data + * @param string $location location name for data + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + public static function decrementCache(string $level, string $app, string $location, int $offset=1, int $intial_value=0, int $expiration=0) + { + //error_log(__METHOD__."('$level', '$app', '$location', $offset, $inital_value, $expiration)"); + switch($level) + { + case self::SESSION: + case self::REQUEST: + break; + + case self::INSTANCE: + case self::TREE: + default: + if (!($provider = self::get_provider($level))) + { + return false; + } + // limit expiration to configured maximum time + if (isset(self::$max_expiration) && (!$expiration || $expiration > self::$max_expiration)) + { + $expiration = self::$max_expiration; + } + return $provider->decrement(self::keys($level, $app, $location), $offset, $intial_value, $expiration); + } + throw new Exception\WrongParameter(__METHOD__."() unsupported level '$level'!"); + } + /** * Content stored to signal one callback is already running and calculating the data, to not run it multiple times */ diff --git a/api/src/Cache/Apcu.php b/api/src/Cache/Apcu.php index 0be1297a46..1cc581e526 100644 --- a/api/src/Cache/Apcu.php +++ b/api/src/Cache/Apcu.php @@ -129,6 +129,53 @@ class Apcu extends Base implements Provider return $data; } + /** + * Increments value in cache + * + * Default implementation emulating increment using get & set. + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function increment(array $keys, int $offset=1, int $intial_value=0, int $expiration=0) + { + $key = self::key($keys); + if ($intial_value !== 0 && !apcu_exists($key)) + { + return apcu_store($key, $intial_value, $expiration) ? $intial_value : false; + } + return apcu_inc($key, $offset, $success, $expiration); + } + + /** + * Decrements value in cache, but never below 0 + * + * If new value would be below 0, 0 will be set as new value! + * Default implementation emulating decrement using get & set. + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function decrement(array $keys, int $offset=1, int $intial_value=0, int $expiration=0) + { + $key = self::key($keys); + if ($intial_value !== 0 && !apcu_exists($key)) + { + return apcu_store($key, $intial_value, $expiration) ? $intial_value : false; + } + if (($value = apcu_dec($key, $offset, $success, $expiration)) < 0) + { + $value = apcu_store($key, 0, $expiration) ? 0 : false; + } + return $value; + } + /** * Delete some data from the cache * diff --git a/api/src/Cache/Base.php b/api/src/Cache/Base.php index 1055933ced..c0f61adac5 100644 --- a/api/src/Cache/Base.php +++ b/api/src/Cache/Base.php @@ -13,7 +13,7 @@ namespace EGroupware\Api\Cache; -/*if (isset($_SERVER['SCRIPT_FILENAME']) && realpath($_SERVER['SCRIPT_FILENAME']) == __FILE__) +/*if (isset($_SERVER['SCRIPT_FILENAME']) && realpath($_SERVER['SCRIPT_FILENAME']) === __FILE__) { require_once dirname(__DIR__).'/loader/common.php'; }*/ @@ -104,7 +104,7 @@ abstract class Base implements Provider ++$failed; } } - elseif (!is_null($data)) // emulation can NOT distinquish between null and not set + elseif (!is_null($data)) // emulation can NOT distinguish between null and not set { $locations[$location] = $data; } @@ -149,6 +149,61 @@ abstract class Base implements Provider ++$failed; } } + + // test increment + $keys = [$level, __CLASS__, 'increment']; + $this->delete($keys); + + if (($val=$this->increment($keys, 3, 8)) !== 8) + { + if ($verbose) echo "$label: increment(\$keys, 3, 8)=".array2string($val)." !== 8 for initial/unset \$keys\n"; + ++$failed; + } + if (($val=$this->get($keys)) != 8) // get always returns string! + { + if ($verbose) echo "$label: get(\$keys)=".array2string($val)." != 8 for reading back incremented value\n"; + ++$failed; + } + if (($val=$this->increment($keys, 2, 5)) !== 10) + { + if ($verbose) echo "$label: increment(\$keys, 2, 5)=".array2string($val)." !== 10 for current \$keys === 8\n"; + ++$failed; + } + if (($val=$this->get($keys)) != 10) + { + if ($verbose) echo "$label: get(\$keys)=".array2string($val)." != 10 for reading back incremented value\n"; + ++$failed; + } + + // test decrement + $keys = [$level, __CLASS__, 'decrement']; + $this->delete($keys); + + if (($val=$this->decrement($keys, 3, 2)) !== 2) + { + if ($verbose) echo "$label: decrement(\$keys, 3, 2)=".array2string($val)." !== 2 for initial/unset \$keys\n"; + ++$failed; + } + if (($val=$this->get($keys)) != 2) + { + if ($verbose) echo "$label: get(\$keys)=".array2string($val)." != 2 for reading back decremented value\n"; + ++$failed; + } + if (($val=$this->decrement($keys, 2, 5)) !== 0) + { + if ($verbose) echo "$label: decrement(\$keys, 2, 5)=".array2string($val)." !== 0 for current \$keys === 2\n"; + ++$failed; + } + if (($val=$this->get($keys)) != 0) + { + if ($verbose) echo "$label: get(\$keys)=".array2string($val)." != 0 for reading back decremented value\n"; + ++$failed; + } + if (($val=$this->decrement($keys, 2, 5)) !== 0) + { + if ($verbose) echo "$label: decrement(\$keys, 2, 5)=".array2string($val)." !== 0 for current \$keys === 0 (value never less then 0!)\n"; + ++$failed; + } } return $failed; @@ -167,15 +222,90 @@ abstract class Base implements Provider unset($keys); // required by function signature return false; } + + /** + * Stores some data in the cache + * + * @param array $keys eg. array($level,$app,$location) + * @param mixed $data + * @param int $expiration =0 + * @return boolean true on success, false on error + */ + abstract function set(array $keys, $data, $expiration=0); + + /** + * Get some data from the cache + * + * @param array $keys eg. array($level,$app,$location) + * @return mixed data stored or NULL if not found in cache + */ + abstract function get(array $keys); + + /** + * Increments value in cache + * + * Default implementation emulating increment using get & set. + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function increment(array $keys, int $offset=1, int $intial_value=0, int $expiration=0) + { + if (($value = $this->get($keys)) === false) + { + return $value; + } + if (!isset($value)) + { + $value = $intial_value; + } + else + { + $value += $offset; + } + return $this->set($keys, $value, $expiration) ? $value : false; + } + + /** + * Decrements value in cache, but never below 0 + * + * If new value would be below 0, 0 will be set as new value! + * Default implementation emulating decrement using get & set. + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function decrement(array $keys, int $offset=1, int $intial_value=0, int $expiration=0) + { + if (($value = $this->get($keys)) === false) + { + return $value; + } + if (!isset($value)) + { + $value = $intial_value; + } + else + { + if (($value -= $offset) < 0) $value = 0; + } + return $this->set($keys, $value, $expiration) ? $value : false; + } } // some testcode, if this file is called via it's URL // can be run on command-line: sudo php -d apc.enable_cli=1 -f api/src/Cache/Base.php -/*if (isset($_SERVER['SCRIPT_FILENAME']) && realpath($_SERVER['SCRIPT_FILENAME']) == __FILE__) +/*if (isset($_SERVER['SCRIPT_FILENAME']) && realpath($_SERVER['SCRIPT_FILENAME']) === __FILE__) { if (!isset($_SERVER['HTTP_HOST'])) { - chdir(dirname(__FILE__)); // to enable our relative pathes to work + chdir(__DIR__); // to enable our relative pathes to work } $GLOBALS['egw_info'] = array( 'flags' => array( diff --git a/api/src/Cache/Memcached.php b/api/src/Cache/Memcached.php index 1ed1b104df..ade61d1ac9 100644 --- a/api/src/Cache/Memcached.php +++ b/api/src/Cache/Memcached.php @@ -45,7 +45,7 @@ class Memcached extends Base implements ProviderMultiple /** * Use Libketama consistent hashing * - * Off by default as for just 2 Memcached servers it creates an extrem + * Off by default as for just 2 Memcached servers it creates an extreme * unbalanced distribution favoring the 2. server and has no benefits * as requests to the failed node can only go to the other one anyway. * @@ -54,7 +54,7 @@ class Memcached extends Base implements ProviderMultiple private $consistent = false; /** - * Retry on node failure: 0: no retry, 1: retry on set/add/delete, 2: allways retry + * Retry on node failure: 0: no retry, 1: retry on set/add/delete, 2: always retry * * @var retry */ @@ -245,6 +245,48 @@ class Memcached extends Base implements ProviderMultiple $this->memcache->delete(self::key($keys)); } + /** + * Increments value in cache + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function increment(array $keys, int $offset=1, int $intial_value=0, int $expiration=0) + { + $ret = $this->memcache->increment($key=self::key($keys), $offset, $intial_value, $expiration); + if ($ret === false && $this->retry > 0) + { + $ret = $this->memcache->increment($key, $offset, $intial_value, $expiration); + } + // fix memcached returning strings + return $ret !== false ? (int)$ret : $ret; + } + + /** + * Decrements value in cache, but never below 0 + * + * If new value would be below 0, 0 will be set as new value! + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to set and return, if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function decrement(array $keys, int $offset=1, int $intial_value=0, int $expiration=0) + { + $ret = $this->memcache->decrement($key=self::key($keys), $offset, $intial_value, $expiration); + if ($ret === false && $this->retry > 0) + { + $ret = $this->memcache->decrement($key, $offset, $intial_value, $expiration); + } + // fix memcached returning strings + return $ret !== false ? (int)$ret : $ret; + } + /** * Create a single key from $keys * diff --git a/api/src/Cache/Provider.php b/api/src/Cache/Provider.php index 77a7ceb6cb..da2802acfb 100644 --- a/api/src/Cache/Provider.php +++ b/api/src/Cache/Provider.php @@ -58,6 +58,30 @@ interface Provider */ function get(array $keys); + /** + * Increments value in cache + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to use if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function increment(array $keys, int $offset=1, int $intial_value=0, int $expiration=0); + + /** + * Decrements value in cache, but never below 0 + * + * If new value would be below 0, 0 will be set as new value! + * + * @param array $keys + * @param int $offset =1 how much to increment by + * @param int $intial_value =0 value to use if not in cache + * @param int $expiration =0 + * @return false|int new value on success, false on error + */ + function decrement(array $keys, int $offset=1, int $intial_value=0, int $expiration=0); + /** * Delete some data from the cache *