Skip to content

Commit

Permalink
bug #23969 [Cache] Use namespace versioning for backends that dont su…
Browse files Browse the repository at this point in the history
…pport clearing by keys (nicolas-grekas)

This PR was merged into the 3.3 branch.

Discussion
----------

[Cache] Use namespace versioning for backends that dont support clearing by keys

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23960
| License       | MIT
| Doc PR        | -

Commits
-------

f8a7518 [Cache] Use namespace versioning for backends that dont support clearing by keys
  • Loading branch information
fabpot committed Aug 31, 2017
2 parents b641a76 + f8a7518 commit 4a0cc29
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
Expand Up @@ -38,7 +38,7 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface
*/
protected function __construct($namespace = '', $defaultLifetime = 0)
{
$this->namespace = '' === $namespace ? '' : $this->getId($namespace).':';
$this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':';
if (null !== $this->maxIdLength && strlen($namespace) > $this->maxIdLength - 24) {
throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s")', $this->maxIdLength - 24, strlen($namespace), $namespace));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Adapter/ProxyAdapter.php
Expand Up @@ -35,7 +35,7 @@ public function __construct(CacheItemPoolInterface $pool, $namespace = '', $defa
{
$this->pool = $pool;
$this->poolHash = $poolHash = spl_object_hash($pool);
$this->namespace = '' === $namespace ? '' : $this->getId($namespace);
$this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace);
$this->namespaceLen = strlen($namespace);
$this->createCacheItem = \Closure::bind(
function ($key, $innerItem) use ($defaultLifetime, $poolHash) {
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/Cache/CacheItem.php
Expand Up @@ -148,6 +148,8 @@ public function getPreviousTags()
*
* @param string $key The key to validate
*
* @return string
*
* @throws InvalidArgumentException When $key is not valid
*/
public static function validateKey($key)
Expand All @@ -161,6 +163,8 @@ public static function validateKey($key)
if (false !== strpbrk($key, '{}()/\@:')) {
throw new InvalidArgumentException(sprintf('Cache key "%s" contains reserved characters {}()/\@:', $key));
}

return $key;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Simple/AbstractCache.php
Expand Up @@ -37,7 +37,7 @@ abstract class AbstractCache implements CacheInterface, LoggerAwareInterface
protected function __construct($namespace = '', $defaultLifetime = 0)
{
$this->defaultLifetime = max(0, (int) $defaultLifetime);
$this->namespace = '' === $namespace ? '' : $this->getId($namespace).':';
$this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':';
if (null !== $this->maxIdLength && strlen($namespace) > $this->maxIdLength - 24) {
throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s")', $this->maxIdLength - 24, strlen($namespace), $namespace));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Tests/CacheItemTest.php
Expand Up @@ -18,7 +18,7 @@ class CacheItemTest extends TestCase
{
public function testValidKey()
{
$this->assertNull(CacheItem::validateKey('foo'));
$this->assertSame('foo', CacheItem::validateKey('foo'));
}

/**
Expand Down
46 changes: 42 additions & 4 deletions src/Symfony/Component/Cache/Traits/AbstractTrait.php
Expand Up @@ -24,6 +24,8 @@ trait AbstractTrait
use LoggerAwareTrait;

private $namespace;
private $namespaceVersion = '';
private $versioningIsEnabled = false;
private $deferred = array();

/**
Expand Down Expand Up @@ -102,10 +104,18 @@ public function hasItem($key)
*/
public function clear()
{
if ($cleared = $this->versioningIsEnabled) {
$this->namespaceVersion = 2;
foreach ($this->doFetch(array('@'.$this->namespace)) as $v) {
$this->namespaceVersion = 1 + (int) $v;
}
$this->namespaceVersion .= ':';
$cleared = $this->doSave(array('@'.$this->namespace => $this->namespaceVersion), 0);
}
$this->deferred = array();

try {
return $this->doClear($this->namespace);
return $this->doClear($this->namespace) || $cleared;
} catch (\Exception $e) {
CacheItem::log($this->logger, 'Failed to clear the cache', array('exception' => $e));

Expand Down Expand Up @@ -158,6 +168,27 @@ public function deleteItems(array $keys)
return $ok;
}

/**
* Enables/disables versioning of items.
*
* When versioning is enabled, clearing the cache is atomic and doesn't require listing existing keys to proceed,
* but old keys may need garbage collection and extra round-trips to the back-end are required.
*
* Calling this method also clears the memoized namespace version and thus forces a resynchonization of it.
*
* @param bool $enable
*
* @return bool the previous state of versioning
*/
public function enableVersioning($enable = true)
{
$wasEnabled = $this->versioningIsEnabled;
$this->versioningIsEnabled = (bool) $enable;
$this->namespaceVersion = '';

return $wasEnabled;
}

/**
* Like the native unserialize() function but throws an exception if anything goes wrong.
*
Expand Down Expand Up @@ -189,11 +220,18 @@ private function getId($key)
{
CacheItem::validateKey($key);

if ($this->versioningIsEnabled && '' === $this->namespaceVersion) {
$this->namespaceVersion = '1:';
foreach ($this->doFetch(array('@'.$this->namespace)) as $v) {
$this->namespaceVersion = $v;
}
}

if (null === $this->maxIdLength) {
return $this->namespace.$key;
return $this->namespace.$this->namespaceVersion.$key;
}
if (strlen($id = $this->namespace.$key) > $this->maxIdLength) {
$id = $this->namespace.substr_replace(base64_encode(hash('sha256', $key, true)), ':', -22);
if (strlen($id = $this->namespace.$this->namespaceVersion.$key) > $this->maxIdLength) {
$id = $this->namespace.$this->namespaceVersion.substr_replace(base64_encode(hash('sha256', $key, true)), ':', -22);
}

return $id;
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Cache/Traits/MemcachedTrait.php
Expand Up @@ -54,6 +54,7 @@ private function init(\Memcached $client, $namespace, $defaultLifetime)
}

parent::__construct($namespace, $defaultLifetime);
$this->enableVersioning();
}

/**
Expand Down Expand Up @@ -242,7 +243,7 @@ protected function doDelete(array $ids)
*/
protected function doClear($namespace)
{
return $this->checkResultCode($this->getClient()->flush());
return false;
}

private function checkResultCode($result)
Expand Down
8 changes: 5 additions & 3 deletions src/Symfony/Component/Cache/Traits/RedisTrait.php
Expand Up @@ -46,7 +46,9 @@ public function init($redisClient, $namespace = '', $defaultLifetime = 0)
if (preg_match('#[^-+_.A-Za-z0-9]#', $namespace, $match)) {
throw new InvalidArgumentException(sprintf('RedisAdapter namespace contains "%s" but only characters in [-+_.A-Za-z0-9] are allowed.', $match[0]));
}
if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\Client) {
if ($redisClient instanceof \RedisCluster) {
$this->enableversioning();
} elseif (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \Predis\Client) {
throw new InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, is_object($redisClient) ? get_class($redisClient) : gettype($redisClient)));
}
$this->redis = $redisClient;
Expand Down Expand Up @@ -171,8 +173,8 @@ protected function doHave($id)
*/
protected function doClear($namespace)
{
// When using a native Redis cluster, clearing the cache cannot work and always returns false.
// Clearing the cache should then be done by any other means (e.g. by restarting the cluster).
// When using a native Redis cluster, clearing the cache is done by versioning in AbstractTrait::clear().
// This means old keys are not really removed until they expire and may need gargage collection.

$cleared = true;
$hosts = array($this->redis);
Expand Down

0 comments on commit 4a0cc29

Please sign in to comment.