Unverified Commit fae5b1a1 authored by 郭伟光's avatar 郭伟光 Committed by GitHub
Browse files

unblockClient: avoid to reset client when the client was shutdown-blocked (#10440)

fix #10439. see https://github.com/redis/redis/pull/9872
When executing SHUTDOWN we pause the client so we can un-pause it
if the shutdown fails.
this could happen during the timeout, if the shutdown is aborted, but could
also happen from withing the initial `call()` to shutdown, if the rdb save fails.
in that case when we return to `call()`, we'll crash if `c->cmd` has been set to NULL.

The call stack is:
```
unblockClient(c)
replyToClientsBlockedOnShutdown()
cancelShutdown()
finishShutdown()
prepareForShutdown()
shutdownCommand()
```

what's special about SHUTDOWN in that respect is that it can be paused,
and then un-paused before the original `call()` returns.
tests where added for both failed shutdown, and a followup successful one.
parent b9656adb
...@@ -204,7 +204,7 @@ void unblockClient(client *c) { ...@@ -204,7 +204,7 @@ void unblockClient(client *c) {
* we do not do it immediately after the command returns (when the * we do not do it immediately after the command returns (when the
* client got blocked) in order to be still able to access the argument * client got blocked) in order to be still able to access the argument
* vector from module callbacks and updateStatsOnUnblock. */ * vector from module callbacks and updateStatsOnUnblock. */
if (c->btype != BLOCKED_POSTPONE) { if (c->btype != BLOCKED_POSTPONE && c->btype != BLOCKED_SHUTDOWN) {
freeClientOriginalArgv(c); freeClientOriginalArgv(c);
resetClient(c); resetClient(c);
} }
......
...@@ -66,3 +66,39 @@ start_server {tags {"shutdown external:skip"}} { ...@@ -66,3 +66,39 @@ start_server {tags {"shutdown external:skip"}} {
} }
} }
} }
start_server {tags {"shutdown external:skip"}} {
set pid [s process_id]
set dump_rdb [file join [lindex [r config get dir] 1] dump.rdb]
test {RDB save will be failed in shutdown} {
for {set i 0} {$i < 20} {incr i} {
r set $i $i
}
# create a folder called 'dump.rdb' to trigger temp-rdb rename failure
# and it will cause rdb save to fail eventually.
if {[file exists $dump_rdb]} {
exec rm -f $dump_rdb
}
exec mkdir -p $dump_rdb
}
test {SHUTDOWN will abort if rdb save failed on signal} {
# trigger a shutdown which will save an rdb
exec kill -SIGINT $pid
wait_for_log_messages 0 {"*Error trying to save the DB, can't exit*"} 0 100 10
}
test {SHUTDOWN will abort if rdb save failed on shutdown command} {
catch {[r shutdown]} err
assert_match {*Errors trying to SHUTDOWN*} $err
# make sure the server is still alive
assert_equal [r ping] {PONG}
}
test {SHUTDOWN can proceed if shutdown command was with nosave} {
catch {[r shutdown nosave]}
wait_for_log_messages 0 {"*ready to exit, bye bye*"} 0 100 10
}
test {Clean up rdb same named folder} {
exec rm -r $dump_rdb
}
}
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