-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Description
Symfony version(s) affected
7.3.4 but probably all
Description
The LockRegistry theoretically allows maximum 25 concurrent cache computations, and maximum 1 per cache-key.
The issue is the lock key is a integer generated from the cache key which is a string. There is a very huge chance of collision for two cache items with different keys generated at the same moment.
This causes problems because when the lock cannot be acquired, the LockRegistry will just do a get and return the value of the cache item after the lock is released instead of computing it, which is fine when the cache key was really being generated by another process, but when it's caused by a collision, the cache data might have not been generated.
In my case it sometimes prevents a cache warmup (with beta=INF) to proceed normally: multiple items have to be generated asynchronously at the same time, with multiple consumers running on the same host. With the lock key collision, sometimes two or more items are not warmed up because the lock cannot be acquired causing random missing cache items.
How to reproduce
Using this simple command on a project with the Cache component:
protected function configure(): void
{
$this->addArgument('key', InputArgument::REQUIRED, 'The cache key to generate');
}
protected function execute(InputInterface $input, OutputInterface $output): int
{
$io = new SymfonyStyle($input, $output);
$key = $input->getArgument('key');
$generated = false;
$this->cache->get($key, function (ItemInterface $item) use (&$generated) {
$item->expiresAfter(3600);
sleep(5);
$generated = true;
return true;
}, INF);
if ($generated) {
$io->success('Cache generated for ' . $key);
return Command::SUCCESS;
} else {
$io->error('Cache not generated for ' . $key);
return Command::FAILURE;
}
}
Launch it twice, at the same time, once with the param test and the other with the param pfeze (randomly found those two cache keys give the same lock key of 21)
One of the two items will not be generated as the lock for 21 was already acquired for the other computation.
Possible Solution
Workarounds:
If the stampede prevention is not needed, it's easy:
- Using the PSR-6 caching avoid this issue as there is not built-in cache stampede prevention for it
- Setting a custom callbackWrapper with an alternative to the LockRegistry would also work I guess?
Solution:
The issue here is to be able to prevent cache stampede without using a fixed list of files that required a numeric key to be generated.
My first guess would be to store the current list of cache computations inside a specific cache item (one managed through the PSR-6 adapter to avoid looping of course). At every cache contract computation, we would need to acquire a lock to write in the list item, add the current key being generated to the list, then release the lock to allow the list to be modified by other processes.
The size of the list in the items allows to know how many concurrents computations are happening, while the keys in it can be used as lock to prevent the same key from being generated at the same time.
After the computation, acquire the lock, remove the key, save and release.
I think this could work but it adds a bottleneck on this specific cache+lock for every cache items that needs computation so I'm not sure if this is viable.
If it works however, it would also allow to make the current 25 maximum a config variable instead of a fixed value.
Additional Context
No response