From 89c78a980807aa1de0a6d0ccde6042450333a783 Mon Sep 17 00:00:00 2001 From: Wang Yuan Date: Tue, 3 Nov 2020 23:16:11 +0800 Subject: [PATCH] Disable rehash when redis has child process (#8007) In redisFork(), we don't set child pid, so updateDictResizePolicy() doesn't take effect, that isn't friendly for copy-on-write. The bug was introduced this in redis 6.0: 56258c6 --- src/aof.c | 1 + src/module.c | 1 + src/rdb.c | 2 ++ src/server.c | 4 +++- tests/unit/other.tcl | 20 ++++++++++++++++++++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/aof.c b/src/aof.c index 0f67f0575..f3176934f 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1715,6 +1715,7 @@ int rewriteAppendOnlyFileBackground(void) { server.aof_rewrite_scheduled = 0; server.aof_rewrite_time_start = time(NULL); server.aof_child_pid = childpid; + updateDictResizePolicy(); /* We set appendseldb to -1 in order to force the next call to the * feedAppendOnlyFile() to issue a SELECT command, so the differences * accumulated by the parent into server.aof_rewrite_buf will start diff --git a/src/module.c b/src/module.c index 65a4932b7..b4a15acae 100644 --- a/src/module.c +++ b/src/module.c @@ -7001,6 +7001,7 @@ int RM_Fork(RedisModuleForkDoneHandler cb, void *user_data) { server.module_child_pid = childpid; moduleForkInfo.done_handler = cb; moduleForkInfo.done_handler_user_data = user_data; + updateDictResizePolicy(); serverLog(LL_VERBOSE, "Module fork started pid: %d ", childpid); } return childpid; diff --git a/src/rdb.c b/src/rdb.c index db5698c1d..3ea55fa67 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1412,6 +1412,7 @@ int rdbSaveBackground(char *filename, rdbSaveInfo *rsi) { server.rdb_save_time_start = time(NULL); server.rdb_child_pid = childpid; server.rdb_child_type = RDB_CHILD_TYPE_DISK; + updateDictResizePolicy(); return C_OK; } return C_OK; /* unreached */ @@ -2618,6 +2619,7 @@ int rdbSaveToSlavesSockets(rdbSaveInfo *rsi) { server.rdb_save_time_start = time(NULL); server.rdb_child_pid = childpid; server.rdb_child_type = RDB_CHILD_TYPE_SOCKET; + updateDictResizePolicy(); close(rdb_pipe_write); /* close write in parent so that it can detect the close on the child. */ if (aeCreateFileEvent(server.el, server.rdb_pipe_read, AE_READABLE, rdbPipeReadHandler,NULL) == AE_ERR) { serverPanic("Unrecoverable error creating server.rdb_pipe_read file event."); diff --git a/src/server.c b/src/server.c index f57bf01a7..ffe2b6be6 100644 --- a/src/server.c +++ b/src/server.c @@ -2052,6 +2052,9 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) { } } } + /* Just for the sake of defensive programming, to avoid forgeting to + * call this function when need. */ + updateDictResizePolicy(); /* AOF postponed flush: Try at every cron cycle if the slow fsync @@ -5072,7 +5075,6 @@ int redisFork(int purpose) { if (childpid == -1) { return -1; } - updateDictResizePolicy(); } return childpid; } diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 20a32e795..3ca70a77a 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -250,3 +250,23 @@ start_server {tags {"other"}} { r save } {OK} } + +start_server {tags {"other"}} { + test {Don't rehash if redis has child proecess} { + r config set save "" + r config set rdb-key-save-delay 1000000 + + populate 4096 "" 1 + r bgsave + r mset k1 v1 k2 v2 + # Hash table should not rehash + assert_no_match "*table size: 8192*" [r debug HTSTATS 9] + exec kill -9 [get_child_pid 0] + after 200 + + # Hash table should rehash since there is no child process, + # size is power of two and over 4098, so it is 16384 + r set k3 v3 + assert_match "*table size: 16384*" [r debug HTSTATS 9] + } +}