diff options
author | Oran Agra <oran@redislabs.com> | 2023-01-10 08:40:40 +0200 |
---|---|---|
committer | Oran Agra <oran@redislabs.com> | 2023-01-16 18:40:35 +0200 |
commit | 3e82bdf73803c084e549085cf9407e5e760630dd (patch) | |
tree | 55c8757043c1b34d3f2f5e21d4f4eeff218709a2 | |
parent | 574a49b96c02f6833bef3b15501aafda1e0fb031 (diff) | |
download | redis-3e82bdf73803c084e549085cf9407e5e760630dd.tar.gz |
Make sure that fork child doesn't do incremental rehashing (#11692)
Turns out that a fork child calling getExpire while persisting keys (and
possibly also a result of some module fork tasks) could cause dictFind
to do incremental rehashing in the child process, which is both a waste
of time, and also causes COW harm.
(cherry picked from commit 2bec254d89ff9266b2e688b15e34a3498da7c92c)
-rw-r--r-- | src/dict.c | 33 | ||||
-rw-r--r-- | src/dict.h | 9 | ||||
-rw-r--r-- | src/server.c | 11 |
3 files changed, 32 insertions, 21 deletions
diff --git a/src/dict.c b/src/dict.c index cb6850027..e2e7b9d93 100644 --- a/src/dict.c +++ b/src/dict.c @@ -47,15 +47,15 @@ #include "zmalloc.h" #include "redisassert.h" -/* Using dictEnableResize() / dictDisableResize() we make possible to - * enable/disable resizing of the hash table as needed. This is very important +/* Using dictEnableResize() / dictDisableResize() we make possible to disable + * resizing and rehashing of the hash table as needed. This is very important * for Redis, as we use copy-on-write and don't want to move too much memory * around when there is a child performing saving operations. * * Note that even when dict_can_resize is set to 0, not all resizes are * prevented: a hash table is still allowed to grow if the ratio between * the number of elements and the buckets > dict_force_resize_ratio. */ -static int dict_can_resize = 1; +static dictResizeEnable dict_can_resize = DICT_RESIZE_ENABLE; static unsigned int dict_force_resize_ratio = 5; /* -------------------------- private prototypes ---------------------------- */ @@ -127,7 +127,7 @@ int dictResize(dict *d) { unsigned long minimal; - if (!dict_can_resize || dictIsRehashing(d)) return DICT_ERR; + if (dict_can_resize != DICT_RESIZE_ENABLE || dictIsRehashing(d)) return DICT_ERR; minimal = d->ht_used[0]; if (minimal < DICT_HT_INITIAL_SIZE) minimal = DICT_HT_INITIAL_SIZE; @@ -210,7 +210,12 @@ int dictTryExpand(dict *d, unsigned long size) { * work it does would be unbound and the function may block for a long time. */ int dictRehash(dict *d, int n) { int empty_visits = n*10; /* Max number of empty buckets to visit. */ - if (!dictIsRehashing(d)) return 0; + if (dict_can_resize == DICT_RESIZE_FORBID || !dictIsRehashing(d)) return 0; + if (dict_can_resize == DICT_RESIZE_AVOID && + (DICTHT_SIZE(d->ht_size_exp[1]) / DICTHT_SIZE(d->ht_size_exp[0]) < dict_force_resize_ratio)) + { + return 0; + } while(n-- && d->ht_used[0] != 0) { dictEntry *de, *nextde; @@ -1000,10 +1005,12 @@ static int _dictExpandIfNeeded(dict *d) * table (global setting) or we should avoid it but the ratio between * elements/buckets is over the "safe" threshold, we resize doubling * the number of buckets. */ - if (d->ht_used[0] >= DICTHT_SIZE(d->ht_size_exp[0]) && - (dict_can_resize || - d->ht_used[0]/ DICTHT_SIZE(d->ht_size_exp[0]) > dict_force_resize_ratio) && - dictTypeExpandAllowed(d)) + if (!dictTypeExpandAllowed(d)) + return DICT_OK; + if ((dict_can_resize == DICT_RESIZE_ENABLE && + d->ht_used[0] >= DICTHT_SIZE(d->ht_size_exp[0])) || + (dict_can_resize != DICT_RESIZE_FORBID && + d->ht_used[0] / DICTHT_SIZE(d->ht_size_exp[0]) > dict_force_resize_ratio)) { return dictExpand(d, d->ht_used[0] + 1); } @@ -1063,12 +1070,8 @@ void dictEmpty(dict *d, void(callback)(dict*)) { d->pauserehash = 0; } -void dictEnableResize(void) { - dict_can_resize = 1; -} - -void dictDisableResize(void) { - dict_can_resize = 0; +void dictSetResizeEnabled(dictResizeEnable enable) { + dict_can_resize = enable; } uint64_t dictGetHash(dict *d, const void *key) { diff --git a/src/dict.h b/src/dict.h index e65fbb583..4a6afa83f 100644 --- a/src/dict.h +++ b/src/dict.h @@ -169,6 +169,12 @@ typedef void (dictScanBucketFunction)(dict *d, dictEntry **bucketref); #define randomULong() random() #endif +typedef enum { + DICT_RESIZE_ENABLE, + DICT_RESIZE_AVOID, + DICT_RESIZE_FORBID, +} dictResizeEnable; + /* API */ dict *dictCreate(dictType *type); int dictExpand(dict *d, unsigned long size); @@ -195,8 +201,7 @@ void dictGetStats(char *buf, size_t bufsize, dict *d); uint64_t dictGenHashFunction(const void *key, size_t len); uint64_t dictGenCaseHashFunction(const unsigned char *buf, size_t len); void dictEmpty(dict *d, void(callback)(dict*)); -void dictEnableResize(void); -void dictDisableResize(void); +void dictSetResizeEnabled(dictResizeEnable enable); int dictRehash(dict *d, int n); int dictRehashMilliseconds(dict *d, int ms); void dictSetHashFunctionSeed(uint8_t *seed); diff --git a/src/server.c b/src/server.c index 33ad776dc..6ceaddec1 100644 --- a/src/server.c +++ b/src/server.c @@ -594,13 +594,15 @@ int incrementallyRehash(int dbid) { * as we want to avoid resizing the hash tables when there is a child in order * to play well with copy-on-write (otherwise when a resize happens lots of * memory pages are copied). The goal of this function is to update the ability - * for dict.c to resize the hash tables accordingly to the fact we have an + * for dict.c to resize or rehash the tables accordingly to the fact we have an * active fork child running. */ void updateDictResizePolicy(void) { - if (!hasActiveChildProcess()) - dictEnableResize(); + if (server.in_fork_child != CHILD_TYPE_NONE) + dictSetResizeEnabled(DICT_RESIZE_FORBID); + else if (hasActiveChildProcess()) + dictSetResizeEnabled(DICT_RESIZE_AVOID); else - dictDisableResize(); + dictSetResizeEnabled(DICT_RESIZE_ENABLE); } const char *strChildType(int type) { @@ -6417,6 +6419,7 @@ int redisFork(int purpose) { server.in_fork_child = purpose; setupChildSignalHandlers(); setOOMScoreAdj(CONFIG_OOM_BGCHILD); + updateDictResizePolicy(); dismissMemoryInChild(); closeChildUnusedResourceAfterFork(); /* Close the reading part, so that if the parent crashes, the child will |