From 5e0fe4c77c556255b738cecb330dc1bae060a3d0 Mon Sep 17 00:00:00 2001
From: Guy Benoish <guy.benoish@redislabs.com>
Date: Wed, 18 Dec 2019 12:27:03 +0530
Subject: [PATCH 01/33] streamReplyWithRangeFromConsumerPEL: Redundant
 streamDecodeID

---
 src/t_stream.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/t_stream.c b/src/t_stream.c
index a499f7381..c35c9d1be 100644
--- a/src/t_stream.c
+++ b/src/t_stream.c
@@ -1043,9 +1043,7 @@ size_t streamReplyWithRangeFromConsumerPEL(client *c, stream *s, streamID *start
              * by the user by other means. In that case we signal it emitting
              * the ID but then a NULL entry for the fields. */
             addReplyArrayLen(c,2);
-            streamID id;
-            streamDecodeID(ri.key,&id);
-            addReplyStreamID(c,&id);
+            addReplyStreamID(c,&thisid);
             addReplyNullArray(c);
         } else {
             streamNACK *nack = ri.data;

From 337dcde9fafa6c4bab78f1ec74f8e8abf253c86c Mon Sep 17 00:00:00 2001
From: "bodong.ybd" <bodong.ybd@alibaba-inc.com>
Date: Sat, 21 Dec 2019 21:27:38 +0800
Subject: [PATCH 02/33] Fix spop return nil #4709

---
 src/t_set.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/t_set.c b/src/t_set.c
index abbf82fde..60cf22d8c 100644
--- a/src/t_set.c
+++ b/src/t_set.c
@@ -415,7 +415,7 @@ void spopWithCountCommand(client *c) {
 
     /* Make sure a key with the name inputted exists, and that it's type is
      * indeed a set. Otherwise, return nil */
-    if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.null[c->resp]))
+    if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.emptyset[c->resp]))
         == NULL || checkType(c,set,OBJ_SET)) return;
 
     /* If count is zero, serve an empty set ASAP to avoid special

From 3848849013e6ec7efa244897f5fdebb843fcfaa4 Mon Sep 17 00:00:00 2001
From: wangyuan21 <wangyuan21@baidu.com>
Date: Tue, 31 Dec 2019 19:53:00 +0800
Subject: [PATCH 03/33] free time event when delete eventloop

---
 src/ae.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/ae.c b/src/ae.c
index 2c1dae512..248096e1f 100644
--- a/src/ae.c
+++ b/src/ae.c
@@ -135,6 +135,13 @@ void aeDeleteEventLoop(aeEventLoop *eventLoop) {
     aeApiFree(eventLoop);
     zfree(eventLoop->events);
     zfree(eventLoop->fired);
+    /* Free time event. */
+    aeTimeEvent *next_te, *te = eventLoop->timeEventHead;
+    while (te) {
+        next_te = te->next;
+        zfree(te);
+        te = next_te;
+    }
     zfree(eventLoop);
 }
 

From 1d4ea00d12885108d936b76cd31097dc4894f5ca Mon Sep 17 00:00:00 2001
From: Ponnuvel Palaniyappan <pponnuvel@gmail.com>
Date: Tue, 14 Jan 2020 08:10:39 +0000
Subject: [PATCH 04/33] Fix a potential overflow with strncpy

---
 src/config.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/config.c b/src/config.c
index 0526de84d..e4b1bf869 100644
--- a/src/config.c
+++ b/src/config.c
@@ -108,12 +108,12 @@ clientBufferLimitsConfig clientBufferLimitsDefaults[CLIENT_TYPE_OBUF_COUNT] = {
 /* Generic config infrastructure function pointers
  * int is_valid_fn(val, err)
  *     Return 1 when val is valid, and 0 when invalid.
- *     Optionslly set err to a static error string.
+ *     Optionally set err to a static error string.
  * int update_fn(val, prev, err)
  *     This function is called only for CONFIG SET command (not at config file parsing)
  *     It is called after the actual config is applied,
  *     Return 1 for success, and 0 for failure.
- *     Optionslly set err to a static error string.
+ *     Optionally set err to a static error string.
  *     On failure the config change will be reverted.
  */
 
@@ -720,7 +720,7 @@ void configSetCommand(client *c) {
      * config_set_memory_field(name,var) */
     } config_set_memory_field(
       "client-query-buffer-limit",server.client_max_querybuf_len) {
-    /* Everyhing else is an error... */
+    /* Everything else is an error... */
     } config_set_else {
         addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s",
             (char*)c->argv[2]->ptr);
@@ -1699,9 +1699,9 @@ static int configEnumLoad(typeData data, sds *argv, int argc, char **err) {
 
         enumerr[sdslen(enumerr) - 2] = '\0';
 
-        /* Make sure we don't overrun the fixed buffer */
-        enumerr[LOADBUF_SIZE - 1] = '\0';
         strncpy(loadbuf, enumerr, LOADBUF_SIZE);
+        /* strncpy does not if null terminate if source string length is >= destination buffer. */
+        loadbuf[LOADBUF_SIZE - 1] = '\0';
 
         sdsfree(enumerr);
         *err = loadbuf;

From 3f6d00d387b85b0eaa459427c2891ca979dc8363 Mon Sep 17 00:00:00 2001
From: hwware <wen.hui.ware@gmail.com>
Date: Thu, 16 Jan 2020 17:33:23 -0500
Subject: [PATCH 05/33] fix potentical memory leaks

---
 src/debug.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/debug.c b/src/debug.c
index a2d37337d..b441fa8f8 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -683,9 +683,12 @@ NULL
         sds stats = sdsempty();
         char buf[4096];
 
-        if (getLongFromObjectOrReply(c, c->argv[2], &dbid, NULL) != C_OK)
+        if (getLongFromObjectOrReply(c, c->argv[2], &dbid, NULL) != C_OK){
+            sdsfree(stats);
             return;
+        }
         if (dbid < 0 || dbid >= server.dbnum) {
+            sdsfree(stats);
             addReplyError(c,"Out of range database");
             return;
         }

From 67ee87522ab5b3c3bae8ee4c84808664b14de2cc Mon Sep 17 00:00:00 2001
From: hwware <wen.hui.ware@gmail.com>
Date: Thu, 16 Jan 2020 17:35:26 -0500
Subject: [PATCH 06/33] format fix

---
 src/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/debug.c b/src/debug.c
index b441fa8f8..5ffbab7b4 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -683,7 +683,7 @@ NULL
         sds stats = sdsempty();
         char buf[4096];
 
-        if (getLongFromObjectOrReply(c, c->argv[2], &dbid, NULL) != C_OK){
+        if (getLongFromObjectOrReply(c, c->argv[2], &dbid, NULL) != C_OK) {
             sdsfree(stats);
             return;
         }

From dd05b7f67f8191aacbbe73a1b415658233715926 Mon Sep 17 00:00:00 2001
From: srzhao <srzhao@sysnew.com>
Date: Fri, 17 Jan 2020 11:46:19 +0800
Subject: [PATCH 07/33] fix impl of aof-child whitelist SIGUSR1 feature.

---
 src/aof.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/aof.c b/src/aof.c
index 9eeb3f1e2..ce1a440f8 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -1797,14 +1797,15 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) {
         serverLog(LL_VERBOSE,
             "Background AOF rewrite signal handler took %lldus", ustime()-now);
     } else if (!bysignal && exitcode != 0) {
+        server.aof_lastbgrewrite_status = C_ERR;
+
+        serverLog(LL_WARNING,
+            "Background AOF rewrite terminated with error");
+    } else {
         /* SIGUSR1 is whitelisted, so we have a way to kill a child without
          * tirggering an error condition. */
         if (bysignal != SIGUSR1)
             server.aof_lastbgrewrite_status = C_ERR;
-        serverLog(LL_WARNING,
-            "Background AOF rewrite terminated with error");
-    } else {
-        server.aof_lastbgrewrite_status = C_ERR;
 
         serverLog(LL_WARNING,
             "Background AOF rewrite terminated by signal %d", bysignal);

From bd60c11bd8ca652f6cb6ecc57d0d10bc8aaa3481 Mon Sep 17 00:00:00 2001
From: qetu3790 <qetu3790@163.com>
Date: Thu, 23 Jan 2020 17:18:07 +0800
Subject: [PATCH 08/33] Fix not used constant in lru_test_mode.

LRU_CYCLE_PERIOD is defined,but not used.
---
 src/redis-cli.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/redis-cli.c b/src/redis-cli.c
index 065c389c6..36ceb16e8 100644
--- a/src/redis-cli.c
+++ b/src/redis-cli.c
@@ -7735,7 +7735,7 @@ static void LRUTestMode(void) {
          * to fill the target instance easily. */
         start_cycle = mstime();
         long long hits = 0, misses = 0;
-        while(mstime() - start_cycle < 1000) {
+        while(mstime() - start_cycle < LRU_CYCLE_PERIOD) {
             /* Write cycle. */
             for (j = 0; j < LRU_CYCLE_PIPELINE_SIZE; j++) {
                 char val[6];

From 1c7a2269d6067de23b0f5d765fdf3a44a3edd4a6 Mon Sep 17 00:00:00 2001
From: Guy Benoish <guy.benoish@redislabs.com>
Date: Tue, 4 Feb 2020 19:28:09 +0530
Subject: [PATCH 09/33] Add RM_CreateStringFromDouble

---
 src/module.c      | 12 ++++++++++++
 src/redismodule.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/src/module.c b/src/module.c
index d2b267be2..91437c6a6 100644
--- a/src/module.c
+++ b/src/module.c
@@ -1037,6 +1037,17 @@ RedisModuleString *RM_CreateStringFromLongLong(RedisModuleCtx *ctx, long long ll
     return RM_CreateString(ctx,buf,len);
 }
 
+/* Like RedisModule_CreatString(), but creates a string starting from a double
+ * integer instead of taking a buffer and its length.
+ *
+ * The returned string must be released with RedisModule_FreeString() or by
+ * enabling automatic memory management. */
+RedisModuleString *RM_CreateStringFromDouble(RedisModuleCtx *ctx, double d) {
+    char buf[128];
+    size_t len = d2string(buf,sizeof(buf),d);
+    return RM_CreateString(ctx,buf,len);
+}
+
 /* Like RedisModule_CreatString(), but creates a string starting from a long
  * double.
  *
@@ -7656,6 +7667,7 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(CreateStringFromCallReply);
     REGISTER_API(CreateString);
     REGISTER_API(CreateStringFromLongLong);
+    REGISTER_API(CreateStringFromDouble);
     REGISTER_API(CreateStringFromLongDouble);
     REGISTER_API(CreateStringFromString);
     REGISTER_API(CreateStringPrintf);
diff --git a/src/redismodule.h b/src/redismodule.h
index 637078f2b..95dd80e8e 100644
--- a/src/redismodule.h
+++ b/src/redismodule.h
@@ -467,6 +467,7 @@ size_t REDISMODULE_API_FUNC(RedisModule_CallReplyLength)(RedisModuleCallReply *r
 RedisModuleCallReply *REDISMODULE_API_FUNC(RedisModule_CallReplyArrayElement)(RedisModuleCallReply *reply, size_t idx);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateString)(RedisModuleCtx *ctx, const char *ptr, size_t len);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromLongLong)(RedisModuleCtx *ctx, long long ll);
+RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromDouble)(RedisModuleCtx *ctx, double d);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromLongDouble)(RedisModuleCtx *ctx, long double ld, int humanfriendly);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromString)(RedisModuleCtx *ctx, const RedisModuleString *str);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringPrintf)(RedisModuleCtx *ctx, const char *fmt, ...);
@@ -726,6 +727,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(CreateStringFromCallReply);
     REDISMODULE_GET_API(CreateString);
     REDISMODULE_GET_API(CreateStringFromLongLong);
+    REDISMODULE_GET_API(CreateStringFromDouble);
     REDISMODULE_GET_API(CreateStringFromLongDouble);
     REDISMODULE_GET_API(CreateStringFromString);
     REDISMODULE_GET_API(CreateStringPrintf);

From 774d8cd721055b768dbffbf5c6b2fa9d6310126e Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Wed, 5 Feb 2020 18:06:33 +0200
Subject: [PATCH 10/33] Optimize temporary memory allocations for
 getKeysFromCommand mechanism

now that we may use it more often (ACL), these excessive calls to malloc
and free can become an overhead.
---
 src/db.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/db.c b/src/db.c
index 88342ac4d..caabeaabb 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1292,6 +1292,8 @@ int expireIfNeeded(redisDb *db, robj *key) {
 /* -----------------------------------------------------------------------------
  * API to get key arguments from commands
  * ---------------------------------------------------------------------------*/
+#define MAX_KEYS_BUFFER 65536
+static int getKeysTempBuffer[MAX_KEYS_BUFFER];
 
 /* The base case is to use the keys position as given in the command table
  * (firstkey, lastkey, step). */
@@ -1306,7 +1308,12 @@ int *getKeysUsingCommandTable(struct redisCommand *cmd,robj **argv, int argc, in
 
     last = cmd->lastkey;
     if (last < 0) last = argc+last;
-    keys = zmalloc(sizeof(int)*((last - cmd->firstkey)+1));
+
+    int count = ((last - cmd->firstkey)+1);
+    keys = getKeysTempBuffer;
+    if (count > MAX_KEYS_BUFFER)
+        keys = zmalloc(sizeof(int)*count);
+
     for (j = cmd->firstkey; j <= last; j += cmd->keystep) {
         if (j >= argc) {
             /* Modules commands, and standard commands with a not fixed number
@@ -1316,7 +1323,7 @@ int *getKeysUsingCommandTable(struct redisCommand *cmd,robj **argv, int argc, in
              * return no keys and expect the command implementation to report
              * an arity or syntax error. */
             if (cmd->flags & CMD_MODULE || cmd->arity < 0) {
-                zfree(keys);
+                getKeysFreeResult(keys);
                 *numkeys = 0;
                 return NULL;
             } else {
@@ -1352,7 +1359,8 @@ int *getKeysFromCommand(struct redisCommand *cmd, robj **argv, int argc, int *nu
 
 /* Free the result of getKeysFromCommand. */
 void getKeysFreeResult(int *result) {
-    zfree(result);
+    if (result != getKeysTempBuffer)
+        zfree(result);
 }
 
 /* Helper function to extract keys from following commands:
@@ -1373,7 +1381,9 @@ int *zunionInterGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *nu
     /* Keys in z{union,inter}store come from two places:
      * argv[1] = storage key,
      * argv[3...n] = keys to intersect */
-    keys = zmalloc(sizeof(int)*(num+1));
+    keys = getKeysTempBuffer;
+    if (num+1>MAX_KEYS_BUFFER)
+        keys = zmalloc(sizeof(int)*(num+1));
 
     /* Add all key positions for argv[3...n] to keys[] */
     for (i = 0; i < num; i++) keys[i] = 3+i;
@@ -1399,7 +1409,10 @@ int *evalGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys)
         return NULL;
     }
 
-    keys = zmalloc(sizeof(int)*num);
+    keys = getKeysTempBuffer;
+    if (num>MAX_KEYS_BUFFER)
+        keys = zmalloc(sizeof(int)*num);
+
     *numkeys = num;
 
     /* Add all key positions for argv[3...n] to keys[] */
@@ -1420,7 +1433,7 @@ int *sortGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys)
     UNUSED(cmd);
 
     num = 0;
-    keys = zmalloc(sizeof(int)*2); /* Alloc 2 places for the worst case. */
+    keys = getKeysTempBuffer; /* Alloc 2 places for the worst case. */
 
     keys[num++] = 1; /* <sort-key> is always present. */
 
@@ -1478,7 +1491,10 @@ int *migrateGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkey
         }
     }
 
-    keys = zmalloc(sizeof(int)*num);
+    keys = getKeysTempBuffer;
+    if (num>MAX_KEYS_BUFFER)
+        keys = zmalloc(sizeof(int)*num);
+
     for (i = 0; i < num; i++) keys[i] = first+i;
     *numkeys = num;
     return keys;
@@ -1511,7 +1527,9 @@ int *georadiusGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numk
      * argv[1] = key,
      * argv[5...n] = stored key if present
      */
-    keys = zmalloc(sizeof(int) * num);
+    keys = getKeysTempBuffer;
+    if (num>MAX_KEYS_BUFFER)
+        keys = zmalloc(sizeof(int) * num);
 
     /* Add all key positions to keys[] */
     keys[0] = 1;
@@ -1529,7 +1547,7 @@ int *memoryGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys
     UNUSED(cmd);
 
     if (argc >= 3 && !strcasecmp(argv[1]->ptr,"usage")) {
-        keys = zmalloc(sizeof(int) * 1);
+        keys = getKeysTempBuffer;
         keys[0] = 2;
         *numkeys = 1;
         return keys;
@@ -1576,7 +1594,10 @@ int *xreadGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys)
     num /= 2; /* We have half the keys as there are arguments because
                  there are also the IDs, one per key. */
 
-    keys = zmalloc(sizeof(int) * num);
+    keys = getKeysTempBuffer;
+    if (num>MAX_KEYS_BUFFER)
+        keys = zmalloc(sizeof(int) * num);
+
     for (i = streams_pos+1; i < argc-num; i++) keys[i-streams_pos-1] = i;
     *numkeys = num;
     return keys;

From 28ef18a8946815e0d83a1c0a9b6baf9d27022461 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Wed, 5 Feb 2020 18:24:14 +0200
Subject: [PATCH 11/33] RM_Scan disable dict rehashing

The callback approach we took is very efficient, the module can do any
filtering of keys without building any list and cloning strings, it can
also read data from the key's value. but if the user tries to re-open
the key, or any other key, this can cause dict re-hashing (dictFind does
that), and that's very bad to do from inside dictScan.

this commit protects the dict from doing any rehashing during scan, but
also warns the user not to attempt any writes or command calls from
within the callback, for fear of unexpected side effects and crashes.
---
 src/dict.c   |  7 +++++++
 src/module.c | 20 ++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/dict.c b/src/dict.c
index 106467ef7..93e6c39a7 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -871,6 +871,10 @@ unsigned long dictScan(dict *d,
 
     if (dictSize(d) == 0) return 0;
 
+    /* Having a safe iterator means no rehashing can happen, see _dictRehashStep.
+     * This is needed in case the scan callback tries to do dictFind or alike. */
+    d->iterators++;
+
     if (!dictIsRehashing(d)) {
         t0 = &(d->ht[0]);
         m0 = t0->sizemask;
@@ -937,6 +941,9 @@ unsigned long dictScan(dict *d,
         } while (v & (m0 ^ m1));
     }
 
+    /* undo the ++ at the top */
+    d->iterators--;
+
     return v;
 }
 
diff --git a/src/module.c b/src/module.c
index 914c50df3..e9bceac47 100644
--- a/src/module.c
+++ b/src/module.c
@@ -6515,9 +6515,13 @@ void RM_ScanCursorDestroy(RedisModuleScanCursor *cursor) {
  *      }
  *      RedisModule_ScanCursorDestroy(c);
  *
- *  The function will return 1 if there are more elements to scan and 0 otherwise,
- *  possibly setting errno if the call failed.
- *  It is also possible to restart and existing cursor using RM_CursorRestart. */
+ * The function will return 1 if there are more elements to scan and 0 otherwise,
+ * possibly setting errno if the call failed.
+ * It is also possible to restart and existing cursor using RM_CursorRestart.
+ *
+ * NOTE: You must avoid doing any database changes from within the callback, you should avoid any
+ * RedisModule_OpenKey or RedisModule_Call, if you need to do these, you need to keep the key name
+ * and do any work you need to do after the call to Scan returns. */
 int RM_Scan(RedisModuleCtx *ctx, RedisModuleScanCursor *cursor, RedisModuleScanCB fn, void *privdata) {
     if (cursor->done) {
         errno = ENOENT;
@@ -6595,9 +6599,13 @@ static void moduleScanKeyCallback(void *privdata, const dictEntry *de) {
  *      RedisModule_CloseKey(key);
  *      RedisModule_ScanCursorDestroy(c);
  *
- *  The function will return 1 if there are more elements to scan and 0 otherwise,
- *  possibly setting errno if the call failed.
- *  It is also possible to restart and existing cursor using RM_CursorRestart. */
+ * The function will return 1 if there are more elements to scan and 0 otherwise,
+ * possibly setting errno if the call failed.
+ * It is also possible to restart and existing cursor using RM_CursorRestart.
+ *
+ * NOTE: You must avoid doing any database changes from within the callback, you should avoid any
+ * RedisModule_OpenKey or RedisModule_Call, if you need to do these, you need to keep the field name
+ * and do any work you need to do after the call to Scan returns. */
 int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleScanKeyCB fn, void *privdata) {
     if (key == NULL || key->value == NULL) {
         errno = EINVAL;

From c8c3281f06acb274e56440e2dec4ec8e2bcd6aa3 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Thu, 6 Feb 2020 14:53:33 +0200
Subject: [PATCH 12/33] add no_auth to COMMAND INFO

---
 src/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/server.c b/src/server.c
index f3b7c37c5..01c048c90 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3764,6 +3764,7 @@ void addReplyCommand(client *c, struct redisCommand *cmd) {
         flagcount += addReplyCommandFlag(c,cmd,CMD_SKIP_SLOWLOG, "skip_slowlog");
         flagcount += addReplyCommandFlag(c,cmd,CMD_ASKING, "asking");
         flagcount += addReplyCommandFlag(c,cmd,CMD_FAST, "fast");
+        flagcount += addReplyCommandFlag(c,cmd,CMD_NO_AUTH, "no_auth");
         if ((cmd->getkeys_proc && !(cmd->flags & CMD_MODULE)) ||
             cmd->flags & CMD_MODULE_GETKEYS)
         {

From 67dbc5f365ccaff65d996486a86c77e5bff390a8 Mon Sep 17 00:00:00 2001
From: "meir@redislabs.com" <meir@redislabs.com>
Date: Mon, 10 Feb 2020 12:10:32 +0200
Subject: [PATCH 13/33] Changed log level for module fork api from 'notice' to
 'verbos'.

---
 src/module.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/module.c b/src/module.c
index ed79ee226..d15aa6c93 100644
--- a/src/module.c
+++ b/src/module.c
@@ -6706,7 +6706,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;
-        serverLog(LL_NOTICE, "Module fork started pid: %d ", childpid);
+        serverLog(LL_VERBOSE, "Module fork started pid: %d ", childpid);
     }
     return childpid;
 }
@@ -6729,7 +6729,7 @@ int TerminateModuleForkChild(int child_pid, int wait) {
         server.module_child_pid != child_pid) return C_ERR;
 
     int statloc;
-    serverLog(LL_NOTICE,"Killing running module fork child: %ld",
+    serverLog(LL_VERBOSE,"Killing running module fork child: %ld",
         (long) server.module_child_pid);
     if (kill(server.module_child_pid,SIGUSR1) != -1 && wait) {
         while(wait4(server.module_child_pid,&statloc,0,NULL) !=

From 6fff2cf9b65effe2846d8383afa32dc0b04dd149 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Sun, 16 Feb 2020 15:43:19 +0200
Subject: [PATCH 14/33] module api docs for aux_save and aux_load

---
 src/module.c | 6 ++++++
 src/rdb.c    | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/module.c b/src/module.c
index aae15d74b..69101161d 100644
--- a/src/module.c
+++ b/src/module.c
@@ -3529,6 +3529,8 @@ void moduleTypeNameByID(char *name, uint64_t moduleid) {
  *          // Optional fields
  *          .digest = myType_DigestCallBack,
  *          .mem_usage = myType_MemUsageCallBack,
+ *          .aux_load = myType_AuxRDBLoadCallBack,
+ *          .aux_save = myType_AuxRDBSaveCallBack,
  *      }
  *
  * * **rdb_load**: A callback function pointer that loads data from RDB files.
@@ -3536,6 +3538,10 @@ void moduleTypeNameByID(char *name, uint64_t moduleid) {
  * * **aof_rewrite**: A callback function pointer that rewrites data as commands.
  * * **digest**: A callback function pointer that is used for `DEBUG DIGEST`.
  * * **free**: A callback function pointer that can free a type value.
+ * * **aux_save**: A callback function pointer that saves out of keyspace data to RDB files.
+ *   'when' argument is either REDISMODULE_AUX_BEFORE_RDB or REDISMODULE_AUX_AFTER_RDB.
+ * * **aux_load**: A callback function pointer that loads out of keyspace data from RDB files.
+ *   Similar to aux_save, returns REDISMODULE_OK on success, and ERR otherwise.
  *
  * The **digest* and **mem_usage** methods should currently be omitted since
  * they are not yet implemented inside the Redis modules core.
diff --git a/src/rdb.c b/src/rdb.c
index 61265433d..cbcea96c6 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -2195,7 +2195,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) {
                 io.ver = 2;
                 /* Call the rdb_load method of the module providing the 10 bit
                  * encoding version in the lower 10 bits of the module ID. */
-                if (mt->aux_load(&io,moduleid&1023, when) || io.error) {
+                if (mt->aux_load(&io,moduleid&1023, when) != REDISMODULE_OK || io.error) {
                     moduleTypeNameByID(name,moduleid);
                     serverLog(LL_WARNING,"The RDB file contains module AUX data for the module type '%s', that the responsible module is not able to load. Check for modules log above for additional clues.", name);
                     exit(1);

From 743cfc0ad604c48bfa8c408ca36c6a5b90996aad Mon Sep 17 00:00:00 2001
From: hwware <wen.hui.ware@gmail.com>
Date: Sat, 22 Feb 2020 11:38:51 -0500
Subject: [PATCH 15/33] add missing file marco

---
 src/sdsalloc.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/sdsalloc.h b/src/sdsalloc.h
index 531d41929..c04ff2a0a 100644
--- a/src/sdsalloc.h
+++ b/src/sdsalloc.h
@@ -36,7 +36,12 @@
  * the include of your alternate allocator if needed (not needed in order
  * to use the default libc allocator). */
 
+#ifndef __SDS_ALLOC_H__
+#define __SDS_ALLOC_H__
+
 #include "zmalloc.h"
 #define s_malloc zmalloc
 #define s_realloc zrealloc
 #define s_free zfree
+
+#endif

From 28bde891723b2bd815663d3228b28376b1e0dcb9 Mon Sep 17 00:00:00 2001
From: Ariel <ashtul@gmail.com>
Date: Sat, 22 Feb 2020 23:49:23 +0200
Subject: [PATCH 16/33] fix ThreadSafeContext lock/unlock function names

---
 src/module.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/module.c b/src/module.c
index 705ffec2c..3856fe50e 100644
--- a/src/module.c
+++ b/src/module.c
@@ -4698,9 +4698,9 @@ int RM_BlockedClientDisconnected(RedisModuleCtx *ctx) {
  *
  * To call non-reply APIs, the thread safe context must be prepared with:
  *
- *     RedisModule_ThreadSafeCallStart(ctx);
+ *     RedisModule_ThreadSafeContextLock(ctx);
  *     ... make your call here ...
- *     RedisModule_ThreadSafeCallStop(ctx);
+ *     RedisModule_ThreadSafeContextUnlock(ctx);
  *
  * This is not needed when using `RedisModule_Reply*` functions, assuming
  * that a blocked client was used when the context was created, otherwise

From 62adabd0e0935cbb1cdedb3cde012c50ad1356a1 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Sun, 23 Feb 2020 12:46:14 +0200
Subject: [PATCH 17/33] Fix latency sensitivity of new defrag test

I saw that the new defag test for list was failing in CI recently, so i
reduce it's threshold from 12 to 60.

besides that, i add / improve the latency test for that other two defrag
tests (add a sensitive latency and digest / save checks)

and fix bad usage of debug populate (can't overrides existing keys).
this was the original intention, which creates higher fragmentation.
---
 tests/unit/memefficiency.tcl | 40 ++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/tests/unit/memefficiency.tcl b/tests/unit/memefficiency.tcl
index ec80c7384..468825a47 100644
--- a/tests/unit/memefficiency.tcl
+++ b/tests/unit/memefficiency.tcl
@@ -39,6 +39,8 @@ start_server {tags {"memefficiency"}} {
 start_server {tags {"defrag"}} {
     if {[string match {*jemalloc*} [s mem_allocator]]} {
         test "Active defrag" {
+            r config set save "" ;# prevent bgsave from interfereing with save below
+            r config set hz 100
             r config set activedefrag no
             r config set active-defrag-threshold-lower 5
             r config set active-defrag-cycle-min 65
@@ -46,8 +48,8 @@ start_server {tags {"defrag"}} {
             r config set active-defrag-ignore-bytes 2mb
             r config set maxmemory 100mb
             r config set maxmemory-policy allkeys-lru
-            r debug populate 700000 asdf 150
-            r debug populate 170000 asdf 300
+            r debug populate 700000 asdf1 150
+            r debug populate 170000 asdf2 300
             r ping ;# trigger eviction following the previous population
             after 120 ;# serverCron only updates the info once in 100ms
             set frag [s allocator_frag_ratio]
@@ -55,6 +57,10 @@ start_server {tags {"defrag"}} {
                 puts "frag $frag"
             }
             assert {$frag >= 1.4}
+
+            r config set latency-monitor-threshold 5
+            r latency reset
+            set digest [r debug digest]
             catch {r config set activedefrag yes} e
             if {![string match {DISABLED*} $e]} {
                 # Wait for the active defrag to start working (decision once a
@@ -78,19 +84,37 @@ start_server {tags {"defrag"}} {
                 # Test the the fragmentation is lower.
                 after 120 ;# serverCron only updates the info once in 100ms
                 set frag [s allocator_frag_ratio]
+                set max_latency 0
+                foreach event [r latency latest] {
+                    lassign $event eventname time latency max
+                    if {$eventname == "active-defrag-cycle"} {
+                        set max_latency $max
+                    }
+                }
                 if {$::verbose} {
                     puts "frag $frag"
+                    puts "max latency $max_latency"
+                    puts [r latency latest]
+                    puts [r latency history active-defrag-cycle]
                 }
                 assert {$frag < 1.1}
+                # due to high fragmentation, 100hz, and active-defrag-cycle-max set to 75,
+                # we expect max latency to be not much higher than 7.5ms but due to rare slowness threshold is set higher
+                assert {$max_latency <= 60}
             } else {
                 set _ ""
             }
-        } {}
+            # verify the data isn't corrupted or changed
+            set newdigest [r debug digest]
+            assert {$digest eq $newdigest}
+            r save ;# saving an rdb iterates over all the data / pointers
+        } {OK}
 
         test "Active defrag big keys" {
             r flushdb
             r config resetstat
             r config set save "" ;# prevent bgsave from interfereing with save below
+            r config set hz 100
             r config set activedefrag no
             r config set active-defrag-max-scan-fields 1000
             r config set active-defrag-threshold-lower 5
@@ -200,9 +224,9 @@ start_server {tags {"defrag"}} {
                     puts [r latency history active-defrag-cycle]
                 }
                 assert {$frag < 1.1}
-                # due to high fragmentation, 10hz, and active-defrag-cycle-max set to 75,
-                # we expect max latency to be not much higher than 75ms
-                assert {$max_latency <= 120}
+                # due to high fragmentation, 100hz, and active-defrag-cycle-max set to 75,
+                # we expect max latency to be not much higher than 7.5ms but due to rare slowness threshold is set higher
+                assert {$max_latency <= 60}
             }
             # verify the data isn't corrupted or changed
             set newdigest [r debug digest]
@@ -292,8 +316,8 @@ start_server {tags {"defrag"}} {
                 }
                 assert {$frag < 1.1}
                 # due to high fragmentation, 100hz, and active-defrag-cycle-max set to 75,
-                # we expect max latency to be not much higher than 7.5ms
-                assert {$max_latency <= 12}
+                # we expect max latency to be not much higher than 7.5ms but due to rare slowness threshold is set higher
+                assert {$max_latency <= 60}
             }
             # verify the data isn't corrupted or changed
             set newdigest [r debug digest]

From a6c7e61e61a253e5cddd4cf919e63da321d4afe8 Mon Sep 17 00:00:00 2001
From: Guy Benoish <guy.benoish@redislabs.com>
Date: Sun, 23 Feb 2020 19:13:09 +0530
Subject: [PATCH 18/33] XREADGROUP should propagate XCALIM/SETID in MULTI/EXEC

Use built-in alsoPropagate mechanism that wraps commands
in MULTI/EXEC before sending them to replica/AOF
---
 src/t_stream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/t_stream.c b/src/t_stream.c
index 0f0f97a1d..fcfc5ca71 100644
--- a/src/t_stream.c
+++ b/src/t_stream.c
@@ -848,7 +848,7 @@ void streamPropagateXCLAIM(client *c, robj *key, streamCG *group, robj *groupnam
     argv[11] = createStringObject("JUSTID",6);
     argv[12] = createStringObject("LASTID",6);
     argv[13] = createObjectFromStreamID(&group->last_id);
-    propagate(server.xclaimCommand,c->db->id,argv,14,PROPAGATE_AOF|PROPAGATE_REPL);
+    alsoPropagate(server.xclaimCommand,c->db->id,argv,14,PROPAGATE_AOF|PROPAGATE_REPL);
     decrRefCount(argv[0]);
     decrRefCount(argv[3]);
     decrRefCount(argv[4]);
@@ -875,7 +875,7 @@ void streamPropagateGroupID(client *c, robj *key, streamCG *group, robj *groupna
     argv[2] = key;
     argv[3] = groupname;
     argv[4] = createObjectFromStreamID(&group->last_id);
-    propagate(server.xgroupCommand,c->db->id,argv,5,PROPAGATE_AOF|PROPAGATE_REPL);
+    alsoPropagate(server.xgroupCommand,c->db->id,argv,5,PROPAGATE_AOF|PROPAGATE_REPL);
     decrRefCount(argv[0]);
     decrRefCount(argv[1]);
     decrRefCount(argv[4]);

From 376a806bfbe545b6aff0b8eb5e0ddf7502c8606b Mon Sep 17 00:00:00 2001
From: Guy Benoish <guy.benoish@redislabs.com>
Date: Wed, 19 Feb 2020 13:24:50 +0530
Subject: [PATCH 19/33] Modules: Do not auto-unblock clients if not blocked on
 keys

---
 src/module.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/module.c b/src/module.c
index aae15d74b..f37d987e0 100644
--- a/src/module.c
+++ b/src/module.c
@@ -4290,12 +4290,15 @@ void unblockClientFromModule(client *c) {
      * We must call moduleUnblockClient in order to free privdata and
      * RedisModuleBlockedClient.
      *
-     * Note that clients implementing threads and working with private data,
-     * should make sure to stop the threads or protect the private data
-     * in some other way in the disconnection and timeout callback, because
-     * here we are going to free the private data associated with the
-     * blocked client. */
-    if (!bc->unblocked)
+     * Note that we only do that for clients that are blocked on keys, for which
+     * the contract is that the module should not call RM_UnblockClient under
+     * normal circumstances.
+     * Clients implementing threads and working with private data should be
+     * aware that calling RM_UnblockClient for every blocked client is their
+     * responsibility, and if they fail to do so memory may leak. Ideally they
+     * should implement the disconnect and timeout callbacks and call
+     * RM_UnblockClient, but any other way is also acceptable. */
+    if (bc->blocked_on_keys && !bc->unblocked)
         moduleUnblockClient(c);
 
     bc->client = NULL;
@@ -4409,6 +4412,10 @@ int moduleTryServeClientBlockedOnKey(client *c, robj *key) {
  *
  *     free_privdata:   called in order to free the private data that is passed
  *                      by RedisModule_UnblockClient() call.
+ *
+ * Note: RedisModule_UnblockClient should be called for every blocked client,
+ *       even if client was killed, timed-out or disconnected. Failing to do so
+ *       will result in memory leaks.
  */
 RedisModuleBlockedClient *RM_BlockClient(RedisModuleCtx *ctx, RedisModuleCmdFunc reply_callback, RedisModuleCmdFunc timeout_callback, void (*free_privdata)(RedisModuleCtx*,void*), long long timeout_ms) {
     return moduleBlockClient(ctx,reply_callback,timeout_callback,free_privdata,timeout_ms, NULL,0,NULL);
@@ -4463,7 +4470,15 @@ RedisModuleBlockedClient *RM_BlockClient(RedisModuleCtx *ctx, RedisModuleCmdFunc
  * freed using the free_privdata callback provided by the user.
  *
  * However the reply callback will be able to access the argument vector of
- * the command, so the private data is often not needed. */
+ * the command, so the private data is often not needed.
+ *
+ * Note: Under normal circumstances RedisModule_UnblockClient should not be
+ *       called for clients that are blocked on keys (Either the key will
+ *       become ready or a timeout will occur). If for some reason you do want
+ *       to call RedisModule_UnblockClient it is possible: Client will be
+ *       handled as if it were timed-out (You must implement the timeout
+ *       callback in that case).
+ */
 RedisModuleBlockedClient *RM_BlockClientOnKeys(RedisModuleCtx *ctx, RedisModuleCmdFunc reply_callback, RedisModuleCmdFunc timeout_callback, void (*free_privdata)(RedisModuleCtx*,void*), long long timeout_ms, RedisModuleString **keys, int numkeys, void *privdata) {
     return moduleBlockClient(ctx,reply_callback,timeout_callback,free_privdata,timeout_ms, keys,numkeys,privdata);
 }

From 0a643efa0c86e3053fd9a7bf7a74dfa4d0610f22 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Thu, 20 Feb 2020 17:56:52 +0200
Subject: [PATCH 20/33] fix race in module api test for fork

in some cases we were trying to kill the fork before it got created
---
 tests/modules/fork.c          | 2 +-
 tests/unit/moduleapi/fork.tcl | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/modules/fork.c b/tests/modules/fork.c
index 1a139ef1b..0443d9ef0 100644
--- a/tests/modules/fork.c
+++ b/tests/modules/fork.c
@@ -42,7 +42,7 @@ int fork_create(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 
     /* child */
     RedisModule_Log(ctx, "notice", "fork child started");
-    usleep(200000);
+    usleep(500000);
     RedisModule_Log(ctx, "notice", "fork child exiting");
     RedisModule_ExitFromChild(code_to_exit_with);
     /* unreachable */
diff --git a/tests/unit/moduleapi/fork.tcl b/tests/unit/moduleapi/fork.tcl
index f7d7e47d5..8535a3382 100644
--- a/tests/unit/moduleapi/fork.tcl
+++ b/tests/unit/moduleapi/fork.tcl
@@ -20,9 +20,8 @@ start_server {tags {"modules"}} {
 
     test {Module fork kill} {
         r fork.create 3
-        after 20
+        after 250
         r fork.kill
-        after 100
 
         assert {[count_log_message "fork child started"] eq "2"}
         assert {[count_log_message "Received SIGUSR1 in child"] eq "1"}

From 73305861f5dc11af6ef6dd268fedbe290b00e396 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Mon, 24 Feb 2020 10:46:23 +0100
Subject: [PATCH 21/33] Test engine: experimental change to avoid busy port
 problems.

---
 tests/support/server.tcl | 149 ++++++++++++++++++++++++---------------
 1 file changed, 92 insertions(+), 57 deletions(-)

diff --git a/tests/support/server.tcl b/tests/support/server.tcl
index eec43e485..d086366dc 100644
--- a/tests/support/server.tcl
+++ b/tests/support/server.tcl
@@ -141,6 +141,18 @@ proc tags {tags code} {
     uplevel 1 $code
     set ::tags [lrange $::tags 0 end-[llength $tags]]
 }
+
+# Write the configuration in the dictionary 'config' in the specified
+# file name.
+proc create_server_config_file {filename config} {
+    set fp [open $filename w+]
+    foreach directive [dict keys $config] {
+        puts -nonewline $fp "$directive "
+        puts $fp [dict get $config $directive]
+    }
+    close $fp
+}
+
 proc start_server {options {code undefined}} {
     # If we are running against an external server, we just push the
     # host/port pair in the stack the first time
@@ -222,68 +234,91 @@ proc start_server {options {code undefined}} {
 
     # write new configuration to temporary file
     set config_file [tmpfile redis.conf]
-    set fp [open $config_file w+]
-    foreach directive [dict keys $config] {
-        puts -nonewline $fp "$directive "
-        puts $fp [dict get $config $directive]
-    }
-    close $fp
+    create_server_config_file $config_file $config
 
     set stdout [format "%s/%s" [dict get $config "dir"] "stdout"]
     set stderr [format "%s/%s" [dict get $config "dir"] "stderr"]
 
-    send_data_packet $::test_server_fd "server-spawning" "port $::port"
-
-    if {$::valgrind} {
-        set pid [exec valgrind --track-origins=yes --suppressions=src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full src/redis-server $config_file > $stdout 2> $stderr &]
-    } elseif ($::stack_logging) {
-        set pid [exec /usr/bin/env MallocStackLogging=1 MallocLogFile=/tmp/malloc_log.txt src/redis-server $config_file > $stdout 2> $stderr &]
-    } else {
-        set pid [exec src/redis-server $config_file > $stdout 2> $stderr &]
-    }
-
-    # Tell the test server about this new instance.
-    send_data_packet $::test_server_fd server-spawned $pid
-
-    # check that the server actually started
-    # ugly but tries to be as fast as possible...
-    if {$::valgrind} {set retrynum 1000} else {set retrynum 100}
-
-    if {$::verbose} {
-        puts -nonewline "=== ($tags) Starting server ${::host}:${::port} "
-    }
-
-    if {$code ne "undefined"} {
-        set serverisup [server_is_up $::host $::port $retrynum]
-    } else {
-        set serverisup 1
-    }
-
-    if {$::verbose} {
-        puts ""
-    }
-
-    if {!$serverisup} {
-        set err {}
-        append err [exec cat $stdout] "\n" [exec cat $stderr]
-        start_server_error $config_file $err
-        return
-    }
-
-    # Wait for actual startup
-    set checkperiod 100; # Milliseconds
-    set maxiter [expr {120*1000/100}] ; # Wait up to 2 minutes.
-    while {![info exists _pid]} {
-        regexp {PID:\s(\d+)} [exec cat $stdout] _ _pid
-        after $checkperiod
-        incr maxiter -1
-        if {$maxiter == 0} {
-            start_server_error $config_file "No PID detected in log $stdout"
-            puts "--- LOG CONTENT ---"
-            puts [exec cat $stdout]
-            puts "-------------------"
-            break
+    # We need a loop here to retry with different ports.
+    set server_started 0
+    while {$server_started == 0} {
+        if {$::verbose} {
+            puts -nonewline "=== ($tags) Starting server ${::host}:${::port} "
         }
+
+        send_data_packet $::test_server_fd "server-spawning" "port $::port"
+
+        if {$::valgrind} {
+            set pid [exec valgrind --track-origins=yes --suppressions=src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full src/redis-server $config_file > $stdout 2> $stderr &]
+        } elseif ($::stack_logging) {
+            set pid [exec /usr/bin/env MallocStackLogging=1 MallocLogFile=/tmp/malloc_log.txt src/redis-server $config_file > $stdout 2> $stderr &]
+        } else {
+            set pid [exec src/redis-server $config_file > $stdout 2> $stderr &]
+        }
+
+        # Tell the test server about this new instance.
+        send_data_packet $::test_server_fd server-spawned $pid
+
+        # check that the server actually started
+        # ugly but tries to be as fast as possible...
+        if {$::valgrind} {set retrynum 1000} else {set retrynum 100}
+
+        # Wait for actual startup
+        set checkperiod 100; # Milliseconds
+        set maxiter [expr {120*1000/100}] ; # Wait up to 2 minutes.
+        set port_busy 0
+        while {![info exists _pid]} {
+            regexp {PID:\s(\d+)} [exec cat $stdout] _ _pid
+            after $checkperiod
+            incr maxiter -1
+            if {$maxiter == 0} {
+                start_server_error $config_file "No PID detected in log $stdout"
+                puts "--- LOG CONTENT ---"
+                puts [exec cat $stdout]
+                puts "-------------------"
+                break
+            }
+
+            # Check if the port is actually busy and the server failed
+            # for this reason.
+            if {[regexp {Could not create server TCP} [exec cat $stdout]]} {
+                set port_busy 1
+                break
+            }
+        }
+
+        # Sometimes we have to try a different port, even if we checked
+        # for availability. Other test clients may grab the port before we
+        # are able to do it for example.
+        if {$port_busy} {
+            puts "Port $::port was already busy, trying another port..."
+            set ::port [find_available_port [expr {$::port+1}]]
+            if {$::tls} {
+                dict set config "tls-port" $::port
+            } else {
+                dict set config port $::port
+            }
+            create_server_config_file $config_file $config
+            continue; # Try again
+        }
+
+        if {$code ne "undefined"} {
+            set serverisup [server_is_up $::host $::port $retrynum]
+        } else {
+            set serverisup 1
+        }
+
+        if {$::verbose} {
+            puts ""
+        }
+
+        if {!$serverisup} {
+            set err {}
+            append err [exec cat $stdout] "\n" [exec cat $stderr]
+            start_server_error $config_file $err
+            return
+        }
+        set server_started 1
     }
 
     # setup properties to be able to initialize a client object

From b6378edcd684fb8194861fbc7506614f987afc78 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 21 Feb 2020 16:39:42 +0100
Subject: [PATCH 22/33] Tracking: optin/out implemented.

---
 src/networking.c | 65 ++++++++++++++++++++++++++++++++++++++++++------
 src/server.h     |  6 ++++-
 src/tracking.c   | 27 ++++++++++++++------
 3 files changed, 82 insertions(+), 16 deletions(-)

diff --git a/src/networking.c b/src/networking.c
index 5b1229fde..4c394af70 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -1365,6 +1365,12 @@ void resetClient(client *c) {
     if (!(c->flags & CLIENT_MULTI) && prevcmd != askingCommand)
         c->flags &= ~CLIENT_ASKING;
 
+    /* We do the same for the CACHING command as well. It also affects
+     * the next command or transaction executed, in a way very similar
+     * to ASKING. */
+    if (!(c->flags & CLIENT_MULTI) && prevcmd != clientCommand)
+        c->flags &= ~CLIENT_TRACKING_CACHING;
+
     /* Remove the CLIENT_REPLY_SKIP flag if any so that the reply
      * to the next command will be sent, but set the flag if the command
      * we just processed was "CLIENT REPLY SKIP". */
@@ -2044,7 +2050,7 @@ void clientCommand(client *c) {
 "REPLY (on|off|skip)    -- Control the replies sent to the current connection.",
 "SETNAME <name>         -- Assign the name <name> to the current connection.",
 "UNBLOCK <clientid> [TIMEOUT|ERROR] -- Unblock the specified blocked client.",
-"TRACKING (on|off) [REDIRECT <id>] [BCAST] [PREFIX first] [PREFIX second] ... -- Enable client keys tracking for client side caching.",
+"TRACKING (on|off) [REDIRECT <id>] [BCAST] [PREFIX first] [PREFIX second] [OPTIN] [OPTOUT]... -- Enable client keys tracking for client side caching.",
 "GETREDIR               -- Return the client ID we are redirecting to when tracking is enabled.",
 NULL
         };
@@ -2221,9 +2227,9 @@ NULL
         addReply(c,shared.ok);
     } else if (!strcasecmp(c->argv[1]->ptr,"tracking") && c->argc >= 3) {
         /* CLIENT TRACKING (on|off) [REDIRECT <id>] [BCAST] [PREFIX first]
-         *                          [PREFIX second] ... */
+         *                          [PREFIX second] [OPTIN] [OPTOUT] ... */
         long long redir = 0;
-        int bcast = 0;
+        uint64_t options = 0;
         robj **prefix = NULL;
         size_t numprefix = 0;
 
@@ -2256,7 +2262,11 @@ NULL
                     return;
                 }
             } else if (!strcasecmp(c->argv[j]->ptr,"bcast")) {
-                bcast = 1;
+                options |= CLIENT_TRACKING_BCAST;
+            } else if (!strcasecmp(c->argv[j]->ptr,"optin")) {
+                options |= CLIENT_TRACKING_OPTIN;
+            } else if (!strcasecmp(c->argv[j]->ptr,"optout")) {
+                options |= CLIENT_TRACKING_OPTOUT;
             } else if (!strcasecmp(c->argv[j]->ptr,"prefix") && moreargs) {
                 j++;
                 prefix = zrealloc(prefix,sizeof(robj*)*(numprefix+1));
@@ -2272,7 +2282,7 @@ NULL
         if (!strcasecmp(c->argv[2]->ptr,"on")) {
             /* Before enabling tracking, make sure options are compatible
              * among each other and with the current state of the client. */
-            if (!bcast && numprefix) {
+            if (!(options & CLIENT_TRACKING_BCAST) && numprefix) {
                 addReplyError(c,
                     "PREFIX option requires BCAST mode to be enabled");
                 zfree(prefix);
@@ -2281,7 +2291,8 @@ NULL
 
             if (c->flags & CLIENT_TRACKING) {
                 int oldbcast = !!(c->flags & CLIENT_TRACKING_BCAST);
-                if (oldbcast != bcast) {
+                int newbcast = !!(options & CLIENT_TRACKING_BCAST);
+                if (oldbcast != newbcast) {
                     addReplyError(c,
                     "You can't switch BCAST mode on/off before disabling "
                     "tracking for this client, and then re-enabling it with "
@@ -2290,7 +2301,17 @@ NULL
                     return;
                 }
             }
-            enableTracking(c,redir,bcast,prefix,numprefix);
+
+            if (options & CLIENT_TRACKING_BCAST &&
+                options & (CLIENT_TRACKING_OPTIN|CLIENT_TRACKING_OPTOUT))
+            {
+                addReplyError(c,
+                "OPTIN and OPTOUT are not compatible with BCAST");
+                zfree(prefix);
+                return;
+            }
+
+            enableTracking(c,redir,options,prefix,numprefix);
         } else if (!strcasecmp(c->argv[2]->ptr,"off")) {
             disableTracking(c);
         } else {
@@ -2300,6 +2321,36 @@ NULL
         }
         zfree(prefix);
         addReply(c,shared.ok);
+    } else if (!strcasecmp(c->argv[1]->ptr,"caching") && c->argc >= 3) {
+        if (!(c->flags & CLIENT_TRACKING)) {
+            addReplyError(c,"CLIENT CACHING can be called only when the "
+                            "client is in tracking mode with OPTIN or "
+                            "OPTOUT mode enabled");
+            return;
+        }
+
+        char *opt = c->argv[2]->ptr;
+        if (!strcasecmp(opt,"yes")) {
+            if (c->flags & CLIENT_TRACKING_OPTIN) {
+                c->flags |= CLIENT_TRACKING_CACHING;
+            } else {
+                addReplyError(c,"CLIENT CACHING YES is only valid when tracking is enabled in OPTIN mode.");
+                return;
+            }
+        } else if (!strcasecmp(opt,"no")) {
+            if (c->flags & CLIENT_TRACKING_OPTOUT) {
+                c->flags |= CLIENT_TRACKING_CACHING;
+            } else {
+                addReplyError(c,"CLIENT CACHING NO is only valid when tracking is enabled in OPTOUT mode.");
+                return;
+            }
+        } else {
+            addReply(c,shared.syntaxerr);
+            return;
+        }
+
+        /* Common reply for when we succeeded. */
+        addReply(c,shared.ok);
     } else if (!strcasecmp(c->argv[1]->ptr,"getredir") && c->argc == 2) {
         /* CLIENT GETREDIR */
         if (c->flags & CLIENT_TRACKING) {
diff --git a/src/server.h b/src/server.h
index bb40ca4e1..87c293c26 100644
--- a/src/server.h
+++ b/src/server.h
@@ -248,6 +248,10 @@ typedef long long ustime_t; /* microsecond time type. */
                                    perform client side caching. */
 #define CLIENT_TRACKING_BROKEN_REDIR (1ULL<<32) /* Target client is invalid. */
 #define CLIENT_TRACKING_BCAST (1ULL<<33) /* Tracking in BCAST mode. */
+#define CLIENT_TRACKING_OPTIN (1ULL<<34)  /* Tracking in opt-in mode. */
+#define CLIENT_TRACKING_OPTOUT (1ULL<<35) /* Tracking in opt-out mode. */
+#define CLIENT_TRACKING_CACHING (1ULL<<36) /* CACHING yes/no was given,
+                                              depending on optin/optout mode. */
 
 /* Client block type (btype field in client structure)
  * if CLIENT_BLOCKED flag is set. */
@@ -1651,7 +1655,7 @@ void addReplyStatusFormat(client *c, const char *fmt, ...);
 #endif
 
 /* Client side caching (tracking mode) */
-void enableTracking(client *c, uint64_t redirect_to, int bcast, robj **prefix, size_t numprefix);
+void enableTracking(client *c, uint64_t redirect_to, uint64_t options, robj **prefix, size_t numprefix);
 void disableTracking(client *c);
 void trackingRememberKeys(client *c);
 void trackingInvalidateKey(robj *keyobj);
diff --git a/src/tracking.c b/src/tracking.c
index 619148f2f..45f83103a 100644
--- a/src/tracking.c
+++ b/src/tracking.c
@@ -93,7 +93,8 @@ void disableTracking(client *c) {
     if (c->flags & CLIENT_TRACKING) {
         server.tracking_clients--;
         c->flags &= ~(CLIENT_TRACKING|CLIENT_TRACKING_BROKEN_REDIR|
-                      CLIENT_TRACKING_BCAST);
+                      CLIENT_TRACKING_BCAST|CLIENT_TRACKING_OPTIN|
+                      CLIENT_TRACKING_OPTOUT);
     }
 }
 
@@ -124,10 +125,11 @@ void enableBcastTrackingForPrefix(client *c, char *prefix, size_t plen) {
  * eventually get freed, we'll send a message to the original client to
  * inform it of the condition. Multiple clients can redirect the invalidation
  * messages to the same client ID. */
-void enableTracking(client *c, uint64_t redirect_to, int bcast, robj **prefix, size_t numprefix) {
+void enableTracking(client *c, uint64_t redirect_to, uint64_t options, robj **prefix, size_t numprefix) {
     if (!(c->flags & CLIENT_TRACKING)) server.tracking_clients++;
     c->flags |= CLIENT_TRACKING;
-    c->flags &= ~(CLIENT_TRACKING_BROKEN_REDIR|CLIENT_TRACKING_BCAST);
+    c->flags &= ~(CLIENT_TRACKING_BROKEN_REDIR|CLIENT_TRACKING_BCAST|
+                  CLIENT_TRACKING_OPTIN|CLIENT_TRACKING_OPTOUT);
     c->client_tracking_redirection = redirect_to;
     if (TrackingTable == NULL) {
         TrackingTable = raxNew();
@@ -135,7 +137,7 @@ void enableTracking(client *c, uint64_t redirect_to, int bcast, robj **prefix, s
         TrackingChannelName = createStringObject("__redis__:invalidate",20);
     }
 
-    if (bcast) {
+    if (options & CLIENT_TRACKING_BCAST) {
         c->flags |= CLIENT_TRACKING_BCAST;
         if (numprefix == 0) enableBcastTrackingForPrefix(c,"",0);
         for (size_t j = 0; j < numprefix; j++) {
@@ -143,14 +145,23 @@ void enableTracking(client *c, uint64_t redirect_to, int bcast, robj **prefix, s
             enableBcastTrackingForPrefix(c,sdsprefix,sdslen(sdsprefix));
         }
     }
+    c->flags |= options & (CLIENT_TRACKING_OPTIN|CLIENT_TRACKING_OPTOUT);
 }
 
 /* This function is called after the execution of a readonly command in the
- * case the client 'c' has keys tracking enabled. It will populate the
- * tracking invalidation table according to the keys the user fetched, so that
- * Redis will know what are the clients that should receive an invalidation
- * message with certain groups of keys are modified. */
+ * case the client 'c' has keys tracking enabled and the tracking is not
+ * in BCAST mode. It will populate the tracking invalidation table according
+ * to the keys the user fetched, so that Redis will know what are the clients
+ * that should receive an invalidation message with certain groups of keys
+ * are modified. */
 void trackingRememberKeys(client *c) {
+    /* Return if we are in optin/out mode and the right CACHING command
+     * was/wasn't given in order to modify the default behavior. */
+    uint64_t optin = c->flags & CLIENT_TRACKING_OPTIN;
+    uint64_t optout = c->flags & CLIENT_TRACKING_OPTOUT;
+    uint64_t caching_given = c->flags & CLIENT_TRACKING_CACHING;
+    if ((optin && !caching_given) || (optout && caching_given)) return;
+
     int numkeys;
     int *keys = getKeysFromCommand(c->cmd,c->argv,c->argc,&numkeys);
     if (keys == NULL) return;

From 1ab550832477319195cd3455e657e35ab33ae985 Mon Sep 17 00:00:00 2001
From: Hengjian Tang <hengjian_tang@TSG.AISE.com>
Date: Tue, 25 Feb 2020 15:55:28 +0800
Subject: [PATCH 23/33] modify the read buf size according to the write buf
 size PROTO_IOBUF_LEN defined before

---
 src/replication.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/replication.c b/src/replication.c
index 4843f97d5..c497051c8 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -1352,7 +1352,7 @@ void disklessLoadRestoreBackups(redisDb *backup, int restore, int empty_db_flags
 /* Asynchronously read the SYNC payload we receive from a master */
 #define REPL_MAX_WRITTEN_BEFORE_FSYNC (1024*1024*8) /* 8 MB */
 void readSyncBulkPayload(connection *conn) {
-    char buf[4096];
+    char buf[PROTO_IOBUF_LEN];
     ssize_t nread, readlen, nwritten;
     int use_diskless_load = useDisklessLoad();
     redisDb *diskless_load_backup = NULL;

From 537893420bc5a51ece060b53e5492b101bcfaba3 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Tue, 25 Feb 2020 13:01:52 +0200
Subject: [PATCH 24/33] fix github actions failing latency test for active
 defrag

seems that github actions are slow, using just one client to reduce
false positives.

also adding verbose, testing only on latest ubuntu, and building on
older one.

when doing that, i can reduce the test threshold back to something saner
---
 .github/workflows/ci.yml     | 23 ++++++++++++-----------
 tests/unit/memefficiency.tcl |  6 +++---
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 847abcf02..559ae61d8 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -3,11 +3,8 @@ name: CI
 on: [push, pull_request]
 
 jobs:
-  build-ubuntu:
-    strategy:
-      matrix:
-        platform: [ubuntu-latest, ubuntu-16.04]
-    runs-on: ${{ matrix.platform }}
+  test-ubuntu-latest:
+    runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v1
     - name: make
@@ -15,13 +12,17 @@ jobs:
     - name: test
       run: |
         sudo apt-get install tcl8.5
-        make test
+        ./runtest --clients 1 --verbose
 
-  build-macos-latest:
-    strategy:
-      matrix:
-        platform: [macos-latest, macOS-10.14]
-    runs-on: ${{ matrix.platform }}
+  test-ubuntu-old:
+    runs-on: ubuntu-16.04
+    steps:
+    - uses: actions/checkout@v1
+    - name: make
+      run: make
+
+  build-macos-latest:
+    runs-on: macos-latest
     steps:
     - uses: actions/checkout@v1
     - name: make
diff --git a/tests/unit/memefficiency.tcl b/tests/unit/memefficiency.tcl
index 468825a47..c899103fd 100644
--- a/tests/unit/memefficiency.tcl
+++ b/tests/unit/memefficiency.tcl
@@ -100,7 +100,7 @@ start_server {tags {"defrag"}} {
                 assert {$frag < 1.1}
                 # due to high fragmentation, 100hz, and active-defrag-cycle-max set to 75,
                 # we expect max latency to be not much higher than 7.5ms but due to rare slowness threshold is set higher
-                assert {$max_latency <= 60}
+                assert {$max_latency <= 30}
             } else {
                 set _ ""
             }
@@ -226,7 +226,7 @@ start_server {tags {"defrag"}} {
                 assert {$frag < 1.1}
                 # due to high fragmentation, 100hz, and active-defrag-cycle-max set to 75,
                 # we expect max latency to be not much higher than 7.5ms but due to rare slowness threshold is set higher
-                assert {$max_latency <= 60}
+                assert {$max_latency <= 30}
             }
             # verify the data isn't corrupted or changed
             set newdigest [r debug digest]
@@ -317,7 +317,7 @@ start_server {tags {"defrag"}} {
                 assert {$frag < 1.1}
                 # due to high fragmentation, 100hz, and active-defrag-cycle-max set to 75,
                 # we expect max latency to be not much higher than 7.5ms but due to rare slowness threshold is set higher
-                assert {$max_latency <= 60}
+                assert {$max_latency <= 30}
             }
             # verify the data isn't corrupted or changed
             set newdigest [r debug digest]

From 2f1a1c3835efefec8de97d3222538a45ccdd257e Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Wed, 26 Feb 2020 08:12:07 +0200
Subject: [PATCH 25/33] fix github actions failing latency test for active
 defrag - part 2

it seems that running two clients at a time is ok too, resuces action
time from 20 minutes to 10. we'll use this for now, and if one day it
won't be enough we'll have to run just the sensitive tests one by one
separately from the others.

this commit also fixes an issue with the defrag test that appears to be
very rare.
---
 .github/workflows/ci.yml     | 4 ++--
 tests/unit/memefficiency.tcl | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 559ae61d8..cc4991606 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -12,9 +12,9 @@ jobs:
     - name: test
       run: |
         sudo apt-get install tcl8.5
-        ./runtest --clients 1 --verbose
+        ./runtest --clients 2 --verbose
 
-  test-ubuntu-old:
+  build-ubuntu-old:
     runs-on: ubuntu-16.04
     steps:
     - uses: actions/checkout@v1
diff --git a/tests/unit/memefficiency.tcl b/tests/unit/memefficiency.tcl
index c899103fd..06b0e07d7 100644
--- a/tests/unit/memefficiency.tcl
+++ b/tests/unit/memefficiency.tcl
@@ -60,6 +60,7 @@ start_server {tags {"defrag"}} {
 
             r config set latency-monitor-threshold 5
             r latency reset
+            r config set maxmemory 110mb ;# prevent further eviction (not to fail the digest test)
             set digest [r debug digest]
             catch {r config set activedefrag yes} e
             if {![string match {DISABLED*} $e]} {
@@ -166,7 +167,7 @@ start_server {tags {"defrag"}} {
             for {set j 0} {$j < 500000} {incr j} {
                 $rd read ; # Discard replies
             }
-            assert {[r dbsize] == 500010}
+            assert_equal [r dbsize] 500010
 
             # create some fragmentation
             for {set j 0} {$j < 500000} {incr j 2} {
@@ -175,7 +176,7 @@ start_server {tags {"defrag"}} {
             for {set j 0} {$j < 500000} {incr j 2} {
                 $rd read ; # Discard replies
             }
-            assert {[r dbsize] == 250010}
+            assert_equal [r dbsize] 250010
 
             # start defrag
             after 120 ;# serverCron only updates the info once in 100ms

From d8ab7e8012771ebcc4dbdc988ce84f7df57688c7 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 27 Feb 2020 17:41:48 +0100
Subject: [PATCH 26/33] Improve aeDeleteEventLoop() top comment grammar.

---
 src/ae.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/ae.c b/src/ae.c
index 248096e1f..d2248fe5c 100644
--- a/src/ae.c
+++ b/src/ae.c
@@ -135,7 +135,8 @@ void aeDeleteEventLoop(aeEventLoop *eventLoop) {
     aeApiFree(eventLoop);
     zfree(eventLoop->events);
     zfree(eventLoop->fired);
-    /* Free time event. */
+
+    /* Free the time events list. */
     aeTimeEvent *next_te, *te = eventLoop->timeEventHead;
     while (te) {
         next_te = te->next;

From 43f39f2e37b3b46683747a8207637f04747c9e94 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 27 Feb 2020 17:45:48 +0100
Subject: [PATCH 27/33] Remove useless comment from enumConfigSet().

---
 src/config.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/config.c b/src/config.c
index 5841ae7a0..8d069f8db 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1674,7 +1674,6 @@ static int enumConfigSet(typeData data, sds value, int update, char **err) {
         enumerr[sdslen(enumerr) - 2] = '\0';
 
         strncpy(loadbuf, enumerr, LOADBUF_SIZE);
-        /* strncpy does not if null terminate if source string length is >= destination buffer. */
         loadbuf[LOADBUF_SIZE - 1] = '\0';
 
         sdsfree(enumerr);

From 9abdc089f84c7b7e16b62b41a7a30eb9814d5b83 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 27 Feb 2020 17:47:50 +0100
Subject: [PATCH 28/33] Fix SDS misuse in enumConfigSet(). Related to #6778.

---
 src/config.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/config.c b/src/config.c
index 8d069f8db..aeb2fea7e 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1666,12 +1666,12 @@ static int enumConfigSet(typeData data, sds value, int update, char **err) {
         sds enumerr = sdsnew("argument must be one of the following: ");
         configEnum *enumNode = data.enumd.enum_value;
         while(enumNode->name != NULL) {
-            enumerr = sdscatlen(enumerr, enumNode->name, strlen(enumNode->name));
+            enumerr = sdscatlen(enumerr, enumNode->name,
+                                strlen(enumNode->name));
             enumerr = sdscatlen(enumerr, ", ", 2);
             enumNode++;
         }
-
-        enumerr[sdslen(enumerr) - 2] = '\0';
+        sdsrange(enumerr,0,-3); /* Remove final ", ". */
 
         strncpy(loadbuf, enumerr, LOADBUF_SIZE);
         loadbuf[LOADBUF_SIZE - 1] = '\0';

From 45f318d2ffaeb7b44b3aebb803a012fef1c36f0a Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 27 Feb 2020 18:21:12 +0100
Subject: [PATCH 29/33] Show Redis version when not understanding a config
 directive.

This makes simpler to give people help when posting such kind of errors
in the mailing list or other help forums, because sometimes the
directive looks well spelled, but the version of Redis they are using is
not able to support it.
---
 src/config.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/config.c b/src/config.c
index aeb2fea7e..fd04b7c87 100644
--- a/src/config.c
+++ b/src/config.c
@@ -518,7 +518,8 @@ void loadServerConfigFromString(char *config) {
     return;
 
 loaderr:
-    fprintf(stderr, "\n*** FATAL CONFIG FILE ERROR ***\n");
+    fprintf(stderr, "\n*** FATAL CONFIG FILE ERROR (Redis %s) ***\n",
+        REDIS_VERSION);
     fprintf(stderr, "Reading the configuration file, at line %d\n", linenum);
     fprintf(stderr, ">>> '%s'\n", lines[i]);
     fprintf(stderr, "%s\n", err);

From 8a3e9a6d446230f7fe0db6651af1ee0314617b0c Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 28 Feb 2020 18:06:30 +0100
Subject: [PATCH 30/33] Modules: more details in RM_Scan API top comment.

---
 src/module.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/module.c b/src/module.c
index b821f0c31..47371fb32 100644
--- a/src/module.c
+++ b/src/module.c
@@ -6557,9 +6557,21 @@ void RM_ScanCursorDestroy(RedisModuleScanCursor *cursor) {
  * possibly setting errno if the call failed.
  * It is also possible to restart and existing cursor using RM_CursorRestart.
  *
- * NOTE: You must avoid doing any database changes from within the callback, you should avoid any
- * RedisModule_OpenKey or RedisModule_Call, if you need to do these, you need to keep the key name
- * and do any work you need to do after the call to Scan returns. */
+ * IMPORTANT: This API is very similar to the Redis SCAN command from the
+ * point of view of the guarantees it provides. This means that the API
+ * may report duplicated keys, but guarantees to report at least one time
+ * every key that was there from the start to the end of the scanning process.
+ *
+ * NOTE: If you do database changes within the callback, you should be aware
+ * that the internal state of the database may change. For instance it is safe
+ * to delete or modify the current key, but may not be safe to delete any
+ * other key.
+ * Moreover playing with the Redis keyspace while iterating may have the
+ * effect of returning more duplicates. A safe pattern is to store the keys
+ * names you want to modify elsewhere, and perform the actions on the keys
+ * later when the iteration is complete. Howerver this can cost a lot of
+ * memory, so it may make sense to just operate on the current key when
+ * possible during the iteration, given that this is safe. */
 int RM_Scan(RedisModuleCtx *ctx, RedisModuleScanCursor *cursor, RedisModuleScanCB fn, void *privdata) {
     if (cursor->done) {
         errno = ENOENT;
@@ -6641,9 +6653,13 @@ static void moduleScanKeyCallback(void *privdata, const dictEntry *de) {
  * possibly setting errno if the call failed.
  * It is also possible to restart and existing cursor using RM_CursorRestart.
  *
- * NOTE: You must avoid doing any database changes from within the callback, you should avoid any
- * RedisModule_OpenKey or RedisModule_Call, if you need to do these, you need to keep the field name
- * and do any work you need to do after the call to Scan returns. */
+ * NOTE: Certain operations are unsafe while iterating the object. For instance
+ * while the API guarantees to return at least one time all the elements that
+ * are present in the data structure consistently from the start to the end
+ * of the iteration (see HSCAN and similar commands documentation), the more
+ * you play with the elements, the more duplicates you may get. In general
+ * deleting the current element of the data structure is safe, while removing
+ * the key you are iterating is not safe. */
 int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleScanKeyCB fn, void *privdata) {
     if (key == NULL || key->value == NULL) {
         errno = EINVAL;

From c8ae90fef64c829cd1ac50c29e5912807c2577d5 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 28 Feb 2020 18:09:46 +0100
Subject: [PATCH 31/33] Modules: reformat RM_Scan() top comment a bit.

---
 src/module.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/module.c b/src/module.c
index 47371fb32..bbd54082c 100644
--- a/src/module.c
+++ b/src/module.c
@@ -6526,24 +6526,32 @@ void RM_ScanCursorDestroy(RedisModuleScanCursor *cursor) {
     zfree(cursor);
 }
 
-/* Scan api that allows a module to scan all the keys and value in the selected db.
+/* Scan API that allows a module to scan all the keys and value in
+ * the selected db.
  *
  * Callback for scan implementation.
- * void scan_callback(RedisModuleCtx *ctx, RedisModuleString *keyname, RedisModuleKey *key, void *privdata);
- * - ctx - the redis module context provided to for the scan.
- * - keyname - owned by the caller and need to be retained if used after this function.
- * - key - holds info on the key and value, it is provided as best effort, in some cases it might
- *   be NULL, in which case the user should (can) use RedisModule_OpenKey (and CloseKey too).
- *   when it is provided, it is owned by the caller and will be free when the callback returns.
- * - privdata - the user data provided to RedisModule_Scan.
+ * void scan_callback(RedisModuleCtx *ctx, RedisModuleString *keyname,
+ *                    RedisModuleKey *key, void *privdata);
+ * ctx - the redis module context provided to for the scan.
+ * keyname - owned by the caller and need to be retained if used after this
+ * function.
+ *
+ * key - holds info on the key and value, it is provided as best effort, in
+ * some cases it might be NULL, in which case the user should (can) use
+ * RedisModule_OpenKey (and CloseKey too).
+ * when it is provided, it is owned by the caller and will be free when the
+ * callback returns.
+ *
+ * privdata - the user data provided to RedisModule_Scan.
  *
  * The way it should be used:
  *      RedisModuleCursor *c = RedisModule_ScanCursorCreate();
  *      while(RedisModule_Scan(ctx, c, callback, privateData));
  *      RedisModule_ScanCursorDestroy(c);
  *
- * It is also possible to use this API from another thread while the lock is acquired durring 
- * the actuall call to RM_Scan:
+ * It is also possible to use this API from another thread while the lock
+ * is acquired durring the actuall call to RM_Scan:
+ *
  *      RedisModuleCursor *c = RedisModule_ScanCursorCreate();
  *      RedisModule_ThreadSafeContextLock(ctx);
  *      while(RedisModule_Scan(ctx, c, callback, privateData)){
@@ -6553,8 +6561,9 @@ void RM_ScanCursorDestroy(RedisModuleScanCursor *cursor) {
  *      }
  *      RedisModule_ScanCursorDestroy(c);
  *
- * The function will return 1 if there are more elements to scan and 0 otherwise,
- * possibly setting errno if the call failed.
+ * The function will return 1 if there are more elements to scan and
+ * 0 otherwise, possibly setting errno if the call failed.
+ *
  * It is also possible to restart and existing cursor using RM_CursorRestart.
  *
  * IMPORTANT: This API is very similar to the Redis SCAN command from the

From 98b23cce295d1ed584de00395695a012ef2dd2ab Mon Sep 17 00:00:00 2001
From: ShooterIT <wangyuancode@163.com>
Date: Sat, 29 Feb 2020 18:28:41 +0800
Subject: [PATCH 32/33] Avoid compiler warnings

---
 src/acl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/acl.c b/src/acl.c
index b046785ff..efe6b96ad 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -1830,6 +1830,7 @@ void aclCommand(client *c) {
             case ACL_DENIED_CMD: reasonstr="command"; break;
             case ACL_DENIED_KEY: reasonstr="key"; break;
             case ACL_DENIED_AUTH: reasonstr="auth"; break;
+            default: reasonstr="unknown";
             }
             addReplyBulkCString(c,reasonstr);
 

From 27641ee490aafdbb38ec506edfcbdb041e829826 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Thu, 5 Mar 2020 16:55:14 +0200
Subject: [PATCH 33/33] fix for flaky psync2 test

*** [err]: PSYNC2: total sum of full synchronizations is exactly 4 in tests/integration/psync2.tcl
Expected 5 == 4 (context: type eval line 6 cmd {assert {$sum == 4}} proc ::test)

issue was that sometime the test got an unexpected full sync since it
tried to switch to the replica before it was in sync with it's master.
---
 tests/integration/psync2.tcl | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tests/integration/psync2.tcl b/tests/integration/psync2.tcl
index d1212b640..333736ffa 100644
--- a/tests/integration/psync2.tcl
+++ b/tests/integration/psync2.tcl
@@ -114,6 +114,27 @@ start_server {} {
             }
         }
 
+        # wait for all the slaves to be in sync with the master
+        set master_ofs [status $R($master_id) master_repl_offset]
+        wait_for_condition 500 100 {
+            $master_ofs == [status $R(0) master_repl_offset] &&
+            $master_ofs == [status $R(1) master_repl_offset] &&
+            $master_ofs == [status $R(2) master_repl_offset] &&
+            $master_ofs == [status $R(3) master_repl_offset] &&
+            $master_ofs == [status $R(4) master_repl_offset]
+        } else {
+            if {$debug_msg} {
+                for {set j 0} {$j < 5} {incr j} {
+                    puts "$j: sync_full: [status $R($j) sync_full]"
+                    puts "$j: id1      : [status $R($j) master_replid]:[status $R($j) master_repl_offset]"
+                    puts "$j: id2      : [status $R($j) master_replid2]:[status $R($j) second_repl_offset]"
+                    puts "$j: backlog  : firstbyte=[status $R($j) repl_backlog_first_byte_offset] len=[status $R($j) repl_backlog_histlen]"
+                    puts "---"
+                }
+            }
+            fail "Slaves are not in sync with the master after too long time."
+        }
+
         # Put down the old master so that it cannot generate more
         # replication stream, this way in the next master switch, the time at
         # which we move slaves away is not important, each will have full