[Cache] fix 2RTT + race condition in AbstractTagAwareAdapter::deleteItems()

This commit is contained in:
Nicolas Grekas 2019-10-11 20:23:30 +02:00
parent 28f95361c6
commit 0613c227ec
3 changed files with 39 additions and 45 deletions

View File

@ -133,12 +133,18 @@ abstract class AbstractTagAwareAdapter implements TagAwareAdapterInterface, TagA
/**
* Removes multiple items from the pool and their corresponding tags.
*
* @param array $ids An array of identifiers that should be removed from the pool
* @param array $tagData Optional array of tag identifiers => key identifiers that should be removed from the pool
* @param array $ids An array of identifiers that should be removed from the pool
*
* @return bool True if the items were successfully removed, false otherwise
*/
abstract protected function doDelete(array $ids, array $tagData = []): bool;
abstract protected function doDelete(array $ids);
/**
* Removes relations between tags and deleted items.
*
* @param array $tagData Array of tag => key identifiers that should be removed from the pool
*/
abstract protected function doDeleteTagRelations(array $tagData): bool;
/**
* Invalidates cached items using tags.
@ -150,21 +156,21 @@ abstract class AbstractTagAwareAdapter implements TagAwareAdapterInterface, TagA
abstract protected function doInvalidate(array $tagIds): bool;
/**
* Returns the tags bound to the provided ids.
* Delete items and yields the tags they were bound to.
*/
protected function doFetchTags(array $ids): iterable
protected function doDeleteYieldTags(array $ids): iterable
{
foreach ($this->doFetch($ids) as $id => $value) {
yield $id => \is_array($value) && \is_array($value['tags'] ?? null) ? $value['tags'] : [];
}
$this->doDelete($ids);
}
/**
* {@inheritdoc}
*
* @return bool
*/
public function commit()
public function commit(): bool
{
$ok = true;
$byLifetime = $this->mergeByLifetime;
@ -223,17 +229,14 @@ abstract class AbstractTagAwareAdapter implements TagAwareAdapterInterface, TagA
/**
* {@inheritdoc}
*
* Overloaded in order to deal with tags for adjusted doDelete() signature.
*
* @return bool
*/
public function deleteItems(array $keys)
public function deleteItems(array $keys): bool
{
if (!$keys) {
return true;
}
$ok = true;
$ids = [];
$tagData = [];
@ -243,24 +246,22 @@ abstract class AbstractTagAwareAdapter implements TagAwareAdapterInterface, TagA
}
try {
foreach ($this->doFetchTags($ids) as $id => $tags) {
foreach ($this->doDeleteYieldTags(array_values($ids)) as $id => $tags) {
foreach ($tags as $tag) {
$tagData[$this->getId(self::TAGS_PREFIX.$tag)][] = $id;
}
}
} catch (\Exception $e) {
// ignore unserialization failures
$ok = false;
}
try {
if ($this->doDelete(array_values($ids), $tagData)) {
if ((!$tagData || $this->doDeleteTagRelations($tagData)) && $ok) {
return true;
}
} catch (\Exception $e) {
}
$ok = true;
// When bulk-delete failed, retry each item individually
foreach ($ids as $key => $id) {
try {

View File

@ -27,7 +27,6 @@ class FilesystemTagAwareAdapter extends AbstractTagAwareAdapter implements Prune
use FilesystemTrait {
doClear as private doClearCache;
doSave as private doSaveCache;
doDelete as private doDeleteCache;
}
/**
@ -133,7 +132,7 @@ class FilesystemTagAwareAdapter extends AbstractTagAwareAdapter implements Prune
/**
* {@inheritdoc}
*/
protected function doFetchTags(array $ids): iterable
protected function doDeleteYieldTags(array $ids): iterable
{
foreach ($ids as $id) {
$file = $this->getFile($id);
@ -141,6 +140,11 @@ class FilesystemTagAwareAdapter extends AbstractTagAwareAdapter implements Prune
continue;
}
if ((\PHP_VERSION_ID >= 70300 || '\\' !== \DIRECTORY_SEPARATOR) && !@unlink($file)) {
fclose($h);
continue;
}
$meta = explode("\n", fread($h, 4096), 3)[2] ?? '';
// detect the compact format used in marshall() using magic numbers in the form 9D-..-..-..-..-00-..-..-..-5F
@ -161,25 +165,26 @@ class FilesystemTagAwareAdapter extends AbstractTagAwareAdapter implements Prune
}
fclose($h);
if (\PHP_VERSION_ID < 70300 && '\\' === \DIRECTORY_SEPARATOR) {
@unlink($file);
}
}
}
/**
* {@inheritdoc}
*/
protected function doDelete(array $ids, array $tagData = []): bool
protected function doDeleteTagRelations(array $tagData): bool
{
$ok = $this->doDeleteCache($ids);
// Remove tags
foreach ($tagData as $tagId => $idMap) {
foreach ($tagData as $tagId => $idList) {
$tagFolder = $this->getTagFolder($tagId);
foreach ($idMap as $id) {
foreach ($idList as $id) {
@unlink($this->getFile($id, false, $tagFolder));
}
}
return $ok;
return true;
}
/**

View File

@ -123,10 +123,11 @@ class RedisTagAwareAdapter extends AbstractTagAwareAdapter
/**
* {@inheritdoc}
*/
protected function doFetchTags(array $ids): iterable
protected function doDeleteYieldTags(array $ids): iterable
{
$lua = <<<'EOLUA'
local v = redis.call('GET', KEYS[1])
redis.call('DEL', KEYS[1])
if not v or v:len() <= 13 or v:byte(1) ~= 0x9D or v:byte(6) ~= 0 or v:byte(10) ~= 0x5F then
return ''
@ -159,25 +160,12 @@ EOLUA;
/**
* {@inheritdoc}
*/
protected function doDelete(array $ids, array $tagData = []): bool
protected function doDeleteTagRelations(array $tagData): bool
{
if (!$ids) {
return true;
}
$predisCluster = $this->redis instanceof \Predis\ClientInterface && $this->redis->getConnection() instanceof PredisCluster;
$this->pipeline(static function () use ($ids, $tagData, $predisCluster) {
if ($predisCluster) {
// Unlike phpredis, Predis does not handle bulk calls for us against cluster
foreach ($ids as $id) {
yield 'del' => [$id];
}
} else {
yield 'del' => $ids;
}
$this->pipeline(static function () use ($tagData) {
foreach ($tagData as $tagId => $idList) {
yield 'sRem' => array_merge([$tagId], $idList);
array_unshift($idList, $tagId);
yield 'sRem' => $idList;
}
})->rewind();