Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Prevent hash table resize while there are active child processes in o…

…rder to play well with copy on write
  • Loading branch information...
commit 884d4b39d475bc15d89ba4415772463f337c7f19 1 parent 5727b9a
@antirez authored
Showing with 57 additions and 2 deletions.
  1. +16 −1 dict.c
  2. +2 −0  dict.h
  3. +21 −1 redis.c
  4. +18 −0 staticsymbols.h
View
17 dict.c
@@ -45,6 +45,12 @@
#include "dict.h"
#include "zmalloc.h"
+/* Using dictEnableResize() / dictDisableResize() we make possible to
+ * enable/disable resizing 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. */
+static int dict_can_resize = 1;
+
/* ---------------------------- Utility funcitons --------------------------- */
static void _dictPanic(const char *fmt, ...)
@@ -147,6 +153,7 @@ int dictResize(dict *ht)
{
int minimal = ht->used;
+ if (!dict_can_resize) return DICT_ERR;
if (minimal < DICT_HT_INITIAL_SIZE)
minimal = DICT_HT_INITIAL_SIZE;
return dictExpand(ht, minimal);
@@ -417,7 +424,7 @@ static int _dictExpandIfNeeded(dict *ht)
* if the table is "full" dobule its size. */
if (ht->size == 0)
return dictExpand(ht, DICT_HT_INITIAL_SIZE);
- if (ht->used == ht->size)
+ if (ht->used >= ht->size && dict_can_resize)
return dictExpand(ht, ht->size*2);
return DICT_OK;
}
@@ -507,6 +514,14 @@ void dictPrintStats(dict *ht) {
}
}
+void dictEnableResize(void) {
+ dict_can_resize = 1;
+}
+
+void dictDisableResize(void) {
+ dict_can_resize = 0;
+}
+
/* ----------------------- StringCopy Hash Table Type ------------------------*/
static unsigned int _dictStringCopyHTHashFunction(const void *key)
View
2  dict.h
@@ -127,6 +127,8 @@ dictEntry *dictGetRandomKey(dict *ht);
void dictPrintStats(dict *ht);
unsigned int dictGenHashFunction(const unsigned char *buf, int len);
void dictEmpty(dict *ht);
+void dictEnableResize(void);
+void dictDisableResize(void);
/* Hash table types */
extern dictType dictTypeHeapStringCopyKey;
View
22 redis.c
@@ -1294,6 +1294,19 @@ void backgroundRewriteDoneHandler(int statloc) {
server.bgrewritechildpid = -1;
}
+/* This function is called once a background process of some kind terminates,
+ * 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 o not
+ * running childs. */
+static void updateDictResizePolicy(void) {
+ if (server.bgsavechildpid == -1 && server.bgrewritechildpid == -1)
+ dictEnableResize();
+ else
+ dictDisableResize();
+}
+
static int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) {
int j, loops = server.cronloops++;
REDIS_NOTUSED(eventLoop);
@@ -1325,7 +1338,11 @@ static int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientD
* if we resize the HT while there is the saving child at work actually
* a lot of memory movements in the parent will cause a lot of pages
* copied. */
- if (server.bgsavechildpid == -1 && !(loops % 10)) tryResizeHashTables();
+ if (server.bgsavechildpid == -1 && server.bgrewritechildpid == -1 &&
+ !(loops % 10))
+ {
+ tryResizeHashTables();
+ }
/* Show information about connected clients */
if (!(loops % 50)) {
@@ -1351,6 +1368,7 @@ static int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientD
} else {
backgroundRewriteDoneHandler(statloc);
}
+ updateDictResizePolicy();
}
} else {
/* If there is not a background saving in progress check if
@@ -3497,6 +3515,7 @@ static int rdbSaveBackground(char *filename) {
}
redisLog(REDIS_NOTICE,"Background saving started by pid %d",childpid);
server.bgsavechildpid = childpid;
+ updateDictResizePolicy();
return REDIS_OK;
}
return REDIS_OK; /* unreached */
@@ -8116,6 +8135,7 @@ static int rewriteAppendOnlyFileBackground(void) {
redisLog(REDIS_NOTICE,
"Background append only file rewriting started by pid %d",childpid);
server.bgrewritechildpid = 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.bgrewritebuf will start
View
18 staticsymbols.h
@@ -8,6 +8,7 @@ static struct redisFunctionSym symsTable[] = {
{"addReplyBulkLen",(unsigned long)addReplyBulkLen},
{"addReplyDouble",(unsigned long)addReplyDouble},
{"addReplyLong",(unsigned long)addReplyLong},
+{"addReplyLongLong",(unsigned long)addReplyLongLong},
{"addReplySds",(unsigned long)addReplySds},
{"addReplyUlong",(unsigned long)addReplyUlong},
{"aofRemoveTempFile",(unsigned long)aofRemoveTempFile},
@@ -80,6 +81,7 @@ static struct redisFunctionSym symsTable[] = {
{"freeIOJob",(unsigned long)freeIOJob},
{"freeListObject",(unsigned long)freeListObject},
{"freeMemoryIfNeeded",(unsigned long)freeMemoryIfNeeded},
+{"freePubsubPattern",(unsigned long)freePubsubPattern},
{"freeSetObject",(unsigned long)freeSetObject},
{"freeStringObject",(unsigned long)freeStringObject},
{"freeZsetObject",(unsigned long)freeZsetObject},
@@ -103,6 +105,7 @@ static struct redisFunctionSym symsTable[] = {
{"hexistsCommand",(unsigned long)hexistsCommand},
{"hgetCommand",(unsigned long)hgetCommand},
{"hgetallCommand",(unsigned long)hgetallCommand},
+{"hincrbyCommand",(unsigned long)hincrbyCommand},
{"hkeysCommand",(unsigned long)hkeysCommand},
{"hlenCommand",(unsigned long)hlenCommand},
{"hsetCommand",(unsigned long)hsetCommand},
@@ -120,6 +123,8 @@ static struct redisFunctionSym symsTable[] = {
{"keysCommand",(unsigned long)keysCommand},
{"lastsaveCommand",(unsigned long)lastsaveCommand},
{"lindexCommand",(unsigned long)lindexCommand},
+{"listMatchObjects",(unsigned long)listMatchObjects},
+{"listMatchPubsubPattern",(unsigned long)listMatchPubsubPattern},
{"llenCommand",(unsigned long)llenCommand},
{"loadServerConfig",(unsigned long)loadServerConfig},
{"lockThreadedIO",(unsigned long)lockThreadedIO},
@@ -147,6 +152,16 @@ static struct redisFunctionSym symsTable[] = {
{"popGenericCommand",(unsigned long)popGenericCommand},
{"processCommand",(unsigned long)processCommand},
{"processInputBuffer",(unsigned long)processInputBuffer},
+{"psubscribeCommand",(unsigned long)psubscribeCommand},
+{"publishCommand",(unsigned long)publishCommand},
+{"pubsubPublishMessage",(unsigned long)pubsubPublishMessage},
+{"pubsubSubscribeChannel",(unsigned long)pubsubSubscribeChannel},
+{"pubsubSubscribePattern",(unsigned long)pubsubSubscribePattern},
+{"pubsubUnsubscribeAllChannels",(unsigned long)pubsubUnsubscribeAllChannels},
+{"pubsubUnsubscribeAllPatterns",(unsigned long)pubsubUnsubscribeAllPatterns},
+{"pubsubUnsubscribeChannel",(unsigned long)pubsubUnsubscribeChannel},
+{"pubsubUnsubscribePattern",(unsigned long)pubsubUnsubscribePattern},
+{"punsubscribeCommand",(unsigned long)punsubscribeCommand},
{"pushGenericCommand",(unsigned long)pushGenericCommand},
{"qsortCompareSetsByCardinality",(unsigned long)qsortCompareSetsByCardinality},
{"qsortCompareZsetopsrcByCardinality",(unsigned long)qsortCompareZsetopsrcByCardinality},
@@ -224,6 +239,7 @@ static struct redisFunctionSym symsTable[] = {
{"stringObjectLen",(unsigned long)stringObjectLen},
{"stringmatch",(unsigned long)stringmatch},
{"stringmatchlen",(unsigned long)stringmatchlen},
+{"subscribeCommand",(unsigned long)subscribeCommand},
{"substrCommand",(unsigned long)substrCommand},
{"sunionCommand",(unsigned long)sunionCommand},
{"sunionDiffGenericCommand",(unsigned long)sunionDiffGenericCommand},
@@ -241,6 +257,8 @@ static struct redisFunctionSym symsTable[] = {
{"typeCommand",(unsigned long)typeCommand},
{"unblockClientWaitingData",(unsigned long)unblockClientWaitingData},
{"unlockThreadedIO",(unsigned long)unlockThreadedIO},
+{"unsubscribeCommand",(unsigned long)unsubscribeCommand},
+{"updateDictResizePolicy",(unsigned long)updateDictResizePolicy},
{"updateSlavesWaitingBgsave",(unsigned long)updateSlavesWaitingBgsave},
{"usage",(unsigned long)usage},
{"version",(unsigned long)version},
Please sign in to comment.
Something went wrong with that request. Please try again.