From 4cfc7273dbfa3efbe3054e5ae8f4af4ba27db81b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 11 Nov 2021 18:29:11 +0100 Subject: [PATCH] [Cache] Decrease the probability of invalidation loss on tag eviction --- .../Cache/Adapter/TagAwareAdapter.php | 113 +++++++++--------- .../Tests/Adapter/TagAwareAdapterTest.php | 20 +--- 2 files changed, 55 insertions(+), 78 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php b/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php index a9048995c2abb..ff22e5a8ac56e 100644 --- a/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php @@ -41,7 +41,7 @@ class TagAwareAdapter implements TagAwareAdapterInterface, TagAwareCacheInterfac private static $createCacheItem; private static $setCacheItemTags; private static $getTagsByKey; - private static $invalidateTags; + private static $saveTags; public function __construct(AdapterInterface $itemsPool, AdapterInterface $tagsPool = null, float $knownTagVersionsTtl = 0.15) { @@ -95,8 +95,10 @@ static function ($deferred) { null, CacheItem::class ); - self::$invalidateTags ?? self::$invalidateTags = \Closure::bind( + self::$saveTags ?? self::$saveTags = \Closure::bind( static function (AdapterInterface $tagsAdapter, array $tags) { + ksort($tags); + foreach ($tags as $v) { $v->expiry = 0; $tagsAdapter->saveDeferred($v); @@ -114,40 +116,14 @@ static function (AdapterInterface $tagsAdapter, array $tags) { */ public function invalidateTags(array $tags) { - $ok = true; - $tagsByKey = []; - $invalidatedTags = []; + $ids = []; foreach ($tags as $tag) { \assert('' !== CacheItem::validateKey($tag)); - $invalidatedTags[$tag] = 0; - } - - if ($this->deferred) { - $items = $this->deferred; - foreach ($items as $key => $item) { - if (!$this->pool->saveDeferred($item)) { - unset($this->deferred[$key]); - $ok = false; - } - } - - $tagsByKey = (self::$getTagsByKey)($items); - $this->deferred = []; - } - - $tagVersions = $this->getTagVersions($tagsByKey, $invalidatedTags); - $f = self::$createCacheItem; - - foreach ($tagsByKey as $key => $tags) { - $this->pool->saveDeferred($f(static::TAGS_PREFIX.$key, array_intersect_key($tagVersions, $tags), $items[$key])); - } - $ok = $this->pool->commit() && $ok; - - if ($invalidatedTags) { - $ok = (self::$invalidateTags)($this->tags, $invalidatedTags) && $ok; + unset($this->knownTagVersions[$tag]); + $ids[] = $tag.static::TAGS_PREFIX; } - return $ok; + return !$tags || $this->tags->deleteItems($ids); } /** @@ -176,7 +152,7 @@ public function hasItem($key) } foreach ($this->getTagVersions([$itemTags]) as $tag => $version) { - if ($itemTags[$tag] !== $version && 1 !== $itemTags[$tag] - $version) { + if ($itemTags[$tag] !== $version) { return false; } } @@ -314,7 +290,30 @@ public function saveDeferred(CacheItemInterface $item) */ public function commit() { - return $this->invalidateTags([]); + if (!$this->deferred) { + return true; + } + + $ok = true; + foreach ($this->deferred as $key => $item) { + if (!$this->pool->saveDeferred($item)) { + unset($this->deferred[$key]); + $ok = false; + } + } + + $items = $this->deferred; + $tagsByKey = (self::$getTagsByKey)($items); + $this->deferred = []; + + $tagVersions = $this->getTagVersions($tagsByKey); + $f = self::$createCacheItem; + + foreach ($tagsByKey as $key => $tags) { + $this->pool->saveDeferred($f(static::TAGS_PREFIX.$key, array_intersect_key($tagVersions, $tags), $items[$key])); + } + + return $this->pool->commit() && $ok; } /** @@ -361,7 +360,7 @@ private function generateItems(iterable $items, array $tagKeys): \Generator foreach ($itemTags as $key => $tags) { foreach ($tags as $tag => $version) { - if ($tagVersions[$tag] !== $version && 1 !== $version - $tagVersions[$tag]) { + if ($tagVersions[$tag] !== $version) { unset($itemTags[$key]); continue 2; } @@ -377,42 +376,32 @@ private function generateItems(iterable $items, array $tagKeys): \Generator } } - private function getTagVersions(array $tagsByKey, array &$invalidatedTags = []) + private function getTagVersions(array $tagsByKey) { - $tagVersions = $invalidatedTags; + $tagVersions = []; + $fetchTagVersions = false; foreach ($tagsByKey as $tags) { $tagVersions += $tags; + + foreach ($tags as $tag => $version) { + if ($tagVersions[$tag] !== $version) { + unset($this->knownTagVersions[$tag]); + } + } } if (!$tagVersions) { return []; } - if (!$fetchTagVersions = 1 !== \func_num_args()) { - foreach ($tagsByKey as $tags) { - foreach ($tags as $tag => $version) { - if ($tagVersions[$tag] > $version) { - $tagVersions[$tag] = $version; - } - } - } - } - $now = microtime(true); $tags = []; foreach ($tagVersions as $tag => $version) { $tags[$tag.static::TAGS_PREFIX] = $tag; - if ($fetchTagVersions || !isset($this->knownTagVersions[$tag])) { + if ($fetchTagVersions || ($this->knownTagVersions[$tag][1] ?? null) !== $version || $now - $this->knownTagVersions[$tag][0] >= $this->knownTagVersionsTtl) { + // reuse previously fetched tag versions up to the ttl $fetchTagVersions = true; - continue; - } - $version -= $this->knownTagVersions[$tag][1]; - if ((0 !== $version && 1 !== $version) || $now - $this->knownTagVersions[$tag][0] >= $this->knownTagVersionsTtl) { - // reuse previously fetched tag versions up to the ttl, unless we are storing items or a potential miss arises - $fetchTagVersions = true; - } else { - $this->knownTagVersions[$tag][1] += $version; } } @@ -420,14 +409,20 @@ private function getTagVersions(array $tagsByKey, array &$invalidatedTags = []) return $tagVersions; } + $newTags = []; + $newVersion = null; foreach ($this->tags->getItems(array_keys($tags)) as $tag => $version) { - $tagVersions[$tag = $tags[$tag]] = $version->get() ?: 0; - if (isset($invalidatedTags[$tag])) { - $invalidatedTags[$tag] = $version->set(++$tagVersions[$tag]); + if (!$version->isHit()) { + $newTags[$tag] = $version->set($newVersion ?? $newVersion = random_int(\PHP_INT_MIN, \PHP_INT_MAX)); } + $tagVersions[$tag = $tags[$tag]] = $version->get(); $this->knownTagVersions[$tag] = [$now, $tagVersions[$tag]]; } + if ($newTags) { + (self::$saveTags)($this->tags, $newTags); + } + return $tagVersions; } } diff --git a/src/Symfony/Component/Cache/Tests/Adapter/TagAwareAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/TagAwareAdapterTest.php index 7e69001648fcc..b688ad46ed440 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/TagAwareAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/TagAwareAdapterTest.php @@ -40,25 +40,6 @@ public static function tearDownAfterClass(): void (new Filesystem())->remove(sys_get_temp_dir().'/symfony-cache'); } - /** - * Test feature specific to TagAwareAdapter as it implicit needs to save deferred when also saving expiry info. - */ - public function testInvalidateCommitsSeperatePools() - { - $pool1 = $this->createCachePool(); - - $foo = $pool1->getItem('foo'); - $foo->tag('tag'); - - $pool1->saveDeferred($foo->set('foo')); - $pool1->invalidateTags(['tag']); - - $pool2 = $this->createCachePool(); - $foo = $pool2->getItem('foo'); - - $this->assertTrue($foo->isHit()); - } - public function testPrune() { $cache = new TagAwareAdapter($this->getPruneableMock()); @@ -84,6 +65,7 @@ public function testKnownTagVersionsTtl() $tag = $this->createMock(CacheItemInterface::class); $tag->expects(self::exactly(2))->method('get')->willReturn(10); + $tag->expects(self::exactly(2))->method('set')->willReturn($tag); $tagsPool->expects(self::exactly(2))->method('getItems')->willReturn([ 'baz'.TagAwareAdapter::TAGS_PREFIX => $tag,