gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

Adding server.call/pcall option to LUA scripting. (#136) (#213)

Browse Source 
This commit does not remove redis.call/pcall just yet. It also does not
rename Redis in error messages such as "Please specify at least one
argument for this redis lib call". This allows users to maintain full
backwards compatibility while introducing an option to use server.call
for new code.

I verified that the unit tests pass. Also manually verified that the
redis-server responds to server.call invocations within lua scripting.
Also verified that function registration works as expected.

```
[ok]: EVAL - is Lua able to call Redis API? (0 ms)
[ok]: EVAL - is Lua able to call Server API? (1 ms)
[ok]: EVAL - No arguments to redis.call/pcall is considered an error (0 ms)
[ok]: EVAL - No arguments to server.call/pcall is considered an error (1 ms)
```

---------

Signed-off-by: Parth Patel <661497+parthpatel@users.noreply.github.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
This commit is contained in:
Parth 2024-04-05 21:17:11 -07:00 committed by GitHub
parent bc28fb4ac0
commit 620d325fdc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 63 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/eval.c
View File

@ -27,6 +27,13 @@
* POSSIBILITY OF SUCH DAMAGE.
*/
/*
* This file initializes the global LUA object and registers functions to call Valkey API from within the LUA language.
* It heavily invokes LUA's C API documented at https://www.lua.org/pil/24.html. There are 2 entrypoint functions in this file:
* 1. evalCommand() - Gets invoked everytime a user runs LUA script via eval command on Valkey.
* 2. scriptingInit() - initServer() function from server.c invokes this to initialize LUA at startup.
* It is also invoked between 2 eval invocations to reset Lua.
*/
#include "server.h"
#include "sha1.h"
#include "rand.h"

5
src/function_lua.c
View File

@ -437,6 +437,11 @@ int luaEngineInitEngine(void) {
luaRegisterVersion(lua_engine_ctx->lua);
luaSetErrorMetatable(lua_engine_ctx->lua);
lua_setfield(lua_engine_ctx->lua, -2, SERVER_API_NAME);
/* Get the server object and also set it to the Redis API
* compatibility namespace. */
lua_getfield(lua_engine_ctx->lua, -1, SERVER_API_NAME);
lua_setfield(lua_engine_ctx->lua, -2, REDIS_API_NAME);
luaSetErrorMetatable(lua_engine_ctx->lua);

12
src/script_lua.c
View File

@ -57,7 +57,8 @@ static char *libraries_allow_list[] = {
/* Redis Lua API globals */
static char *redis_api_allow_list[] = {
"redis",
SERVER_API_NAME,
REDIS_API_NAME,
"__redis__err__handler", /* error handler for eval, currently located on globals.
Should move to registry. */
NULL,
@ -1504,9 +1505,14 @@ void luaRegisterRedisAPI(lua_State* lua) {
lua_pushcfunction(lua,luaRedisAclCheckCmdPermissionsCommand);
lua_settable(lua,-3);
/* Finally set the table as 'redis' global var. */
/* Finally set the table as 'server' global var.
* We will also alias it to 'redis' global var for backwards compatibility. */
lua_setglobal(lua,SERVER_API_NAME);
/* lua_getglobal invocation retrieves the 'server' variable value to the stack.
* lua_setglobal invocation uses the value from stack to set 'redis' global variable.
* This is not a deep copy but is enough for our purpose here. */
lua_getglobal(lua,SERVER_API_NAME);
lua_setglobal(lua,REDIS_API_NAME);
/* Replace math.random and math.randomseed with our implementations. */
lua_getglobal(lua,"math");

1
src/script_lua.h
View File

@ -57,6 +57,7 @@
#define REGISTRY_RUN_CTX_NAME "__RUN_CTX__"
#define REGISTRY_SET_GLOBALS_PROTECTION_NAME "__GLOBAL_PROTECTION__"
#define REDIS_API_NAME "redis"
#define SERVER_API_NAME "server"
typedef struct errorInfo {
sds msg;

53
tests/unit/scripting.tcl
View File

@ -1,13 +1,38 @@
foreach is_eval {0 1} {
foreach script_compatibility_api {server redis} {
# We run the tests using both the server APIs, e.g. server.call(), and redis APIs, e.g. redis.call(),
# in order to ensure compatibility.
if {$script_compatibility_api eq "server"} {
proc replace_script_redis_api_with_server {args} {
set new_string [regsub -all {redis\.} [lindex $args 0] {server.}]
return lreplace $args 0 0 $new_string
}
proc get_script_api_name {} {
return "server"
}
} else {
proc replace_script_redis_api_with_server {args} {
return {*}$args
}
proc get_script_api_name {} {
return "redis"
}
}
if {$is_eval == 1} {
proc run_script {args} {
set args [replace_script_redis_api_with_server $args]
r eval {*}$args
}
proc run_script_ro {args} {
set args [replace_script_redis_api_with_server $args]
r eval_ro {*}$args
}
proc run_script_on_connection {args} {
set args [replace_script_redis_api_with_server $args]
[lindex $args 0] eval {*}[lrange $args 1 end]
}
proc kill_script {args} {
@ -15,7 +40,8 @@ if {$is_eval == 1} {
}
} else {
proc run_script {args} {
r function load replace [format "#!lua name=test\nredis.register_function('test', function(KEYS, ARGV)\n %s \nend)" [lindex $args 0]]
set args [replace_script_redis_api_with_server $args]
r function load replace [format "#!lua name=test\n%s.register_function('test', function(KEYS, ARGV)\n %s \nend)" [get_script_api_name] [lindex $args 0]]
if {[r readingraw] eq 1} {
# read name
assert_equal {test} [r read]
@ -23,7 +49,8 @@ if {$is_eval == 1} {
r fcall test {*}[lrange $args 1 end]
}
proc run_script_ro {args} {
r function load replace [format "#!lua name=test\nredis.register_function{function_name='test', callback=function(KEYS, ARGV)\n %s \nend, flags={'no-writes'}}" [lindex $args 0]]
set args [replace_script_redis_api_with_server $args]
r function load replace [format "#!lua name=test\n%s.register_function{function_name='test', callback=function(KEYS, ARGV)\n %s \nend, flags={'no-writes'}}" [get_script_api_name] [lindex $args 0]]
if {[r readingraw] eq 1} {
# read name
assert_equal {test} [r read]
@ -31,8 +58,9 @@ if {$is_eval == 1} {
r fcall_ro test {*}[lrange $args 1 end]
}
proc run_script_on_connection {args} {
set args [replace_script_redis_api_with_server $args]
set rd [lindex $args 0]
$rd function load replace [format "#!lua name=test\nredis.register_function('test', function(KEYS, ARGV)\n %s \nend)" [lindex $args 1]]
$rd function load replace [format "#!lua name=test\n%s.register_function('test', function(KEYS, ARGV)\n %s \nend)" [get_script_api_name] [lindex $args 1]]
# read name
$rd read
$rd fcall test {*}[lrange $args 2 end]
@ -44,7 +72,7 @@ if {$is_eval == 1} {
start_server {tags {"scripting"}} {
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
test {Script - disallow write on OOM} {
r config set maxmemory 1
@ -113,7 +141,7 @@ start_server {tags {"scripting"}} {
run_script {return redis.call('get',KEYS[1])} 1 mykey
} {myval}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
# eval sha is only relevant for is_eval Lua
test {EVALSHA - Can we call a SHA1 if already defined?} {
r evalsha fd758d1589d044dd850a6f05d52f2eefd27f033f 1 mykey
@ -572,7 +600,7 @@ start_server {tags {"scripting"}} {
set e
} {ERR Write commands are not allowed from read-only scripts*}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
# script command is only relevant for is_eval Lua
test {SCRIPTING FLUSH - is able to clear the scripts cache?} {
r set mykey myval
@ -712,7 +740,7 @@ start_server {tags {"scripting"}} {
set res
} {4 3 2 2 2}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
# random handling is only relevant for is_eval Lua
test {random numbers are random now} {
set rand1 [r eval {return tostring(math.random())} 0]
@ -754,7 +782,7 @@ start_server {tags {"scripting"}} {
r get x
} {10000}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
test {SPOP: We can call scripts rewriting client->argv from Lua} {
set repl [attach_to_replication_stream]
#this sadd operation is for external-cluster test. If myset doesn't exist, 'del myset' won't get propagated.
@ -1300,7 +1328,7 @@ start_server {tags {"scripting"}} {
}
}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
test "Now use EVALSHA against the master, with both SHAs" {
# The server should replicate successful and unsuccessful
# commands as EVAL instead of EVALSHA.
@ -1336,7 +1364,7 @@ start_server {tags {"scripting"}} {
set res
} {a 1}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
test "EVALSHA replication when first call is readonly" {
r del x
r eval {if tonumber(ARGV[1]) > 0 then redis.call('incr', KEYS[1]) end} 1 x 0
@ -1440,7 +1468,7 @@ start_server {tags {"scripting repl external:skip"}} {
}
test "PRNG is seeded randomly for command replication" {
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
# on is_eval Lua we need to call redis.replicate_commands() to get real randomization
set a [
run_script {
@ -1485,7 +1513,7 @@ start_server {tags {"scripting repl external:skip"}} {
}
}
if {$is_eval eq 1} {
if {$is_eval eq 1 && $script_compatibility_api == "redis"} {
start_server {tags {"scripting external:skip"}} {
r script debug sync
r eval {return 'hello'} 0
@ -1876,6 +1904,7 @@ start_server {tags {"scripting needs:debug"}} {
r debug set-disable-deny-scripts 0
}
} ;# foreach is_eval
} ;# foreach script_compatibility_api
# Scripting "shebang" notation tests