closeio / tasktiger

Python task queue using Redis
MIT License
1.42k stars 80 forks source link

Support Redis >= 6.2.7 (Lua readonly global tables) #268

Closed nsaje closed 1 year ago

nsaje commented 1 year ago

On redis >= 6.2.7 we currently encounter an "Attempt to modify a readonly table" error (more info here) This error is introduced by https://github.com/redis/redis/pull/10651 which hardened Lua script execution by not allowing modifications of global tables and is affecting Redis versions >= 6.2.7.

It happens because when trying to execute a pipeline using the execute_pipeline Lua script, we try to set the KEYS and ARGV global variables before calling another script using EVALSHA.

Executing pipelines via a Lua script was introduced because Redis transactions don't abort execution of commands when one of the commands in the transaction fails.

The fix would probably be to use Redis functions for Redis 7.0 and above, because they're directly callable by providing keys and argv in the call itself using FCALL <func_name> <keys> <argv>

nsaje commented 1 year ago

cc @thomasst , any thoughts on this? 😄

thomasst commented 1 year ago

The fix would probably be to use Redis functions for Redis 7.0 and above, because they're directly callable by providing keys and argv in the call itself using FCALL <func_name> <keys> <argv>

I have not played around with Redis 7 but that looks good. The way we're doing it currently is a hack.

joeleclems commented 1 year ago

``Hi, For information, we encountered the same problem with redis 6.2.6. We also have redis databases in version 6.2.5, and no problems with theses databases. So i think the problem is on redis >= 6.2.6

Code that create problem for us is :

string.startswith = function(self, str) 
    return self:find('^' .. str) ~= nil
end

And we solved it by creating our own function (no so good... but it works)

local function string_start_with(str, startwith)
    return string.find(str,'^' .. startwith) ~= nil
end