Redis 4.0 Crash using lua scripting and module redisearch #4127
Comments
|
hey @chameleonbr, can you share the lua script creating this crash? (I'm the author of the module. I've never tested them together but it might be an inherent problem with the execution model of redisearch) |
|
@antirez I've managed to recreate it - calling redis searches from lua crashes redis. looks like something with the fact that the search blocks the client. |
|
Yes. -- key produtos
-- FT.CREATE produtos SCHEMA cnpj TEXT cditem TEXT unidade TEXT desc TEXT local GEO timestamp NUMERIC SORTABLE uf TEXT valor NUMERIC SORTABLE valor_tabela NUMERIC idnfe TEXT cdanp TEXT ncm TEXT nprot TEXT gtin TEXT valor_desconto NUMERIC
local function split(pString, pPattern)
local Table = {}
local fpat = "(.-)" .. pPattern
local last_end = 1
local s, e, cap = pString:find(fpat, 1)
while s do
if s ~= 1 or cap ~= "" then
table.insert(Table,cap)
end
last_end = e+1
s, e, cap = pString:find(fpat, last_end)
end
if last_end <= #pString then
cap = pString:sub(last_end)
table.insert(Table, cap)
end
return Table
end
local term1 = '@'..KEYS[1]..':'..ARGV[1]
local term2 = nil
local limit = nil
local order = nil
local result = nil
if KEYS[2] == "estab" then
term2 = '@'..KEYS[2]..':'..ARGV[2]
end
if KEYS[3] then
limit = split(KEYS[3],',')
end
if ARGV[3] then
order = split(ARGV[3],',')
end
if KEYS[2] == "local" then
local geo = split(ARGV[2],',')
result = redis.call('FT.SEARCH','produtos',term1,'GEOFILTER','local',geo[1],geo[2],geo[3],'km','LIMIT',limit[1],limit[2], 'ORDERBY',order[1],order[2])
else
result = redis.call('FT.SEARCH','produtos',term1,term2,'LIMIT',limit[1],limit[2], 'ORDERBY',order[1],order[2])
end
return result
-- or return cjson.encode(result) |
|
Seizing the opportunity, is it in your plans to order by radius? |
|
Doesn't seem to be directly related to redisearch actually. the crash is here: in scripting.c /* Try to use a cached object. */
407 if (j < LUA_CMD_OBJCACHE_SIZE && cached_objects[j] &&
408 cached_objects_len[j] >= obj_len)
409 {
410 sds s = cached_objects[j]->ptr;
411 argv[j] = cached_objects[j];
412 cached_objects[j] = NULL;
413 ==========>>> memcpy(s,obj_s,obj_len+1);
414 sdssetlen(s, obj_len);
415 } else {
416 argv[j] = createStringObject(obj_s, obj_len);
417 }
and @chameleonbr no such plans currently, but you can write your own scoring function and extend redisearch with it! PS full stack trace: EDIT: Looks like something with blocking commands. the helloworld module worked fine with lua, the helloblock didn't. |
|
Looks like someone else got smacked by this or similar: https://www.reddit.com/r/redis/comments/6on853/what_is_the_module_response_format_specifically/ |
|
HI all, any one-liner to reproduce it with clear instructions? |
|
|
@dvirsky ok so it's a blocking thing? All the Redis blocking commands are flagged as "s", not executable in Lua scripts, so looks like the problem is that we are allowing to do that... There is no explicit blocking flag in the Redis command flag, but we can add a flag in the module API that will just map to the "s" flag (no scripting). |
|
I wonder if there is now a case for actually doing it - the search query being the use case. it's blocking just for the sake of concurrency, it's not a long running command. Calling it from lua will just make it non concurrent, but will do no harm, it's not SUBSCRIBE or BLPOP. If it's possible technically maybe it's worth allowing this explicitly by the module author. |
|
Not sure if it is possible to dynamically trap the fact that blockClient() is called in the context of a Lua script and try to recover from that. Looks hard-ish since the command cannot fail, but there are potentially things we could do actually. Like create a blocked client context with "client" set to NULL (this happens if a client is terminated between blocking and unblocking), and actually replying to the Lua client ASAP with "-ERR module blocking command called from Lua script". I can give it a try... |
|
@dvirsky if we allow the command to sense a Lua script, it can just avoid blocking. |
|
didn't @oranagra add this to his fork and suggest it as an addition? |
|
yes, he suggested this addition, and I agree, via the RedisModule_GetClientFlags() API, so that multiple conditions are returned: lua, master, slave, ... |
|
So immediate fix - fail in lua somehow? longer term fix - allow the module to know it's a lua script and avoid blocking if it can. right? |
|
Yes, looks like it's the way to go. I'm trying to implement the first part so to release 4.0.1 between today and tomorrow with the other two important fixes (already inside). |
|
Thanks. BTW in one of the cases of people calling search from lua it turned out the user wanted to implement in Lua stuff that redisearch already does, so a little guidance on advanced RS capabilities allowed them to achieve what they wanted initially. |
|
Good, many things will likely be indeed false positives, and modules doc is not stellar... All the part about async contexts is missing (but is also marked as experimental). I'll try to make it better in the next weeks, but more likely I'll be able to improve it significantly after mid September. |
|
Just pushed a fix, looks good and uses already built-in mechanisms so should not fail spectacularly I hope. |
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue #4127 in the short term.
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue #4127 in the short term.
|
I would have thought of solving this with an explicit flag in command registration. |
|
@itamarhaber as outlined in the commit message, you can't trust modules to behave, and if they do not flag commands appropriately, this resulting into a crash is unfortunate given that this can be avoided with such a small patch. So an additional mechanism (that will more likely be the ability to detect Lua scripts) is welcome but this fundamental check is needed as well in my opinion. |
|
Both then - well-behaved modules can declare it and a failsafe check against anyone else is a perfect combo :) |
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue redis#4127 in the short term.
|
NOTE: This is fixed but left open for further considering what to do in the future. |
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue redis#4127 in the short term.
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue redis#4127 in the short term.
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue redis#4127 in the short term.
|
@gkorland @swilly22 @MeirShpilraien - is there anything else left to discuss here, from your POVs? |
Lua scripting does not support calling blocking commands, however all
the native Redis commands are flagged as "s" (no scripting flag), so
this is not possible at all. With modules there is no such mechanism in
order to flag a command as non callable by the Lua scripting engine,
moreover we cannot trust the modules users from complying all the times:
it is likely that modules will be released to have blocking commands
without such commands being flagged correctly, even if we provide a way to
signal this fact.
This commit attempts to address the problem in a short term way, by
detecting that a module is trying to block in the context of the Lua
scripting engine client, and preventing to do this. The module will
actually believe to block as usually, but what happens is that the Lua
script receives an error immediately, and the background call is ignored
by the Redis engine (if not for the cleanup callbacks, once it
unblocks).
Long term, the more likely solution, is to introduce a new call called
RedisModule_GetClientFlags(), so that a command can detect if the caller
is a Lua script, and return an error, or avoid blocking at all.
Being the blocking API experimental right now, more work is needed in
this regard in order to reach a level well blocking module commands and
all the other Redis subsystems interact peacefully.
Now the effect is like the following:
127.0.0.1:6379> eval "redis.call('hello.block',1,5000)" 0
(error) ERR Error running script (call to
f_b5ba35ff97bc1ef23debc4d6e9fd802da187ed53): @user_script:1: ERR
Blocking module command called from Lua script
This commit fixes issue redis#4127 in the short term.
Hello, I don't know if is redis bug or module bug, look the crash report:
The text was updated successfully, but these errors were encountered: