mirror of https://mirror.osredm.com/root/redis.git
Fix RM_Yield bug (#10548)
The bug was when using REDISMODULE_YIELD_FLAG_CLIENTS. in that case we would have only set the CLIENTS type flag in server.busy_module_yield_flags and then clear that flag when exiting RM_Yield, so we would never call unblockPostponedClients when the context is destroyed. This didn't really have any actual implication, which is why the tests couldn't (and still can't) find that since the bug only happens when using CLIENT, but in this case we won't have any clients to un-postpone i.e. clients will get rejected with BUSY error, rather than being postponed. Unrelated: * Adding tests for nested contexts, just in case. * Avoid nested RM_Yield calls
This commit is contained in:
parent
3e09a8c097
commit
451531f1c8
12
src/module.c
12
src/module.c
|
@ -2079,6 +2079,12 @@ int RM_BlockedClientMeasureTimeEnd(RedisModuleBlockedClient *bc) {
|
||||||
* the -LOADING error)
|
* the -LOADING error)
|
||||||
*/
|
*/
|
||||||
void RM_Yield(RedisModuleCtx *ctx, int flags, const char *busy_reply) {
|
void RM_Yield(RedisModuleCtx *ctx, int flags, const char *busy_reply) {
|
||||||
|
static int yield_nesting = 0;
|
||||||
|
/* Avoid nested calls to RM_Yield */
|
||||||
|
if (yield_nesting)
|
||||||
|
return;
|
||||||
|
yield_nesting++;
|
||||||
|
|
||||||
long long now = getMonotonicUs();
|
long long now = getMonotonicUs();
|
||||||
if (now >= ctx->next_yield_time) {
|
if (now >= ctx->next_yield_time) {
|
||||||
/* In loading mode, there's no need to handle busy_module_yield_reply,
|
/* In loading mode, there's no need to handle busy_module_yield_reply,
|
||||||
|
@ -2092,10 +2098,11 @@ void RM_Yield(RedisModuleCtx *ctx, int flags, const char *busy_reply) {
|
||||||
server.busy_module_yield_reply = busy_reply;
|
server.busy_module_yield_reply = busy_reply;
|
||||||
/* start the blocking operation if not already started. */
|
/* start the blocking operation if not already started. */
|
||||||
if (!server.busy_module_yield_flags) {
|
if (!server.busy_module_yield_flags) {
|
||||||
server.busy_module_yield_flags = flags & REDISMODULE_YIELD_FLAG_CLIENTS ?
|
server.busy_module_yield_flags = BUSY_MODULE_YIELD_EVENTS;
|
||||||
BUSY_MODULE_YIELD_CLIENTS : BUSY_MODULE_YIELD_EVENTS;
|
|
||||||
blockingOperationStarts();
|
blockingOperationStarts();
|
||||||
}
|
}
|
||||||
|
if (flags & REDISMODULE_YIELD_FLAG_CLIENTS)
|
||||||
|
server.busy_module_yield_flags |= BUSY_MODULE_YIELD_CLIENTS;
|
||||||
|
|
||||||
/* Let redis process events */
|
/* Let redis process events */
|
||||||
processEventsWhileBlocked();
|
processEventsWhileBlocked();
|
||||||
|
@ -2110,6 +2117,7 @@ void RM_Yield(RedisModuleCtx *ctx, int flags, const char *busy_reply) {
|
||||||
/* decide when the next event should fire. */
|
/* decide when the next event should fire. */
|
||||||
ctx->next_yield_time = now + 1000000 / server.hz;
|
ctx->next_yield_time = now + 1000000 / server.hz;
|
||||||
}
|
}
|
||||||
|
yield_nesting --;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Set flags defining capabilities or behavior bit flags.
|
/* Set flags defining capabilities or behavior bit flags.
|
||||||
|
|
|
@ -90,7 +90,8 @@ start_server {tags {"modules"}} {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
test {Busy module command} {
|
foreach call_type {nested normal} {
|
||||||
|
test "Busy module command - $call_type" {
|
||||||
set busy_time_limit 50
|
set busy_time_limit 50
|
||||||
set old_time_limit [lindex [r config get busy-reply-threshold] 1]
|
set old_time_limit [lindex [r config get busy-reply-threshold] 1]
|
||||||
r config set busy-reply-threshold $busy_time_limit
|
r config set busy-reply-threshold $busy_time_limit
|
||||||
|
@ -98,7 +99,11 @@ start_server {tags {"modules"}} {
|
||||||
|
|
||||||
# run command that blocks until released
|
# run command that blocks until released
|
||||||
set start [clock clicks -milliseconds]
|
set start [clock clicks -milliseconds]
|
||||||
$rd slow_fg_command 0
|
if {$call_type == "nested"} {
|
||||||
|
$rd do_rm_call slow_fg_command 0
|
||||||
|
} else {
|
||||||
|
$rd slow_fg_command 0
|
||||||
|
}
|
||||||
$rd flush
|
$rd flush
|
||||||
|
|
||||||
# make sure we get BUSY error, and that we didn't get it too early
|
# make sure we get BUSY error, and that we didn't get it too early
|
||||||
|
@ -114,9 +119,13 @@ start_server {tags {"modules"}} {
|
||||||
}
|
}
|
||||||
$rd read
|
$rd read
|
||||||
|
|
||||||
#run command that blocks for 200ms
|
# run command that blocks for 200ms
|
||||||
set start [clock clicks -milliseconds]
|
set start [clock clicks -milliseconds]
|
||||||
$rd slow_fg_command 200000
|
if {$call_type == "nested"} {
|
||||||
|
$rd do_rm_call slow_fg_command 200000
|
||||||
|
} else {
|
||||||
|
$rd slow_fg_command 200000
|
||||||
|
}
|
||||||
$rd flush
|
$rd flush
|
||||||
after 10 ;# try to make sure redis started running the command before we proceed
|
after 10 ;# try to make sure redis started running the command before we proceed
|
||||||
|
|
||||||
|
@ -128,6 +137,7 @@ start_server {tags {"modules"}} {
|
||||||
$rd close
|
$rd close
|
||||||
r config set busy-reply-threshold $old_time_limit
|
r config set busy-reply-threshold $old_time_limit
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
test {RM_Call from blocked client} {
|
test {RM_Call from blocked client} {
|
||||||
set busy_time_limit 50
|
set busy_time_limit 50
|
||||||
|
|
Loading…
Reference in New Issue