matthiasmullie/scrapbook

[Redis] TTL gets set while add() fails.

daaan opened this issue · 1 comments

Hi!

I ran into an issue this morning with mutex keys. It might be that its a product of our code and the way it works with Redis, but I found some code in this library unlogical from my point of view :)

I'm talking about src/Adapters/Redis.php:add(), which ends with this:

$this->client->multi();
$this->client->setnx($key, $value);
$this->client->expire($key, $ttl);

/** @var bool[] $return */
$return = (array) $this->client->exec();

return !in_array(false, $return, true);

My problem here is that both command (setnx and expire) get executed. The first command fails when the key already exists, the second command succeeds anyway. This results in $return being [false, true]. This in its turn makes the return false because there is a false in the array.

In my code I get a failure adding the key and make the assumption that the key already exists and the program will try again a bit later. But as the TTL gets updated on the failing add() call it will be there forever while scripts check the key periodically.

The solution I found is largely the one you mentioned in the comments above the code. Except for the use of xx, which means 'Only set the key if it already exists.' while it should be nx 'Only set the key if it does not already exist.'. The nx is also in accordance with the use of setnx() as is the current situation. This results in this simple line:

return $this->client->set($key, $value, ['ex' => $ttl, 'nx']);

Again, it might be that my use case is slightly incompatible with your library... but I find it strange that a function that fails still mutates silently.

I also tried to use set() but the 3rd parameter does not accept arrays.

Thanks

Thanks for the detailed bug report, @daaan - I just released a new version to include the fix to this!