Unverified Commit 036963a7 authored by Meir Shpilraien (Spielrein)'s avatar Meir Shpilraien (Spielrein) Committed by GitHub
Browse files

Restore old client 'processCommandAndResetClient' to fix false dead client indicator (#8715)

'processCommandAndResetClient' returns 1 if client is dead. It does it
by checking if serve.current_client is NULL. On script timeout, Redis will re-enter
'processCommandAndResetClient' and when finish we will set server.current_client
to NULL. This will cause later to falsely return 1 and think that the client that
sent the timed-out script is dead (Redis to stop reading from the client buffer).
parent 54fd3d40
...@@ -1990,12 +1990,20 @@ void commandProcessed(client *c) { ...@@ -1990,12 +1990,20 @@ void commandProcessed(client *c) {
* of processing the command, otherwise C_OK is returned. */ * of processing the command, otherwise C_OK is returned. */
int processCommandAndResetClient(client *c) { int processCommandAndResetClient(client *c) {
int deadclient = 0; int deadclient = 0;
client *old_client = server.current_client;
server.current_client = c; server.current_client = c;
if (processCommand(c) == C_OK) { if (processCommand(c) == C_OK) {
commandProcessed(c); commandProcessed(c);
} }
if (server.current_client == NULL) deadclient = 1; if (server.current_client == NULL) deadclient = 1;
server.current_client = NULL; /*
* Restore the old client, this is needed because when a script
* times out, we will get into this code from processEventsWhileBlocked.
* Which will cause to set the server.current_client. If not restored
* we will return 1 to our caller which will falsely indicate the client
* is dead and will stop reading from its buffer.
*/
server.current_client = old_client;
/* performEvictions may flush slave output buffers. This may /* performEvictions may flush slave output buffers. This may
* result in a slave, that may be the active client, to be * result in a slave, that may be the active client, to be
* freed. */ * freed. */
......
...@@ -640,6 +640,43 @@ start_server {tags {"scripting"}} { ...@@ -640,6 +640,43 @@ start_server {tags {"scripting"}} {
assert_match {*killed by user*} $res assert_match {*killed by user*} $res
} }
test {Timedout script does not cause a false dead client} {
set rd [redis_deferring_client]
r config set lua-time-limit 10
# senging (in a pipeline):
# 1. eval "while 1 do redis.call('ping') end" 0
# 2. ping
set buf "*3\r\n\$4\r\neval\r\n\$33\r\nwhile 1 do redis.call('ping') end\r\n\$1\r\n0\r\n"
append buf "*1\r\n\$4\r\nping\r\n"
$rd write $buf
$rd flush
wait_for_condition 50 100 {
[catch {r ping} e] == 1
} else {
fail "Can't wait for script to start running"
}
catch {r ping} e
assert_match {BUSY*} $e
r script kill
wait_for_condition 50 100 {
[catch {r ping} e] == 0
} else {
fail "Can't wait for script to be killed"
}
assert_equal [r ping] "PONG"
catch {$rd read} res
assert_match {*killed by user*} $res
set res [$rd read]
assert_match {*PONG*} $res
$rd close
}
test {Timedout script link is still usable after Lua returns} { test {Timedout script link is still usable after Lua returns} {
r config set lua-time-limit 10 r config set lua-time-limit 10
r eval {for i=1,100000 do redis.call('ping') end return 'ok'} 0 r eval {for i=1,100000 do redis.call('ping') end return 'ok'} 0
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment