From cc7a01ebc48949ebd2c53111b5c73b00b6cde420 Mon Sep 17 00:00:00 2001 From: PragmaTwice Date: Fri, 10 Mar 2023 16:58:22 +0800 Subject: [PATCH 1/5] Improve code style in scripting --- src/server/server.cc | 4 +- src/storage/scripting.cc | 126 +++++++++++++++++++-------------------- src/storage/scripting.h | 8 ++- 3 files changed, 68 insertions(+), 70 deletions(-) diff --git a/src/server/server.cc b/src/server/server.cc index 73ac722506e..d2d8209d9e1 100644 --- a/src/server/server.cc +++ b/src/server/server.cc @@ -1507,9 +1507,7 @@ Status Server::ScriptGet(const std::string &sha, std::string *body) { auto cf = storage_->GetCFHandle(Engine::kPropagateColumnFamilyName); auto s = storage_->Get(rocksdb::ReadOptions(), cf, func_name, body); if (!s.ok()) { - if (s.IsNotFound()) return {Status::NotFound}; - - return {Status::NotOK, s.ToString()}; + return {s.IsNotFound() ? Status::NotFound : Status::NotOK, s.ToString()}; } return Status::OK(); } diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc index abe0e186257..20455c47585 100644 --- a/src/storage/scripting.cc +++ b/src/storage/scripting.cc @@ -25,6 +25,7 @@ #include +#include #include #include "commands/commander.h" @@ -109,12 +110,10 @@ void loadFuncs(lua_State *lua, bool read_only) { lua_pushcfunction(lua, redisStatusReplyCommand); lua_settable(lua, -3); - if (read_only) { - /* redis.read_only */ - lua_pushstring(lua, "read_only"); - lua_pushboolean(lua, 1); - lua_settable(lua, -3); - } + /* redis.read_only */ + lua_pushstring(lua, "read_only"); + lua_pushboolean(lua, read_only); + lua_settable(lua, -3); lua_setglobal(lua, "redis"); @@ -165,7 +164,7 @@ void loadFuncs(lua_State *lua, bool read_only) { } int redisLogCommand(lua_State *lua) { - int j = 0, level = 0, argc = lua_gettop(lua); + int argc = lua_gettop(lua); if (argc < 2) { lua_pushstring(lua, "redis.log() requires two arguments or more."); @@ -175,26 +174,20 @@ int redisLogCommand(lua_State *lua) { lua_pushstring(lua, "First argument must be a number (log level)."); return lua_error(lua); } - level = static_cast(lua_tonumber(lua, -argc)); + int level = static_cast(lua_tonumber(lua, -argc)); if (level < LL_DEBUG || level > LL_WARNING) { lua_pushstring(lua, "Invalid debug level."); return lua_error(lua); } - if (level < GetServer()->GetConfig()->log_level) { - return 0; - } std::string log_message; - for (j = 1; j < argc; j++) { + for (int j = 1; j < argc; j++) { size_t len = 0; - const char *s = nullptr; - s = lua_tolstring(lua, (-argc) + j, &len); - if (s) { + if (const char *s = lua_tolstring(lua, j - argc, &len)) { if (j != 1) { - log_message += " " + std::string(s, len); - } else { - log_message = std::string(s, len); + log_message += " "; } + log_message += std::string(s, len); } } @@ -213,16 +206,12 @@ int redisLogCommand(lua_State *lua) { Status evalGenericCommand(Redis::Connection *conn, const std::vector &args, bool evalsha, std::string *output, bool read_only) { - int64_t numkeys = 0; - char funcname[43]; Server *srv = conn->GetServer(); - lua_State *lua = srv->Lua(); - if (read_only) { - // Use the worker's private Lua VM when entering the read-only mode - lua = conn->Owner()->Lua(); - } - numkeys = GET_OR_RET(ParseInt(args[2], 10)); + // Use the worker's private Lua VM when entering the read-only mode + lua_State *lua = read_only ? conn->Owner()->Lua() : srv->Lua(); + + int64_t numkeys = GET_OR_RET(ParseInt(args[2], 10)); if (numkeys > int64_t(args.size() - 3)) { return {Status::NotOK, "Number of keys can't be greater than number of args"}; } else if (numkeys < -1) { @@ -231,16 +220,14 @@ Status evalGenericCommand(Redis::Connection *conn, const std::vector= 'A' && sha[j] <= 'Z') ? static_cast(sha[j] + 'a' - 'A') : sha[j]; + funcname[j + 2] = static_cast(tolower(args[1][j])); } - funcname[42] = '\0'; } /* Push the pcall error handler function on the stack. */ @@ -260,6 +247,7 @@ Status evalGenericCommand(Redis::Connection *conn, const std::vector(args.begin() + 3, args.begin() + 3 + numkeys)); setGlobalArray(lua, "ARGV", std::vector(args.begin() + 3 + numkeys, args.end())); - int err = lua_pcall(lua, 0, 1, -2); - if (err) { - std::string msg = std::string("ERR running script (call to ") + funcname + "): " + lua_tostring(lua, -1); + + if (lua_pcall(lua, 0, 1, -2)) { + auto msg = fmt::format("ERR running script (call to {}): {}", funcname, lua_tostring(lua, -1)); *output = Redis::Error(msg); lua_pop(lua, 2); } else { @@ -306,33 +294,36 @@ Status evalGenericCommand(Redis::Connection *conn, const std::vector args; lua_getglobal(lua, "redis"); lua_getfield(lua, -1, "read_only"); int read_only = lua_toboolean(lua, -1); lua_pop(lua, 2); + int argc = lua_gettop(lua); if (argc == 0) { pushError(lua, "Please specify at least one argument for redis.call()"); return raise_error ? raiseError(lua) : 1; } - for (j = 0; j < argc; j++) { - if (lua_type(lua, j + 1) == LUA_TNUMBER) { - lua_Number num = lua_tonumber(lua, j + 1); + + std::vector args; + for (int j = 1; j <= argc; j++) { + if (lua_type(lua, j) == LUA_TNUMBER) { + lua_Number num = lua_tonumber(lua, j); args.emplace_back(fmt::format("{:.17g}", static_cast(num))); } else { size_t obj_len = 0; - const char *obj_s = lua_tolstring(lua, j + 1, &obj_len); - if (obj_s == nullptr) break; /* no a string */ + const char *obj_s = lua_tolstring(lua, j, &obj_len); + if (obj_s == nullptr) { + pushError(lua, "Lua redis() command arguments must be strings or integers"); + return raise_error ? raiseError(lua) : 1; + } args.emplace_back(obj_s, obj_len); } } - if (j != argc) { - pushError(lua, "Lua redis() command arguments must be strings or integers"); - return raise_error ? raiseError(lua) : 1; - } auto commands = Redis::GetCommands(); auto cmd_iter = commands->find(Util::ToLower(args[0])); @@ -340,14 +331,17 @@ int redisGenericCommand(lua_State *lua, int raise_error) { pushError(lua, "Unknown Redis command called from Lua script"); return raise_error ? raiseError(lua) : 1; } + auto redisCmd = cmd_iter->second; - if (read_only && redisCmd->is_write()) { + if (read_only && ~(redisCmd->flags & Redis::kCmdReadOnly)) { pushError(lua, "Write commands are not allowed from read-only scripts"); return raise_error ? raiseError(lua) : 1; } + auto cmd = redisCmd->factory(); cmd->SetAttributes(redisCmd); cmd->SetArgs(args); + int arity = cmd->GetAttributes()->arity; if (((arity > 0 && argc != arity) || (arity < 0 && argc < -arity))) { pushError(lua, "Wrong number of args calling Redis command From Lua script"); @@ -359,9 +353,10 @@ int redisGenericCommand(lua_State *lua, int raise_error) { return raise_error ? raiseError(lua) : 1; } - std::string output, cmd_name = Util::ToLower(args[0]); + std::string cmd_name = Util::ToLower(args[0]); Server *srv = GetServer(); Config *config = srv->GetConfig(); + Redis::Connection *conn = srv->GetCurrentConnection(); if (config->cluster_enabled) { auto s = srv->cluster_->CanExecByMySelf(attributes, args, conn); @@ -370,10 +365,12 @@ int redisGenericCommand(lua_State *lua, int raise_error) { return raise_error ? raiseError(lua) : 1; } } + if (config->slave_readonly && srv->IsSlave() && attributes->is_write()) { pushError(lua, "READONLY You can't write against a read only slave."); return raise_error ? raiseError(lua) : 1; } + if (!config->slave_serve_stale_data && srv->IsSlave() && cmd_name != "info" && cmd_name != "slaveof" && srv->GetReplicationState() != kReplConnected) { pushError(lua, @@ -381,14 +378,17 @@ int redisGenericCommand(lua_State *lua, int raise_error) { "and slave-serve-stale-data is set to 'no'."); return raise_error ? raiseError(lua) : 1; } + auto s = cmd->Parse(args); - if (!s.IsOK()) { + if (!s) { pushError(lua, s.Msg().data()); return raise_error ? raiseError(lua) : 1; } + srv->stats_.IncrCalls(cmd_name); auto start = std::chrono::high_resolution_clock::now(); bool is_profiling = conn->isProfilingEnabled(cmd_name); + std::string output; s = cmd->Execute(GetServer(), srv->GetCurrentConnection(), &output); auto end = std::chrono::high_resolution_clock::now(); uint64_t duration = std::chrono::duration_cast(end - start).count(); @@ -396,10 +396,11 @@ int redisGenericCommand(lua_State *lua, int raise_error) { srv->SlowlogPushEntryIfNeeded(&args, duration); srv->stats_.IncrLatency(static_cast(duration), cmd_name); srv->FeedMonitorConns(conn, args); - if (!s.IsOK()) { + if (!s) { pushError(lua, s.Msg().data()); return raise_error ? raiseError(lua) : 1; } + redisProtocolToLuaType(lua, output.data()); return 1; } @@ -443,6 +444,7 @@ void loadLibraries(lua_State *lua) { lua_pushstring(lua, libname); lua_call(lua, 1, 0); }; + loadLib(lua, "", luaopen_base); loadLib(lua, LUA_TABLIBNAME, luaopen_table); loadLib(lua, LUA_STRLIBNAME, luaopen_string); @@ -484,16 +486,16 @@ int redisStatusReplyCommand(lua_State *lua) { return redisReturnSingleFieldTable * function used for sha1ing lua scripts. */ int redisSha1hexCommand(lua_State *lua) { int argc = lua_gettop(lua); - char digest[41]; - size_t len = 0; - const char *s = nullptr; if (argc != 1) { lua_pushstring(lua, "wrong number of arguments"); return lua_error(lua); } - s = static_cast(lua_tolstring(lua, 1, &len)); + size_t len = 0; + const char *s = static_cast(lua_tolstring(lua, 1, &len)); + + char digest[41]; SHA1Hex(digest, s, len); lua_pushstring(lua, digest); return 1; @@ -513,13 +515,12 @@ void SHA1Hex(char *digest, const char *script, size_t len) { SHA1_CTX ctx; unsigned char hash[20]; const char *cset = "0123456789abcdef"; - int j = 0; SHA1Init(&ctx); SHA1Update(&ctx, (const unsigned char *)script, len); SHA1Final(hash, &ctx); - for (j = 0; j < 20; j++) { + for (int j = 0; j < 20; j++) { digest[j * 2] = cset[((hash[j] & 0xF0) >> 4)]; digest[j * 2 + 1] = cset[(hash[j] & 0xF)]; } @@ -872,29 +873,22 @@ int redisMathRandomSeed(lua_State *L) { * If 'c' is not NULL, on error the client is informed with an appropriate * error describing the nature of the problem and the Lua interpreter error. */ Status createFunction(Server *srv, const std::string &body, std::string *sha, lua_State *lua) { - char funcname[43]; + char funcname[2 + 40 + 1] = "f_"; - funcname[0] = 'f'; - funcname[1] = '_'; SHA1Hex(funcname + 2, body.c_str(), body.size()); *sha = funcname + 2; - std::string funcdef; - funcdef += "function "; - funcdef += funcname; - funcdef += "() "; - funcdef += body; - funcdef += "\nend"; + auto funcdef = fmt::format("function {}() {}\nend", funcname, body); if (luaL_loadbuffer(lua, funcdef.c_str(), funcdef.size(), "@user_script")) { std::string errMsg = lua_tostring(lua, -1); lua_pop(lua, 1); - return Status(Status::NotOK, "Error compiling script (new function): " + errMsg + "\n"); + return {Status::NotOK, "Error compiling script (new function): " + errMsg + "\n"}; } if (lua_pcall(lua, 0, 0, 0)) { std::string errMsg = lua_tostring(lua, -1); lua_pop(lua, 1); - return Status(Status::NotOK, "Error running script (new function): " + errMsg + "\n"); + return {Status::NotOK, "Error running script (new function): " + errMsg + "\n"}; } // would store lua function into propagate column family and propagate those scripts to slaves return srv->ScriptSet(*sha, body); diff --git a/src/storage/scripting.h b/src/storage/scripting.h index 0f850a15b16..466fda4f19d 100644 --- a/src/storage/scripting.h +++ b/src/storage/scripting.h @@ -36,15 +36,17 @@ void loadFuncs(lua_State *lua, bool read_only = false); void loadLibraries(lua_State *lua); void removeUnsupportedFunctions(lua_State *lua); void enableGlobalsProtection(lua_State *lua); + int redisCallCommand(lua_State *lua); int redisPCallCommand(lua_State *lua); int redisGenericCommand(lua_State *lua, int raise_error); int redisSha1hexCommand(lua_State *lua); int redisStatusReplyCommand(lua_State *lua); int redisErrorReplyCommand(lua_State *lua); +int redisLogCommand(lua_State *lua); + Status createFunction(Server *srv, const std::string &body, std::string *sha, lua_State *lua); -int redisLogCommand(lua_State *lua); Status evalGenericCommand(Redis::Connection *conn, const std::vector &args, bool evalsha, std::string *output, bool read_only = false); @@ -57,9 +59,12 @@ const char *redisProtocolToLuaType_Aggregate(lua_State *lua, const char *reply, const char *redisProtocolToLuaType_Null(lua_State *lua, const char *reply); const char *redisProtocolToLuaType_Bool(lua_State *lua, const char *reply, int tf); const char *redisProtocolToLuaType_Double(lua_State *lua, const char *reply); + std::string replyToRedisReply(lua_State *lua); + void pushError(lua_State *lua, const char *err); [[noreturn]] int raiseError(lua_State *lua); + void sortArray(lua_State *lua); void setGlobalArray(lua_State *lua, const std::string &var, const std::vector &elems); @@ -67,4 +72,5 @@ void SHA1Hex(char *digest, const char *script, size_t len); int redisMathRandom(lua_State *L); int redisMathRandomSeed(lua_State *L); + } // namespace Lua From dd12ba8e8240e922e9af558ddd3970f1176e991a Mon Sep 17 00:00:00 2001 From: PragmaTwice Date: Fri, 10 Mar 2023 17:04:17 +0800 Subject: [PATCH 2/5] format --- src/storage/scripting.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc index 20455c47585..480b551ebfd 100644 --- a/src/storage/scripting.cc +++ b/src/storage/scripting.cc @@ -370,7 +370,7 @@ int redisGenericCommand(lua_State *lua, int raise_error) { pushError(lua, "READONLY You can't write against a read only slave."); return raise_error ? raiseError(lua) : 1; } - + if (!config->slave_serve_stale_data && srv->IsSlave() && cmd_name != "info" && cmd_name != "slaveof" && srv->GetReplicationState() != kReplConnected) { pushError(lua, @@ -400,7 +400,7 @@ int redisGenericCommand(lua_State *lua, int raise_error) { pushError(lua, s.Msg().data()); return raise_error ? raiseError(lua) : 1; } - + redisProtocolToLuaType(lua, output.data()); return 1; } From 601653c9088567ed3dc54cea0ddfab6403d32726 Mon Sep 17 00:00:00 2001 From: PragmaTwice Date: Fri, 10 Mar 2023 20:43:22 +0800 Subject: [PATCH 3/5] fix --- src/storage/scripting.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc index 480b551ebfd..6222344b4a3 100644 --- a/src/storage/scripting.cc +++ b/src/storage/scripting.cc @@ -333,7 +333,7 @@ int redisGenericCommand(lua_State *lua, int raise_error) { } auto redisCmd = cmd_iter->second; - if (read_only && ~(redisCmd->flags & Redis::kCmdReadOnly)) { + if (read_only && !(redisCmd->flags & Redis::kCmdReadOnly)) { pushError(lua, "Write commands are not allowed from read-only scripts"); return raise_error ? raiseError(lua) : 1; } From 4dea5c0e1d901b81dc975bc8e5fad60c199fba77 Mon Sep 17 00:00:00 2001 From: PragmaTwice Date: Fri, 10 Mar 2023 21:55:02 +0800 Subject: [PATCH 4/5] remove useless script store --- src/commands/cmd_script.cc | 2 +- src/storage/scripting.cc | 6 +++--- src/storage/scripting.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/commands/cmd_script.cc b/src/commands/cmd_script.cc index 5652b0019bc..28bba262aff 100644 --- a/src/commands/cmd_script.cc +++ b/src/commands/cmd_script.cc @@ -98,7 +98,7 @@ class CommandScript : public Commander { } } else if (args_.size() == 3 && subcommand_ == "load") { std::string sha; - auto s = Lua::createFunction(svr, args_[2], &sha, svr->Lua()); + auto s = Lua::createFunction(svr, args_[2], &sha, svr->Lua(), true); if (!s.IsOK()) { return s; } diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc index 6222344b4a3..17fc23012dc 100644 --- a/src/storage/scripting.cc +++ b/src/storage/scripting.cc @@ -249,7 +249,7 @@ Status evalGenericCommand(Redis::Connection *conn, const std::vectorScriptSet(*sha, body); + return need_to_store ? srv->ScriptSet(*sha, body) : Status::OK(); } } // namespace Lua diff --git a/src/storage/scripting.h b/src/storage/scripting.h index 466fda4f19d..fbe063221ae 100644 --- a/src/storage/scripting.h +++ b/src/storage/scripting.h @@ -45,7 +45,7 @@ int redisStatusReplyCommand(lua_State *lua); int redisErrorReplyCommand(lua_State *lua); int redisLogCommand(lua_State *lua); -Status createFunction(Server *srv, const std::string &body, std::string *sha, lua_State *lua); +Status createFunction(Server *srv, const std::string &body, std::string *sha, lua_State *lua, bool need_to_store); Status evalGenericCommand(Redis::Connection *conn, const std::vector &args, bool evalsha, std::string *output, bool read_only = false); From ce4bacdaff26f7eb5bff58c3bc5be6a1742a60d4 Mon Sep 17 00:00:00 2001 From: PragmaTwice Date: Fri, 10 Mar 2023 23:06:47 +0800 Subject: [PATCH 5/5] fix script exists --- src/server/server.cc | 7 +++++++ src/storage/scripting.cc | 4 ++-- src/storage/scripting.h | 2 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/server/server.cc b/src/server/server.cc index d2d8209d9e1..55ee5303f46 100644 --- a/src/server/server.cc +++ b/src/server/server.cc @@ -36,6 +36,7 @@ #include "commands/commander.h" #include "config.h" #include "fmt/format.h" +#include "lua.h" #include "redis_connection.h" #include "redis_request.h" #include "storage/compaction_checker.h" @@ -1498,6 +1499,12 @@ Status Server::LookupAndCreateCommand(const std::string &cmd_name, std::unique_p } Status Server::ScriptExists(const std::string &sha) { + lua_getglobal(lua_, (REDIS_LUA_FUNC_SHA_PREFIX + sha).c_str()); + if (!lua_isnil(lua_, -1)) { + return Status::OK(); + } + lua_pop(lua_, 1); + std::string body; return ScriptGet(sha, &body); } diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc index 17fc23012dc..e1aa723364c 100644 --- a/src/storage/scripting.cc +++ b/src/storage/scripting.cc @@ -220,7 +220,7 @@ Status evalGenericCommand(Redis::Connection *conn, const std::vector