Cache: Inconsistent behaviour regarding locks when clearing cache

[pxlrbt]

Laravel Version

11.19.0

PHP Version

8.3

Database Driver & Version

Redis

Description

There is an inconsistency when clearing the cache between the Redis store and the file store. The file store also clears locks, while the redis store doesn't. This affects unique queued jobs. While you can reset the job locks through artisan cache:clear when using the file store, this doesn't work with the redis store, leaving you confused why no jobs are pushed to the queue.

Steps To Reproduce

When CACHE_STORE is set to redis Laravel behaves like this:

$lock = cache()->lock('test');
$lock->get(); // true
$lock->get(); // false

cache()->flush(); // or artisan cache:clear

$lock->get(); // false

When CACHE_STORE is set to file Laravel behaves like this:

$lock = cache()->lock('test');
$lock->get(); // true
$lock->get(); // false

cache()->flush(); // or artisan cache:clear

$lock->get(); // true

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[rust17]

Hi everyone, Could you review this PR and check if the way to reproduce the issue is acceptable? Thanks.

[Katalam]

That is kinda by design. We have two configs for the redis cache

We have this

framework/config/cache.php

Lines 74 to 78 in cfe5f84

	'redis' => [
	'driver' => 'redis',
	'connection' => env('REDIS_CACHE_CONNECTION', 'cache'),
	'lock_connection' => env('REDIS_CACHE_LOCK_CONNECTION', 'default'),
	],

That will determine which connection is used for the cache. Be advised that there is a difference in the default values of these two entries

And then we have this

framework/config/database.php

Lines 153 to 169 in cfe5f84

	'default' => [
	'url' => env('REDIS_URL'),
	'host' => env('REDIS_HOST', '127.0.0.1'),
	'username' => env('REDIS_USERNAME'),
	'password' => env('REDIS_PASSWORD'),
	'port' => env('REDIS_PORT', '6379'),
	'database' => env('REDIS_DB', '0'),
	],
	'cache' => [
	'url' => env('REDIS_URL'),
	'host' => env('REDIS_HOST', '127.0.0.1'),
	'username' => env('REDIS_USERNAME'),
	'password' => env('REDIS_PASSWORD'),
	'port' => env('REDIS_PORT', '6379'),
	'database' => env('REDIS_CACHE_DB', '1'),
	],

The cache:clear will use FLUSHDB (https://redis.io/docs/latest/commands/flushdb/) as shown here

framework/src/Illuminate/Cache/RedisStore.php

Line 248 in cfe5f84

$this->connection()->flushdb();

This will only flush the current database, because we don't want to interfere with other databases inside redis. You can change that behaviour by either change the database for locks in redis or change the behaviour of the cache clear command in your own application and replace the RedisConnection with your own class here

framework/src/Illuminate/Redis/Connections/PhpRedisConnection.php

Lines 496 to 505 in cfe5f84

	public function flushdb()
	{
	$arguments = func_get_args();
	if (strtoupper((string) ($arguments[0] ?? null)) === 'ASYNC') {
	return $this->command('flushdb', [true]);
	}
	return $this->command('flushdb');
	}

. be advised, there are other things that will impact that change.

[pxlrbt]

Yes, I found that solutions with two Redis connections after hours of debugging. I'm okay with it being by design, but what threw me off was the different behaviour between drivers.

I just checked the config/cache.php and there are separate connections for DB and Redis, but not for the others. At least for the File Driver it's just a config change. Never used the others, so not sure whether it's even possible to have the same behaviour for all the drivers

[rust17]

It seems odd, in order to keep consistency, the best solution may be to set REDIS_CACHE_DB to 0.
From multiple testing code executions, I've noticed that when REDIS_CACHE_DB is set to 1, the actual redis database that executing the commands is as bellow:

Cache::store('redis')->lock('foo')->forceRelease(); // actual database is 0

$lock = Cache::store('redis')->lock('foo'); // actual database is 0
$this->assertTrue($lock->get()); // actual database is 0
$this->assertFalse($lock->get()); // actual database is 0

Cache::store('redis')->flush(); // actual database is 1

$this->assertTrue($lock->get()); // actual database is 0

When REDIS_CACHE_DB is set to 0, the actual Redis database that executing the commands is as follows:

Cache::store('redis')->lock('foo')->forceRelease(); // actual database is 0

$lock = Cache::store('redis')->lock('foo'); // actual database is 0
$this->assertTrue($lock->get()); // actual database is 0
$this->assertFalse($lock->get()); // actual database is 0

Cache::store('redis')->flush(); // actual database is 0

$this->assertTrue($lock->get()); // actual database is 0

The inconsistency arises because only the redis instance executing the flush command is different, preventing the lock from being released.

[Katalam]

But that is indeed how it is supposed to work. In order to separate the locks from the actual values be use different databases, these values are saved inside database 1. The cache clear command is intended to clear the values from database 1. In order to flush all locks we need to have a separate logic that will flush database 0 for us. You can do: Redis::command("flushall") do achieve that type of behavior. We want to prevent that an actual value key is colliding with a lock key. And to do that, we use different databases. The different behavior in different drivers comes from the actual implementation and it is kinda expected that different drivers work slightly differently. I think we can try to add an extra layer for the Redis Connection to also flush the lock database while cache:clear

[Katalam]

While I wrote the answer, I though about it again and have to say that dropping the locks is kinda an expected behaviour, because we also drop the data at the moment

[rust17]

Oh, I get it now 😯, I initially thought it was a bug.

[crynobone]

Thabks @Katalam

[pxlrbt]

@crynobone Not sure why this was closed. It's not really solved.

I think we should find a consensus on whether it's expected to clear locks when clearing the cache or not. Since it might break existing workflows, maybe the best idea is to introduce a new command and mention it in the docs?

[crynobone]

I think we should find a consensus on whether it's expected to clear locks when clearing the cache or not.

issues should focus on confirmed bug. Feel free to continue the discussion or submit a PR to the documentation etc.