diff --git a/src/quicklist.c b/src/quicklist.c index 0385e0bf9..d23212b34 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -110,7 +110,7 @@ quicklist *quicklistCreate(void) { return quicklist; } -#define COMPRESS_MAX (1 << QL_COMP_BITS) +#define COMPRESS_MAX ((1 << QL_COMP_BITS)-1) void quicklistSetCompressDepth(quicklist *quicklist, int compress) { if (compress > COMPRESS_MAX) { compress = COMPRESS_MAX; @@ -120,7 +120,7 @@ void quicklistSetCompressDepth(quicklist *quicklist, int compress) { quicklist->compress = compress; } -#define FILL_MAX (1 << (QL_FILL_BITS-1)) +#define FILL_MAX ((1 << (QL_FILL_BITS-1))-1) void quicklistSetFill(quicklist *quicklist, int fill) { if (fill > FILL_MAX) { fill = FILL_MAX; diff --git a/src/redismodule.h b/src/redismodule.h index 3f08df509..33b4367ae 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -474,7 +474,11 @@ RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromLongLong)(Re RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromDouble)(RedisModuleCtx *ctx, double d); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromLongDouble)(RedisModuleCtx *ctx, long double ld, int humanfriendly); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromString)(RedisModuleCtx *ctx, const RedisModuleString *str); +#ifdef __GNUC__ +RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringPrintf)(RedisModuleCtx *ctx, const char *fmt, ...) __attribute__ ((format (printf, 2, 3))); +#else RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringPrintf)(RedisModuleCtx *ctx, const char *fmt, ...); +#endif void REDISMODULE_API_FUNC(RedisModule_FreeString)(RedisModuleCtx *ctx, RedisModuleString *str); const char *REDISMODULE_API_FUNC(RedisModule_StringPtrLen)(const RedisModuleString *str, size_t *len); int REDISMODULE_API_FUNC(RedisModule_ReplyWithError)(RedisModuleCtx *ctx, const char *err); @@ -558,8 +562,13 @@ void REDISMODULE_API_FUNC(RedisModule_SaveLongDouble)(RedisModuleIO *io, long do long double REDISMODULE_API_FUNC(RedisModule_LoadLongDouble)(RedisModuleIO *io); void *REDISMODULE_API_FUNC(RedisModule_LoadDataTypeFromString)(const RedisModuleString *str, const RedisModuleType *mt); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_SaveDataTypeToString)(RedisModuleCtx *ctx, void *data, const RedisModuleType *mt); +#ifdef __GNUC__ +void REDISMODULE_API_FUNC(RedisModule_Log)(RedisModuleCtx *ctx, const char *level, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +void REDISMODULE_API_FUNC(RedisModule_LogIOError)(RedisModuleIO *io, const char *levelstr, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +#else void REDISMODULE_API_FUNC(RedisModule_Log)(RedisModuleCtx *ctx, const char *level, const char *fmt, ...); void REDISMODULE_API_FUNC(RedisModule_LogIOError)(RedisModuleIO *io, const char *levelstr, const char *fmt, ...); +#endif void REDISMODULE_API_FUNC(RedisModule__Assert)(const char *estr, const char *file, int line); void REDISMODULE_API_FUNC(RedisModule_LatencyAddSample)(const char *event, mstime_t latency); int REDISMODULE_API_FUNC(RedisModule_StringAppendBuffer)(RedisModuleCtx *ctx, RedisModuleString *str, const char *buf, size_t len); diff --git a/src/scripting.cpp b/src/scripting.cpp index 224b32931..f84c7386f 100644 --- a/src/scripting.cpp +++ b/src/scripting.cpp @@ -671,12 +671,11 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) { !g_pserver->loading && /* Don't care about mem if loading. */ !listLength(g_pserver->masters) && /* Slave must execute the script. */ g_pserver->lua_write_dirty == 0 && /* Script had no side effects so far. */ + g_pserver->lua_oom && /* Detected OOM when script started. */ (cmd->flags & CMD_DENYOOM)) { - if (getMaxmemoryState(NULL,NULL,NULL,NULL) != C_OK) { - luaPushError(lua, (char*)ptrFromObj(shared.oomerr)); - goto cleanup; - } + luaPushError(lua, (char*)ptrFromObj(shared.oomerr)); + goto cleanup; } if (cmd->flags & CMD_RANDOM) g_pserver->lua_random_dirty = 1; diff --git a/src/server.cpp b/src/server.cpp index 10048519f..091dcd2a3 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -705,15 +705,15 @@ struct redisCommand redisCommandTable[] = { 0,NULL,1,1,1,0,0,0}, {"multi",multiCommand,1, - "no-script fast @transaction", + "no-script fast ok-loading ok-stale @transaction", 0,NULL,0,0,0,0,0,0}, {"exec",execCommand,1, - "no-script no-monitor no-slowlog @transaction", + "no-script no-monitor no-slowlog ok-loading ok-stale @transaction", 0,NULL,0,0,0,0,0,0}, {"discard",discardCommand,1, - "no-script fast @transaction", + "no-script fast ok-loading ok-stale @transaction", 0,NULL,0,0,0,0,0,0}, {"sync",syncCommand,1, @@ -980,11 +980,11 @@ struct redisCommand redisCommandTable[] = { 0,NULL,1,1,1,0,0,0}, {"xread",xreadCommand,-4, - "read-only no-script @stream @blocking", + "read-only @stream @blocking", 0,xreadGetKeys,1,1,1,0,0,0}, {"xreadgroup",xreadCommand,-7, - "write no-script @stream @blocking", + "write @stream @blocking", 0,xreadGetKeys,1,1,1,0,0,0}, {"xgroup",xgroupCommand,-2, @@ -3692,6 +3692,13 @@ int processCommand(client *c, int callFlags) { addReply(c, shared.oomerr); return C_OK; } + + /* Save out_of_memory result at script start, otherwise if we check OOM + * untill first write within script, memory used by lua stack and + * arguments might interfere. */ + if (c->cmd->proc == evalCommand || c->cmd->proc == evalShaCommand) { + g_pserver->lua_oom = out_of_memory; + } } /* Make sure to use a reasonable amount of memory for client side diff --git a/src/server.h b/src/server.h index e911034d2..bb4bfcc37 100644 --- a/src/server.h +++ b/src/server.h @@ -1935,6 +1935,7 @@ struct redisServer { execution. */ int lua_kill; /* Kill the script if true. */ int lua_always_replicate_commands; /* Default replication type. */ + int lua_oom; /* OOM detected when script start? */ /* Lazy free */ int lazyfree_lazy_eviction; int lazyfree_lazy_expire; diff --git a/src/t_stream.cpp b/src/t_stream.cpp index ab06f733c..44e61efa3 100644 --- a/src/t_stream.cpp +++ b/src/t_stream.cpp @@ -1382,6 +1382,11 @@ void xreadCommand(client *c) { int moreargs = c->argc-i-1; char *o = szFromObj(c->argv[i]); if (!strcasecmp(o,"BLOCK") && moreargs) { + if (c->flags & CLIENT_LUA) { + /* There is no sense to use BLOCK option within LUA */ + addReplyErrorFormat(c, "%s command is not allowed with BLOCK option from scripts", szFromObj(c->argv[0])); + return; + } i++; if (getTimeoutFromObjectOrReply(c,c->argv[i],&timeout, UNIT_MILLISECONDS) != C_OK) return; diff --git a/tests/support/server.tcl b/tests/support/server.tcl index 69326bb20..9739f2ae6 100644 --- a/tests/support/server.tcl +++ b/tests/support/server.tcl @@ -159,12 +159,9 @@ proc start_server {options {code undefined}} { if {$::external} { if {[llength $::servers] == 0} { set srv {} - # In test_server_main(tests/test_helper.tcl:215~218), increase the value of start_port - # and assign it to ::port through the `--port` option, so we need to reduce it. - set baseport [expr {$::port-100}] dict set srv "host" $::host - dict set srv "port" $baseport - set client [redis $::host $baseport 0 $::tls] + dict set srv "port" $::port + set client [redis $::host $::port 0 $::tls] dict set srv "client" $client $client select 9 diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index 24f6b82e0..0892c8bec 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -217,13 +217,19 @@ proc test_server_main {} { # Start the client instances set ::clients_pids {} - set start_port [expr {$::port+100}] - for {set j 0} {$j < $::numclients} {incr j} { - set start_port [find_available_port $start_port] + if {$::external} { set p [exec $tclsh [info script] {*}$::argv \ - --client $port --port $start_port &] + --client $port --port $::port &] lappend ::clients_pids $p - incr start_port 10 + } else { + set start_port [expr {$::port+100}] + for {set j 0} {$j < $::numclients} {incr j} { + set start_port [find_available_port $start_port] + set p [exec $tclsh [info script] {*}$::argv \ + --client $port --port $start_port &] + lappend ::clients_pids $p + incr start_port 10 + } } # Setup global state for the test server @@ -511,9 +517,6 @@ for {set j 0} {$j < [llength $argv]} {incr j} { } elseif {$opt eq {--host}} { set ::external 1 set ::host $arg - # If we use an external server, we can only set numclients to 1, - # otherwise the port will be miscalculated. - set ::numclients 1 incr j } elseif {$opt eq {--port}} { set ::port $arg diff --git a/tests/unit/moduleapi/blockonkeys.tcl b/tests/unit/moduleapi/blockonkeys.tcl index c8b8f23ed..5e5d93da3 100644 --- a/tests/unit/moduleapi/blockonkeys.tcl +++ b/tests/unit/moduleapi/blockonkeys.tcl @@ -11,6 +11,12 @@ start_server {tags {"modules"}} { $rd1 fsl.bpoppush src dst 0 $rd2 fsl.bpoppush dst src 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {2} + } else { + fail "Clients are not blocked" + } r fsl.push src 42 @@ -24,7 +30,12 @@ start_server {tags {"modules"}} { r del src $rd1 fsl.bpoppush src src 0 - + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r fsl.push src 42 assert_equal {42} [r fsl.getall src] @@ -48,6 +59,12 @@ start_server {tags {"modules"}} { r del k set rd [redis_deferring_client] $rd fsl.bpop k 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r fsl.push k 34 assert_equal {34} [$rd read] } @@ -76,6 +93,12 @@ start_server {tags {"modules"}} { set cid [$rd read] r fsl.push k 33 $rd fsl.bpopgt k 33 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r fsl.push k 34 assert_equal {34} [$rd read] r client kill id $cid ;# try to smoke-out client-related memory leak @@ -85,6 +108,12 @@ start_server {tags {"modules"}} { r del k set rd [redis_deferring_client] $rd fsl.bpopgt k 35 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r fsl.push k 33 r fsl.push k 34 r fsl.push k 35 @@ -98,6 +127,12 @@ start_server {tags {"modules"}} { $rd client id set cid [$rd read] $rd fsl.bpopgt k 35 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r client kill id $cid ;# try to smoke-out client-related memory leak } @@ -107,6 +142,12 @@ start_server {tags {"modules"}} { $rd client id set cid [$rd read] $rd fsl.bpopgt k 35 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r client unblock $cid timeout ;# try to smoke-out client-related memory leak assert_equal {Request timedout} [$rd read] } @@ -117,6 +158,12 @@ start_server {tags {"modules"}} { $rd client id set cid [$rd read] $rd fsl.bpopgt k 35 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r client unblock $cid error ;# try to smoke-out client-related memory leak assert_error "*unblocked*" {$rd read} } @@ -125,6 +172,12 @@ start_server {tags {"modules"}} { r del k set rd [redis_deferring_client] $rd fsl.bpop k 0 + ;# wait until clients are actually blocked + wait_for_condition 50 100 { + [s 0 blocked_clients] eq {1} + } else { + fail "Clients are not blocked" + } r lpush k 12 r lpush k 13 r lpush k 14 diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index fb36d0b80..8b364b287 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -146,6 +146,17 @@ start_server {tags {"scripting"}} { set e } {*not allowed*} + test {EVAL - Scripts can't run XREAD and XREADGROUP with BLOCK option} { + r del s + r xgroup create s g $ MKSTREAM + set res [r eval {return redis.pcall('xread','STREAMS','s','$')} 1 s] + assert {$res eq {}} + assert_error "*xread command is not allowed with BLOCK option from scripts" {r eval {return redis.pcall('xread','BLOCK',0,'STREAMS','s','$')} 1 s} + set res [r eval {return redis.pcall('xreadgroup','group','g','c','STREAMS','s','>')} 1 s] + assert {$res eq {}} + assert_error "*xreadgroup command is not allowed with BLOCK option from scripts" {r eval {return redis.pcall('xreadgroup','group','g','c','BLOCK',0,'STREAMS','s','>')} 1 s} + } + test {EVAL - Scripts can't run certain commands} { set e {} r debug lua-always-replicate-commands 0