You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by ti...@apache.org on 2022/09/21 02:59:20 UTC
[incubator-kvrocks] branch unstable updated: Add EVAL_RO (#782)
This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 87644ab Add EVAL_RO (#782)
87644ab is described below
commit 87644ab5672fdcb8fe7d27e811ab115a88eb897f
Author: xiaobiaozhao <52...@users.noreply.github.com>
AuthorDate: Wed Sep 21 10:59:14 2022 +0800
Add EVAL_RO (#782)
* โจ feat: update lua vm
* โจ feat: update eval_ro
* ๐งช test: add test case
* ๐ fix: fix not set connection
* ๐งช test: add *sha test case
* ๐ style: rm ;
* ๐งช test: command 180 -> 182
* ๐ fix: fix exclusive issue
* Update command_test.go
Co-authored-by: tison <wa...@gmail.com>
Co-authored-by: hulk <hu...@gmail.com>
---
src/redis_cmd.cc | 26 ++++++++++++++++++++++-
src/redis_connection.cc | 20 +++++++++++++-----
src/scripting.cc | 34 ++++++++++++++++++++++++-------
src/scripting.h | 12 +++++------
src/worker.cc | 5 ++++-
src/worker.h | 4 ++++
tests/gocase/unit/command/command_test.go | 7 -------
tests/tcl/tests/unit/scripting.tcl | 22 ++++++++++++++++++++
8 files changed, 103 insertions(+), 27 deletions(-)
diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc
index 9ece7bf..553c77f 100644
--- a/src/redis_cmd.cc
+++ b/src/redis_cmd.cc
@@ -4875,6 +4875,28 @@ class CommandEvalSHA : public Commander {
}
};
+class CommandEvalRO : public Commander {
+ public:
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ return Lua::evalGenericCommand(conn, args_, false, output, true);
+ }
+};
+
+class CommandEvalSHARO : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args[1].size() != 40) {
+ return Status(Status::NotOK,
+ "NOSCRIPT No matching script. Please use EVAL");
+ }
+ return Status::OK();
+ }
+
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ return Lua::evalGenericCommand(conn, args_, true, output, true);
+ }
+};
+
class CommandScript : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
@@ -4904,7 +4926,7 @@ class CommandScript : public Commander {
}
} else if (args_.size() == 3 && subcommand_ == "load") {
std::string sha;
- auto s = Lua::createFunction(svr, args_[2], &sha);
+ auto s = Lua::createFunction(svr, args_[2], &sha, svr->Lua());
if (!s.IsOK()) {
return s;
}
@@ -5962,6 +5984,8 @@ CommandAttributes redisCommandTable[] = {
ADD_CMD("eval", -3, "exclusive write no-script", 0, 0, 0, CommandEval),
ADD_CMD("evalsha", -3, "exclusive write no-script", 0, 0, 0, CommandEvalSHA),
+ ADD_CMD("eval_ro", -3, "read-only no-script", 0, 0, 0, CommandEvalRO),
+ ADD_CMD("evalsha_ro", -3, "read-only no-script", 0, 0, 0, CommandEvalSHARO),
ADD_CMD("script", -2, "exclusive no-script", 0, 0, 0, CommandScript),
ADD_CMD("compact", 1, "read-only no-script", 0, 0, 0, CommandCompact),
diff --git a/src/redis_connection.cc b/src/redis_connection.cc
index db37b28..bf686b2 100644
--- a/src/redis_connection.cc
+++ b/src/redis_connection.cc
@@ -347,19 +347,29 @@ void Connection::ExecuteCommands(std::deque<CommandTokens> *to_process_cmds) {
if (IsFlagEnabled(Connection::kMultiExec) && attributes->name != "exec") {
// No lock guard, because 'exec' command has acquired 'WorkExclusivityGuard'
} else if (attributes->is_exclusive() ||
- (cmd_name == "config" && cmd_tokens.size() == 2 && !strcasecmp(cmd_tokens[1].c_str(), "set")) ||
- (config->cluster_enabled && (cmd_name == "clusterx" || cmd_name == "cluster")
- && cmd_tokens.size() >= 2 && Cluster::SubCommandIsExecExclusive(cmd_tokens[1]))) {
+ (cmd_name == "config" && cmd_tokens.size() == 2 &&
+ !strcasecmp(cmd_tokens[1].c_str(), "set")) ||
+ (config->cluster_enabled &&
+ (cmd_name == "clusterx" || cmd_name == "cluster") &&
+ cmd_tokens.size() >= 2 &&
+ Cluster::SubCommandIsExecExclusive(cmd_tokens[1]))) {
exclusivity = svr_->WorkExclusivityGuard();
// When executing lua script commands that have "exclusive" attribute,
- // we need to know current connection, but we should set current connection
- // after acquiring the WorkExclusivityGuard to make it thread-safe
+ // we need to know current connection, but we should set current
+ // connection after acquiring the WorkExclusivityGuard to make it
+ // thread-safe
svr_->SetCurrentConnection(this);
} else {
concurrency = svr_->WorkConcurrencyGuard();
}
+ if (cmd_name == "eval_ro" || cmd_name == "evalsha_ro") {
+ // if executing read only lua script commands, set current
+ // connection.
+ svr_->SetCurrentConnection(this);
+ }
+
if (svr_->IsLoading() && attributes->is_ok_loading() == false) {
Reply(Redis::Error("LOADING kvrocks is restoring the db from backup"));
if (IsFlagEnabled(Connection::kMultiExec)) multi_error_ = true;
diff --git a/src/scripting.cc b/src/scripting.cc
index 19d1d04..652f606 100644
--- a/src/scripting.cc
+++ b/src/scripting.cc
@@ -75,11 +75,11 @@ enum {
};
namespace Lua {
- lua_State* CreateState() {
+ lua_State *CreateState(bool read_only) {
lua_State *lua = lua_open();
loadLibraries(lua);
removeUnsupportedFunctions(lua);
- loadFuncs(lua);
+ loadFuncs(lua, read_only);
enableGlobalsProtection(lua);
return lua;
}
@@ -89,7 +89,7 @@ namespace Lua {
lua_close(lua);
}
- void loadFuncs(lua_State *lua) {
+ void loadFuncs(lua_State *lua, bool read_only) {
lua_newtable(lua);
/* redis.call */
@@ -136,6 +136,13 @@ namespace Lua {
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);
+ }
+
lua_setglobal(lua, "redis");
/* Replace math.random and math.randomseed with our implementations. */
@@ -233,11 +240,16 @@ namespace Lua {
Status evalGenericCommand(Redis::Connection *conn,
const std::vector<std::string> &args,
bool evalsha,
- std::string *output) {
+ 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();
+ }
auto s = Util::DecimalStringToNum(args[2], &numkeys);
if (!s.IsOK()) {
@@ -281,7 +293,7 @@ namespace Lua {
body = args[1];
}
std::string sha;
- s = createFunction(srv, body, &sha);
+ s = createFunction(srv, body, &sha, lua);
if (!s.IsOK()) {
lua_pop(lua, 1); /* remove the error handler from the stack. */
return s;
@@ -333,6 +345,10 @@ namespace Lua {
int redisGenericCommand(lua_State *lua, int raise_error) {
int j, argc = lua_gettop(lua);
std::vector<std::string> args;
+ lua_getglobal(lua, "redis");
+ lua_getfield(lua, -1, "read_only");
+ int read_only = lua_toboolean(lua, -1);
+ lua_pop(lua, 2);
if (argc == 0) {
pushError(lua, "Please specify at least one argument for redis.call()");
@@ -364,6 +380,10 @@ namespace Lua {
return raise_error ? raiseError(lua) : 1;
}
auto redisCmd = cmd_iter->second;
+ if (read_only && redisCmd->is_write()) {
+ 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);
@@ -883,7 +903,8 @@ 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) {
+Status createFunction(Server *srv, const std::string &body, std::string *sha,
+ lua_State *lua) {
char funcname[43];
funcname[0] = 'f';
@@ -898,7 +919,6 @@ Status createFunction(Server *srv, const std::string &body, std::string *sha) {
funcdef += body;
funcdef += "\nend";
- lua_State *lua = srv->Lua();
if (luaL_loadbuffer(lua, funcdef.c_str(), funcdef.size(), "@user_script")) {
std::string errMsg = lua_tostring(lua, -1);
lua_pop(lua, 1);
diff --git a/src/scripting.h b/src/scripting.h
index eee1f02..459662d 100644
--- a/src/scripting.h
+++ b/src/scripting.h
@@ -29,10 +29,10 @@
namespace Lua {
-lua_State* CreateState();
+lua_State* CreateState(bool read_only = false);
void DestroyState(lua_State *lua);
-void loadFuncs(lua_State *lua);
+void loadFuncs(lua_State *lua, bool read_only = false);
void loadLibraries(lua_State *lua);
void removeUnsupportedFunctions(lua_State *lua);
void enableGlobalsProtection(lua_State *lua);
@@ -42,13 +42,13 @@ int redisGenericCommand(lua_State *lua, int raise_error);
int redisSha1hexCommand(lua_State *lua);
int redisStatusReplyCommand(lua_State *lua);
int redisErrorReplyCommand(lua_State *lua);
-Status createFunction(Server *srv, const std::string &body, std::string *sha);
+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<std::string> &args,
- bool evalsha,
- std::string *output);
+ const std::vector<std::string> &args, bool evalsha,
+ std::string *output, bool read_only = false);
const char *redisProtocolToLuaType(lua_State *lua, const char *reply);
const char *redisProtocolToLuaType_Int(lua_State *lua, const char *reply);
diff --git a/src/worker.cc b/src/worker.cc
index 7b9d9a4..5657e9e 100644
--- a/src/worker.cc
+++ b/src/worker.cc
@@ -38,8 +38,9 @@
#include <openssl/err.h>
#endif
-#include "redis_request.h"
#include "redis_connection.h"
+#include "redis_request.h"
+#include "scripting.h"
#include "server.h"
#include "util.h"
@@ -65,6 +66,7 @@ Worker::Worker(Server *svr, Config *config, bool repl) : svr_(svr) {
LOG(INFO) << "[worker] Listening on: " << bind << ":" << *port;
}
}
+ lua_ = Lua::CreateState(true);
}
Worker::~Worker() {
@@ -86,6 +88,7 @@ Worker::~Worker() {
ev_token_bucket_cfg_free(rate_limit_group_cfg_);
}
event_base_free(base_);
+ Lua::DestroyState(lua_);
}
void Worker::TimerCB(int, int16_t events, void *ctx) {
diff --git a/src/worker.h b/src/worker.h
index 400fa1b..81c74bb 100644
--- a/src/worker.h
+++ b/src/worker.h
@@ -33,6 +33,8 @@
#include <event2/listener.h>
#include <event2/util.h>
#include "storage.h"
+
+#include "lua.hpp"
#include "redis_connection.h"
class Server;
@@ -62,6 +64,7 @@ class Worker {
Status ListenUnixSocket(const std::string &path, int perm, int backlog);
+ lua_State *Lua() { return lua_; }
Server *svr_;
private:
@@ -85,6 +88,7 @@ class Worker {
struct bufferevent_rate_limit_group *rate_limit_group_ = nullptr;
struct ev_token_bucket_cfg *rate_limit_group_cfg_ = nullptr;
+ lua_State* lua_;
};
class WorkerThread {
diff --git a/tests/gocase/unit/command/command_test.go b/tests/gocase/unit/command/command_test.go
index b5a87fc..a1eb556 100644
--- a/tests/gocase/unit/command/command_test.go
+++ b/tests/gocase/unit/command/command_test.go
@@ -35,13 +35,6 @@ func TestCommand(t *testing.T) {
rdb := srv.NewClient()
defer func() { require.NoError(t, rdb.Close()) }()
- t.Run("Kvrocks supports 182 commands currently", func(t *testing.T) {
- r := rdb.Do(ctx, "COMMAND", "COUNT")
- v, err := r.Int()
- require.NoError(t, err)
- require.Equal(t, 182, v)
- })
-
t.Run("acquire GET command info by COMMAND INFO", func(t *testing.T) {
r := rdb.Do(ctx, "COMMAND", "INFO", "GET")
vs, err := r.Slice()
diff --git a/tests/tcl/tests/unit/scripting.tcl b/tests/tcl/tests/unit/scripting.tcl
index 4138faf..41268be 100644
--- a/tests/tcl/tests/unit/scripting.tcl
+++ b/tests/tcl/tests/unit/scripting.tcl
@@ -387,6 +387,28 @@ start_server {tags {"scripting"}} {
set v [r eval { return redis.log(redis.LOG_WARNING, 'warning level'); } 0]
assert_equal "" $v
} {}
+
+ test {EVAL_RO - Successful case} {
+ r set foo bar
+ assert_equal bar [r eval_ro {return redis.call('get', KEYS[1]);} 1 foo]
+ }
+
+ test {EVALSHA_RO - Successful case} {
+ r set foo bar
+ assert_equal bar [r evalsha_ro 796941151549c416aa77522fb347487236c05e46 1 foo]
+ }
+
+ test {EVAL_RO - Cannot run write commands} {
+ r set foo bar
+ catch {r eval_ro {redis.call('del', KEYS[1]);} 1 foo} e
+ set e
+ } {ERR * Write commands are not allowed from read-only scripts}
+
+ test {EVALSHA_RO - Cannot run write commands} {
+ r set foo bar
+ catch {r evalsha_ro a1e63e1cd1bd1d5413851949332cfb9da4ee6dc0 1 foo} e
+ set e
+ } {ERR * Write commands are not allowed from read-only scripts}
}
start_server {tags {"repl"}} {