From 0bad00d0499d79c6367f340e7dfc641071f002b8 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 2 Mar 2021 09:39:37 +0200 Subject: [PATCH 001/131] Fix errors when loading RDB with missing modules. (#8579) Fixes #8574 --- src/module.c | 6 ++++++ src/rdb.c | 13 ++++++++----- src/server.h | 1 + 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/module.c b/src/module.c index 274210590..871399cff 100644 --- a/src/module.c +++ b/src/module.c @@ -4264,6 +4264,12 @@ void moduleTypeNameByID(char *name, uint64_t moduleid) { } } +/* Return the name of the module that owns the specified moduleType. */ +const char *moduleTypeModuleName(moduleType *mt) { + if (!mt || !mt->module) return NULL; + return mt->module->name; +} + /* Create a copy of a module type value using the copy callback. If failed * or not supported, produce an error reply and return NULL. */ diff --git a/src/rdb.c b/src/rdb.c index 630417302..94f568bb3 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2176,16 +2176,17 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { return NULL; } moduleType *mt = moduleTypeLookupModuleByID(moduleid); - char name[10]; if (rdbCheckMode && rdbtype == RDB_TYPE_MODULE_2) { + char name[10]; moduleTypeNameByID(name,moduleid); return rdbLoadCheckModuleValue(rdb,name); } if (mt == NULL) { + char name[10]; moduleTypeNameByID(name,moduleid); - rdbReportCorruptRDB("The RDB file contains module data I can't load: no matching module '%s'", name); + rdbReportCorruptRDB("The RDB file contains module data I can't load: no matching module type '%s'", name); return NULL; } RedisModuleIO io; @@ -2212,7 +2213,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { return NULL; } if (eof != RDB_MODULE_OPCODE_EOF) { - rdbReportCorruptRDB("The RDB file contains module data for the module '%s' that is not terminated by the proper module value EOF marker", name); + rdbReportCorruptRDB("The RDB file contains module data for the module '%s' that is not terminated by " + "the proper module value EOF marker", moduleTypeModuleName(mt)); if (ptr) { o = createModuleObject(mt,ptr); /* creating just in order to easily destroy */ decrRefCount(o); @@ -2222,8 +2224,9 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { } if (ptr == NULL) { - moduleTypeNameByID(name,moduleid); - rdbReportCorruptRDB("The RDB file contains module data for the module type '%s', that the responsible module is not able to load. Check for modules log above for additional clues.", name); + rdbReportCorruptRDB("The RDB file contains module data for the module type '%s', that the responsible " + "module is not able to load. Check for modules log above for additional clues.", + moduleTypeModuleName(mt)); return NULL; } o = createModuleObject(mt,ptr); diff --git a/src/server.h b/src/server.h index e241bad70..17fd9fc9a 100644 --- a/src/server.h +++ b/src/server.h @@ -1744,6 +1744,7 @@ void moduleLoadFromQueue(void); int moduleGetCommandKeysViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result); moduleType *moduleTypeLookupModuleByID(uint64_t id); void moduleTypeNameByID(char *name, uint64_t moduleid); +const char *moduleTypeModuleName(moduleType *mt); void moduleFreeContext(struct RedisModuleCtx *ctx); void unblockClientFromModule(client *c); void moduleHandleBlockedClients(void); From 5d180d28349b36a177f9d70edb12bb27e88e8c61 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 2 Mar 2021 18:12:11 +0200 Subject: [PATCH 002/131] Fix potential replication-4 test race condition. (#8583) Co-authored-by: Oran Agra --- tests/integration/replication-4.tcl | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/integration/replication-4.tcl b/tests/integration/replication-4.tcl index c867001b8..8715ae999 100644 --- a/tests/integration/replication-4.tcl +++ b/tests/integration/replication-4.tcl @@ -79,12 +79,16 @@ start_server {tags {"repl"}} { $master config set min-slaves-max-lag 2 $master config set min-slaves-to-write 1 assert {[$master set foo bar] eq {OK}} - $slave deferred 1 - $slave debug sleep 6 - after 4000 - catch {$master set foo bar} e - set e - } {NOREPLICAS*} + exec kill -SIGSTOP [srv 0 pid] + wait_for_condition 100 100 { + [catch {$master set foo bar}] != 0 + } else { + fail "Master didn't become readonly" + } + catch {$master set foo bar} err + assert_match {NOREPLICAS*} $err + exec kill -SIGCONT [srv 0 pid] + } } } From 257009afff0ca9b299bd6e70ed439bab9186be56 Mon Sep 17 00:00:00 2001 From: Wen Hui Date: Wed, 3 Mar 2021 01:36:08 -0500 Subject: [PATCH 003/131] generalize error check for SENTINEL MONITOR rumtime config (#8590) --- src/sentinel.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/sentinel.c b/src/sentinel.c index 8597a10df..164ff15b2 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -3710,17 +3710,7 @@ NULL ri = createSentinelRedisInstance(c->argv[2]->ptr,SRI_MASTER, c->argv[3]->ptr,port,quorum,NULL); if (ri == NULL) { - switch(errno) { - case EBUSY: - addReplyError(c,"Duplicated master name"); - break; - case EINVAL: - addReplyError(c,"Invalid port number"); - break; - default: - addReplyError(c,"Unspecified error adding the instance"); - break; - } + addReplyError(c,sentinelCheckCreateInstanceErrors(SRI_MASTER)); } else { sentinelFlushConfig(); sentinelEvent(LL_WARNING,"+monitor",ri,"%@ quorum %d",ri->quorum); From fba391ae66a6527ca5dba9025b6fb57c35dd65d9 Mon Sep 17 00:00:00 2001 From: yoav-steinberg Date: Wed, 3 Mar 2021 09:36:27 +0200 Subject: [PATCH 004/131] Fix client_recent_max_input/output_buffer in 'INFO CLIENTS' when all clients drop. (#8588) When no connected clients variables stopped being updated instead of being zeroed over time. The other changes are cleanup and optimization (avoiding an unnecessary division per client) --- src/server.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/server.c b/src/server.c index 4784431ba..20ada1404 100644 --- a/src/server.c +++ b/src/server.c @@ -1729,33 +1729,17 @@ int clientsCronResizeQueryBuffer(client *c) { * When we want to know what was recently the peak memory usage, we just scan * such few slots searching for the maximum value. */ #define CLIENTS_PEAK_MEM_USAGE_SLOTS 8 -size_t ClientsPeakMemInput[CLIENTS_PEAK_MEM_USAGE_SLOTS]; -size_t ClientsPeakMemOutput[CLIENTS_PEAK_MEM_USAGE_SLOTS]; +size_t ClientsPeakMemInput[CLIENTS_PEAK_MEM_USAGE_SLOTS] = {0}; +size_t ClientsPeakMemOutput[CLIENTS_PEAK_MEM_USAGE_SLOTS] = {0}; -int clientsCronTrackExpansiveClients(client *c) { +int clientsCronTrackExpansiveClients(client *c, int time_idx) { size_t in_usage = sdsZmallocSize(c->querybuf) + c->argv_len_sum + (c->argv ? zmalloc_size(c->argv) : 0); size_t out_usage = getClientOutputBufferMemoryUsage(c); - int i = server.unixtime % CLIENTS_PEAK_MEM_USAGE_SLOTS; - int zeroidx = (i+1) % CLIENTS_PEAK_MEM_USAGE_SLOTS; - - /* Always zero the next sample, so that when we switch to that second, we'll - * only register samples that are greater in that second without considering - * the history of such slot. - * - * Note: our index may jump to any random position if serverCron() is not - * called for some reason with the normal frequency, for instance because - * some slow command is called taking multiple seconds to execute. In that - * case our array may end containing data which is potentially older - * than CLIENTS_PEAK_MEM_USAGE_SLOTS seconds: however this is not a problem - * since here we want just to track if "recently" there were very expansive - * clients from the POV of memory usage. */ - ClientsPeakMemInput[zeroidx] = 0; - ClientsPeakMemOutput[zeroidx] = 0; /* Track the biggest values observed so far in this slot. */ - if (in_usage > ClientsPeakMemInput[i]) ClientsPeakMemInput[i] = in_usage; - if (out_usage > ClientsPeakMemOutput[i]) ClientsPeakMemOutput[i] = out_usage; + if (in_usage > ClientsPeakMemInput[time_idx]) ClientsPeakMemInput[time_idx] = in_usage; + if (out_usage > ClientsPeakMemOutput[time_idx]) ClientsPeakMemOutput[time_idx] = out_usage; return 0; /* This function never terminates the client. */ } @@ -1828,6 +1812,24 @@ void clientsCron(void) { iterations = (numclients < CLIENTS_CRON_MIN_ITERATIONS) ? numclients : CLIENTS_CRON_MIN_ITERATIONS; + + int curr_peak_mem_usage_slot = server.unixtime % CLIENTS_PEAK_MEM_USAGE_SLOTS; + /* Always zero the next sample, so that when we switch to that second, we'll + * only register samples that are greater in that second without considering + * the history of such slot. + * + * Note: our index may jump to any random position if serverCron() is not + * called for some reason with the normal frequency, for instance because + * some slow command is called taking multiple seconds to execute. In that + * case our array may end containing data which is potentially older + * than CLIENTS_PEAK_MEM_USAGE_SLOTS seconds: however this is not a problem + * since here we want just to track if "recently" there were very expansive + * clients from the POV of memory usage. */ + int zeroidx = (curr_peak_mem_usage_slot+1) % CLIENTS_PEAK_MEM_USAGE_SLOTS; + ClientsPeakMemInput[zeroidx] = 0; + ClientsPeakMemOutput[zeroidx] = 0; + + while(listLength(server.clients) && iterations--) { client *c; listNode *head; @@ -1843,7 +1845,7 @@ void clientsCron(void) { * terminated. */ if (clientsCronHandleTimeout(c,now)) continue; if (clientsCronResizeQueryBuffer(c)) continue; - if (clientsCronTrackExpansiveClients(c)) continue; + if (clientsCronTrackExpansiveClients(c, curr_peak_mem_usage_slot)) continue; if (clientsCronTrackClientsMemUsage(c)) continue; } } From ea1b48bd12d4cbd6634e5a478da99c6a1f14be8e Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Wed, 3 Mar 2021 10:08:06 +0200 Subject: [PATCH 005/131] Improve SSL cleanup handling. (#8589) This solves the problem of /dev/random and /dev/urandom open file descriptors leaking to childs with some versions of OpenSSL. --- src/sentinel.c | 1 + src/server.h | 1 + src/tls.c | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/sentinel.c b/src/sentinel.c index 164ff15b2..90b97940c 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -870,6 +870,7 @@ void sentinelRunPendingScripts(void) { sj->pid = 0; } else if (pid == 0) { /* Child */ + tlsCleanup(); execve(sj->argv[0],sj->argv,environ); /* If we are here an error occurred. */ _exit(2); /* Don't retry execution. */ diff --git a/src/server.h b/src/server.h index 17fd9fc9a..da4440a21 100644 --- a/src/server.h +++ b/src/server.h @@ -2715,6 +2715,7 @@ void makeThreadKillable(void); /* TLS stuff */ void tlsInit(void); +void tlsCleanup(void); int tlsConfigure(redisTLSContextConfig *ctx_config); #define redisDebug(fmt, ...) \ diff --git a/src/tls.c b/src/tls.c index bcfe53a35..810f0faab 100644 --- a/src/tls.c +++ b/src/tls.c @@ -147,7 +147,7 @@ void tlsInit(void) { #if OPENSSL_VERSION_NUMBER < 0x10100000L OPENSSL_config(NULL); #else - OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG|OPENSSL_INIT_ATFORK, NULL); #endif ERR_load_crypto_strings(); SSL_load_error_strings(); @@ -164,6 +164,21 @@ void tlsInit(void) { pending_list = listCreate(); } +void tlsCleanup(void) { + if (redis_tls_ctx) { + SSL_CTX_free(redis_tls_ctx); + redis_tls_ctx = NULL; + } + if (redis_tls_client_ctx) { + SSL_CTX_free(redis_tls_client_ctx); + redis_tls_client_ctx = NULL; + } + + #if OPENSSL_VERSION_NUMBER >= 0x10100000L + OPENSSL_cleanup(); + #endif +} + /* Create a *base* SSL_CTX using the SSL configuration provided. The base context * includes everything that's common for both client-side and server-side connections. */ @@ -948,6 +963,9 @@ sds connTLSGetPeerCert(connection *conn_) { void tlsInit(void) { } +void tlsCleanup(void) { +} + int tlsConfigure(redisTLSContextConfig *ctx_config) { UNUSED(ctx_config); return C_OK; From ac91822952c512f6881d79fdbf2106e609f5cb0e Mon Sep 17 00:00:00 2001 From: YaacovHazan <31382944+YaacovHazan@users.noreply.github.com> Date: Thu, 4 Mar 2021 13:02:23 +0200 Subject: [PATCH 006/131] Fix RedisModule_IsAOFClient Redis Module API (#8596) Since the API declared (as #define) in redismodule.h and uses the CLIENT_ID_AOF that declared in the server.h, when a module will want to make use of this API, it will get a compilation error (module doesn't include the server.h). The API was broken by d6eb3af (failed attempt for a cleanup). Revert to the original version of RedisModule_IsAOFClient that uses UINT64_MAX instead of CLIENT_ID_AOF --- src/redismodule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redismodule.h b/src/redismodule.h index ea271b82b..e590ec48d 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -835,7 +835,7 @@ REDISMODULE_API int (*RedisModule_DefragCursorSet)(RedisModuleDefragCtx *ctx, un REDISMODULE_API int (*RedisModule_DefragCursorGet)(RedisModuleDefragCtx *ctx, unsigned long *cursor) REDISMODULE_ATTR; #endif -#define RedisModule_IsAOFClient(id) ((id) == CLIENT_ID_AOF) +#define RedisModule_IsAOFClient(id) ((id) == UINT64_MAX) /* This is included inline inside each Redis module. */ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int apiver) REDISMODULE_ATTR_UNUSED; From f07b7393a0f4b1348342c02849b4c389ac45372e Mon Sep 17 00:00:00 2001 From: sundb Date: Thu, 4 Mar 2021 19:12:00 +0800 Subject: [PATCH 007/131] Fix memory overlap in quicklistRotate (#8599) When the length of the quicklist is 1(only one zipmap), the rotate operation will cause memory overlap when moving an entity from the tail of the zipmap to the head. quicklistRotate is a dead code, so it has no impact on the existing code. --- src/quicklist.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/quicklist.c b/src/quicklist.c index 7b7aa7839..1095f8791 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -1296,17 +1296,24 @@ void quicklistRotate(quicklist *quicklist) { /* First, get the tail entry */ unsigned char *p = ziplistIndex(quicklist->tail->zl, -1); - unsigned char *value; + unsigned char *value, *tmp; long long longval; unsigned int sz; char longstr[32] = {0}; - ziplistGet(p, &value, &sz, &longval); + ziplistGet(p, &tmp, &sz, &longval); /* If value found is NULL, then ziplistGet populated longval instead */ - if (!value) { + if (!tmp) { /* Write the longval as a string so we can re-add it */ sz = ll2string(longstr, sizeof(longstr), longval); value = (unsigned char *)longstr; + } else if (quicklist->len == 1) { + /* Copy buffer since there could be a memory overlap when move + * entity from tail to head in the same ziplist. */ + value = zmalloc(sz); + memcpy(value, tmp, sz); + } else { + value = tmp; } /* Add tail entry to head (must happen before tail is deleted). */ @@ -1321,6 +1328,8 @@ void quicklistRotate(quicklist *quicklist) { /* Remove tail entry. */ quicklistDelIndex(quicklist, quicklist->tail, &p); + if (value != (unsigned char*)longstr && value != tmp) + zfree(value); } /* pop from quicklist and return result in 'data' ptr. Value of 'data' From 3c7d6a185329f2dc20c82a29bdbe125d5ad4140b Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Thu, 4 Mar 2021 15:03:49 +0200 Subject: [PATCH 008/131] Improve redis-cli non-binary safe string handling. (#8566) * The `redis-cli --scan` output should honor output mode (set explicitly or implicitly), and quote key names when not in raw mode. * Technically this is a breaking change, but it should be very minor since raw mode is by default on for non-tty output. * It should only affect TTY output (human users) or non-tty output if `--no-raw` is specified. * Added `--quoted-input` option to treat all arguments as potentially quoted strings. * Added `--quoted-pattern` option to accept a potentially quoted pattern. Unquoting is applied to potentially quoted input only if single or double quotes are used. Fixes #8561, #8563 --- src/redis-cli.c | 121 ++++++++++++++++++++------------ tests/integration/redis-cli.tcl | 36 ++++++++++ 2 files changed, 113 insertions(+), 44 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 63d31f79a..63cd4e463 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -227,7 +227,7 @@ static struct config { int scan_mode; int intrinsic_latency_mode; int intrinsic_latency_duration; - char *pattern; + sds pattern; char *rdb_filename; int bigkeys; int memkeys; @@ -237,6 +237,7 @@ static struct config { char *auth; int askpass; char *user; + int quoted_input; /* Force input args to be treated as quoted strings */ int output; /* output mode, see OUTPUT_* defines */ int push_output; /* Should we display spontaneous PUSH replies */ sds mb_delim; @@ -763,6 +764,23 @@ static void freeHintsCallback(void *ptr) { * Networking / parsing *--------------------------------------------------------------------------- */ +/* Unquote a null-terminated string and return it as a binary-safe sds. */ +static sds unquoteCString(char *str) { + int count; + sds *unquoted = sdssplitargs(str, &count); + sds res = NULL; + + if (unquoted && count == 1) { + res = unquoted[0]; + unquoted[0] = NULL; + } + + if (unquoted) + sdsfreesplitres(unquoted, count); + + return res; +} + /* Send AUTH command to the server */ static int cliAuth(redisContext *ctx, char *user, char *auth) { redisReply *reply; @@ -1533,6 +1551,8 @@ static int parseOptions(int argc, char **argv) { config.output = OUTPUT_RAW; } else if (!strcmp(argv[i],"--no-raw")) { config.output = OUTPUT_STANDARD; + } else if (!strcmp(argv[i],"--quoted-input")) { + config.quoted_input = 1; } else if (!strcmp(argv[i],"--csv")) { config.output = OUTPUT_CSV; } else if (!strcmp(argv[i],"--latency")) { @@ -1557,7 +1577,15 @@ static int parseOptions(int argc, char **argv) { } else if (!strcmp(argv[i],"--scan")) { config.scan_mode = 1; } else if (!strcmp(argv[i],"--pattern") && !lastarg) { - config.pattern = argv[++i]; + sdsfree(config.pattern); + config.pattern = sdsnew(argv[++i]); + } else if (!strcmp(argv[i],"--quoted-pattern") && !lastarg) { + sdsfree(config.pattern); + config.pattern = unquoteCString(argv[++i]); + if (!config.pattern) { + fprintf(stderr,"Invalid quoted string specified for --quoted-pattern.\n"); + exit(1); + } } else if (!strcmp(argv[i],"--intrinsic-latency") && !lastarg) { config.intrinsic_latency_mode = 1; config.intrinsic_latency_duration = atoi(argv[++i]); @@ -1841,6 +1869,7 @@ static void usage(void) { " --raw Use raw formatting for replies (default when STDOUT is\n" " not a tty).\n" " --no-raw Force formatted output even when STDOUT is not a tty.\n" +" --quoted-input Force input to be handled as quoted strings.\n" " --csv Output in CSV format.\n" " --show-pushes Whether to print RESP3 PUSH messages. Enabled by default when\n" " STDOUT is a tty but can be overriden with --show-pushes no.\n" @@ -1876,6 +1905,8 @@ static void usage(void) { " --scan List all keys using the SCAN command.\n" " --pattern Keys pattern when using the --scan, --bigkeys or --hotkeys\n" " options (default: *).\n" +" --quoted-pattern Same as --pattern, but the specified string can be\n" +" quoted, in order to pass an otherwise non binary-safe string.\n" " --intrinsic-latency Run a test to measure intrinsic system latency.\n" " The test will run for the specified amount of seconds.\n" " --eval Send an EVAL command using the Lua script at .\n" @@ -1901,6 +1932,7 @@ static void usage(void) { " redis-cli get mypasswd\n" " redis-cli -r 100 lpush mylist x\n" " redis-cli -r 100 -i 1 info | grep used_memory_human:\n" +" redis-cli --quoted-input set '\"null-\\x00-separated\"' value\n" " redis-cli --eval myscript.lua key1 key2 , arg1 arg2 arg3\n" " redis-cli --scan --pattern '*:12345*'\n" "\n" @@ -1930,22 +1962,28 @@ static int confirmWithYes(char *msg, int ignore_force) { return (nread != 0 && !strcmp("yes", buf)); } -/* Turn the plain C strings into Sds strings */ -static char **convertToSds(int count, char** args) { - int j; - char **sds = zmalloc(sizeof(char*)*count); +/* Create an sds array from argv, either as-is or by dequoting every + * element. When quoted is non-zero, may return a NULL to indicate an + * invalid quoted string. + */ +static sds *getSdsArrayFromArgv(int argc, char **argv, int quoted) { + sds *res = sds_malloc(sizeof(sds) * argc); - for(j = 0; j < count; j++) - sds[j] = sdsnew(args[j]); + for (int j = 0; j < argc; j++) { + if (quoted) { + sds unquoted = unquoteCString(argv[j]); + if (!unquoted) { + while (--j >= 0) sdsfree(res[j]); + sds_free(res); + return NULL; + } + res[j] = unquoted; + } else { + res[j] = sdsnew(argv[j]); + } + } - return sds; -} - -static void freeConvertedSds(int count, char **sds) { - int j; - for (j = 0; j < count; j++) - sdsfree(sds[j]); - zfree(sds); + return res; } static int issueCommandRepeat(int argc, char **argv, long repeat) { @@ -2178,17 +2216,19 @@ static void repl(void) { static int noninteractive(int argc, char **argv) { int retval = 0; - - argv = convertToSds(argc, argv); - if (config.stdinarg) { - argv = zrealloc(argv, (argc+1)*sizeof(char*)); - argv[argc] = readArgFromStdin(); - retval = issueCommand(argc+1, argv); - sdsfree(argv[argc]); - } else { - retval = issueCommand(argc, argv); + sds *sds_args = getSdsArrayFromArgv(argc, argv, config.quoted_input); + if (!sds_args) { + printf("Invalid quoted string\n"); + return 1; } - freeConvertedSds(argc, argv); + if (config.stdinarg) { + sds_args = sds_realloc(sds_args, (argc + 1) * sizeof(sds)); + sds_args[argc] = readArgFromStdin(); + argc++; + } + + retval = issueCommand(argc, sds_args); + sdsfreesplitres(sds_args, argc); return retval; } @@ -7332,8 +7372,8 @@ static redisReply *sendScan(unsigned long long *it) { redisReply *reply; if (config.pattern) - reply = redisCommand(context,"SCAN %llu MATCH %s", - *it,config.pattern); + reply = redisCommand(context, "SCAN %llu MATCH %b", + *it, config.pattern, sdslen(config.pattern)); else reply = redisCommand(context,"SCAN %llu",*it); @@ -7945,23 +7985,16 @@ static void scanMode(void) { unsigned long long cur = 0; do { - if (config.pattern) - reply = redisCommand(context,"SCAN %llu MATCH %s", - cur,config.pattern); - else - reply = redisCommand(context,"SCAN %llu",cur); - if (reply == NULL) { - printf("I/O error\n"); - exit(1); - } else if (reply->type == REDIS_REPLY_ERROR) { - printf("ERROR: %s\n", reply->str); - exit(1); - } else { - unsigned int j; - - cur = strtoull(reply->element[0]->str,NULL,10); - for (j = 0; j < reply->element[1]->elements; j++) + reply = sendScan(&cur); + for (unsigned int j = 0; j < reply->element[1]->elements; j++) { + if (config.output == OUTPUT_STANDARD) { + sds out = sdscatrepr(sdsempty(), reply->element[1]->element[j]->str, + reply->element[1]->element[j]->len); + printf("%s\n", out); + sdsfree(out); + } else { printf("%s\n", reply->element[1]->element[j]->str); + } } freeReplyObject(reply); } while(cur != 0); diff --git a/tests/integration/redis-cli.tcl b/tests/integration/redis-cli.tcl index d877542ed..7e8b41fca 100644 --- a/tests/integration/redis-cli.tcl +++ b/tests/integration/redis-cli.tcl @@ -207,6 +207,28 @@ start_server {tags {"cli"}} { assert_equal "foo\nbar" [run_cli lrange list 0 -1] } + test_nontty_cli "Quoted input arguments" { + r set "\x00\x00" "value" + assert_equal "value" [run_cli --quoted-input get {"\x00\x00"}] + } + + test_nontty_cli "No accidental unquoting of input arguments" { + run_cli --quoted-input set {"\x41\x41"} quoted-val + run_cli set {"\x41\x41"} unquoted-val + + assert_equal "quoted-val" [r get AA] + assert_equal "unquoted-val" [r get {"\x41\x41"}] + } + + test_nontty_cli "Invalid quoted input arguments" { + catch {run_cli --quoted-input set {"Unterminated}} err + assert_match {*exited abnormally*} $err + + # A single arg that unquotes to two arguments is also not expected + catch {run_cli --quoted-input set {"arg1" "arg2"}} err + assert_match {*exited abnormally*} $err + } + test_nontty_cli "Read last argument from pipe" { assert_equal "OK" [run_cli_with_input_pipe "echo foo" set key] assert_equal "foo\n" [r get key] @@ -247,6 +269,20 @@ start_server {tags {"cli"}} { test_redis_cli_rdb_dump } + test "Scan mode" { + r flushdb + populate 1000 key: 1 + + # basic use + assert_equal 1000 [llength [split [run_cli --scan]]] + + # pattern + assert_equal {key:2} [run_cli --scan --pattern "*:2"] + + # pattern matching with a quoted string + assert_equal {key:2} [run_cli --scan --quoted-pattern {"*:\x32"}] + } + test "Connecting as a replica" { set fd [open_cli "--replica"] wait_for_condition 500 500 { From 55c74617d754ee3b3112fa5146aeba425ac53db8 Mon Sep 17 00:00:00 2001 From: uriyage <78144248+uriyage@users.noreply.github.com> Date: Fri, 5 Mar 2021 19:29:52 +0200 Subject: [PATCH 009/131] remove unused latency variable (#8597) Remove unused latency variable from redis-cli --- src/redis-benchmark.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 186739766..5a9b37d27 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -102,7 +102,6 @@ static struct config { int showerrors; long long start; long long totlatency; - long long *latency; const char *title; list *clients; int quiet; From 3d9c6a7ce60f58ca087cbd533d9b769a8a455309 Mon Sep 17 00:00:00 2001 From: yanchaozhong Date: Sat, 6 Mar 2021 01:32:11 +0800 Subject: [PATCH 010/131] Fix typo in sentinel.c (#8600) --- src/sentinel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentinel.c b/src/sentinel.c index 90b97940c..266462466 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -1732,7 +1732,7 @@ char *sentinelGetInstanceTypeString(sentinelRedisInstance *ri) { * with CONFIG and SLAVEOF commands renamed for security concerns. In that * case we check the ri->renamed_command table (or if the instance is a slave, * we check the one of the master), and map the command that we should send - * to the set of renamed commads. However, if the command was not renamed, + * to the set of renamed commands. However, if the command was not renamed, * we just return "command" itself. */ char *sentinelInstanceMapCommand(sentinelRedisInstance *ri, char *command) { sds sc = sdsnew(command); From 367ba4125fa08440f1e33799ccf2fedfe88129aa Mon Sep 17 00:00:00 2001 From: Huang Zhw Date: Sat, 6 Mar 2021 01:54:34 +0800 Subject: [PATCH 011/131] Remove some dead code (#8605) --- src/config.c | 2 -- src/server.h | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/config.c b/src/config.c index 1d6ce6b8c..2f4a13ae8 100644 --- a/src/config.c +++ b/src/config.c @@ -229,8 +229,6 @@ typedef union typeData { typedef struct typeInterface { /* Called on server start, to init the server with default value */ void (*init)(typeData data); - /* Called on server start, should return 1 on success, 0 on error and should set err */ - int (*load)(typeData data, sds *argc, int argv, const char **err); /* Called on server startup and CONFIG SET, returns 1 on success, 0 on error * and can set a verbose err string, update is true when called from CONFIG SET */ int (*set)(typeData data, sds value, int update, const char **err); diff --git a/src/server.h b/src/server.h index da4440a21..ef1ac1e48 100644 --- a/src/server.h +++ b/src/server.h @@ -735,8 +735,6 @@ typedef struct multiState { int cmd_inv_flags; /* Same as cmd_flags, OR-ing the ~flags. so that it is possible to know if all the commands have a certain flag. */ - int minreplicas; /* MINREPLICAS for synchronous replication */ - time_t minreplicas_timeout; /* MINREPLICAS timeout as unixtime. */ } multiState; /* This structure holds the blocking operation state for a client. @@ -762,7 +760,6 @@ typedef struct blockingState { size_t xread_count; /* XREAD COUNT option. */ robj *xread_group; /* XREADGROUP group name. */ robj *xread_consumer; /* XREADGROUP consumer name. */ - mstime_t xread_retry_time, xread_retry_ttl; int xread_group_noack; /* BLOCKED_WAIT */ From e8e6ca630934ff2fa5581aced1dc0dad110d8b40 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Sun, 7 Mar 2021 14:14:23 +0200 Subject: [PATCH 012/131] Fix FreeBSD <12.x builds. (#8603) --- src/zmalloc.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/zmalloc.h b/src/zmalloc.h index d44c7b389..3c0ba95d4 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -71,12 +71,21 @@ */ #ifndef ZMALLOC_LIB #define ZMALLOC_LIB "libc" + #if !defined(NO_MALLOC_USABLE_SIZE) && \ (defined(__GLIBC__) || defined(__FreeBSD__) || \ defined(USE_MALLOC_USABLE_SIZE)) + +/* Includes for malloc_usable_size() */ +#ifdef __FreeBSD__ +#include +#else #include +#endif + #define HAVE_MALLOC_SIZE 1 #define zmalloc_size(p) malloc_usable_size(p) + #endif #endif From f491eee2a1ef44b0b62380ee07f873e0c22be5e0 Mon Sep 17 00:00:00 2001 From: Huang Zhw Date: Mon, 8 Mar 2021 00:09:12 +0800 Subject: [PATCH 013/131] Cleanup: dictEncObjHash remove redundant conditional statement (#8488) --- src/server.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/server.c b/src/server.c index 20ada1404..26fe57499 100644 --- a/src/server.c +++ b/src/server.c @@ -1332,21 +1332,14 @@ uint64_t dictEncObjHash(const void *key) { if (sdsEncodedObject(o)) { return dictGenHashFunction(o->ptr, sdslen((sds)o->ptr)); + } else if (o->encoding == OBJ_ENCODING_INT) { + char buf[32]; + int len; + + len = ll2string(buf,32,(long)o->ptr); + return dictGenHashFunction((unsigned char*)buf, len); } else { - if (o->encoding == OBJ_ENCODING_INT) { - char buf[32]; - int len; - - len = ll2string(buf,32,(long)o->ptr); - return dictGenHashFunction((unsigned char*)buf, len); - } else { - uint64_t hash; - - o = getDecodedObject(o); - hash = dictGenHashFunction(o->ptr, sdslen((sds)o->ptr)); - decrRefCount(o); - return hash; - } + serverPanic("Unknown string encoding"); } } From 20377a6f3d7e5159df22b91555f70292e3b39f04 Mon Sep 17 00:00:00 2001 From: Pavlo Yatsukhnenko Date: Mon, 8 Mar 2021 12:57:27 +0200 Subject: [PATCH 014/131] Wrong usage sdscatprintf in redis-cli. (#8604) The result of `sdscatprintf` is doubled when using argument twice. --- src/redis-cli.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 63cd4e463..fa6905cc0 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -2953,8 +2953,12 @@ static int clusterManagerGetAntiAffinityScore(clusterManagerNodeArray *ipnodes, else types = sdsempty(); /* Master type 'm' is always set as the first character of the * types string. */ - if (!node->replicate) types = sdscatprintf(types, "m%s", types); - else types = sdscat(types, "s"); + if (node->replicate) types = sdscat(types, "s"); + else { + sds s = sdscatsds(sdsnew("m"), types); + sdsfree(types); + types = s; + } dictReplace(related, key, types); } /* Now it's trivial to check, for each related group having the From e58118cda6cacb5a845a9cf8eda45074470e31b2 Mon Sep 17 00:00:00 2001 From: guybe7 Date: Mon, 8 Mar 2021 18:00:19 +0100 Subject: [PATCH 015/131] Fix edge-case when a module client is unblocked (#8618) Scenario: 1. A module client is blocked on keys with a timeout 2. Shortly before the timeout expires, the key is being populated and signaled as ready 3. Redis calls moduleTryServeClientBlockedOnKey (which replies to client) and then moduleUnblockClient 4. moduleUnblockClient doesn't really unblock the client, it writes to server.module_blocked_pipe and only marks the BC as unblocked. 5. beforeSleep kics in, by this time the client still exists and techincally timed-out. beforeSleep replies to the timeout client (double reply) and only then moduleHandleBlockedClients is called, reading from module_blocked_pipe and calling unblockClient The solution is similar to what was done in moduleTryServeClientBlockedOnKey: we should avoid re-processing an already-unblocked client --- src/module.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/module.c b/src/module.c index 871399cff..b4fc3e856 100644 --- a/src/module.c +++ b/src/module.c @@ -5550,6 +5550,12 @@ void moduleHandleBlockedClients(void) { * API to unblock the client and the memory will be released. */ void moduleBlockedClientTimedOut(client *c) { RedisModuleBlockedClient *bc = c->bpop.module_blocked_handle; + + /* Protect against re-processing: don't serve clients that are already + * in the unblocking list for any reason (including RM_UnblockClient() + * explicit call). See #6798. */ + if (bc->unblocked) return; + RedisModuleCtx ctx = REDISMODULE_CTX_INIT; ctx.flags |= REDISMODULE_CTX_BLOCKED_TIMEOUT; ctx.module = bc->module; From 7d81f39222360cf537f86f7c35111fbe53287885 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Mon, 8 Mar 2021 20:53:53 +0200 Subject: [PATCH 016/131] Fix flaky unit/maxmemory test on MacOS/BSD. (#8619) It seems like non-Linux sockets may be less greedy, resulting with more transient client output buffers. Haven't proven this but empirically when stressing this test on non-Linux tends to exhibit increased mem_clients_normal values. --- tests/unit/maxmemory.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/maxmemory.tcl b/tests/unit/maxmemory.tcl index ed5657860..99321acfe 100644 --- a/tests/unit/maxmemory.tcl +++ b/tests/unit/maxmemory.tcl @@ -178,7 +178,7 @@ proc test_slave_buffers {test_name cmd_count payload_len limit_memory pipeline} set orig_client_buf [s -1 mem_clients_normal] set orig_mem_not_counted_for_evict [s -1 mem_not_counted_for_evict] set orig_used_no_repl [expr {$orig_used - $orig_mem_not_counted_for_evict}] - set limit [expr {$orig_used - $orig_mem_not_counted_for_evict + 20*1024}] + set limit [expr {$orig_used - $orig_mem_not_counted_for_evict + 32*1024}] if {$limit_memory==1} { $master config set maxmemory $limit From 817894c012ee67cdf8e33e0a098025f65521ba42 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Mon, 8 Mar 2021 21:22:08 +0200 Subject: [PATCH 017/131] Fix test false positive due to a race condition. (#8616) --- tests/unit/other.tcl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 16ed092be..b35172aba 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -309,6 +309,12 @@ start_server {tags {"other"}} { populate 4096 "" 1 r bgsave + wait_for_condition 10 100 { + [s rdb_bgsave_in_progress] eq 1 + } else { + fail "bgsave did not start in time" + } + r mset k1 v1 k2 v2 # Hash table should not rehash assert_no_match "*table size: 8192*" [r debug HTSTATS 9] From 9b4edfdf08a99adda0b6da5b1434d14884d6419b Mon Sep 17 00:00:00 2001 From: Huang Zhw Date: Tue, 9 Mar 2021 03:43:09 +0800 Subject: [PATCH 018/131] __quicklistCompress may compress more node than required (#8311) When a quicklist has quicklist->compress * 2 nodes, then call __quicklistCompress, all nodes will be decompressed and the middle two nodes will be recompressed again. This violates the fact that quicklist->compress * 2 nodes are uncompressed. It's harmless because when visit a node, we always try to uncompress node first. This only happened when a quicklist has quicklist->compress * 2 + 1 nodes, then delete a node. For other scenarios like insert node and iterate this will not happen. --- src/quicklist.c | 74 ++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/src/quicklist.c b/src/quicklist.c index 1095f8791..ae9010b9d 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -315,7 +315,9 @@ REDIS_STATIC void __quicklistCompress(const quicklist *quicklist, if (forward == node || reverse == node) in_depth = 1; - if (forward == reverse) + /* We passed into compress depth of opposite side of the quicklist + * so there's no need to compress anything and we can exit. */ + if (forward == reverse || forward->next == reverse) return; forward = forward->next; @@ -325,11 +327,9 @@ REDIS_STATIC void __quicklistCompress(const quicklist *quicklist, if (!in_depth) quicklistCompressNode(node); - if (depth > 2) { - /* At this point, forward and reverse are one node beyond depth */ - quicklistCompressNode(forward); - quicklistCompressNode(reverse); - } + /* At this point, forward and reverse are one node beyond depth */ + quicklistCompressNode(forward); + quicklistCompressNode(reverse); } #define quicklistCompress(_ql, _node) \ @@ -380,10 +380,11 @@ REDIS_STATIC void __quicklistInsertNode(quicklist *quicklist, quicklist->head = quicklist->tail = new_node; } + /* Update len first, so in __quicklistCompress we know exactly len */ + quicklist->len++; + if (old_node) quicklistCompress(quicklist, old_node); - - quicklist->len++; } /* Wrappers for node inserting around existing node. */ @@ -602,15 +603,16 @@ REDIS_STATIC void __quicklistDelNode(quicklist *quicklist, quicklist->head = node->next; } + /* Update len first, so in __quicklistCompress we know exactly len */ + quicklist->len--; + quicklist->count -= node->count; + /* If we deleted a node within our compress depth, we * now have compressed nodes needing to be decompressed. */ __quicklistCompress(quicklist, NULL); - quicklist->count -= node->count; - zfree(node->zl); zfree(node); - quicklist->len--; } /* Delete one entry from list given the node for the entry and a pointer @@ -2706,30 +2708,40 @@ int quicklistTest(int argc, char *argv[]) { quicklistPushHead(ql, genstr("hello HEAD", i + 1), 64); } - quicklistNode *node = ql->head; - unsigned int low_raw = ql->compress; - unsigned int high_raw = ql->len - ql->compress; - - for (unsigned int at = 0; at < ql->len; - at++, node = node->next) { - if (at < low_raw || at >= high_raw) { - if (node->encoding != QUICKLIST_NODE_ENCODING_RAW) { - ERR("Incorrect compression: node %d is " - "compressed at depth %d ((%u, %u); total " - "nodes: %lu; size: %u)", - at, depth, low_raw, high_raw, ql->len, - node->sz); + for (int step = 0; step < 2; step++) { + /* test remove node */ + if (step == 1) { + for (int i = 0; i < list_sizes[list] / 2; i++) { + quicklistPop(ql, QUICKLIST_HEAD, NULL, NULL, NULL); + quicklistPop(ql, QUICKLIST_TAIL, NULL, NULL, NULL); } - } else { - if (node->encoding != QUICKLIST_NODE_ENCODING_LZF) { - ERR("Incorrect non-compression: node %d is NOT " - "compressed at depth %d ((%u, %u); total " - "nodes: %lu; size: %u; attempted: %d)", - at, depth, low_raw, high_raw, ql->len, - node->sz, node->attempted_compress); + } + quicklistNode *node = ql->head; + unsigned int low_raw = ql->compress; + unsigned int high_raw = ql->len - ql->compress; + + for (unsigned int at = 0; at < ql->len; + at++, node = node->next) { + if (at < low_raw || at >= high_raw) { + if (node->encoding != QUICKLIST_NODE_ENCODING_RAW) { + ERR("Incorrect compression: node %d is " + "compressed at depth %d ((%u, %u); total " + "nodes: %lu; size: %u)", + at, depth, low_raw, high_raw, ql->len, + node->sz); + } + } else { + if (node->encoding != QUICKLIST_NODE_ENCODING_LZF) { + ERR("Incorrect non-compression: node %d is NOT " + "compressed at depth %d ((%u, %u); total " + "nodes: %lu; size: %u; attempted: %d)", + at, depth, low_raw, high_raw, ql->len, + node->sz, node->attempted_compress); + } } } } + quicklistRelease(ql); } } From af2175326c87231d16c5633dd8febbc86a258540 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 9 Mar 2021 11:33:32 +0200 Subject: [PATCH 019/131] Fix memory info on FreeBSD. (#8620) The obtained process_rss was incorrect (the OS reports pages, not bytes), resulting with many other fields getting corrupted. This has been tested on FreeBSD but not other platforms. --- src/zmalloc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/zmalloc.c b/src/zmalloc.c index 1dc662e57..e212583a4 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -414,9 +414,9 @@ size_t zmalloc_get_rss(void) { if (sysctl(mib, 4, &info, &infolen, NULL, 0) == 0) #if defined(__FreeBSD__) - return (size_t)info.ki_rssize; + return (size_t)info.ki_rssize * getpagesize(); #else - return (size_t)info.kp_vm_rssize; + return (size_t)info.kp_vm_rssize * getpagesize(); #endif return 0L; @@ -436,7 +436,7 @@ size_t zmalloc_get_rss(void) { mib[4] = sizeof(info); mib[5] = 1; if (sysctl(mib, 4, &info, &infolen, NULL, 0) == 0) - return (size_t)info.p_vm_rssize; + return (size_t)info.p_vm_rssize * getpagesize(); return 0L; } From 25f8d4fb4180f0eb893b1ea0e6ba973bab77b0f5 Mon Sep 17 00:00:00 2001 From: Harkrishn Patro <30795839+hpatro@users.noreply.github.com> Date: Tue, 9 Mar 2021 11:35:41 +0100 Subject: [PATCH 020/131] Run daily workflows only on redis/redis repo. (#8625) Co-authored-by: Harkrishn Patro --- .github/workflows/daily.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 59d236d9a..49a1e5b4b 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -259,6 +259,7 @@ jobs: test-alpine-jemalloc: runs-on: ubuntu-latest + if: github.repository == 'redis/redis' container: alpine:latest steps: - uses: actions/checkout@v2 @@ -279,6 +280,7 @@ jobs: test-alpine-libc-malloc: runs-on: ubuntu-latest + if: github.repository == 'redis/redis' container: alpine:latest steps: - uses: actions/checkout@v2 From 1ccfd6a1f79aee9e601092b2b0e6e1c68901deaf Mon Sep 17 00:00:00 2001 From: luhuachao Date: Tue, 9 Mar 2021 20:50:04 +0800 Subject: [PATCH 021/131] Clean redis-benchmark multi-threaded output (#8615) --- src/redis-benchmark.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 5a9b37d27..351335862 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -1655,7 +1655,10 @@ int showThroughput(struct aeEventLoop *eventLoop, long long id, void *clientData const float instantaneous_rps = (float)(requests_finished-previous_requests_finished)/instantaneous_dt; config.previous_tick = current_tick; atomicSet(config.previous_requests_finished,requests_finished); - config.last_printed_bytes = printf("%s: rps=%.1f (overall: %.1f) avg_msec=%.3f (overall: %.3f)\r", config.title, instantaneous_rps, rps, hdr_mean(config.current_sec_latency_histogram)/1000.0f, hdr_mean(config.latency_histogram)/1000.0f); + int printed_bytes = printf("%s: rps=%.1f (overall: %.1f) avg_msec=%.3f (overall: %.3f)\r", config.title, instantaneous_rps, rps, hdr_mean(config.current_sec_latency_histogram)/1000.0f, hdr_mean(config.latency_histogram)/1000.0f); + if (printed_bytes > config.last_printed_bytes){ + config.last_printed_bytes = printed_bytes; + } hdr_reset(config.current_sec_latency_histogram); fflush(stdout); return 250; /* every 250ms */ From 53774e69fa68450859a566e84f06d4b8a7a53eca Mon Sep 17 00:00:00 2001 From: uriyage <78144248+uriyage@users.noreply.github.com> Date: Tue, 9 Mar 2021 21:57:14 +0200 Subject: [PATCH 022/131] remove redundant check for evalCommand (#8565) --- src/cluster.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 4605e3ea9..06102141b 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -5824,18 +5824,14 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in * cluster is down. */ if (error_code) *error_code = CLUSTER_REDIR_DOWN_STATE; return NULL; - } else if ((cmd->flags & CMD_WRITE) && !(cmd->proc == evalCommand) - && !(cmd->proc == evalShaCommand)) - { - /* The cluster is configured to allow read only commands - * but this command is neither readonly, nor EVAL or - * EVALSHA. */ + } else if (cmd->flags & CMD_WRITE) { + /* The cluster is configured to allow read only commands */ if (error_code) *error_code = CLUSTER_REDIR_DOWN_RO_STATE; return NULL; } else { /* Fall through and allow the command to be executed: * this happens when server.cluster_allow_reads_when_down is - * true and the command is a readonly command or EVAL / EVALSHA. */ + * true and the command is not a write command */ } } @@ -5876,7 +5872,7 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in int is_write_command = (c->cmd->flags & CMD_WRITE) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_WRITE)); if (c->flags & CLIENT_READONLY && - (!is_write_command || cmd->proc == evalCommand || cmd->proc == evalShaCommand) && + !is_write_command && nodeIsSlave(myself) && myself->slaveof == n) { From 95d6297db868fc5400fb833c6753c8d25da486c7 Mon Sep 17 00:00:00 2001 From: sundb Date: Wed, 10 Mar 2021 15:13:11 +0800 Subject: [PATCH 023/131] Add run all test support with define REDIS_TEST (#8570) 1. Add `redis-server test all` support to run all tests. 2. Add redis test to daily ci. 3. Add `--accurate` option to run slow tests for more iterations (so that by default we run less cycles (shorter time, and less prints). 4. Move dict benchmark to REDIS_TEST. 5. fix some leaks in tests 6. make quicklist tests run on a specific fill set of options rather than huge ranges 7. move some prints in quicklist test outside their loops to reduce prints 8. removing sds.h from dict.c since it is now used in both redis-server and redis-cli (uses hiredis sds) --- .github/workflows/daily.yml | 14 +- src/Makefile | 5 +- src/crc64.c | 3 +- src/crc64.h | 2 +- src/dict.c | 64 +++++---- src/dict.h | 4 + src/endianconv.c | 3 +- src/endianconv.h | 2 +- src/intset.c | 13 +- src/intset.h | 2 +- src/quicklist.c | 255 +++++++++++++++--------------------- src/quicklist.h | 2 +- src/sds.c | 3 +- src/sds.h | 2 +- src/server.c | 86 ++++++++---- src/sha1.c | 3 +- src/sha1.h | 2 +- src/util.c | 3 +- src/util.h | 2 +- src/ziplist.c | 22 ++-- src/ziplist.h | 2 +- src/zipmap.c | 4 +- src/zipmap.h | 2 +- src/zmalloc.c | 3 +- src/zmalloc.h | 2 +- 25 files changed, 276 insertions(+), 229 deletions(-) diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 49a1e5b4b..ee9ac1bbf 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -17,7 +17,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: make - run: make + run: make REDIS_CFLAGS='-Werror -DREDIS_TEST' - name: test run: | sudo apt-get install tcl8.6 @@ -28,6 +28,8 @@ jobs: run: ./runtest-sentinel - name: cluster tests run: ./runtest-cluster + - name: unittest + run: ./src/redis-server test all test-ubuntu-libc-malloc: runs-on: ubuntu-latest @@ -76,7 +78,7 @@ jobs: - name: make run: | sudo apt-get update && sudo apt-get install libc6-dev-i386 - make 32bit + make 32bit REDIS_CFLAGS='-Werror -DREDIS_TEST' - name: test run: | sudo apt-get install tcl8.6 @@ -89,6 +91,8 @@ jobs: run: ./runtest-sentinel - name: cluster tests run: ./runtest-cluster + - name: unittest + run: ./src/redis-server test all test-ubuntu-tls: runs-on: ubuntu-latest @@ -142,7 +146,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: make - run: make valgrind + run: make valgrind REDIS_CFLAGS='-Werror -DREDIS_TEST' - name: test run: | sudo apt-get update @@ -150,6 +154,10 @@ jobs: ./runtest --valgrind --verbose --clients 1 --dump-logs - name: module api test run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 + - name: unittest + run: | + valgrind --track-origins=yes --suppressions=./src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full --log-file=err.txt ./src/redis-server test all + if grep -q 0x err.txt; then cat err.txt; exit 1; fi test-valgrind-no-malloc-usable-size: runs-on: ubuntu-latest diff --git a/src/Makefile b/src/Makefile index ecd69295f..591a06ad5 100644 --- a/src/Makefile +++ b/src/Makefile @@ -351,9 +351,6 @@ $(REDIS_CLI_NAME): $(REDIS_CLI_OBJ) $(REDIS_BENCHMARK_NAME): $(REDIS_BENCHMARK_OBJ) $(REDIS_LD) -o $@ $^ ../deps/hiredis/libhiredis.a ../deps/hdr_histogram/hdr_histogram.o $(FINAL_LIBS) -dict-benchmark: dict.c zmalloc.c sds.c siphash.c mt19937-64.c - $(REDIS_CC) $(FINAL_CFLAGS) $^ -D DICT_BENCHMARK_MAIN -o $@ $(FINAL_LIBS) - DEP = $(REDIS_SERVER_OBJ:%.o=%.d) $(REDIS_CLI_OBJ:%.o=%.d) $(REDIS_BENCHMARK_OBJ:%.o=%.d) -include $(DEP) @@ -364,7 +361,7 @@ DEP = $(REDIS_SERVER_OBJ:%.o=%.d) $(REDIS_CLI_OBJ:%.o=%.d) $(REDIS_BENCHMARK_OBJ $(REDIS_CC) -MMD -o $@ -c $< clean: - rm -rf $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) $(REDIS_CHECK_RDB_NAME) $(REDIS_CHECK_AOF_NAME) *.o *.gcda *.gcno *.gcov redis.info lcov-html Makefile.dep dict-benchmark + rm -rf $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) $(REDIS_CHECK_RDB_NAME) $(REDIS_CHECK_AOF_NAME) *.o *.gcda *.gcno *.gcov redis.info lcov-html Makefile.dep rm -f $(DEP) .PHONY: clean diff --git a/src/crc64.c b/src/crc64.c index 6c9432c4a..d4db4158e 100644 --- a/src/crc64.c +++ b/src/crc64.c @@ -127,9 +127,10 @@ uint64_t crc64(uint64_t crc, const unsigned char *s, uint64_t l) { #include #define UNUSED(x) (void)(x) -int crc64Test(int argc, char *argv[]) { +int crc64Test(int argc, char *argv[], int accurate) { UNUSED(argc); UNUSED(argv); + UNUSED(accurate); crc64_init(); printf("[calcula]: e9c6d914c4b8d9ca == %016" PRIx64 "\n", (uint64_t)_crc64(0, "123456789", 9)); diff --git a/src/crc64.h b/src/crc64.h index 60c42345f..38b0b6387 100644 --- a/src/crc64.h +++ b/src/crc64.h @@ -7,7 +7,7 @@ void crc64_init(void); uint64_t crc64(uint64_t crc, const unsigned char *s, uint64_t l); #ifdef REDIS_TEST -int crc64Test(int argc, char *argv[]); +int crc64Test(int argc, char *argv[], int accurate); #endif #endif diff --git a/src/dict.c b/src/dict.c index 4a9f3fb0a..21c616e6f 100644 --- a/src/dict.c +++ b/src/dict.c @@ -45,11 +45,7 @@ #include "dict.h" #include "zmalloc.h" -#ifndef DICT_BENCHMARK_MAIN #include "redisassert.h" -#else -#include -#endif /* Using dictEnableResize() / dictDisableResize() we make possible to * enable/disable resizing of the hash table as needed. This is very important @@ -1175,20 +1171,18 @@ void dictGetStats(char *buf, size_t bufsize, dict *d) { /* ------------------------------- Benchmark ---------------------------------*/ -#ifdef DICT_BENCHMARK_MAIN - -#include "sds.h" +#ifdef REDIS_TEST uint64_t hashCallback(const void *key) { - return dictGenHashFunction((unsigned char*)key, sdslen((char*)key)); + return dictGenHashFunction((unsigned char*)key, strlen((char*)key)); } int compareCallback(void *privdata, const void *key1, const void *key2) { int l1,l2; DICT_NOTUSED(privdata); - l1 = sdslen((sds)key1); - l2 = sdslen((sds)key2); + l1 = strlen((char*)key1); + l2 = strlen((char*)key2); if (l1 != l2) return 0; return memcmp(key1, key2, l1) == 0; } @@ -1196,7 +1190,19 @@ int compareCallback(void *privdata, const void *key1, const void *key2) { void freeCallback(void *privdata, void *val) { DICT_NOTUSED(privdata); - sdsfree(val); + zfree(val); +} + +char *stringFromLongLong(long long value) { + char buf[32]; + int len; + char *s; + + len = sprintf(buf,"%lld",value); + s = zmalloc(len+1); + memcpy(s, buf, len); + s[len] = '\0'; + return s; } dictType BenchmarkDictType = { @@ -1215,22 +1221,26 @@ dictType BenchmarkDictType = { printf(msg ": %ld items in %lld ms\n", count, elapsed); \ } while(0) -/* dict-benchmark [count] */ -int main(int argc, char **argv) { +/* ./redis-server test dict [ | --accurate] */ +int dictTest(int argc, char **argv, int accurate) { long j; long long start, elapsed; dict *dict = dictCreate(&BenchmarkDictType,NULL); long count = 0; - if (argc == 2) { - count = strtol(argv[1],NULL,10); + if (argc == 4) { + if (accurate) { + count = 5000000; + } else { + count = strtol(argv[3],NULL,10); + } } else { - count = 5000000; + count = 5000; } start_benchmark(); for (j = 0; j < count; j++) { - int retval = dictAdd(dict,sdsfromlonglong(j),(void*)j); + int retval = dictAdd(dict,stringFromLongLong(j),(void*)j); assert(retval == DICT_OK); } end_benchmark("Inserting"); @@ -1243,28 +1253,28 @@ int main(int argc, char **argv) { start_benchmark(); for (j = 0; j < count; j++) { - sds key = sdsfromlonglong(j); + char *key = stringFromLongLong(j); dictEntry *de = dictFind(dict,key); assert(de != NULL); - sdsfree(key); + zfree(key); } end_benchmark("Linear access of existing elements"); start_benchmark(); for (j = 0; j < count; j++) { - sds key = sdsfromlonglong(j); + char *key = stringFromLongLong(j); dictEntry *de = dictFind(dict,key); assert(de != NULL); - sdsfree(key); + zfree(key); } end_benchmark("Linear access of existing elements (2nd round)"); start_benchmark(); for (j = 0; j < count; j++) { - sds key = sdsfromlonglong(rand() % count); + char *key = stringFromLongLong(rand() % count); dictEntry *de = dictFind(dict,key); assert(de != NULL); - sdsfree(key); + zfree(key); } end_benchmark("Random access of existing elements"); @@ -1277,17 +1287,17 @@ int main(int argc, char **argv) { start_benchmark(); for (j = 0; j < count; j++) { - sds key = sdsfromlonglong(rand() % count); + char *key = stringFromLongLong(rand() % count); key[0] = 'X'; dictEntry *de = dictFind(dict,key); assert(de == NULL); - sdsfree(key); + zfree(key); } end_benchmark("Accessing missing"); start_benchmark(); for (j = 0; j < count; j++) { - sds key = sdsfromlonglong(j); + char *key = stringFromLongLong(j); int retval = dictDelete(dict,key); assert(retval == DICT_OK); key[0] += 17; /* Change first number to letter. */ @@ -1295,5 +1305,7 @@ int main(int argc, char **argv) { assert(retval == DICT_OK); } end_benchmark("Removing and adding"); + dictRelease(dict); + return 0; } #endif diff --git a/src/dict.h b/src/dict.h index bd57f859e..7e2258960 100644 --- a/src/dict.h +++ b/src/dict.h @@ -201,4 +201,8 @@ extern dictType dictTypeHeapStringCopyKey; extern dictType dictTypeHeapStrings; extern dictType dictTypeHeapStringCopyKeyValue; +#ifdef REDIS_TEST +int dictTest(int argc, char *argv[], int accurate); +#endif + #endif /* __DICT_H */ diff --git a/src/endianconv.c b/src/endianconv.c index 918844e25..98ed405a5 100644 --- a/src/endianconv.c +++ b/src/endianconv.c @@ -105,11 +105,12 @@ uint64_t intrev64(uint64_t v) { #include #define UNUSED(x) (void)(x) -int endianconvTest(int argc, char *argv[]) { +int endianconvTest(int argc, char *argv[], int accurate) { char buf[32]; UNUSED(argc); UNUSED(argv); + UNUSED(accurate); sprintf(buf,"ciaoroma"); memrev16(buf); diff --git a/src/endianconv.h b/src/endianconv.h index 475f72b08..004749786 100644 --- a/src/endianconv.h +++ b/src/endianconv.h @@ -72,7 +72,7 @@ uint64_t intrev64(uint64_t v); #endif #ifdef REDIS_TEST -int endianconvTest(int argc, char *argv[]); +int endianconvTest(int argc, char *argv[], int accurate); #endif #endif diff --git a/src/intset.c b/src/intset.c index 74de87acb..5498227f4 100644 --- a/src/intset.c +++ b/src/intset.c @@ -392,7 +392,7 @@ static void checkConsistency(intset *is) { } #define UNUSED(x) (void)(x) -int intsetTest(int argc, char **argv) { +int intsetTest(int argc, char **argv, int accurate) { uint8_t success; int i; intset *is; @@ -400,6 +400,7 @@ int intsetTest(int argc, char **argv) { UNUSED(argc); UNUSED(argv); + UNUSED(accurate); printf("Value encodings: "); { assert(_intsetValueEncoding(-32768) == INTSET_ENC_INT16); @@ -424,6 +425,7 @@ int intsetTest(int argc, char **argv) { is = intsetAdd(is,4,&success); assert(success); is = intsetAdd(is,4,&success); assert(!success); ok(); + zfree(is); } printf("Large number of random adds: "); { @@ -436,6 +438,7 @@ int intsetTest(int argc, char **argv) { assert(intrev32ifbe(is->length) == inserts); checkConsistency(is); ok(); + zfree(is); } printf("Upgrade from int16 to int32: "); { @@ -447,6 +450,7 @@ int intsetTest(int argc, char **argv) { assert(intsetFind(is,32)); assert(intsetFind(is,65535)); checkConsistency(is); + zfree(is); is = intsetNew(); is = intsetAdd(is,32,NULL); @@ -457,6 +461,7 @@ int intsetTest(int argc, char **argv) { assert(intsetFind(is,-65535)); checkConsistency(is); ok(); + zfree(is); } printf("Upgrade from int16 to int64: "); { @@ -468,6 +473,7 @@ int intsetTest(int argc, char **argv) { assert(intsetFind(is,32)); assert(intsetFind(is,4294967295)); checkConsistency(is); + zfree(is); is = intsetNew(); is = intsetAdd(is,32,NULL); @@ -478,6 +484,7 @@ int intsetTest(int argc, char **argv) { assert(intsetFind(is,-4294967295)); checkConsistency(is); ok(); + zfree(is); } printf("Upgrade from int32 to int64: "); { @@ -489,6 +496,7 @@ int intsetTest(int argc, char **argv) { assert(intsetFind(is,65535)); assert(intsetFind(is,4294967295)); checkConsistency(is); + zfree(is); is = intsetNew(); is = intsetAdd(is,65535,NULL); @@ -499,6 +507,7 @@ int intsetTest(int argc, char **argv) { assert(intsetFind(is,-4294967295)); checkConsistency(is); ok(); + zfree(is); } printf("Stress lookups: "); { @@ -512,6 +521,7 @@ int intsetTest(int argc, char **argv) { for (i = 0; i < num; i++) intsetSearch(is,rand() % ((1<len == 0 && !errors) { - OK; return errors; } @@ -1690,8 +1687,6 @@ static int _ql_verify(quicklist *ql, uint32_t len, uint32_t count, } } - if (!errors) - OK; return errors; } @@ -1703,9 +1698,10 @@ static char *genstr(char *prefix, int i) { } /* main test, but callable from other files */ -int quicklistTest(int argc, char *argv[]) { +int quicklistTest(int argc, char *argv[], int accurate) { UNUSED(argc); UNUSED(argv); + UNUSED(accurate); unsigned int err = 0; int optimize_start = @@ -1714,11 +1710,14 @@ int quicklistTest(int argc, char *argv[]) { printf("Starting optimization offset at: %d\n", optimize_start); int options[] = {0, 1, 2, 3, 4, 5, 6, 10}; + int fills[] = {-5, -4, -3, -2, -1, 0, + 1, 2, 32, 66, 128, 999}; size_t option_count = sizeof(options) / sizeof(*options); + int fill_count = (int)(sizeof(fills) / sizeof(*fills)); long long runtime[option_count]; for (int _i = 0; _i < (int)option_count; _i++) { - printf("Testing Option %d\n", options[_i]); + printf("Testing Compression option %d\n", options[_i]); long long start = mstime(); TEST("create list") { @@ -1743,57 +1742,53 @@ int quicklistTest(int argc, char *argv[]) { quicklistRelease(ql); } - for (int f = optimize_start; f < 32; f++) { - TEST_DESC("add to tail 5x at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("add to tail 5x at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 5; i++) quicklistPushTail(ql, genstr("hello", i), 32); if (ql->count != 5) ERROR; - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 1, 5, 5, 5); quicklistRelease(ql); } } - for (int f = optimize_start; f < 32; f++) { - TEST_DESC("add to head 5x at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("add to head 5x at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 5; i++) quicklistPushHead(ql, genstr("hello", i), 32); if (ql->count != 5) ERROR; - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 1, 5, 5, 5); quicklistRelease(ql); } } - for (int f = optimize_start; f < 512; f++) { - TEST_DESC("add to tail 500x at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("add to tail 500x at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushTail(ql, genstr("hello", i), 64); if (ql->count != 500) ERROR; - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 16, 500, 32, 20); quicklistRelease(ql); } } - for (int f = optimize_start; f < 512; f++) { - TEST_DESC("add to head 500x at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("add to head 500x at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushHead(ql, genstr("hello", i), 32); if (ql->count != 500) ERROR; - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 16, 500, 20, 32); quicklistRelease(ql); } @@ -1806,9 +1801,9 @@ int quicklistTest(int argc, char *argv[]) { quicklistRelease(ql); } - for (int f = optimize_start; f < 32; f++) { - TEST("rotate one val once") { - quicklist *ql = quicklistNew(f, options[_i]); + TEST("rotate one val once") { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); quicklistPushHead(ql, "hello", 6); quicklistRotate(ql); /* Ignore compression verify because ziplist is @@ -1818,10 +1813,9 @@ int quicklistTest(int argc, char *argv[]) { } } - for (int f = optimize_start; f < 3; f++) { - TEST_DESC("rotate 500 val 5000 times at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("rotate 500 val 5000 times at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); quicklistPushHead(ql, "900", 3); quicklistPushHead(ql, "7000", 4); quicklistPushHead(ql, "-1200", 5); @@ -1833,11 +1827,11 @@ int quicklistTest(int argc, char *argv[]) { ql_info(ql); quicklistRotate(ql); } - if (f == 1) + if (fills[f] == 1) ql_verify(ql, 504, 504, 1, 1); - else if (f == 2) + else if (fills[f] == 2) ql_verify(ql, 252, 504, 2, 2); - else if (f == 32) + else if (fills[f] == 32) ql_verify(ql, 16, 504, 32, 24); quicklistRelease(ql); } @@ -2014,11 +2008,10 @@ int quicklistTest(int argc, char *argv[]) { quicklistRelease(ql); } - for (int f = optimize_start; f < 12; f++) { - TEST_DESC("insert once in elements while iterating at fill %d at " - "compress %d\n", - f, options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("insert once in elements while iterating at compress %d", + options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); quicklistPushTail(ql, "abc", 3); quicklistSetFill(ql, 1); quicklistPushTail(ql, "def", 3); /* force to unique node */ @@ -2070,12 +2063,10 @@ int quicklistTest(int argc, char *argv[]) { } } - for (int f = optimize_start; f < 1024; f++) { - TEST_DESC( - "insert [before] 250 new in middle of 500 elements at fill" - " %d at compress %d", - f, options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("insert [before] 250 new in middle of 500 elements at compress %d", + options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushTail(ql, genstr("hello", i), 32); for (int i = 0; i < 250; i++) { @@ -2083,17 +2074,16 @@ int quicklistTest(int argc, char *argv[]) { quicklistIndex(ql, 250, &entry); quicklistInsertBefore(ql, &entry, genstr("abc", i), 32); } - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 25, 750, 32, 20); quicklistRelease(ql); } } - for (int f = optimize_start; f < 1024; f++) { - TEST_DESC("insert [after] 250 new in middle of 500 elements at " - "fill %d at compress %d", - f, options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("insert [after] 250 new in middle of 500 elements at compress %d", + options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushHead(ql, genstr("hello", i), 32); for (int i = 0; i < 250; i++) { @@ -2105,7 +2095,7 @@ int quicklistTest(int argc, char *argv[]) { if (ql->count != 750) ERR("List size not 750, but rather %ld", ql->count); - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 26, 750, 20, 32); quicklistRelease(ql); } @@ -2143,70 +2133,58 @@ int quicklistTest(int argc, char *argv[]) { quicklistRelease(copy); } - for (int f = optimize_start; f < 512; f++) { + for (int f = 0; f < fill_count; f++) { TEST_DESC("index 1,200 from 500 list at fill %d at compress %d", f, options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushTail(ql, genstr("hello", i + 1), 32); quicklistEntry entry; quicklistIndex(ql, 1, &entry); - if (!strcmp((char *)entry.value, "hello2")) - OK; - else + if (strcmp((char *)entry.value, "hello2") != 0) ERR("Value: %s", entry.value); quicklistIndex(ql, 200, &entry); - if (!strcmp((char *)entry.value, "hello201")) - OK; - else + if (strcmp((char *)entry.value, "hello201") != 0) ERR("Value: %s", entry.value); quicklistRelease(ql); } - TEST_DESC("index -1,-2 from 500 list at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("index -1,-2 from 500 list at fill %d at compress %d", + fills[f], options[_i]) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushTail(ql, genstr("hello", i + 1), 32); quicklistEntry entry; quicklistIndex(ql, -1, &entry); - if (!strcmp((char *)entry.value, "hello500")) - OK; - else + if (strcmp((char *)entry.value, "hello500") != 0) ERR("Value: %s", entry.value); quicklistIndex(ql, -2, &entry); - if (!strcmp((char *)entry.value, "hello499")) - OK; - else + if (strcmp((char *)entry.value, "hello499") != 0) ERR("Value: %s", entry.value); quicklistRelease(ql); } - TEST_DESC("index -100 from 500 list at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("index -100 from 500 list at fill %d at compress %d", + fills[f], options[_i]) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 500; i++) quicklistPushTail(ql, genstr("hello", i + 1), 32); quicklistEntry entry; quicklistIndex(ql, -100, &entry); - if (!strcmp((char *)entry.value, "hello401")) - OK; - else + if (strcmp((char *)entry.value, "hello401") != 0) ERR("Value: %s", entry.value); quicklistRelease(ql); } TEST_DESC("index too big +1 from 50 list at fill %d at compress %d", - f, options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + fills[f], options[_i]) { + quicklist *ql = quicklistNew(fills[f], options[_i]); for (int i = 0; i < 50; i++) quicklistPushTail(ql, genstr("hello", i + 1), 32); quicklistEntry entry; if (quicklistIndex(ql, 50, &entry)) ERR("Index found at 50 with 50 list: %.*s", entry.sz, entry.value); - else - OK; quicklistRelease(ql); } } @@ -2378,12 +2356,11 @@ int quicklistTest(int argc, char *argv[]) { quicklistReplaceAtIndex(ql, 1, "foo", 3); quicklistReplaceAtIndex(ql, -1, "bar", 3); quicklistRelease(ql); - OK; } - for (int f = optimize_start; f < 16; f++) { - TEST_DESC("lrem test at fill %d at compress %d", f, options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("lrem test at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); char *words[] = {"abc", "foo", "bar", "foobar", "foobared", "zap", "bar", "test", "foo"}; char *result[] = {"abc", "foo", "foobar", "foobared", @@ -2408,14 +2385,12 @@ int quicklistTest(int argc, char *argv[]) { /* check result of lrem 0 bar */ iter = quicklistGetIterator(ql, AL_START_HEAD); i = 0; - int ok = 1; while (quicklistNext(iter, &entry)) { /* Result must be: abc, foo, foobar, foobared, zap, test, * foo */ if (strncmp((char *)entry.value, result[i], entry.sz)) { ERR("No match at position %d, got %.*s instead of %s", i, entry.sz, entry.value, result[i]); - ok = 0; } i++; } @@ -2452,23 +2427,18 @@ int quicklistTest(int argc, char *argv[]) { entry.sz)) { ERR("No match at position %d, got %.*s instead of %s", i, entry.sz, entry.value, resultB[resB - 1 - i]); - ok = 0; } i++; } quicklistReleaseIterator(iter); - /* final result of all tests */ - if (ok) - OK; quicklistRelease(ql); } } - for (int f = optimize_start; f < 16; f++) { - TEST_DESC("iterate reverse + delete at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("iterate reverse + delete at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); quicklistPushTail(ql, "abc", 3); quicklistPushTail(ql, "def", 3); quicklistPushTail(ql, "hij", 3); @@ -2505,10 +2475,9 @@ int quicklistTest(int argc, char *argv[]) { } } - for (int f = optimize_start; f < 800; f++) { - TEST_DESC("iterator at index test at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("iterator at index test at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); char num[32]; long long nums[5000]; for (int i = 0; i < 760; i++) { @@ -2532,10 +2501,9 @@ int quicklistTest(int argc, char *argv[]) { } } - for (int f = optimize_start; f < 40; f++) { - TEST_DESC("ltrim test A at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("ltrim test A at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); char num[32]; long long nums[5000]; for (int i = 0; i < 32; i++) { @@ -2543,7 +2511,7 @@ int quicklistTest(int argc, char *argv[]) { int sz = ll2string(num, sizeof(num), nums[i]); quicklistPushTail(ql, num, sz); } - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 1, 32, 32, 32); /* ltrim 25 53 (keep [25,32] inclusive = 7 remaining) */ quicklistDelRange(ql, 0, 25); @@ -2556,18 +2524,17 @@ int quicklistTest(int argc, char *argv[]) { "%lld", entry.longval, nums[25 + i]); } - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 1, 7, 7, 7); quicklistRelease(ql); } } - for (int f = optimize_start; f < 40; f++) { - TEST_DESC("ltrim test B at fill %d at compress %d", f, - options[_i]) { + TEST_DESC("ltrim test B at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { /* Force-disable compression because our 33 sequential * integers don't compress and the check always fails. */ - quicklist *ql = quicklistNew(f, QUICKLIST_NOCOMPRESS); + quicklist *ql = quicklistNew(fills[f], QUICKLIST_NOCOMPRESS); char num[32]; long long nums[5000]; for (int i = 0; i < 33; i++) { @@ -2575,24 +2542,20 @@ int quicklistTest(int argc, char *argv[]) { int sz = ll2string(num, sizeof(num), nums[i]); quicklistPushTail(ql, num, sz); } - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 2, 33, 32, 1); /* ltrim 5 16 (keep [5,16] inclusive = 12 remaining) */ quicklistDelRange(ql, 0, 5); quicklistDelRange(ql, -16, 16); - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 1, 12, 12, 12); quicklistEntry entry; quicklistIndex(ql, 0, &entry); if (entry.longval != 5) ERR("A: longval not 5, but %lld", entry.longval); - else - OK; quicklistIndex(ql, -1, &entry); if (entry.longval != 16) ERR("B! got instead: %lld", entry.longval); - else - OK; quicklistPushTail(ql, "bobobob", 7); quicklistIndex(ql, -1, &entry); if (strncmp((char *)entry.value, "bobobob", 7)) @@ -2609,10 +2572,9 @@ int quicklistTest(int argc, char *argv[]) { } } - for (int f = optimize_start; f < 40; f++) { - TEST_DESC("ltrim test C at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("ltrim test C at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); char num[32]; long long nums[5000]; for (int i = 0; i < 33; i++) { @@ -2620,28 +2582,25 @@ int quicklistTest(int argc, char *argv[]) { int sz = ll2string(num, sizeof(num), nums[i]); quicklistPushTail(ql, num, sz); } - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 2, 33, 32, 1); /* ltrim 3 3 (keep [3,3] inclusive = 1 remaining) */ quicklistDelRange(ql, 0, 3); quicklistDelRange(ql, -29, 4000); /* make sure not loop forever */ - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 1, 1, 1, 1); quicklistEntry entry; quicklistIndex(ql, 0, &entry); if (entry.longval != -5157318210846258173) ERROR; - else - OK; quicklistRelease(ql); } } - for (int f = optimize_start; f < 40; f++) { - TEST_DESC("ltrim test D at fill %d at compress %d", f, - options[_i]) { - quicklist *ql = quicklistNew(f, options[_i]); + TEST_DESC("ltrim test D at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { + quicklist *ql = quicklistNew(fills[f], options[_i]); char num[32]; long long nums[5000]; for (int i = 0; i < 33; i++) { @@ -2649,7 +2608,7 @@ int quicklistTest(int argc, char *argv[]) { int sz = ll2string(num, sizeof(num), nums[i]); quicklistPushTail(ql, num, sz); } - if (f == 32) + if (fills[f] == 32) ql_verify(ql, 2, 33, 32, 1); quicklistDelRange(ql, -12, 3); if (ql->count != 30) @@ -2659,9 +2618,8 @@ int quicklistTest(int argc, char *argv[]) { } } - for (int f = optimize_start; f < 72; f++) { - TEST_DESC("create quicklist from ziplist at fill %d at compress %d", - f, options[_i]) { + TEST_DESC("create quicklist from ziplist at compress %d", options[_i]) { + for (int f = 0; f < fill_count; f++) { unsigned char *zl = ziplistNew(); long long nums[64]; char num[64]; @@ -2675,12 +2633,12 @@ int quicklistTest(int argc, char *argv[]) { zl = ziplistPush(zl, (unsigned char *)genstr("hello", i), 32, ZIPLIST_TAIL); } - quicklist *ql = quicklistCreateFromZiplist(f, options[_i], zl); - if (f == 1) + quicklist *ql = quicklistCreateFromZiplist(fills[f], options[_i], zl); + if (fills[f] == 1) ql_verify(ql, 66, 66, 1, 1); - else if (f == 32) + else if (fills[f] == 32) ql_verify(ql, 3, 66, 32, 2); - else if (f == 66) + else if (fills[f] == 66) ql_verify(ql, 1, 66, 66, 66); quicklistRelease(ql); } @@ -2693,16 +2651,14 @@ int quicklistTest(int argc, char *argv[]) { /* Run a longer test of compression depth outside of primary test loop. */ int list_sizes[] = {250, 251, 500, 999, 1000}; long long start = mstime(); - for (int list = 0; list < (int)(sizeof(list_sizes) / sizeof(*list_sizes)); - list++) { - for (int f = optimize_start; f < 128; f++) { - for (int depth = 1; depth < 40; depth++) { - /* skip over many redundant test cases */ - TEST_DESC("verify specific compression of interior nodes with " - "%d list " - "at fill %d at compress %d", - list_sizes[list], f, depth) { - quicklist *ql = quicklistNew(f, depth); + int list_count = accurate ? (int)(sizeof(list_sizes) / sizeof(*list_sizes)) : 1; + for (int list = 0; list < list_count; list++) { + TEST_DESC("verify specific compression of interior nodes with %d list ", + list_sizes[list]) { + for (int f = 0; f < fill_count; f++) { + for (int depth = 1; depth < 40; depth++) { + /* skip over many redundant test cases */ + quicklist *ql = quicklistNew(fills[f], depth); for (int i = 0; i < list_sizes[list]; i++) { quicklistPushTail(ql, genstr("hello TAIL", i + 1), 64); quicklistPushHead(ql, genstr("hello HEAD", i + 1), 64); @@ -2712,8 +2668,11 @@ int quicklistTest(int argc, char *argv[]) { /* test remove node */ if (step == 1) { for (int i = 0; i < list_sizes[list] / 2; i++) { - quicklistPop(ql, QUICKLIST_HEAD, NULL, NULL, NULL); - quicklistPop(ql, QUICKLIST_TAIL, NULL, NULL, NULL); + unsigned char *data; + quicklistPop(ql, QUICKLIST_HEAD, &data, NULL, NULL); + zfree(data); + quicklistPop(ql, QUICKLIST_TAIL, &data, NULL, NULL); + zfree(data); } } quicklistNode *node = ql->head; diff --git a/src/quicklist.h b/src/quicklist.h index fd9878af0..c9f493d80 100644 --- a/src/quicklist.h +++ b/src/quicklist.h @@ -199,7 +199,7 @@ quicklistNode *quicklistBookmarkFind(quicklist *ql, const char *name); void quicklistBookmarksClear(quicklist *ql); #ifdef REDIS_TEST -int quicklistTest(int argc, char *argv[]); +int quicklistTest(int argc, char *argv[], int accurate); #endif /* Directions for iterators */ diff --git a/src/sds.c b/src/sds.c index 6385ab14b..2ec3aa733 100644 --- a/src/sds.c +++ b/src/sds.c @@ -1234,9 +1234,10 @@ static sds sdsTestTemplateCallback(sds varname, void *arg) { else return NULL; } -int sdsTest(int argc, char **argv) { +int sdsTest(int argc, char **argv, int accurate) { UNUSED(argc); UNUSED(argv); + UNUSED(accurate); { sds x = sdsnew("foo"), y; diff --git a/src/sds.h b/src/sds.h index 85dc0b680..7f8710745 100644 --- a/src/sds.h +++ b/src/sds.h @@ -277,7 +277,7 @@ void *sds_realloc(void *ptr, size_t size); void sds_free(void *ptr); #ifdef REDIS_TEST -int sdsTest(int argc, char *argv[]); +int sdsTest(int argc, char *argv[], int accurate); #endif #endif diff --git a/src/server.c b/src/server.c index 26fe57499..e7c9787b1 100644 --- a/src/server.c +++ b/src/server.c @@ -6030,36 +6030,78 @@ int iAmMaster(void) { (server.cluster_enabled && nodeIsMaster(server.cluster->myself))); } +#ifdef REDIS_TEST +typedef int redisTestProc(int argc, char **argv, int accurate); +struct redisTest { + char *name; + redisTestProc *proc; + int failed; +} redisTests[] = { + {"ziplist", ziplistTest}, + {"quicklist", quicklistTest}, + {"intset", intsetTest}, + {"zipmap", zipmapTest}, + {"sha1test", sha1Test}, + {"util", utilTest}, + {"endianconv", endianconvTest}, + {"crc64", crc64Test}, + {"zmalloc", zmalloc_test}, + {"sds", sdsTest}, + {"dict", dictTest} +}; +redisTestProc *getTestProcByName(const char *name) { + int numtests = sizeof(redisTests)/sizeof(struct redisTest); + for (int j = 0; j < numtests; j++) { + if (!strcasecmp(name,redisTests[j].name)) { + return redisTests[j].proc; + } + } + return NULL; +} +#endif + int main(int argc, char **argv) { struct timeval tv; int j; char config_from_stdin = 0; #ifdef REDIS_TEST - if (argc == 3 && !strcasecmp(argv[1], "test")) { - if (!strcasecmp(argv[2], "ziplist")) { - return ziplistTest(argc, argv); - } else if (!strcasecmp(argv[2], "quicklist")) { - quicklistTest(argc, argv); - } else if (!strcasecmp(argv[2], "intset")) { - return intsetTest(argc, argv); - } else if (!strcasecmp(argv[2], "zipmap")) { - return zipmapTest(argc, argv); - } else if (!strcasecmp(argv[2], "sha1test")) { - return sha1Test(argc, argv); - } else if (!strcasecmp(argv[2], "util")) { - return utilTest(argc, argv); - } else if (!strcasecmp(argv[2], "endianconv")) { - return endianconvTest(argc, argv); - } else if (!strcasecmp(argv[2], "crc64")) { - return crc64Test(argc, argv); - } else if (!strcasecmp(argv[2], "zmalloc")) { - return zmalloc_test(argc, argv); - } else if (!strcasecmp(argv[2], "sds")) { - return sdsTest(argc, argv); + if (argc >= 3 && !strcasecmp(argv[1], "test")) { + int accurate = 0; + for (j = 3; j < argc; j++) { + if (!strcasecmp(argv[j], "--accurate")) { + accurate = 1; + } } - return -1; /* test not found */ + if (!strcasecmp(argv[2], "all")) { + int numtests = sizeof(redisTests)/sizeof(struct redisTest); + for (j = 0; j < numtests; j++) { + redisTests[j].failed = (redisTests[j].proc(argc,argv,accurate) != 0); + } + + /* Report tests result */ + int failed_num = 0; + for (j = 0; j < numtests; j++) { + if (redisTests[j].failed) { + failed_num++; + printf("[failed] Test - %s\n", redisTests[j].name); + } else { + printf("[ok] Test - %s\n", redisTests[j].name); + } + } + + printf("%d tests, %d passed, %d failed\n", numtests, + numtests-failed_num, failed_num); + + return failed_num == 0 ? 0 : 1; + } else { + redisTestProc *proc = getTestProcByName(argv[2]); + if (!proc) return -1; /* test not found */ + return proc(argc,argv,accurate); + } + + return 0; } #endif diff --git a/src/sha1.c b/src/sha1.c index ce487e367..f2423c052 100644 --- a/src/sha1.c +++ b/src/sha1.c @@ -201,7 +201,7 @@ void SHA1Final(unsigned char digest[20], SHA1_CTX* context) #define BUFSIZE 4096 #define UNUSED(x) (void)(x) -int sha1Test(int argc, char **argv) +int sha1Test(int argc, char **argv, int accurate) { SHA1_CTX ctx; unsigned char hash[20], buf[BUFSIZE]; @@ -209,6 +209,7 @@ int sha1Test(int argc, char **argv) UNUSED(argc); UNUSED(argv); + UNUSED(accurate); for(i=0;i --accurate */ +int ziplistTest(int argc, char **argv, int accurate) { unsigned char *zl, *p; unsigned char *entry; unsigned int elen; long long value; + int iteration; /* If an argument is given, use it as the random seed. */ - if (argc == 2) - srand(atoi(argv[1])); + if (argc >= 4) + srand(atoi(argv[3])); zl = createIntList(); ziplistRepr(zl); @@ -2339,7 +2341,8 @@ int ziplistTest(int argc, char **argv) { unsigned int slen; long long sval; - for (i = 0; i < 20000; i++) { + iteration = accurate ? 20000 : 20; + for (i = 0; i < iteration; i++) { zl = ziplistNew(); ref = listCreate(); listSetFreeMethod(ref,(void (*)(void*))sdsfree); @@ -2405,15 +2408,17 @@ int ziplistTest(int argc, char **argv) { printf("Stress with variable ziplist size:\n"); { unsigned long long start = usec(); - stress(ZIPLIST_HEAD,100000,16384,256); - stress(ZIPLIST_TAIL,100000,16384,256); + int maxsize = accurate ? 16384 : 16; + stress(ZIPLIST_HEAD,100000,maxsize,256); + stress(ZIPLIST_TAIL,100000,maxsize,256); printf("Done. usec=%lld\n\n", usec()-start); } /* Benchmarks */ { zl = ziplistNew(); - for (int i=0; i<100000; i++) { + iteration = accurate ? 100000 : 100; + for (int i=0; i %d:%.*s\n", klen, klen, key, vlen, vlen, value); } } + zfree(zm); return 0; } #endif diff --git a/src/zipmap.h b/src/zipmap.h index daf8430a0..1b34a32ae 100644 --- a/src/zipmap.h +++ b/src/zipmap.h @@ -48,7 +48,7 @@ void zipmapRepr(unsigned char *p); int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep); #ifdef REDIS_TEST -int zipmapTest(int argc, char *argv[]); +int zipmapTest(int argc, char *argv[], int accurate); #endif #endif diff --git a/src/zmalloc.c b/src/zmalloc.c index e212583a4..77eeea174 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -675,11 +675,12 @@ size_t zmalloc_get_memory_size(void) { #ifdef REDIS_TEST #define UNUSED(x) ((void)(x)) -int zmalloc_test(int argc, char **argv) { +int zmalloc_test(int argc, char **argv, int accurate) { void *ptr; UNUSED(argc); UNUSED(argv); + UNUSED(accurate); printf("Malloc prefix size: %d\n", (int) PREFIX_SIZE); printf("Initial used memory: %zu\n", zmalloc_used_memory()); ptr = zmalloc(123); diff --git a/src/zmalloc.h b/src/zmalloc.h index 3c0ba95d4..bb4cbddbb 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -135,7 +135,7 @@ size_t zmalloc_usable_size(void *ptr); #endif #ifdef REDIS_TEST -int zmalloc_test(int argc, char **argv); +int zmalloc_test(int argc, char **argv, int accurate); #endif #endif /* __ZMALLOC_H */ From 62a197516b0c114f96122d40f81f79a45d614981 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Wed, 10 Mar 2021 13:20:06 +0200 Subject: [PATCH 024/131] key miss stat increment was misplaced (#8630) The implication is that OBJECT command would was not updating stat_keyspace_misses --- src/db.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db.c b/src/db.c index 57705f003..2660ad876 100644 --- a/src/db.c +++ b/src/db.c @@ -139,9 +139,9 @@ robj *lookupKeyReadWithFlags(redisDb *db, robj *key, int flags) { keymiss: if (!(flags & LOOKUP_NONOTIFY)) { - server.stat_keyspace_misses++; notifyKeyspaceEvent(NOTIFY_KEY_MISS, "keymiss", key, db->id); } + server.stat_keyspace_misses++; return NULL; } From 7778f1b4b03a6a416388fe28d9bc93374384a65b Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Wed, 10 Mar 2021 15:25:16 +0200 Subject: [PATCH 025/131] strip % sign from current_fork_perc info field (#8628) `master_sync_perc` and `loading_loaded_perc` don't have that sign, and i think the info field should be a raw floating point number (the name suggests its units). we already have `used_memory_peak_perc` and `used_memory_dataset_perc` which do add the `%` sign, but: 1) i think it was a mistake but maybe too late to fix now, and maybe not too late to fix for `current_fork_perc` 2) it is more important to be consistent with the two other "progress" "prec" metrics, and not with the "utilization" metric. --- src/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.c b/src/server.c index e7c9787b1..81e23d8ee 100644 --- a/src/server.c +++ b/src/server.c @@ -4801,7 +4801,7 @@ sds genRedisInfoString(const char *section) { "# Persistence\r\n" "loading:%d\r\n" "current_cow_size:%zu\r\n" - "current_fork_perc:%.2f%%\r\n" + "current_fork_perc:%.2f\r\n" "current_save_keys_processed:%zu\r\n" "current_save_keys_total:%zu\r\n" "rdb_changes_since_last_save:%lld\r\n" From 61a73de64dd8e900c0fe720b0fbf063ddf77676b Mon Sep 17 00:00:00 2001 From: guybe7 Date: Wed, 10 Mar 2021 15:09:43 +0100 Subject: [PATCH 026/131] Cleanup ZADD_* flags (#8559) Have a clear separation between in and out flags Other changes: delete dead code in RM_ZsetIncrby: if zsetAdd returned error (happens only if the result of the operation is NAN or if score is NAN) we return immediately so there is no way that zsetAdd succeeded and returned NAN in the out-flags --- src/module.c | 37 ++++++++++------------- src/server.h | 25 +++++++--------- src/t_zset.c | 83 +++++++++++++++++++++++++--------------------------- 3 files changed, 67 insertions(+), 78 deletions(-) diff --git a/src/module.c b/src/module.c index b4fc3e856..8d04215d6 100644 --- a/src/module.c +++ b/src/module.c @@ -2516,19 +2516,19 @@ RedisModuleString *RM_ListPop(RedisModuleKey *key, int where) { * so that we have everything decoupled. */ int moduleZsetAddFlagsToCoreFlags(int flags) { int retflags = 0; - if (flags & REDISMODULE_ZADD_XX) retflags |= ZADD_XX; - if (flags & REDISMODULE_ZADD_NX) retflags |= ZADD_NX; - if (flags & REDISMODULE_ZADD_GT) retflags |= ZADD_GT; - if (flags & REDISMODULE_ZADD_LT) retflags |= ZADD_LT; + if (flags & REDISMODULE_ZADD_XX) retflags |= ZADD_IN_XX; + if (flags & REDISMODULE_ZADD_NX) retflags |= ZADD_IN_NX; + if (flags & REDISMODULE_ZADD_GT) retflags |= ZADD_IN_GT; + if (flags & REDISMODULE_ZADD_LT) retflags |= ZADD_IN_LT; return retflags; } /* See previous function comment. */ int moduleZsetAddFlagsFromCoreFlags(int flags) { int retflags = 0; - if (flags & ZADD_ADDED) retflags |= REDISMODULE_ZADD_ADDED; - if (flags & ZADD_UPDATED) retflags |= REDISMODULE_ZADD_UPDATED; - if (flags & ZADD_NOP) retflags |= REDISMODULE_ZADD_NOP; + if (flags & ZADD_OUT_ADDED) retflags |= REDISMODULE_ZADD_ADDED; + if (flags & ZADD_OUT_UPDATED) retflags |= REDISMODULE_ZADD_UPDATED; + if (flags & ZADD_OUT_NOP) retflags |= REDISMODULE_ZADD_NOP; return retflags; } @@ -2565,16 +2565,16 @@ int moduleZsetAddFlagsFromCoreFlags(int flags) { * * 'score' double value is not a number (NaN). */ int RM_ZsetAdd(RedisModuleKey *key, double score, RedisModuleString *ele, int *flagsptr) { - int flags = 0; + int in_flags = 0, out_flags = 0; if (!(key->mode & REDISMODULE_WRITE)) return REDISMODULE_ERR; if (key->value && key->value->type != OBJ_ZSET) return REDISMODULE_ERR; if (key->value == NULL) moduleCreateEmptyKey(key,REDISMODULE_KEYTYPE_ZSET); - if (flagsptr) flags = moduleZsetAddFlagsToCoreFlags(*flagsptr); - if (zsetAdd(key->value,score,ele->ptr,&flags,NULL) == 0) { + if (flagsptr) in_flags = moduleZsetAddFlagsToCoreFlags(*flagsptr); + if (zsetAdd(key->value,score,ele->ptr,in_flags,&out_flags,NULL) == 0) { if (flagsptr) *flagsptr = 0; return REDISMODULE_ERR; } - if (flagsptr) *flagsptr = moduleZsetAddFlagsFromCoreFlags(flags); + if (flagsptr) *flagsptr = moduleZsetAddFlagsFromCoreFlags(out_flags); return REDISMODULE_OK; } @@ -2592,22 +2592,17 @@ int RM_ZsetAdd(RedisModuleKey *key, double score, RedisModuleString *ele, int *f * with the new score of the element after the increment, if no error * is returned. */ int RM_ZsetIncrby(RedisModuleKey *key, double score, RedisModuleString *ele, int *flagsptr, double *newscore) { - int flags = 0; + int in_flags = 0, out_flags = 0; if (!(key->mode & REDISMODULE_WRITE)) return REDISMODULE_ERR; if (key->value && key->value->type != OBJ_ZSET) return REDISMODULE_ERR; if (key->value == NULL) moduleCreateEmptyKey(key,REDISMODULE_KEYTYPE_ZSET); - if (flagsptr) flags = moduleZsetAddFlagsToCoreFlags(*flagsptr); - flags |= ZADD_INCR; - if (zsetAdd(key->value,score,ele->ptr,&flags,newscore) == 0) { + if (flagsptr) in_flags = moduleZsetAddFlagsToCoreFlags(*flagsptr); + in_flags |= ZADD_IN_INCR; + if (zsetAdd(key->value,score,ele->ptr,in_flags,&out_flags,newscore) == 0) { if (flagsptr) *flagsptr = 0; return REDISMODULE_ERR; } - /* zsetAdd() may signal back that the resulting score is not a number. */ - if (flagsptr && (*flagsptr & ZADD_NAN)) { - *flagsptr = 0; - return REDISMODULE_ERR; - } - if (flagsptr) *flagsptr = moduleZsetAddFlagsFromCoreFlags(flags); + if (flagsptr) *flagsptr = moduleZsetAddFlagsFromCoreFlags(out_flags); return REDISMODULE_OK; } diff --git a/src/server.h b/src/server.h index ef1ac1e48..11fe55397 100644 --- a/src/server.h +++ b/src/server.h @@ -2111,21 +2111,18 @@ void ACLUpdateDefaultUserPassword(sds password); /* Sorted sets data type */ /* Input flags. */ -#define ZADD_NONE 0 -#define ZADD_INCR (1<<0) /* Increment the score instead of setting it. */ -#define ZADD_NX (1<<1) /* Don't touch elements not already existing. */ -#define ZADD_XX (1<<2) /* Only touch elements already existing. */ -#define ZADD_GT (1<<7) /* Only update existing when new scores are higher. */ -#define ZADD_LT (1<<8) /* Only update existing when new scores are lower. */ +#define ZADD_IN_NONE 0 +#define ZADD_IN_INCR (1<<0) /* Increment the score instead of setting it. */ +#define ZADD_IN_NX (1<<1) /* Don't touch elements not already existing. */ +#define ZADD_IN_XX (1<<2) /* Only touch elements already existing. */ +#define ZADD_IN_GT (1<<3) /* Only update existing when new scores are higher. */ +#define ZADD_IN_LT (1<<4) /* Only update existing when new scores are lower. */ /* Output flags. */ -#define ZADD_NOP (1<<3) /* Operation not performed because of conditionals.*/ -#define ZADD_NAN (1<<4) /* Only touch elements already existing. */ -#define ZADD_ADDED (1<<5) /* The element was new and was added. */ -#define ZADD_UPDATED (1<<6) /* The element already existed, score updated. */ - -/* Flags only used by the ZADD command but not by zsetAdd() API: */ -#define ZADD_CH (1<<16) /* Return num of elements added or updated. */ +#define ZADD_OUT_NOP (1<<0) /* Operation not performed because of conditionals.*/ +#define ZADD_OUT_NAN (1<<1) /* Only touch elements already existing. */ +#define ZADD_OUT_ADDED (1<<2) /* The element was new and was added. */ +#define ZADD_OUT_UPDATED (1<<3) /* The element already existed, score updated. */ /* Struct to hold an inclusive/exclusive range spec by score comparison. */ typedef struct { @@ -2156,7 +2153,7 @@ void zsetConvert(robj *zobj, int encoding); void zsetConvertToZiplistIfNeeded(robj *zobj, size_t maxelelen); int zsetScore(robj *zobj, sds member, double *score); unsigned long zslGetRank(zskiplist *zsl, double score, sds o); -int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore); +int zsetAdd(robj *zobj, double score, sds ele, int in_flags, int *out_flags, double *newscore); long zsetRank(robj *zobj, sds ele, int reverse); int zsetDel(robj *zobj, sds ele); robj *zsetDup(robj *o); diff --git a/src/t_zset.c b/src/t_zset.c index 6df21e300..dd65d3c67 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -1279,9 +1279,7 @@ int zsetScore(robj *zobj, sds member, double *score) { /* Add a new element or update the score of an existing element in a sorted * set, regardless of its encoding. * - * The set of flags change the command behavior. They are passed with an integer - * pointer since the function will clear the flags and populate them with - * other flags to indicate different conditions. + * The set of flags change the command behavior. * * The input flags are the following: * @@ -1323,19 +1321,19 @@ int zsetScore(robj *zobj, sds member, double *score) { * * The function does not take ownership of the 'ele' SDS string, but copies * it if needed. */ -int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { +int zsetAdd(robj *zobj, double score, sds ele, int in_flags, int *out_flags, double *newscore) { /* Turn options into simple to check vars. */ - int incr = (*flags & ZADD_INCR) != 0; - int nx = (*flags & ZADD_NX) != 0; - int xx = (*flags & ZADD_XX) != 0; - int gt = (*flags & ZADD_GT) != 0; - int lt = (*flags & ZADD_LT) != 0; - *flags = 0; /* We'll return our response flags. */ + int incr = (in_flags & ZADD_IN_INCR) != 0; + int nx = (in_flags & ZADD_IN_NX) != 0; + int xx = (in_flags & ZADD_IN_XX) != 0; + int gt = (in_flags & ZADD_IN_GT) != 0; + int lt = (in_flags & ZADD_IN_LT) != 0; + *out_flags = 0; /* We'll return our response flags. */ double curscore; /* NaN as input is an error regardless of all the other parameters. */ if (isnan(score)) { - *flags = ZADD_NAN; + *out_flags = ZADD_OUT_NAN; return 0; } @@ -1346,7 +1344,7 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { if ((eptr = zzlFind(zobj->ptr,ele,&curscore)) != NULL) { /* NX? Return, same element already exists. */ if (nx) { - *flags |= ZADD_NOP; + *out_flags |= ZADD_OUT_NOP; return 1; } @@ -1354,7 +1352,7 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { if (incr) { score += curscore; if (isnan(score)) { - *flags |= ZADD_NAN; + *out_flags |= ZADD_OUT_NAN; return 0; } if (newscore) *newscore = score; @@ -1369,7 +1367,7 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { { zobj->ptr = zzlDelete(zobj->ptr,eptr); zobj->ptr = zzlInsert(zobj->ptr,ele,score); - *flags |= ZADD_UPDATED; + *out_flags |= ZADD_OUT_UPDATED; } return 1; } else if (!xx) { @@ -1380,10 +1378,10 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { sdslen(ele) > server.zset_max_ziplist_value) zsetConvert(zobj,OBJ_ENCODING_SKIPLIST); if (newscore) *newscore = score; - *flags |= ZADD_ADDED; + *out_flags |= ZADD_OUT_ADDED; return 1; } else { - *flags |= ZADD_NOP; + *out_flags |= ZADD_OUT_NOP; return 1; } } else if (zobj->encoding == OBJ_ENCODING_SKIPLIST) { @@ -1395,7 +1393,7 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { if (de != NULL) { /* NX? Return, same element already exists. */ if (nx) { - *flags |= ZADD_NOP; + *out_flags |= ZADD_OUT_NOP; return 1; } curscore = *(double*)dictGetVal(de); @@ -1404,7 +1402,7 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { if (incr) { score += curscore; if (isnan(score)) { - *flags |= ZADD_NAN; + *out_flags |= ZADD_OUT_NAN; return 0; } if (newscore) *newscore = score; @@ -1422,18 +1420,18 @@ int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore) { * the hash table representing the sorted set, so we just * update the score. */ dictGetVal(de) = &znode->score; /* Update score ptr. */ - *flags |= ZADD_UPDATED; + *out_flags |= ZADD_OUT_UPDATED; } return 1; } else if (!xx) { ele = sdsdup(ele); znode = zslInsert(zs->zsl,score,ele); serverAssert(dictAdd(zs->dict,ele,&znode->score) == DICT_OK); - *flags |= ZADD_ADDED; + *out_flags |= ZADD_OUT_ADDED; if (newscore) *newscore = score; return 1; } else { - *flags |= ZADD_NOP; + *out_flags |= ZADD_OUT_NOP; return 1; } } else { @@ -1712,7 +1710,7 @@ void zaddGenericCommand(client *c, int flags) { robj *zobj; sds ele; double score = 0, *scores = NULL; - int j, elements; + int j, elements, ch = 0; int scoreidx = 0; /* The following vars are used in order to track what the command actually * did during the execution, to reply to the client and to trigger the @@ -1727,23 +1725,22 @@ void zaddGenericCommand(client *c, int flags) { scoreidx = 2; while(scoreidx < c->argc) { char *opt = c->argv[scoreidx]->ptr; - if (!strcasecmp(opt,"nx")) flags |= ZADD_NX; - else if (!strcasecmp(opt,"xx")) flags |= ZADD_XX; - else if (!strcasecmp(opt,"ch")) flags |= ZADD_CH; - else if (!strcasecmp(opt,"incr")) flags |= ZADD_INCR; - else if (!strcasecmp(opt,"gt")) flags |= ZADD_GT; - else if (!strcasecmp(opt,"lt")) flags |= ZADD_LT; + if (!strcasecmp(opt,"nx")) flags |= ZADD_IN_NX; + else if (!strcasecmp(opt,"xx")) flags |= ZADD_IN_XX; + else if (!strcasecmp(opt,"ch")) ch = 1; /* Return num of elements added or updated. */ + else if (!strcasecmp(opt,"incr")) flags |= ZADD_IN_INCR; + else if (!strcasecmp(opt,"gt")) flags |= ZADD_IN_GT; + else if (!strcasecmp(opt,"lt")) flags |= ZADD_IN_LT; else break; scoreidx++; } /* Turn options into simple to check vars. */ - int incr = (flags & ZADD_INCR) != 0; - int nx = (flags & ZADD_NX) != 0; - int xx = (flags & ZADD_XX) != 0; - int ch = (flags & ZADD_CH) != 0; - int gt = (flags & ZADD_GT) != 0; - int lt = (flags & ZADD_LT) != 0; + int incr = (flags & ZADD_IN_INCR) != 0; + int nx = (flags & ZADD_IN_NX) != 0; + int xx = (flags & ZADD_IN_XX) != 0; + int gt = (flags & ZADD_IN_GT) != 0; + int lt = (flags & ZADD_IN_LT) != 0; /* After the options, we expect to have an even number of args, since * we expect any number of score-element pairs. */ @@ -1801,17 +1798,17 @@ void zaddGenericCommand(client *c, int flags) { for (j = 0; j < elements; j++) { double newscore; score = scores[j]; - int retflags = flags; + int retflags = 0; ele = c->argv[scoreidx+1+j*2]->ptr; - int retval = zsetAdd(zobj, score, ele, &retflags, &newscore); + int retval = zsetAdd(zobj, score, ele, flags, &retflags, &newscore); if (retval == 0) { addReplyError(c,nanerr); goto cleanup; } - if (retflags & ZADD_ADDED) added++; - if (retflags & ZADD_UPDATED) updated++; - if (!(retflags & ZADD_NOP)) processed++; + if (retflags & ZADD_OUT_ADDED) added++; + if (retflags & ZADD_OUT_UPDATED) updated++; + if (!(retflags & ZADD_OUT_NOP)) processed++; score = newscore; } server.dirty += (added+updated); @@ -1836,11 +1833,11 @@ cleanup: } void zaddCommand(client *c) { - zaddGenericCommand(c,ZADD_NONE); + zaddGenericCommand(c,ZADD_IN_NONE); } void zincrbyCommand(client *c) { - zaddGenericCommand(c,ZADD_INCR); + zaddGenericCommand(c,ZADD_IN_INCR); } void zremCommand(client *c) { @@ -2941,7 +2938,7 @@ static void zrangeResultEmitCBufferForStore(zrange_result_handler *handler, double newscore; int retflags = 0; sds ele = sdsnewlen(value, value_length_in_bytes); - int retval = zsetAdd(handler->dstobj, score, ele, &retflags, &newscore); + int retval = zsetAdd(handler->dstobj, score, ele, ZADD_IN_NONE, &retflags, &newscore); sdsfree(ele); serverAssert(retval); } @@ -2952,7 +2949,7 @@ static void zrangeResultEmitLongLongForStore(zrange_result_handler *handler, double newscore; int retflags = 0; sds ele = sdsfromlonglong(value); - int retval = zsetAdd(handler->dstobj, score, ele, &retflags, &newscore); + int retval = zsetAdd(handler->dstobj, score, ele, ZADD_IN_NONE, &retflags, &newscore); sdsfree(ele); serverAssert(retval); } From ceb3a7a8f6100ac8c88491ad029ac93965ced36e Mon Sep 17 00:00:00 2001 From: Itamar Haber Date: Wed, 10 Mar 2021 16:19:48 +0200 Subject: [PATCH 027/131] Adds `LEN` to `STRALGO`'s comment (#8626) Ref: https://github.com/redis/redis-doc/pull/1529 --- src/t_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/t_string.c b/src/t_string.c index 3f73363e0..0967e30e1 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -723,7 +723,7 @@ void stralgoCommand(client *c) { } } -/* STRALGO [IDX] [MINMATCHLEN ] [WITHMATCHLEN] +/* STRALGO [IDX] [LEN] [MINMATCHLEN ] [WITHMATCHLEN] * STRINGS | KEYS */ void stralgoLCS(client *c) { From 3d0b427c30610b45c00b2377ee28bb69974ccea2 Mon Sep 17 00:00:00 2001 From: guybe7 Date: Wed, 10 Mar 2021 17:02:17 +0100 Subject: [PATCH 028/131] Fix some issues with modules and MULTI/EXEC (#8617) Bug 1: When a module ctx is freed moduleHandlePropagationAfterCommandCallback is called and handles propagation. We want to prevent it from propagating commands that were not replicated by the same context. Example: 1. module1.foo does: RM_Replicate(cmd1); RM_Call(cmd2); RM_Replicate(cmd3) 2. RM_Replicate(cmd1) propagates MULTI and adds cmd1 to also_propagagte 3. RM_Call(cmd2) create a new ctx, calls call() and destroys the ctx. 4. moduleHandlePropagationAfterCommandCallback is called, calling alsoPropagates EXEC (Note: EXEC is still not written to socket), setting server.in_trnsaction = 0 5. RM_Replicate(cmd3) is called, propagagting yet another MULTI (now we have nested MULTI calls, which is no good) and then cmd3 We must prevent RM_Call(cmd2) from resetting server.in_transaction. REDISMODULE_CTX_MULTI_EMITTED was revived for that purpose. Bug 2: Fix issues with nested RM_Call where some have '!' and some don't. Example: 1. module1.foo does RM_Call of module2.bar without replication (i.e. no '!') 2. module2.bar internally calls RM_Call of INCR with '!' 3. at the end of module1.foo we call RM_ReplicateVerbatim We want the replica/AOF to see only module1.foo and not the INCR from module2.bar Introduced a global replication_allowed flag inside RM_Call to determine whether we need to replicate or not (even if '!' was specified) Other changes: Split beforePropagateMultiOrExec to beforePropagateMulti afterPropagateExec just for better readability --- src/module.c | 27 +++++++++-- src/multi.c | 26 +++++------ src/server.c | 5 ++ src/server.h | 4 +- tests/modules/propagate.c | 74 ++++++++++++++++++++++++++++++ tests/unit/moduleapi/propagate.tcl | 53 +++++++++++++++++++++ 6 files changed, 170 insertions(+), 19 deletions(-) diff --git a/src/module.c b/src/module.c index 8d04215d6..d42207bbd 100644 --- a/src/module.c +++ b/src/module.c @@ -169,6 +169,7 @@ typedef struct RedisModuleCtx RedisModuleCtx; #define REDISMODULE_CTX_THREAD_SAFE (1<<4) #define REDISMODULE_CTX_BLOCKED_DISCONNECTED (1<<5) #define REDISMODULE_CTX_MODULE_COMMAND_CALL (1<<6) +#define REDISMODULE_CTX_MULTI_EMITTED (1<<7) /* This represents a Redis key opened with RM_OpenKey(). */ struct RedisModuleKey { @@ -599,6 +600,10 @@ void moduleHandlePropagationAfterCommandCallback(RedisModuleCtx *ctx) { /* We don't need to do anything here if the context was never used * in order to propagate commands. */ + if (!(ctx->flags & REDISMODULE_CTX_MULTI_EMITTED)) return; + + /* We don't need to do anything here if the server isn't inside + * a transaction. */ if (!server.propagate_in_transaction) return; /* If this command is executed from with Lua or MULTI/EXEC we do noy @@ -607,9 +612,9 @@ void moduleHandlePropagationAfterCommandCallback(RedisModuleCtx *ctx) { /* Handle the replication of the final EXEC, since whatever a command * emits is always wrapped around MULTI/EXEC. */ - beforePropagateMultiOrExec(0); alsoPropagate(server.execCommand,c->db->id,&shared.exec,1, PROPAGATE_AOF|PROPAGATE_REPL); + afterPropagateExec(); /* If this is not a module command context (but is instead a simple * callback context), we have to handle directly the "also propagate" @@ -1707,10 +1712,12 @@ void moduleReplicateMultiIfNeeded(RedisModuleCtx *ctx) { * context, we have to setup the op array for the "also propagate" API * so that RM_Replicate() will work. */ if (!(ctx->flags & REDISMODULE_CTX_MODULE_COMMAND_CALL)) { + serverAssert(ctx->saved_oparray.ops == NULL); ctx->saved_oparray = server.also_propagate; redisOpArrayInit(&server.also_propagate); } execCommandPropagateMulti(ctx->client->db->id); + ctx->flags |= REDISMODULE_CTX_MULTI_EMITTED; } /* Replicate the specified command and arguments to slaves and AOF, as effect @@ -4062,20 +4069,30 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch } } - /* If we are using single commands replication, we need to wrap what - * we propagate into a MULTI/EXEC block, so that it will be atomic like - * a Lua script in the context of AOF and slaves. */ - if (replicate) moduleReplicateMultiIfNeeded(ctx); + /* We need to use a global replication_allowed flag in order to prevent + * replication of nested RM_Calls. Example: + * 1. module1.foo does RM_Call of module2.bar without replication (i.e. no '!') + * 2. module2.bar internally calls RM_Call of INCR with '!' + * 3. at the end of module1.foo we call RM_ReplicateVerbatim + * We want the replica/AOF to see only module1.foo and not the INCR from module2.bar */ + int prev_replication_allowed = server.replication_allowed; + server.replication_allowed = replicate && server.replication_allowed; /* Run the command */ int call_flags = CMD_CALL_SLOWLOG | CMD_CALL_STATS | CMD_CALL_NOWRAP; if (replicate) { + /* If we are using single commands replication, we need to wrap what + * we propagate into a MULTI/EXEC block, so that it will be atomic like + * a Lua script in the context of AOF and slaves. */ + moduleReplicateMultiIfNeeded(ctx); + if (!(flags & REDISMODULE_ARGV_NO_AOF)) call_flags |= CMD_CALL_PROPAGATE_AOF; if (!(flags & REDISMODULE_ARGV_NO_REPLICAS)) call_flags |= CMD_CALL_PROPAGATE_REPL; } call(c,call_flags); + server.replication_allowed = prev_replication_allowed; serverAssert((c->flags & CLIENT_BLOCKED) == 0); diff --git a/src/multi.c b/src/multi.c index d88c5f1b8..5d690b71f 100644 --- a/src/multi.c +++ b/src/multi.c @@ -113,30 +113,30 @@ void discardCommand(client *c) { addReply(c,shared.ok); } -void beforePropagateMultiOrExec(int multi) { - if (multi) { - /* Propagating MULTI */ - serverAssert(!server.propagate_in_transaction); - server.propagate_in_transaction = 1; - } else { - /* Propagating EXEC */ - serverAssert(server.propagate_in_transaction == 1); - server.propagate_in_transaction = 0; - } +void beforePropagateMulti() { + /* Propagating MULTI */ + serverAssert(!server.propagate_in_transaction); + server.propagate_in_transaction = 1; +} + +void afterPropagateExec() { + /* Propagating EXEC */ + serverAssert(server.propagate_in_transaction == 1); + server.propagate_in_transaction = 0; } /* Send a MULTI command to all the slaves and AOF file. Check the execCommand * implementation for more information. */ void execCommandPropagateMulti(int dbid) { - beforePropagateMultiOrExec(1); + beforePropagateMulti(); propagate(server.multiCommand,dbid,&shared.multi,1, PROPAGATE_AOF|PROPAGATE_REPL); } void execCommandPropagateExec(int dbid) { - beforePropagateMultiOrExec(0); propagate(server.execCommand,dbid,&shared.exec,1, PROPAGATE_AOF|PROPAGATE_REPL); + afterPropagateExec(); } /* Aborts a transaction, with a specific error message. @@ -254,7 +254,6 @@ void execCommand(client *c) { if (server.propagate_in_transaction) { int is_master = server.masterhost == NULL; server.dirty++; - beforePropagateMultiOrExec(0); /* If inside the MULTI/EXEC block this instance was suddenly * switched from master to slave (using the SLAVEOF command), the * initial MULTI was propagated into the replication backlog, but the @@ -264,6 +263,7 @@ void execCommand(client *c) { char *execcmd = "*1\r\n$4\r\nEXEC\r\n"; feedReplicationBacklog(execcmd,strlen(execcmd)); } + afterPropagateExec(); } server.in_exec = 0; diff --git a/src/server.c b/src/server.c index 81e23d8ee..3ca87b684 100644 --- a/src/server.c +++ b/src/server.c @@ -3158,6 +3158,7 @@ void initServer(void) { server.clients_pending_write = listCreate(); server.clients_pending_read = listCreate(); server.clients_timeout_table = raxNew(); + server.replication_allowed = 1; server.slaveseldb = -1; /* Force to emit the first SELECT command. */ server.unblocked_clients = listCreate(); server.ready_keys = listCreate(); @@ -3502,6 +3503,7 @@ void redisOpArrayFree(redisOpArray *oa) { zfree(op->argv); } zfree(oa->ops); + oa->ops = NULL; } /* ====================== Commands lookup and execution ===================== */ @@ -3552,6 +3554,9 @@ struct redisCommand *lookupCommandOrOriginal(sds name) { void propagate(struct redisCommand *cmd, int dbid, robj **argv, int argc, int flags) { + if (!server.replication_allowed) + return; + /* Propagate a MULTI request once we encounter the first command which * is a write command. * This way we'll deliver the MULTI/..../EXEC block as a whole and diff --git a/src/server.h b/src/server.h index 11fe55397..a18e09ef0 100644 --- a/src/server.h +++ b/src/server.h @@ -1400,6 +1400,7 @@ struct redisServer { int child_info_nread; /* Num of bytes of the last read from pipe */ /* Propagation of commands in AOF / replication */ redisOpArray also_propagate; /* Additional command to propagate. */ + int replication_allowed; /* Are we allowed to replicate? */ /* Logging */ char *logfile; /* Path of log file */ int syslog_enabled; /* Is syslog enabled? */ @@ -1932,7 +1933,8 @@ void flagTransaction(client *c); void execCommandAbort(client *c, sds error); void execCommandPropagateMulti(int dbid); void execCommandPropagateExec(int dbid); -void beforePropagateMultiOrExec(int multi); +void beforePropagateMulti(); +void afterPropagateExec(); /* Redis object implementation */ void decrRefCount(robj *o); diff --git a/tests/modules/propagate.c b/tests/modules/propagate.c index 70cddacbd..e07f8efe7 100644 --- a/tests/modules/propagate.c +++ b/tests/modules/propagate.c @@ -70,6 +70,43 @@ int propagateTestTimerCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int return REDISMODULE_OK; } +/* Timer callback. */ +void timerNestedHandler(RedisModuleCtx *ctx, void *data) { + int repl = (long long)data; + + /* The goal is the trigger a module command that calls RM_Replicate + * in order to test MULTI/EXEC structre */ + RedisModule_Replicate(ctx,"INCRBY","cc","timer-nested-start","1"); + RedisModule_Call(ctx,"propagate-test.nested", repl? "!" : ""); + RedisModule_Replicate(ctx,"INCRBY","cc","timer-nested-end","1"); +} + +int propagateTestTimerNestedCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) +{ + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + + RedisModuleTimerID timer_id = + RedisModule_CreateTimer(ctx,100,timerNestedHandler,(void*)0); + REDISMODULE_NOT_USED(timer_id); + + RedisModule_ReplyWithSimpleString(ctx,"OK"); + return REDISMODULE_OK; +} + +int propagateTestTimerNestedReplCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) +{ + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + + RedisModuleTimerID timer_id = + RedisModule_CreateTimer(ctx,100,timerNestedHandler,(void*)1); + REDISMODULE_NOT_USED(timer_id); + + RedisModule_ReplyWithSimpleString(ctx,"OK"); + return REDISMODULE_OK; +} + /* The thread entry point. */ void *threadMain(void *arg) { REDISMODULE_NOT_USED(arg); @@ -131,6 +168,28 @@ int propagateTestMixedCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int return REDISMODULE_OK; } +int propagateTestNestedCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) +{ + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + RedisModuleCallReply *reply; + + /* This test mixes multiple propagation systems. */ + reply = RedisModule_Call(ctx, "INCR", "c!", "using-call"); + RedisModule_FreeCallReply(reply); + + RedisModule_Call(ctx,"propagate-test.simple", "!"); + + RedisModule_Replicate(ctx,"INCR","c","counter-3"); + RedisModule_Replicate(ctx,"INCR","c","counter-4"); + + reply = RedisModule_Call(ctx, "INCR", "c!", "after-call"); + RedisModule_FreeCallReply(reply); + + RedisModule_ReplyWithSimpleString(ctx,"OK"); + return REDISMODULE_OK; +} + int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); @@ -143,6 +202,16 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) "",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx,"propagate-test.timer-nested", + propagateTestTimerNestedCommand, + "",1,1,1) == REDISMODULE_ERR) + return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"propagate-test.timer-nested-repl", + propagateTestTimerNestedReplCommand, + "",1,1,1) == REDISMODULE_ERR) + return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx,"propagate-test.thread", propagateTestThreadCommand, "",1,1,1) == REDISMODULE_ERR) @@ -158,5 +227,10 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) "",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx,"propagate-test.nested", + propagateTestNestedCommand, + "",1,1,1) == REDISMODULE_ERR) + return REDISMODULE_ERR; + return REDISMODULE_OK; } diff --git a/tests/unit/moduleapi/propagate.tcl b/tests/unit/moduleapi/propagate.tcl index adebd37a6..1277d3970 100644 --- a/tests/unit/moduleapi/propagate.tcl +++ b/tests/unit/moduleapi/propagate.tcl @@ -42,6 +42,59 @@ tags "modules" { close_replication_stream $repl } + test {module propagates nested ctx case1} { + set repl [attach_to_replication_stream] + + $master del timer-nested-start + $master del timer-nested-end + $master propagate-test.timer-nested + + wait_for_condition 5000 10 { + [$replica get timer-nested-end] eq "1" + } else { + fail "The two counters don't match the expected value." + } + + assert_replication_stream $repl { + {select *} + {multi} + {incrby timer-nested-start 1} + {incrby timer-nested-end 1} + {exec} + } + close_replication_stream $repl + } + + test {module propagates nested ctx case2} { + set repl [attach_to_replication_stream] + + $master del timer-nested-start + $master del timer-nested-end + $master propagate-test.timer-nested-repl + + wait_for_condition 5000 10 { + [$replica get timer-nested-end] eq "1" + } else { + fail "The two counters don't match the expected value." + } + + # Note the 'after-call' and 'timer-nested-start' propagation below is out of order (known limitation) + assert_replication_stream $repl { + {select *} + {multi} + {incr using-call} + {incr counter-1} + {incr counter-2} + {incr after-call} + {incr counter-3} + {incr counter-4} + {incrby timer-nested-start 1} + {incrby timer-nested-end 1} + {exec} + } + close_replication_stream $repl + } + test {module propagates from thread} { set repl [attach_to_replication_stream] From c945e1126a0b7ccba642af7c51c60f2d86290e17 Mon Sep 17 00:00:00 2001 From: cheese1 Date: Wed, 10 Mar 2021 18:11:16 +0100 Subject: [PATCH 029/131] Changes http to https in texts (#8495) --- 00-RELEASENOTES | 2 +- README.md | 10 +++++----- redis.conf | 6 +++--- src/asciilogo.h | 4 ++-- src/latency.c | 4 ++-- utils/whatisdoing.sh | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/00-RELEASENOTES b/00-RELEASENOTES index ce472159e..f745d70a3 100644 --- a/00-RELEASENOTES +++ b/00-RELEASENOTES @@ -11,6 +11,6 @@ to download the latest stable release here: http://download.redis.io/releases/redis-stable.tar.gz -More information is available at http://redis.io +More information is available at https://redis.io Happy hacking! diff --git a/README.md b/README.md index d892f4881..ab911b731 100644 --- a/README.md +++ b/README.md @@ -15,10 +15,10 @@ Another good example is to think of Redis as a more complex version of memcached If you want to know more, this is a list of selected starting points: -* Introduction to Redis data types. http://redis.io/topics/data-types-intro +* Introduction to Redis data types. https://redis.io/topics/data-types-intro * Try Redis directly inside your browser. http://try.redis.io -* The full list of Redis commands. http://redis.io/commands -* There is much more inside the official Redis documentation. http://redis.io/documentation +* The full list of Redis commands. https://redis.io/commands +* There is much more inside the official Redis documentation. https://redis.io/documentation Building Redis -------------- @@ -184,7 +184,7 @@ then in another terminal try the following: (integer) 2 redis> -You can find the list of all the available commands at http://redis.io/commands. +You can find the list of all the available commands at https://redis.io/commands. Installing Redis ----------------- @@ -453,7 +453,7 @@ Other C files * `scripting.c` implements Lua scripting. It is completely self-contained and isolated from the rest of the Redis implementation and is simple enough to understand if you are familiar with the Lua API. * `cluster.c` implements the Redis Cluster. Probably a good read only after being very familiar with the rest of the Redis code base. If you want to read `cluster.c` make sure to read the [Redis Cluster specification][3]. -[3]: http://redis.io/topics/cluster-spec +[3]: https://redis.io/topics/cluster-spec Anatomy of a Redis command --- diff --git a/redis.conf b/redis.conf index 465d56fc0..6bdef8843 100644 --- a/redis.conf +++ b/redis.conf @@ -1225,7 +1225,7 @@ disable-thp yes # If the AOF is enabled on startup Redis will load the AOF, that is the file # with the better durability guarantees. # -# Please check http://redis.io/topics/persistence for more information. +# Please check https://redis.io/topics/persistence for more information. appendonly no @@ -1480,7 +1480,7 @@ lua-time-limit 5000 # cluster-allow-reads-when-down no # In order to setup your cluster make sure to read the documentation -# available at http://redis.io web site. +# available at https://redis.io web site. ########################## CLUSTER DOCKER/NAT support ######################## @@ -1563,7 +1563,7 @@ latency-monitor-threshold 0 ############################# EVENT NOTIFICATION ############################## # Redis can notify Pub/Sub clients about events happening in the key space. -# This feature is documented at http://redis.io/topics/notifications +# This feature is documented at https://redis.io/topics/notifications # # For instance if keyspace events notification is enabled, and a client # performs a DEL operation on key "foo" stored in the Database 0, two diff --git a/src/asciilogo.h b/src/asciilogo.h index 044ca0c55..a62f68cf9 100644 --- a/src/asciilogo.h +++ b/src/asciilogo.h @@ -31,13 +31,13 @@ const char *ascii_logo = " _._ \n" " _.-``__ ''-._ \n" " _.-`` `. `_. ''-._ Redis %s (%s/%d) %s bit\n" -" .-`` .-```. ```\\/ _.,_ ''-._ \n" +" .-`` .-```. ```\\/ _.,_ ''-._ \n" " ( ' , .-` | `, ) Running in %s mode\n" " |`-._`-...-` __...-.``-._|'` _.-'| Port: %d\n" " | `-._ `._ / _.-' | PID: %ld\n" " `-._ `-._ `-./ _.-' _.-' \n" " |`-._`-._ `-.__.-' _.-'_.-'| \n" -" | `-._`-._ _.-'_.-' | http://redis.io \n" +" | `-._`-._ _.-'_.-' | https://redis.io \n" " `-._ `-._`-.__.-'_.-' _.-' \n" " |`-._`-._ `-.__.-' _.-'_.-'| \n" " | `-._`-._ _.-'_.-' | \n" diff --git a/src/latency.c b/src/latency.c index d447b2b5b..a5dfc5a7d 100644 --- a/src/latency.c +++ b/src/latency.c @@ -256,7 +256,7 @@ sds createLatencyReport(void) { if (dictSize(server.latency_events) == 0 && server.latency_monitor_threshold == 0) { - report = sdscat(report,"I'm sorry, Dave, I can't do that. Latency monitoring is disabled in this Redis instance. You may use \"CONFIG SET latency-monitor-threshold .\" in order to enable it. If we weren't in a deep space mission I'd suggest to take a look at http://redis.io/topics/latency-monitor.\n"); + report = sdscat(report,"I'm sorry, Dave, I can't do that. Latency monitoring is disabled in this Redis instance. You may use \"CONFIG SET latency-monitor-threshold .\" in order to enable it. If we weren't in a deep space mission I'd suggest to take a look at https://redis.io/topics/latency-monitor.\n"); return report; } @@ -426,7 +426,7 @@ sds createLatencyReport(void) { } if (advise_slowlog_inspect) { - report = sdscat(report,"- Check your Slow Log to understand what are the commands you are running which are too slow to execute. Please check http://redis.io/commands/slowlog for more information.\n"); + report = sdscat(report,"- Check your Slow Log to understand what are the commands you are running which are too slow to execute. Please check https://redis.io/commands/slowlog for more information.\n"); } /* Intrinsic latency. */ diff --git a/utils/whatisdoing.sh b/utils/whatisdoing.sh index e4059caed..68d7f7cca 100755 --- a/utils/whatisdoing.sh +++ b/utils/whatisdoing.sh @@ -4,7 +4,7 @@ # Software Watchdog, which provides a similar functionality but in # a more reliable / easy to use way. # -# Check http://redis.io/topics/latency for more information. +# Check https://redis.io/topics/latency for more information. #!/bin/bash nsamples=1 From b70d81f60b29b1137d6644da1c609dcf45d9e4bc Mon Sep 17 00:00:00 2001 From: Harkrishn Patro <30795839+hpatro@users.noreply.github.com> Date: Thu, 11 Mar 2021 06:19:35 +0100 Subject: [PATCH 030/131] Process hello command even if the default user has no permissions. (#8633) Co-authored-by: Harkrishn Patro --- src/acl.c | 6 +++--- tests/unit/acl.tcl | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/acl.c b/src/acl.c index 445409ecd..f48fb405e 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1180,9 +1180,9 @@ int ACLCheckCommandPerm(client *c, int *keyidxptr) { /* If there is no associated user, the connection can run anything. */ if (u == NULL) return ACL_OK; - /* Check if the user can execute this command. */ - if (!(u->flags & USER_FLAG_ALLCOMMANDS) && - c->cmd->proc != authCommand) + /* Check if the user can execute this command or if the command + * doesn't need to be authenticated (hello, auth). */ + if (!(u->flags & USER_FLAG_ALLCOMMANDS) && !(c->cmd->flags & CMD_NO_AUTH)) { /* If the bit is not set we have to check further, in case the * command is allowed just with that specific subcommand. */ diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl index 7c09195a1..a6afd3f9e 100644 --- a/tests/unit/acl.tcl +++ b/tests/unit/acl.tcl @@ -409,6 +409,14 @@ start_server {tags {"acl"}} { set e } {*NOAUTH*} + test {When default user has no command permission, hello command still works for other users} { + r ACL setuser secure-user >supass on +@all + r ACL setuser default -@all + r HELLO 2 AUTH secure-user supass + r ACL setuser default nopass +@all + r AUTH default "" + } + test {ACL HELP should not have unexpected options} { catch {r ACL help xxx} e assert_match "*Unknown subcommand or wrong number of arguments*" $e From 4ae1bea323ffc62ebf78a41fef1e50e4f268f8e2 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 11 Mar 2021 07:52:27 +0200 Subject: [PATCH 031/131] allow RESET during busy scripts (#8629) same as DISCARD, this command (RESET) is about client state, not server state --- src/server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server.c b/src/server.c index 3ca87b684..145d515c4 100644 --- a/src/server.c +++ b/src/server.c @@ -4144,6 +4144,7 @@ int processCommand(client *c) { c->cmd->proc != discardCommand && c->cmd->proc != watchCommand && c->cmd->proc != unwatchCommand && + c->cmd->proc != resetCommand && !(c->cmd->proc == shutdownCommand && c->argc == 2 && tolower(((char*)c->argv[1]->ptr)[0]) == 'n') && From a4f03bd7eb44580b44f9499c53417df45e1edfb0 Mon Sep 17 00:00:00 2001 From: guybe7 Date: Thu, 11 Mar 2021 12:50:13 +0100 Subject: [PATCH 032/131] Fix some memory leaks in propagagte.c (#8636) Introduced by 3d0b427c30610b45c00b2377ee28bb69974ccea2 --- tests/modules/propagate.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/modules/propagate.c b/tests/modules/propagate.c index e07f8efe7..7a63ed807 100644 --- a/tests/modules/propagate.c +++ b/tests/modules/propagate.c @@ -77,7 +77,8 @@ void timerNestedHandler(RedisModuleCtx *ctx, void *data) { /* The goal is the trigger a module command that calls RM_Replicate * in order to test MULTI/EXEC structre */ RedisModule_Replicate(ctx,"INCRBY","cc","timer-nested-start","1"); - RedisModule_Call(ctx,"propagate-test.nested", repl? "!" : ""); + RedisModuleCallReply *reply = RedisModule_Call(ctx,"propagate-test.nested", repl? "!" : ""); + RedisModule_FreeCallReply(reply); RedisModule_Replicate(ctx,"INCRBY","cc","timer-nested-end","1"); } @@ -178,7 +179,8 @@ int propagateTestNestedCommand(RedisModuleCtx *ctx, RedisModuleString **argv, in reply = RedisModule_Call(ctx, "INCR", "c!", "using-call"); RedisModule_FreeCallReply(reply); - RedisModule_Call(ctx,"propagate-test.simple", "!"); + reply = RedisModule_Call(ctx,"propagate-test.simple", "!"); + RedisModule_FreeCallReply(reply); RedisModule_Replicate(ctx,"INCR","c","counter-3"); RedisModule_Replicate(ctx,"INCR","c","counter-4"); From 3c09ce26fbead90d1d8e4e9c0ef1003de472f9c7 Mon Sep 17 00:00:00 2001 From: Wen Hui Date: Thu, 11 Mar 2021 15:28:21 -0500 Subject: [PATCH 033/131] Sentinel: fix info_refresh time before sentinel get first response (#8567) --- src/sentinel.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/sentinel.c b/src/sentinel.c index 266462466..9c40e3d06 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -3344,7 +3344,8 @@ void addReplySentinelRedisInstance(client *c, sentinelRedisInstance *ri) { /* Masters and Slaves */ if (ri->flags & (SRI_MASTER|SRI_SLAVE)) { addReplyBulkCString(c,"info-refresh"); - addReplyBulkLongLong(c,mstime() - ri->info_refresh); + addReplyBulkLongLong(c, + ri->info_refresh ? (mstime() - ri->info_refresh) : 0); fields++; addReplyBulkCString(c,"role-reported"); @@ -3811,7 +3812,8 @@ NULL addReplyBulkCBuffer(c,ri->name,strlen(ri->name)); addReplyArrayLen(c,dictSize(ri->slaves) + 1); /* +1 for self */ addReplyArrayLen(c,2); - addReplyLongLong(c, now - ri->info_refresh); + addReplyLongLong(c, + ri->info_refresh ? (now - ri->info_refresh) : 0); if (ri->info) addReplyBulkCBuffer(c,ri->info,sdslen(ri->info)); else @@ -3823,7 +3825,8 @@ NULL while ((sde = dictNext(sdi)) != NULL) { sentinelRedisInstance *sri = dictGetVal(sde); addReplyArrayLen(c,2); - addReplyLongLong(c, now - sri->info_refresh); + addReplyLongLong(c, + ri->info_refresh ? (now - sri->info_refresh) : 0); if (sri->info) addReplyBulkCBuffer(c,sri->info,sdslen(sri->info)); else From 5b48d900498c85bbf4772c1d466c214439888115 Mon Sep 17 00:00:00 2001 From: KinWaiYuen <529054658@qq.com> Date: Fri, 12 Mar 2021 14:40:35 +0800 Subject: [PATCH 034/131] Optimize CLUSTER SLOTS reply by reducing unneeded loops (#8541) This commit more efficiently computes the cluster bulk slots response by looping over the entire slot space once, instead of for each node. --- src/cluster.c | 100 ++++++++---------- .../cluster/tests/19-cluster-nodes-slots.tcl | 9 ++ 2 files changed, 51 insertions(+), 58 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 06102141b..123ea1054 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -4313,7 +4313,31 @@ int getSlotOrReply(client *c, robj *o) { return (int) slot; } -void clusterReplyMultiBulkSlots(client *c) { +void addNodeReplyForClusterSlot(client *c, clusterNode *node, int start_slot, int end_slot) { + int i, nested_elements = 3; /* slots (2) + master addr (1) */ + void *nested_replylen = addReplyDeferredLen(c); + addReplyLongLong(c, start_slot); + addReplyLongLong(c, end_slot); + addReplyArrayLen(c, 3); + addReplyBulkCString(c, node->ip); + addReplyLongLong(c, node->port); + addReplyBulkCBuffer(c, node->name, CLUSTER_NAMELEN); + + /* Remaining nodes in reply are replicas for slot range */ + for (i = 0; i < node->numslaves; i++) { + /* This loop is copy/pasted from clusterGenNodeDescription() + * with modifications for per-slot node aggregation. */ + if (nodeFailed(node->slaves[i])) continue; + addReplyArrayLen(c, 3); + addReplyBulkCString(c, node->slaves[i]->ip); + addReplyLongLong(c, node->slaves[i]->port); + addReplyBulkCBuffer(c, node->slaves[i]->name, CLUSTER_NAMELEN); + nested_elements++; + } + setDeferredArrayLen(c, nested_replylen, nested_elements); +} + +void clusterReplyMultiBulkSlots(client * c) { /* Format: 1) 1) start slot * 2) end slot * 3) 1) master IP @@ -4324,69 +4348,29 @@ void clusterReplyMultiBulkSlots(client *c) { * 3) node ID * ... continued until done */ - - int num_masters = 0; + clusterNode *n = NULL; + int num_masters = 0, start = -1; void *slot_replylen = addReplyDeferredLen(c); - dictEntry *de; - dictIterator *di = dictGetSafeIterator(server.cluster->nodes); - while((de = dictNext(di)) != NULL) { - clusterNode *node = dictGetVal(de); - int j = 0, start = -1; - int i, nested_elements = 0; - - /* Skip slaves (that are iterated when producing the output of their - * master) and masters not serving any slot. */ - if (!nodeIsMaster(node) || node->numslots == 0) continue; - - for(i = 0; i < node->numslaves; i++) { - if (nodeFailed(node->slaves[i])) continue; - nested_elements++; + for (int i = 0; i <= CLUSTER_SLOTS; i++) { + /* Find start node and slot id. */ + if (n == NULL) { + if (i == CLUSTER_SLOTS) break; + n = server.cluster->slots[i]; + start = i; + continue; } - for (j = 0; j < CLUSTER_SLOTS; j++) { - int bit, i; - - if ((bit = clusterNodeGetSlotBit(node,j)) != 0) { - if (start == -1) start = j; - } - if (start != -1 && (!bit || j == CLUSTER_SLOTS-1)) { - addReplyArrayLen(c, nested_elements + 3); /* slots (2) + master addr (1). */ - - if (bit && j == CLUSTER_SLOTS-1) j++; - - /* If slot exists in output map, add to it's list. - * else, create a new output map for this slot */ - if (start == j-1) { - addReplyLongLong(c, start); /* only one slot; low==high */ - addReplyLongLong(c, start); - } else { - addReplyLongLong(c, start); /* low */ - addReplyLongLong(c, j-1); /* high */ - } - start = -1; - - /* First node reply position is always the master */ - addReplyArrayLen(c, 3); - addReplyBulkCString(c, node->ip); - addReplyLongLong(c, node->port); - addReplyBulkCBuffer(c, node->name, CLUSTER_NAMELEN); - - /* Remaining nodes in reply are replicas for slot range */ - for (i = 0; i < node->numslaves; i++) { - /* This loop is copy/pasted from clusterGenNodeDescription() - * with modifications for per-slot node aggregation */ - if (nodeFailed(node->slaves[i])) continue; - addReplyArrayLen(c, 3); - addReplyBulkCString(c, node->slaves[i]->ip); - addReplyLongLong(c, node->slaves[i]->port); - addReplyBulkCBuffer(c, node->slaves[i]->name, CLUSTER_NAMELEN); - } - num_masters++; - } + /* Add cluster slots info when occur different node with start + * or end of slot. */ + if (i == CLUSTER_SLOTS || n != server.cluster->slots[i]) { + addNodeReplyForClusterSlot(c, n, start, i-1); + num_masters++; + if (i == CLUSTER_SLOTS) break; + n = server.cluster->slots[i]; + start = i; } } - dictReleaseIterator(di); setDeferredArrayLen(c, slot_replylen, num_masters); } diff --git a/tests/cluster/tests/19-cluster-nodes-slots.tcl b/tests/cluster/tests/19-cluster-nodes-slots.tcl index ca0b3ce0d..80f68d5d0 100644 --- a/tests/cluster/tests/19-cluster-nodes-slots.tcl +++ b/tests/cluster/tests/19-cluster-nodes-slots.tcl @@ -37,26 +37,35 @@ set master2 [Rn 1] test "Continuous slots distribution" { assert_match "* 0-8191*" [$master1 CLUSTER NODES] assert_match "* 8192-16383*" [$master2 CLUSTER NODES] + assert_match "*0 8191*" [$master1 CLUSTER SLOTS] + assert_match "*8192 16383*" [$master2 CLUSTER SLOTS] $master1 CLUSTER DELSLOTS 4096 assert_match "* 0-4095 4097-8191*" [$master1 CLUSTER NODES] + assert_match "*0 4095*4097 8191*" [$master1 CLUSTER SLOTS] + $master2 CLUSTER DELSLOTS 12288 assert_match "* 8192-12287 12289-16383*" [$master2 CLUSTER NODES] + assert_match "*8192 12287*12289 16383*" [$master2 CLUSTER SLOTS] } test "Discontinuous slots distribution" { # Remove middle slots $master1 CLUSTER DELSLOTS 4092 4094 assert_match "* 0-4091 4093 4095 4097-8191*" [$master1 CLUSTER NODES] + assert_match "*0 4091*4093 4093*4095 4095*4097 8191*" [$master1 CLUSTER SLOTS] $master2 CLUSTER DELSLOTS 12284 12286 assert_match "* 8192-12283 12285 12287 12289-16383*" [$master2 CLUSTER NODES] + assert_match "*8192 12283*12285 12285*12287 12287*12289 16383*" [$master2 CLUSTER SLOTS] # Remove head slots $master1 CLUSTER DELSLOTS 0 2 assert_match "* 1 3-4091 4093 4095 4097-8191*" [$master1 CLUSTER NODES] + assert_match "*1 1*3 4091*4093 4093*4095 4095*4097 8191*" [$master1 CLUSTER SLOTS] # Remove tail slots $master2 CLUSTER DELSLOTS 16380 16382 16383 assert_match "* 8192-12283 12285 12287 12289-16379 16381*" [$master2 CLUSTER NODES] + assert_match "*8192 12283*12285 12285*12287 12287*12289 16379*16381 16381*" [$master2 CLUSTER SLOTS] } From 3a5905fa8575ccc7b0e115dee0a80f774bb6bf85 Mon Sep 17 00:00:00 2001 From: Guillem Jover Date: Sun, 14 Mar 2021 07:46:26 +0100 Subject: [PATCH 035/131] Send the readiness notification when we are ready to accept connections (#8409) On a replica we do accept connections, even though commands accessing the database will operate in read-only mode. But the server is still already operational and processing commands. Not sending the readiness notification means that on a HA setup where the nodes all start as replicas (with replicaof in the config) with a replica that cannot connect to the master server and which might not come back in a predictable amount of time or at all, the service supervisor will end up timing out the service and terminating it, with no option to promote it to be the main instance. This seems counter to what the readiness notification is supposed to be signaling. Instead send the readiness notification when we start accepting commands, and then send the various server status changes as that. Fixes: commit 641c64ada10404356fc76c0b56a69b32c76f253c Fixes: commit dfb598cf3304818e608ceb6b5d9529a293345c4a --- src/replication.c | 6 ++---- src/server.c | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/replication.c b/src/replication.c index eb5fa54c0..7bbba1a9e 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1873,8 +1873,7 @@ void readSyncBulkPayload(connection *conn) { serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: Finished with success"); if (server.supervised_mode == SUPERVISED_SYSTEMD) { - redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Finished with success. Ready to accept connections.\n"); - redisCommunicateSystemd("READY=1\n"); + redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Finished with success. Ready to accept connections in read-write mode.\n"); } /* Send the initial ACK immediately to put this replica in online state. */ @@ -2434,8 +2433,7 @@ void syncWithMaster(connection *conn) { if (psync_result == PSYNC_CONTINUE) { serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: Master accepted a Partial Resynchronization."); if (server.supervised_mode == SUPERVISED_SYSTEMD) { - redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Partial Resynchronization accepted. Ready to accept connections.\n"); - redisCommunicateSystemd("READY=1\n"); + redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Partial Resynchronization accepted. Ready to accept connections in read-write mode.\n"); } return; } diff --git a/src/server.c b/src/server.c index 145d515c4..7e0111cea 100644 --- a/src/server.c +++ b/src/server.c @@ -6294,10 +6294,10 @@ int main(int argc, char **argv) { if (server.supervised_mode == SUPERVISED_SYSTEMD) { if (!server.masterhost) { redisCommunicateSystemd("STATUS=Ready to accept connections\n"); - redisCommunicateSystemd("READY=1\n"); } else { - redisCommunicateSystemd("STATUS=Waiting for MASTER <-> REPLICA sync\n"); + redisCommunicateSystemd("STATUS=Ready to accept connections in read-only mode. Waiting for MASTER <-> REPLICA sync\n"); } + redisCommunicateSystemd("READY=1\n"); } } else { ACLLoadUsersAtStartup(); From 84d056d0f742444819355d6018a2329ede838215 Mon Sep 17 00:00:00 2001 From: Huang Zhw Date: Sun, 14 Mar 2021 15:41:43 +0800 Subject: [PATCH 036/131] Fix typo and outdated comments. (#8640) --- src/aof.c | 2 +- src/module.c | 2 +- src/multi.c | 4 ++-- src/replication.c | 37 +++++++++++++++++++++++++++---------- src/rio.c | 4 ++-- src/server.h | 2 +- 6 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/aof.c b/src/aof.c index f1586cf90..237c4034c 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1590,7 +1590,7 @@ int rewriteAppendOnlyFile(char *filename) { if (write(server.aof_pipe_write_ack_to_parent,"!",1) != 1) goto werr; if (anetNonBlock(NULL,server.aof_pipe_read_ack_from_parent) != ANET_OK) goto werr; - /* We read the ACK from the server using a 10 seconds timeout. Normally + /* We read the ACK from the server using a 5 seconds timeout. Normally * it should reply ASAP, but just in case we lose its reply, we are sure * the child will eventually get terminated. */ if (syncRead(server.aof_pipe_read_ack_from_parent,&byte,1,5000) != 1 || diff --git a/src/module.c b/src/module.c index d42207bbd..c602dbddd 100644 --- a/src/module.c +++ b/src/module.c @@ -606,7 +606,7 @@ void moduleHandlePropagationAfterCommandCallback(RedisModuleCtx *ctx) { * a transaction. */ if (!server.propagate_in_transaction) return; - /* If this command is executed from with Lua or MULTI/EXEC we do noy + /* If this command is executed from with Lua or MULTI/EXEC we do not * need to propagate EXEC */ if (server.in_eval || server.in_exec) return; diff --git a/src/multi.c b/src/multi.c index 5d690b71f..4abdb7499 100644 --- a/src/multi.c +++ b/src/multi.c @@ -140,7 +140,7 @@ void execCommandPropagateExec(int dbid) { } /* Aborts a transaction, with a specific error message. - * The transaction is always aboarted with -EXECABORT so that the client knows + * The transaction is always aborted with -EXECABORT so that the client knows * the server exited the multi state, but the actual reason for the abort is * included too. * Note: 'error' may or may not end with \r\n. see addReplyErrorFormat. */ @@ -202,7 +202,7 @@ void execCommand(client *c) { c->cmd = c->mstate.commands[j].cmd; /* ACL permissions are also checked at the time of execution in case - * they were changed after the commands were ququed. */ + * they were changed after the commands were queued. */ int acl_errpos; int acl_retval = ACLCheckCommandPerm(c,&acl_errpos); if (acl_retval == ACL_OK && c->cmd->proc == publishCommand) diff --git a/src/replication.c b/src/replication.c index 7bbba1a9e..79c6000ad 100644 --- a/src/replication.c +++ b/src/replication.c @@ -892,17 +892,34 @@ void syncCommand(client *c) { } /* REPLCONF