diff --git a/src/config.c b/src/config.c index 8dc241ed7a5..84b218cd724 100644 --- a/src/config.c +++ b/src/config.c @@ -1085,7 +1085,8 @@ dictType optionToLineDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - dictListDestructor /* val destructor */ + dictListDestructor, /* val destructor */ + NULL /* allow to expand */ }; dictType optionSetDictType = { @@ -1094,7 +1095,8 @@ dictType optionSetDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* The config rewrite state. */ diff --git a/src/db.c b/src/db.c index b52c6f3f244..45865b3e8d9 100644 --- a/src/db.c +++ b/src/db.c @@ -464,7 +464,7 @@ dbBackup *backupDb(void) { for (int i=0; idbarray[i] = server.db[i]; server.db[i].dict = dictCreate(&dbDictType,NULL); - server.db[i].expires = dictCreate(&keyptrDictType,NULL); + server.db[i].expires = dictCreate(&dbExpiresDictType,NULL); } /* Backup cluster slots to keys map if enable cluster. */ diff --git a/src/dict.c b/src/dict.c index 57287442aa7..cca5aa921d2 100644 --- a/src/dict.c +++ b/src/dict.c @@ -951,6 +951,16 @@ unsigned long dictScan(dict *d, /* ------------------------- private functions ------------------------------ */ +/* Because we may need to allocate huge memory chunk at once when dict + * expands, we will check this allocation is allowed or not if the dict + * type has expandAllowed member function. */ +static int dictTypeExpandAllowed(dict *d) { + if (d->type->expandAllowed == NULL) return 1; + return d->type->expandAllowed( + _dictNextPower(d->ht[0].used + 1) * sizeof(dictEntry*), + (double)d->ht[0].used / d->ht[0].size); +} + /* Expand the hash table if needed */ static int _dictExpandIfNeeded(dict *d) { @@ -966,9 +976,10 @@ static int _dictExpandIfNeeded(dict *d) * the number of buckets. */ if (d->ht[0].used >= d->ht[0].size && (dict_can_resize || - d->ht[0].used/d->ht[0].size > dict_force_resize_ratio)) + d->ht[0].used/d->ht[0].size > dict_force_resize_ratio) && + dictTypeExpandAllowed(d)) { - return dictExpand(d, d->ht[0].used*2); + return dictExpand(d, d->ht[0].used + 1); } return DICT_OK; } @@ -1173,6 +1184,7 @@ dictType BenchmarkDictType = { NULL, compareCallback, freeCallback, + NULL, NULL }; diff --git a/src/dict.h b/src/dict.h index dec60f6373c..fca924abe71 100644 --- a/src/dict.h +++ b/src/dict.h @@ -62,6 +62,7 @@ typedef struct dictType { int (*keyCompare)(void *privdata, const void *key1, const void *key2); void (*keyDestructor)(void *privdata, void *key); void (*valDestructor)(void *privdata, void *obj); + int (*expandAllowed)(size_t moreMem, double usedRatio); } dictType; /* This is our hash table structure. Every dictionary has two of this as we diff --git a/src/evict.c b/src/evict.c index d14603aefe9..3de328fc059 100644 --- a/src/evict.c +++ b/src/evict.c @@ -412,6 +412,20 @@ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *lev return C_ERR; } +/* Return 1 if used memory is more than maxmemory after allocating more memory, + * return 0 if not. Redis may reject user's requests or evict some keys if used + * memory exceeds maxmemory, especially, when we allocate huge memory at once. */ +int overMaxmemoryAfterAlloc(size_t moremem) { + if (!server.maxmemory) return 0; /* No limit. */ + + /* Check quickly. */ + size_t mem_used = zmalloc_used_memory(); + if (mem_used + moremem <= server.maxmemory) return 0; + + size_t overhead = freeMemoryGetNotCountedMemory(); + mem_used = (mem_used > overhead) ? mem_used - overhead : 0; + return mem_used + moremem > server.maxmemory; +} /* The evictionTimeProc is started when "maxmemory" has been breached and * could not immediately be resolved. This will spin the event loop with short diff --git a/src/expire.c b/src/expire.c index cac2085734b..5433f46ca5a 100644 --- a/src/expire.c +++ b/src/expire.c @@ -433,7 +433,8 @@ void rememberSlaveKeyWithExpire(redisDb *db, robj *key) { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; slaveKeysWithExpire = dictCreate(&dt,NULL); } diff --git a/src/latency.c b/src/latency.c index c661c5d6de6..535e48c2b33 100644 --- a/src/latency.c +++ b/src/latency.c @@ -53,7 +53,8 @@ dictType latencyTimeSeriesDictType = { NULL, /* val dup */ dictStringKeyCompare, /* key compare */ dictVanillaFree, /* key destructor */ - dictVanillaFree /* val destructor */ + dictVanillaFree, /* val destructor */ + NULL /* allow to expand */ }; /* ------------------------- Utility functions ------------------------------ */ diff --git a/src/lazyfree.c b/src/lazyfree.c index 641ab4e640d..125e6a1b09c 100644 --- a/src/lazyfree.c +++ b/src/lazyfree.c @@ -153,7 +153,7 @@ void freeObjAsync(robj *key, robj *obj) { void emptyDbAsync(redisDb *db) { dict *oldht1 = db->dict, *oldht2 = db->expires; db->dict = dictCreate(&dbDictType,NULL); - db->expires = dictCreate(&keyptrDictType,NULL); + db->expires = dictCreate(&dbExpiresDictType,NULL); atomicIncr(lazyfree_objects,dictSize(oldht1)); bioCreateBackgroundJob(BIO_LAZY_FREE,NULL,oldht1,oldht2); } diff --git a/src/module.c b/src/module.c index 647272b597f..51536358316 100644 --- a/src/module.c +++ b/src/module.c @@ -7552,7 +7552,8 @@ dictType moduleAPIDictType = { NULL, /* val dup */ dictCStringKeyCompare, /* key compare */ NULL, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; int moduleRegisterApi(const char *funcname, void *funcptr) { diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index d8c62daa3d7..5567c4af5ee 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -1299,7 +1299,8 @@ static int fetchClusterSlotsConfiguration(client c) { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* printf("[%d] fetchClusterSlotsConfiguration\n", c->thread_id); */ dict *masters = dictCreate(&dtype, NULL); diff --git a/src/redis-cli.c b/src/redis-cli.c index cd10d22e444..e140435d861 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -2294,7 +2294,8 @@ static dictType clusterManagerDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor */ - dictSdsDestructor /* val destructor */ + dictSdsDestructor, /* val destructor */ + NULL /* allow to expand */ }; static dictType clusterManagerLinkDictType = { @@ -2303,7 +2304,8 @@ static dictType clusterManagerLinkDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - dictListDestructor /* val destructor */ + dictListDestructor, /* val destructor */ + NULL /* allow to expand */ }; typedef int clusterManagerCommandProc(int argc, char **argv); @@ -7360,7 +7362,8 @@ static dictType typeinfoDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor (owned by the value)*/ - type_free /* val destructor */ + type_free, /* val destructor */ + NULL /* allow to expand */ }; static void getKeyTypes(dict *types_dict, redisReply *keys, typeinfo **types) { diff --git a/src/sentinel.c b/src/sentinel.c index 87fc20e9fbe..e05be438455 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -418,7 +418,8 @@ dictType instancesDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor */ - dictInstancesValDestructor /* val destructor */ + dictInstancesValDestructor,/* val destructor */ + NULL /* allow to expand */ }; /* Instance runid (sds) -> votes (long casted to void*) @@ -431,7 +432,8 @@ dictType leaderVotesDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Instance renamed commands table. */ @@ -441,7 +443,8 @@ dictType renamedCommandsDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - dictSdsDestructor /* val destructor */ + dictSdsDestructor, /* val destructor */ + NULL /* allow to expand */ }; /* =========================== Initialization =============================== */ diff --git a/src/server.c b/src/server.c index 16e7d3fba5e..bfd25631056 100644 --- a/src/server.c +++ b/src/server.c @@ -1298,6 +1298,20 @@ uint64_t dictEncObjHash(const void *key) { } } +/* Return 1 if currently we allow dict to expand. Dict may allocate huge + * memory to contain hash buckets when dict expands, that may lead redis + * rejects user's requests or evicts some keys, we can stop dict to expand + * provisionally if used memory will be over maxmemory after dict expands, + * but to guarantee the performance of redis, we still allow dict to expand + * if dict load factor exceeds HASHTABLE_MAX_LOAD_FACTOR. */ +int dictExpandAllowed(size_t moreMem, double usedRatio) { + if (usedRatio <= HASHTABLE_MAX_LOAD_FACTOR) { + return !overMaxmemoryAfterAlloc(moreMem); + } else { + return 1; + } +} + /* Generic hash table type where keys are Redis Objects, Values * dummy pointers. */ dictType objectKeyPointerValueDictType = { @@ -1306,7 +1320,8 @@ dictType objectKeyPointerValueDictType = { NULL, /* val dup */ dictEncObjKeyCompare, /* key compare */ dictObjectDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Like objectKeyPointerValueDictType(), but values can be destroyed, if @@ -1317,7 +1332,8 @@ dictType objectKeyHeapPointerValueDictType = { NULL, /* val dup */ dictEncObjKeyCompare, /* key compare */ dictObjectDestructor, /* key destructor */ - dictVanillaFree /* val destructor */ + dictVanillaFree, /* val destructor */ + NULL /* allow to expand */ }; /* Set dictionary type. Keys are SDS strings, values are not used. */ @@ -1337,7 +1353,8 @@ dictType zsetDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* Note: SDS string shared & freed by skiplist */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Db->dict, keys are sds strings, vals are Redis objects. */ @@ -1347,7 +1364,8 @@ dictType dbDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - dictObjectDestructor /* val destructor */ + dictObjectDestructor, /* val destructor */ + dictExpandAllowed /* allow to expand */ }; /* server.lua_scripts sha (as sds string) -> scripts (as robj) cache. */ @@ -1357,17 +1375,19 @@ dictType shaScriptObjectDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - dictObjectDestructor /* val destructor */ + dictObjectDestructor, /* val destructor */ + NULL /* allow to expand */ }; /* Db->expires */ -dictType keyptrDictType = { +dictType dbExpiresDictType = { dictSdsHash, /* hash function */ NULL, /* key dup */ NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + dictExpandAllowed /* allow to expand */ }; /* Command table. sds string -> command struct pointer. */ @@ -1377,7 +1397,8 @@ dictType commandTableDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Hash type hash table (note that small hashes are represented with ziplists) */ @@ -1387,7 +1408,8 @@ dictType hashDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - dictSdsDestructor /* val destructor */ + dictSdsDestructor, /* val destructor */ + NULL /* allow to expand */ }; /* Keylist hash table type has unencoded redis objects as keys and @@ -1399,7 +1421,8 @@ dictType keylistDictType = { NULL, /* val dup */ dictObjKeyCompare, /* key compare */ dictObjectDestructor, /* key destructor */ - dictListDestructor /* val destructor */ + dictListDestructor, /* val destructor */ + NULL /* allow to expand */ }; /* Cluster nodes hash table, mapping nodes addresses 1.2.3.4:6379 to @@ -1410,7 +1433,8 @@ dictType clusterNodesDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Cluster re-addition blacklist. This maps node IDs to the time @@ -1422,7 +1446,8 @@ dictType clusterNodesBlackListDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Modules system dictionary type. Keys are module name, @@ -1433,7 +1458,8 @@ dictType modulesDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Migrate cache dict type. */ @@ -1443,7 +1469,8 @@ dictType migrateCacheDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* Replication cached script dict (server.repl_scriptcache_dict). @@ -1455,7 +1482,8 @@ dictType replScriptCacheDictType = { NULL, /* val dup */ dictSdsKeyCaseCompare, /* key compare */ dictSdsDestructor, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; int htNeedsResize(dict *dict) { @@ -3004,7 +3032,7 @@ void initServer(void) { /* Create the Redis databases, and initialize other internal state. */ for (j = 0; j < server.dbnum; j++) { server.db[j].dict = dictCreate(&dbDictType,NULL); - server.db[j].expires = dictCreate(&keyptrDictType,NULL); + server.db[j].expires = dictCreate(&dbExpiresDictType,NULL); server.db[j].expires_cursor = 0; server.db[j].blocking_keys = dictCreate(&keylistDictType,NULL); server.db[j].ready_keys = dictCreate(&objectKeyPointerValueDictType,NULL); diff --git a/src/server.h b/src/server.h index a858d583373..772775bead7 100644 --- a/src/server.h +++ b/src/server.h @@ -161,6 +161,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; /* Hash table parameters */ #define HASHTABLE_MIN_FILL 10 /* Minimal hash table fill 10% */ +#define HASHTABLE_MAX_LOAD_FACTOR 1.618 /* Maximum hash table load factor. */ /* Command flags. Please check the command table defined in the server.c file * for more information about the meaning of every flag. */ @@ -1638,7 +1639,7 @@ extern dictType shaScriptObjectDictType; extern double R_Zero, R_PosInf, R_NegInf, R_Nan; extern dictType hashDictType; extern dictType replScriptCacheDictType; -extern dictType keyptrDictType; +extern dictType dbExpiresDictType; extern dictType modulesDictType; /*----------------------------------------------------------------------------- @@ -2070,6 +2071,7 @@ int zslLexValueLteMax(sds value, zlexrangespec *spec); /* Core functions */ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *level); size_t freeMemoryGetNotCountedMemory(); +int overMaxmemoryAfterAlloc(size_t moremem); int processCommand(client *c); void setupSignalHandlers(void); void removeSignalHandlers(void); diff --git a/src/t_zset.c b/src/t_zset.c index 382d3b9ceac..0b3cc061e3d 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -2439,7 +2439,8 @@ dictType setAccumulatorDictType = { NULL, /* val dup */ dictSdsKeyCompare, /* key compare */ NULL, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; /* The zunionInterDiffGenericCommand() function is called in order to implement the diff --git a/tests/unit/maxmemory.tcl b/tests/unit/maxmemory.tcl index 0f64ddc18f6..19983de9b96 100644 --- a/tests/unit/maxmemory.tcl +++ b/tests/unit/maxmemory.tcl @@ -241,3 +241,22 @@ test_slave_buffers {slave buffer are counted correctly} 1000000 10 0 1 # test again with fewer (and bigger) commands without pipeline, but with eviction test_slave_buffers "replica buffer don't induce eviction" 100000 100 1 0 +start_server {tags {"maxmemory"}} { + test {Don't rehash if used memory exceeds maxmemory after rehash} { + r config set maxmemory 0 + r config set maxmemory-policy allkeys-random + + # Next rehash size is 8192, that will eat 64k memory + populate 4096 "" 1 + + set used [s used_memory] + set limit [expr {$used + 10*1024}] + r config set maxmemory $limit + r set k1 v1 + # Next writing command will trigger evicting some keys if last + # command trigger DB dict rehash + r set k2 v2 + # There must be 4098 keys because redis doesn't evict keys. + r dbsize + } {4098} +} diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 96e9ee3da77..d98dc1bd4d3 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -316,8 +316,8 @@ start_server {tags {"other"}} { after 200 # Hash table should rehash since there is no child process, - # size is power of two and over 4098, so it is 16384 + # size is power of two and over 4098, so it is 8192 r set k3 v3 - assert_match "*table size: 16384*" [r debug HTSTATS 9] + assert_match "*table size: 8192*" [r debug HTSTATS 9] } } diff --git a/utils/hashtable/rehashing.c b/utils/hashtable/rehashing.c index b57a9043a30..3c0acb84c66 100644 --- a/utils/hashtable/rehashing.c +++ b/utils/hashtable/rehashing.c @@ -30,7 +30,8 @@ dictType dictTypeTest = { NULL, /* val dup */ dictKeyCompare, /* key compare */ NULL, /* key destructor */ - NULL /* val destructor */ + NULL, /* val destructor */ + NULL /* allow to expand */ }; void showBuckets(dictht ht) {