Prevent hash table resize while there are active child processes in order to play well with copy on write

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)

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;

redis.c

@@ -1294,6 +1294,19 @@ cleanup:
     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

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},