[Lock] compatiblity with redis cluster 7 #54252

bastnic · Mar 12, 2024

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
License	MIT

While upgrading from redis 5 (no cluster) to AWS Elasticache redis 7 (cluster serverless), I stumble upon a fail.

This Redis command is not allowed from script script

Investigating, the fail is here, TIME is not available in cluster mode.

symfony/src/Symfony/Component/Lock/Store/RedisStore.php

Lines 272 to 281 in d508d5f

    
                   if (!isset($this->supportTime)) { 
        
                       // Redis < 5.0 does not support TIME (not deterministic) in script. 
        
                       // https://redis.io/commands/eval#replicating-commands-instead-of-scripts 
        
                       // This code asserts TIME can be use, otherwise will fallback to a timestamp generated by the PHP process. 
        
                       $script = ' 
        
                           local now = redis.call("TIME") 
        
                           redis.call("SET", KEYS[1], "1", "PX", 1) 
        
           	            return 1 
        
                       ';

redis > EVAL 'local now = redis.call("TIME"); redis.call("SET", KEYS[1], "1", "PX", 1); return 1;' 1 pouet
(error) ERR This Redis command is not allowed from script script: b0e701c57bb0187cbd6ec8e404a7f6ba19b11a94, on @user_script:1.

BUT the error does not contains commands not allowed after non deterministic anymore, so no fallback on the PHP implementation.

Hence, the new exception catch.

See https://redis.io/docs/interact/programmability/eval-intro/#replicating-commands-instead-of-scripts

nicolas-grekas · Mar 13, 2024

Thank you @bastnic.

bastnic requested a review from jderusse as a code owner March 12, 2024 13:25

carsonbot added Status: Needs Review Bug Lock labels Mar 12, 2024

carsonbot added this to the 5.4 milestone Mar 12, 2024

bastnic force-pushed the feat/lock-redis-7-cluster branch from a317352 to 9ebf742 Compare March 12, 2024 13:27

[Lock] compatiblity with redis cluster 7

ee5bd73

nicolas-grekas force-pushed the feat/lock-redis-7-cluster branch from 9ebf742 to ee5bd73 Compare March 13, 2024 12:23

nicolas-grekas merged commit c44cc73 into symfony:5.4 Mar 13, 2024

bastnic deleted the feat/lock-redis-7-cluster branch March 14, 2024 09:58

This was referenced Apr 2, 2024

Release v5.4.38 #54462

Merged

Release v6.4.6 #54466

Merged

Release v7.0.6 #54467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lock] compatiblity with redis cluster 7 #54252

[Lock] compatiblity with redis cluster 7 #54252

bastnic commented Mar 12, 2024 •

edited

Loading

nicolas-grekas commented Mar 13, 2024

	if (!isset($this->supportTime)) {
	// Redis < 5.0 does not support TIME (not deterministic) in script.
	// https://redis.io/commands/eval#replicating-commands-instead-of-scripts
	// This code asserts TIME can be use, otherwise will fallback to a timestamp generated by the PHP process.
	$script = '
	local now = redis.call("TIME")
	redis.call("SET", KEYS[1], "1", "PX", 1)

	return 1
	';

Search code, repositories, users, issues, pull requests...

[Lock] compatiblity with redis cluster 7 #54252

[Lock] compatiblity with redis cluster 7 #54252

Conversation

bastnic commented Mar 12, 2024 • edited Loading

nicolas-grekas commented Mar 13, 2024

bastnic commented Mar 12, 2024 •

edited

Loading