Predis using transaction to execute two commands, but one of them failed

Question

Describe the bug

I use redis list to do a limiter, it works as expected most times, but recently I found that there are some keys without an expiry.Ideally, I "rpush" the value to the list and set expiry as well in one transaction, and I use "watch" as well before the transaction start.

To Reproduce

I haven't reproduced the bug in my local environment, even I use jmeter to request the related api in batch, for example 1 seconds 500 requests

Versions:

Predis: v2.1.2 PHP 7.4 Redis Server 5.0.10

Code sample

$redisClient->watch($key);
$current = $redisClient->llen($key);

// Transaction start
$tx = $redisClient->transaction();
if ($current >= $limitNum) {
    $redisClient->unwatch();
    return false;
} else {
  
    if ($redisClient->exists($key)) {
        $tx->rpush($key, $now);

        try {
             $replies = $tx->execute();
             return true;
        } catch (\Exception $e) {
            return false;
        }
    } else {
        // Using transaction to let rpush and expire to be an atomic operation
        $tx->rpush($key, $now);
        $tx->expire($key, $expiryTime);

        try {
             $replies = $tx->execute();
             return true;
        } catch (\Exception $e) {
            return false;
        }
    }
}

Others

here is the expected actions in my local redis server

Redis transaction is atomic. Atomic means either all of the commands or none are processed. So one key should have a expiry in my case.

Answer 1

Redis transactions aren't atomic like that. They are atomic in the sense that no other process has access to the keyspace while the transactions in your command execute. If a command in a transaction fails then subsequent commands will be executed and there isn't a rollback.

Example, let's do a transaction that has a bad command in it:

127.0.0.1:6379> exists mylist
(integer) 0
127.0.0.1:6379> lpush mylist a b c
(integer) 3
127.0.0.1:6379> multi
OK
127.0.0.1:6379> rpop mylist
QUEUED
127.0.0.1:6379> sadd mylist d
QUEUED
127.0.0.1:6379> expire mylist 300
QUEUED
127.0.0.1:6379> exec
1) "a"
2) (error) WRONGTYPE Operation against a key holding the wrong kind of value
3) (integer) 1
127.0.0.1:6379> ttl mylist
(integer) 297

Here we check if a list exists, and add some initial items to it. Then, in a transaction, we pop an item from the list, erroneously try to add a new item thinking the key mylist holds a set, then set the time to live on the key mylist. The first and third commands succeed and at the end, mylist has a time to live set. The second command fails. There is no rollback built into Redis for this - your application would need to use optimistic locking with the watch command... this is to detect other processes changing keys your transaction wants to change, before your transaction gets exclusive access to the server. It isn't a rollback mechanism.

Details: https://redis.io/docs/interact/transactions/