Use transaction to run multiple commands atomically, but one of the commands has not been executed

[oik-jinn]

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

Expected behavior
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.

Versions (please complete the following information):

• 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;
            }
        }
    }

[tillkruss]

Try using multi() instead.

[oik-jinn]

Hi @tillkruss , thanks for your answer, but what's the difference between using "$redisClient->transaction()" and "$redisClient->multi()" to start a transaction in predis. And btw, when I use "$redisClient->transaction()", I saw

the commands are expected run in redis server, commands "rpush" and "expire" are in the transaction.

[vladvildanov]

@oik-jinn Hi! I recommend you check CAS transaction example that we provide. I believe you need to enable CAS mode first and run your code within a callback, in CAS mode MULTI should be explicitly called before every transaction.

https://github.com/predis/predis/blob/v2.x/examples/transaction_using_cas.php

Here's code sample that should works correct now:

    $options = [
        'cas' => true,
        'watch' => $key,
        'retry' => 3,
    ];

    try {
        $response = $redisClient->transaction($options, static function (Predis\Transaction\MultiExec $tx) use ($key, $limitNum, $now, $expiryTime) {
            $current = $tx->llen($key);

            if ($current >= $limitNum) {
                $tx->unwatch();
            } else {
                $tx->multi(); // With CAS, MULTI *must* be explicitly invoked.
                $tx->rpush($key, $now);
                
                if (!$tx->exists($key)) {
                    $tx->expire($key, $expiryTime);
                }
            }
        });
    } catch (Throwable $e) {
        return false;
    }

    return null !== $response;

Monitor output: