From bde35e3aaa4d8a079b5b84dad30a0ee25ff611d7 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: e70fbad (cherry picked from commit 16e3af9d23bb4791a7571f889aad8f2c85546521) --- 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 6e2ef5ca0..9eb6c093f 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1652,6 +1652,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 a253309ed..3c96c601b 100644 --- a/src/module.c +++ b/src/module.c @@ -6988,6 +6988,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 ee9418c6b..8cb217b73 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 */ @@ -2590,6 +2591,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(server.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 831349a76..3b73d045d 100644 --- a/src/server.c +++ b/src/server.c @@ -2019,6 +2019,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 @@ -4969,7 +4972,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] + } +}