8000 [Lock] "Cannot use 'SCRIPT' with redis-cluster" after updating to symfony/lock 7.2.* when using a redis cluster with RedisStore · Issue #59795 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Lock] "Cannot use 'SCRIPT' with redis-cluster" after updating to symfony/lock 7.2.* when using a redis cluster with RedisStore #59795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
acelaya opened this issue Feb 17, 2025 · 5 comments
Open

[Lock] "Cannot use 'SCRIPT' with redis-cluster" after updating to symfony/lock 7.2.* when using a redis cluster with RedisStore #59795

acelaya opened this issue Feb 17, 2025 · 5 comments
Labels
Bug Lock Status: Needs Review

Comments

@acelaya
Copy link
acelaya commented Feb 17, 2025 •
edited
Loading

Symfony version(s) affected

7.2.*

Description

When trying to acquire a lock with symfony lock using a RedisStore with predis and a redis cluster, the error Cannot use 'SCRIPT' with redis-cluster is thrown.

Everything works as expected on symfony/lock 7.1.6, so I guess the issue is somewhere here symfony/lock@v7.1.6...v7.2.0

On symfony/lock 7.2.3, a different error is thrown though, which was already reported in #59686, and solved via a PR.

I tested with the changes introduced in that PR, and the error goes back to what I'm reporting here.

How to reproduce

Create a folder with the following files:

docker-compose.yml

services:
    redis:
        image: redis:7.4-alpine
        ports:
            - "6381:6379"

composer.json

{
    "name": "acelaya/symfony-lock-error-repro",
    "type": "project",
    "require": {
        "symfony/lock": "7.2.0",
        "predis/predis": "^2.3"
    }
}

index.php

<?php

declare(strict_types=1);

use Predis\Client;
use Symfony\Component\Lock\LockFactory;
use Symfony\Component\Lock\Store\RedisStore;

require __DIR__ . '/vendor/autoload.php';

$predisClient = new Client(
    ['tcp://127.0.0.1:6381', 'tcp://127.0.0.1:6381'],
    ['cluster' => 'redis'],
);
$lockStore = new RedisStore($predisClient);
$lockFactory = new LockFactory($lockStore);

$lock = $lockFactory->createLock('test');
$lock->acquire();
$lock->release();

Then run the following commands:

  1. docker compose up -d
  2. composer install
  3. php index.php

You will see the following error:

PHP Fatal error:  Uncaught Predis\NotSupportedException: Cannot use 'SCRIPT' with redis-cluster. in /symfony-lock-bug/vendor/predis/predis/src/Connection/Cluster/RedisCluster.php:354
Stack trace:
#0 /symfony-lock-bug/vendor/predis/predis/src/Connection/Cluster/RedisCluster.php(531): Predis\Connection\Cluster\RedisCluster->getConnectionByCommand()
#1 /symfony-lock-bug/vendor/predis/predis/src/Connection/Cluster/RedisCluster.php(591): Predis\Connection\Cluster\RedisCluster->retryCommandOnFailure()
#2 /symfony-lock-bug/vendor/predis/predis/src/Client.php(381): Predis\Connection\Cluster\RedisCluster->executeCommand()
#3 /symfony-lock-bug/vendor/predis/predis/src/Client.php(335): Predis\Client->executeCommand()
#4 /symfony-lock-bug/vendor/symfony/lock/Store/RedisStore.php(297): Predis\Client->__call()
#5 /symfony-lock-bug/vendor/symfony/lock/Store/RedisStore.php(332): Symfony\Component\Lock\Store\RedisStore->evaluate()
#6 /symfony-lock-bug/vendor/symfony/lock/Store/RedisStore.php(60): Symfony\Component\Lock\Store\RedisStore->getNowCode()
#7 /symfony-lock-bug/vendor/symfony/lock/Lock.php(85): Symfony\Component\Lock\Store\RedisStore->save()
#8 /symfony-lock-bug/index.php(16): Symfony\Component\Lock\Lock->acquire()
#9 {main}

Next Symfony\Component\Lock\Exception\LockAcquiringException: Failed to acquire the "test" lock. in /symfony-lock-bug/vendor/symfony/lock/Lock.php:116
Stack trace:
#0 /symfony-lock-bug/index.php(16): Symfony\Component\Lock\Lock->acquire()
#1 {main}
  thrown in /symfony-lock-bug/vendor/symfony/lock/Lock.php on line 116

If you update the composer.json like this

{
    "name": "acelaya/symfony-lock-error-repro",
    "type": "project",
    "require": {
-        "symfony/lock": "7.2.0",
+        "symfony/lock": "7.1.6",
        "predis/predis": "^2.3"
    }
}

Then run composer update and then php index.php, you'll experience no such error.

Possible Solution

No response

Additional Context

Originally reported in shlinkio/shlink#2366
@acelaya acelaya added the Bug label Feb 17, 2025
Closed
@acelaya acelaya changed the title [Lock] "Cannot use 'SCRIPT' with redis-cluster" after updating to symfony/lock 7.2.* when using a redis cluster with RedisAdapter [Lock] "Cannot use 'SCRIPT' with redis-cluster" after updating to symfony/lock 7.2.* when using a redis cluster with RedisStore Feb 18, 2025
@nicolas-grekas
Copy link
Member

Could you work on a fix maybe? That might be the most effective way to solve this ;)

@acelaya
Copy link
Author
acelaya commented Feb 18, 2025

Could you work on a fix maybe? That might be the most effective way to solve this ;)

Can't promise anything, but I could definitely try 😅

I'll report back if I face any blocker or have questions.

@acelaya
Copy link
Author
acelaya commented Feb 18, 2025

Ok so, by my debugging, I think the issue was introduced by the changes in #58533

In there, instead of evaluating a script every time, the logic loads that script in redis and evaluates it via its hash in subsequent attempts, to reduce network I/O.

That's nice, but doesn't seem to work with redis clusters, at least when using predis, although that same PR tries to load the script in the master instance when using \RedisCluster instead of predis, so perhaps there's in fact a way.

One possible solution I see is checking if the predis connection is an aggregate one, and use the former approach in which the same script was evaluated every time. When testing that locally, it seems to work.

On top of that, there's two supported predis versions that need to be tested, and different ways to clusterize redis (master/slave, sentinels, etc). I'll try to test all possible scenarios and see if there's in fact a way to take advantage of loaded scripts.

@acelaya
Copy link
Author
acelaya commented Feb 19, 2025 •
edited
Loading

Note 2 follow up PRs are already merged; #59348 #59661

These don't seem to fix the issue reported here unfortunately.

The debugging I did yesterday was against latest changes in the 7.3 branch, where the issue is still reproducible.

IIUC redis cluster tests are missing here then?

It seems so, yeah.

@acelaya acelaya mentioned this issue Feb 19, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Lock Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nicolas-grekas @acelaya @carsonbot
0