From b7289e912cbe1a011a5569cd67929e83731b9660 Mon Sep 17 00:00:00 2001 From: valentinogeron Date: Thu, 27 Aug 2020 09:19:24 +0300 Subject: [PATCH] EXEC with only read commands should not be rejected when OOM (#7696) If the server gets MULTI command followed by only read commands, and right before it gets the EXEC it reaches OOM, the client will get OOM response. So, from now on, it will get OOM response only if there was at least one command that was tagged with `use-memory` flag --- src/server.c | 21 +++++++++++++-------- tests/unit/multi.tcl | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/server.c b/src/server.c index 70307ff0b..42fa87898 100644 --- a/src/server.c +++ b/src/server.c @@ -3625,14 +3625,19 @@ int processCommand(client *c) { * into a slave, that may be the active client, to be freed. */ if (server.current_client == NULL) return C_ERR; - /* It was impossible to free enough memory, and the command the client - * is trying to execute is denied during OOM conditions or the client - * is in MULTI/EXEC context? Error. */ - if (out_of_memory && - (is_denyoom_command || - (c->flags & CLIENT_MULTI && - c->cmd->proc != discardCommand))) - { + int reject_cmd_on_oom = is_denyoom_command; + /* If client is in MULTI/EXEC context, queuing may consume an unlimited + * amount of memory, so we want to stop that. + * However, we never want to reject DISCARD, or even EXEC (unless it + * contains denied commands, in which case is_denyoom_command is already + * set. */ + if (c->flags & CLIENT_MULTI && + c->cmd->proc != execCommand && + c->cmd->proc != discardCommand) { + reject_cmd_on_oom = 1; + } + + if (out_of_memory && reject_cmd_on_oom) { rejectCommand(c, shared.oomerr); return C_OK; } diff --git a/tests/unit/multi.tcl b/tests/unit/multi.tcl index 44a822ba6..817d509c5 100644 --- a/tests/unit/multi.tcl +++ b/tests/unit/multi.tcl @@ -466,4 +466,42 @@ start_server {tags {"multi"}} { assert { $xx == 1 } $r1 close; } + + test {EXEC with only read commands should not be rejected when OOM} { + set r2 [redis_client] + + r set x value + r multi + r get x + r ping + + # enforcing OOM + $r2 config set maxmemory 1 + + # finish the multi transaction with exec + assert { [r exec] == {value PONG} } + + # releasing OOM + $r2 config set maxmemory 0 + $r2 close + } + + test {EXEC with at least one use-memory command should fail} { + set r2 [redis_client] + + r multi + r set x 1 + r get x + + # enforcing OOM + $r2 config set maxmemory 1 + + # finish the multi transaction with exec + catch {r exec} e + assert_match {EXECABORT*OOM*} $e + + # releasing OOM + $r2 config set maxmemory 0 + $r2 close + } }