From ad1c21283e7c69a359fedef0c4417e75e33a3b77 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 10:14:32 +0200 Subject: [PATCH 01/21] move restartAOFAfterSYNC from replicaofCommand to replicationUnsetMaster replicationUnsetMaster can be called from other places, not just replicaofCOmmand, and all of these need to restart AOF --- src/replication.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/replication.c b/src/replication.c index b7e77184a..5499ebc57 100644 --- a/src/replication.c +++ b/src/replication.c @@ -2399,6 +2399,10 @@ void replicationUnsetMaster(void) { moduleFireServerEvent(REDISMODULE_EVENT_REPLICATION_ROLE_CHANGED, REDISMODULE_EVENT_REPLROLECHANGED_NOW_MASTER, NULL); + + /* Restart the AOF subsystem in case we shut it down during a sync when + * we were still a slave. */ + if (server.aof_enabled && server.aof_state == AOF_OFF) restartAOFAfterSYNC(); } /* This function is called when the slave lose the connection with the @@ -2436,9 +2440,6 @@ void replicaofCommand(client *c) { serverLog(LL_NOTICE,"MASTER MODE enabled (user request from '%s')", client); sdsfree(client); - /* Restart the AOF subsystem in case we shut it down during a sync when - * we were still a slave. */ - if (server.aof_enabled && server.aof_state == AOF_OFF) restartAOFAfterSYNC(); } } else { long port; From 5e3b4c978b8ad018e468da241e757a1d73558301 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 10:07:17 +0200 Subject: [PATCH 02/21] freeClientAsync don't lock mutex if there's just one thread --- src/networking.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/networking.c b/src/networking.c index 496b0f3dc..2b0f7464a 100644 --- a/src/networking.c +++ b/src/networking.c @@ -1163,9 +1163,14 @@ void freeClientAsync(client *c) { * may access the list while Redis uses I/O threads. All the other accesses * are in the context of the main thread while the other threads are * idle. */ - static pthread_mutex_t async_free_queue_mutex = PTHREAD_MUTEX_INITIALIZER; if (c->flags & CLIENT_CLOSE_ASAP || c->flags & CLIENT_LUA) return; c->flags |= CLIENT_CLOSE_ASAP; + if (server.io_threads_num == 1) { + /* no need to bother with locking if there's just one thread (the main thread) */ + listAddNodeTail(server.clients_to_close,c); + return; + } + static pthread_mutex_t async_free_queue_mutex = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_lock(&async_free_queue_mutex); listAddNodeTail(server.clients_to_close,c); pthread_mutex_unlock(&async_free_queue_mutex); From 2af4039b3eace586564b35dba7b885316365a62d Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 09:41:45 +0200 Subject: [PATCH 03/21] reduce repeated calls to use_diskless_load this function possibly iterates on the module list --- src/replication.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/replication.c b/src/replication.c index 5499ebc57..a006d0ad1 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1354,7 +1354,7 @@ void disklessLoadRestoreBackups(redisDb *backup, int restore, int empty_db_flags void readSyncBulkPayload(connection *conn) { char buf[4096]; ssize_t nread, readlen, nwritten; - int use_diskless_load; + int use_diskless_load = useDisklessLoad(); redisDb *diskless_load_backup = NULL; int empty_db_flags = server.repl_slave_lazy_flush ? EMPTYDB_ASYNC : EMPTYDB_NO_FLAGS; @@ -1411,19 +1411,18 @@ void readSyncBulkPayload(connection *conn) { server.repl_transfer_size = 0; serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: receiving streamed RDB from master with EOF %s", - useDisklessLoad()? "to parser":"to disk"); + use_diskless_load? "to parser":"to disk"); } else { usemark = 0; server.repl_transfer_size = strtol(buf+1,NULL,10); serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: receiving %lld bytes from master %s", (long long) server.repl_transfer_size, - useDisklessLoad()? "to parser":"to disk"); + use_diskless_load? "to parser":"to disk"); } return; } - use_diskless_load = useDisklessLoad(); if (!use_diskless_load) { /* Read the data from the socket, store it to a file and search * for the EOF. */ From e92983ed3f83c54489d7988095b9fa56f7c17d1e Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 09:37:04 +0200 Subject: [PATCH 04/21] moduleRDBLoadError, add key name, and use panic rather than exit using panic rather than exit means you get s stack trace of the code path that experianced the error, and possibly other info. --- src/module.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/module.c b/src/module.c index 914c50df3..16b191535 100644 --- a/src/module.c +++ b/src/module.c @@ -3649,14 +3649,15 @@ void moduleRDBLoadError(RedisModuleIO *io) { io->error = 1; return; } - serverLog(LL_WARNING, + serverPanic( "Error loading data from RDB (short read or EOF). " "Read performed by module '%s' about type '%s' " - "after reading '%llu' bytes of a value.", + "after reading '%llu' bytes of a value " + "for key named: '%s'.", io->type->module->name, io->type->name, - (unsigned long long)io->bytes); - exit(1); + (unsigned long long)io->bytes, + io->key? (char*)io->key->ptr: "(null)"); } /* Returns 0 if there's at least one registered data type that did not declare From a98e5a1fc31d922016558762537a104f33555789 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 09:33:20 +0200 Subject: [PATCH 05/21] Fix client flags to be int64 in module.c currently there's no bug since the flags these functions handle are always lower than 32bit, but still better fix the type to prevent future bugs. --- src/module.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/module.c b/src/module.c index 16b191535..3516b8f13 100644 --- a/src/module.c +++ b/src/module.c @@ -714,9 +714,9 @@ void RM_KeyAtPos(RedisModuleCtx *ctx, int pos) { * flags into the command flags used by the Redis core. * * It returns the set of flags, or -1 if unknown flags are found. */ -int commandFlagsFromString(char *s) { +int64_t commandFlagsFromString(char *s) { int count, j; - int flags = 0; + int64_t flags = 0; sds *tokens = sdssplitlen(s,strlen(s)," ",1,&count); for (j = 0; j < count; j++) { char *t = tokens[j]; @@ -798,7 +798,7 @@ int commandFlagsFromString(char *s) { * to authenticate a client. */ int RM_CreateCommand(RedisModuleCtx *ctx, const char *name, RedisModuleCmdFunc cmdfunc, const char *strflags, int firstkey, int lastkey, int keystep) { - int flags = strflags ? commandFlagsFromString((char*)strflags) : 0; + int64_t flags = strflags ? commandFlagsFromString((char*)strflags) : 0; if (flags == -1) return REDISMODULE_ERR; if ((flags & CMD_MODULE_NO_CLUSTER) && server.cluster_enabled) return REDISMODULE_ERR; From 600d02e1a73661405a13dea274318cd1a22411f0 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 09:23:22 +0200 Subject: [PATCH 06/21] fix maxmemory config warning the warning condition was if usage > limit (saying it'll cause eviction or oom), but in fact the eviction and oom depends on used minus slave buffers. other than fixing the condition, i add info about the current usage and limit, which may be useful when looking at the log. --- src/config.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/config.c b/src/config.c index d6be75795..1e8d0ffe2 100644 --- a/src/config.c +++ b/src/config.c @@ -1995,8 +1995,9 @@ static int updateMaxmemory(long long val, long long prev, char **err) { UNUSED(prev); UNUSED(err); if (val) { - if ((unsigned long long)val < zmalloc_used_memory()) { - serverLog(LL_WARNING,"WARNING: the new maxmemory value set via CONFIG SET is smaller than the current memory usage. This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy."); + size_t used = zmalloc_used_memory()-freeMemoryGetNotCountedMemory(); + if ((unsigned long long)val < used) { + serverLog(LL_WARNING,"WARNING: the new maxmemory value set via CONFIG SET (%llu) is smaller than the current memory usage (%zu). This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy.", server.maxmemory, used); } freeMemoryIfNeededAndSafe(); } From da8ec22fc62f85000b3ec045bc00d5bea1354e20 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 09:15:31 +0200 Subject: [PATCH 07/21] Memory leak when bind config is provided twice --- src/config.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/config.c b/src/config.c index 1e8d0ffe2..a6b374817 100644 --- a/src/config.c +++ b/src/config.c @@ -349,6 +349,10 @@ void loadServerConfigFromString(char *config) { if (addresses > CONFIG_BINDADDR_MAX) { err = "Too many bind addresses specified"; goto loaderr; } + /* Free old bind addresses */ + for (j = 0; j < server.bindaddr_count; j++) { + zfree(server.bindaddr[j]); + } for (j = 0; j < addresses; j++) server.bindaddr[j] = zstrdup(argv[j+1]); server.bindaddr_count = addresses; From ca22e14d8a409968f66d0ba72cd924c9d0aa34f5 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 08:53:23 +0200 Subject: [PATCH 08/21] A few non-data commands that should be allowed while loading or stale SELECT, and HELLO are commands that may be executed by the client as soon as it connects, there's no reason to block them, preventing the client from doing the rest of his sequence (which might just be INFO or CONFIG, etc). MONITOR, DEBUG, SLOWLOG, TIME, LASTSAVE are all non-data accessing commands, which there's no reason to block. --- src/server.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/server.c b/src/server.c index 1a3da249f..f87dfba46 100644 --- a/src/server.c +++ b/src/server.c @@ -579,7 +579,7 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"select",selectCommand,2, - "ok-loading fast @keyspace", + "ok-loading fast ok-stale @keyspace", 0,NULL,0,0,0,0,0,0}, {"swapdb",swapdbCommand,3, @@ -660,7 +660,7 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"lastsave",lastsaveCommand,1, - "read-only random fast @admin @dangerous", + "read-only random fast ok-loading ok-stale @admin @dangerous", 0,NULL,0,0,0,0,0,0}, {"type",typeCommand,2, @@ -708,7 +708,7 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"monitor",monitorCommand,1, - "admin no-script", + "admin no-script ok-loading ok-stale", 0,NULL,0,0,0,0,0,0}, {"ttl",ttlCommand,2, @@ -740,7 +740,7 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"debug",debugCommand,-2, - "admin no-script", + "admin no-script ok-loading ok-stale", 0,NULL,0,0,0,0,0,0}, {"config",configCommand,-2, @@ -820,11 +820,11 @@ struct redisCommand redisCommandTable[] = { 0,memoryGetKeys,0,0,0,0,0,0}, {"client",clientCommand,-2, - "admin no-script random @connection", + "admin no-script random ok-loading ok-stale @connection", 0,NULL,0,0,0,0,0,0}, {"hello",helloCommand,-2, - "no-auth no-script fast no-monitor no-slowlog @connection", + "no-auth no-script fast no-monitor ok-loading ok-stale no-slowlog @connection", 0,NULL,0,0,0,0,0,0}, /* EVAL can modify the dataset, however it is not flagged as a write @@ -838,7 +838,7 @@ struct redisCommand redisCommandTable[] = { 0,evalGetKeys,0,0,0,0,0,0}, {"slowlog",slowlogCommand,-2, - "admin random", + "admin random ok-loading ok-stale", 0,NULL,0,0,0,0,0,0}, {"script",scriptCommand,-2, @@ -846,7 +846,7 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"time",timeCommand,1, - "read-only random fast", + "read-only random fast ok-loading ok-stale", 0,NULL,0,0,0,0,0,0}, {"bitop",bitopCommand,-4, From e7e595192ab187032c8383517e916017bdda5222 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Wed, 5 Feb 2020 21:13:21 +0200 Subject: [PATCH 09/21] TLS: Update documentation. --- README.md | 18 ++++++++++++++++++ TLS.md | 45 ++++++++++++++------------------------------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 3442659e6..c08013416 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,11 @@ It is as simple as: % make +To build with TLS support, you'll need OpenSSL development libraries (e.g. +libssl-dev on Debian/Ubuntu) and run: + + % make BUILD_TLS=yes + You can run a 32 bit Redis binary using: % make 32bit @@ -43,6 +48,13 @@ After building Redis, it is a good idea to test it using: % make test +If TLS is built, running the tests with TLS enabled (you will need `tcl-tls` +installed): + + % ./utils/gen-test-certs.sh + % ./runtest --tls + + Fixing build problems with dependencies or cached build options --------- @@ -125,6 +137,12 @@ as options using the command line. Examples: All the options in redis.conf are also supported as options using the command line, with exactly the same name. +Running Redis with TLS: +------------------ + +Please consult the [TLS.md](TLS.md) file for more information on +how to use Redis with TLS. + Playing with Redis ------------------ diff --git a/TLS.md b/TLS.md index 76fe0be2e..e480c1e9d 100644 --- a/TLS.md +++ b/TLS.md @@ -1,8 +1,5 @@ -TLS Support -- Work In Progress -=============================== - -This is a brief note to capture current thoughts/ideas and track pending action -items. +TLS Support +=========== Getting Started --------------- @@ -69,37 +66,23 @@ probably not be so hard. For cluster keys migration it might be more difficult, but there are probably other good reasons to improve that part anyway. To-Do List -========== +---------- -Additional TLS Features ------------------------ +- [ ] Add session caching support. Check if/how it's handled by clients to + assess how useful/important it is. +- [ ] redis-benchmark support. The current implementation is a mix of using + hiredis for parsing and basic networking (establishing connections), but + directly manipulating sockets for most actions. This will need to be cleaned + up for proper TLS support. The best approach is probably to migrate to hiredis + async mode. +- [ ] redis-cli `--slave` and `--rdb` support. -1. Add metrics to INFO? -2. Add session caching support. Check if/how it's handled by clients to assess - how useful/important it is. - -redis-benchmark ---------------- - -The current implementation is a mix of using hiredis for parsing and basic -networking (establishing connections), but directly manipulating sockets for -most actions. - -This will need to be cleaned up for proper TLS support. The best approach is -probably to migrate to hiredis async mode. - -redis-cli ---------- - -1. Add support for TLS in --slave and --rdb modes. - -Others ------- +Multi-port +---------- Consider the implications of allowing TLS to be configured on a separate port, -making Redis listening on multiple ports. +making Redis listening on multiple ports: -This impacts many things, like 1. Startup banner port notification 2. Proctitle 3. How slaves announce themselves From 1f9520a75da9130cac9bd11d60aaef2bbc871d32 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Wed, 5 Feb 2020 19:47:09 +0200 Subject: [PATCH 10/21] Add handling of short read of module id in rdb --- src/rdb.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rdb.c b/src/rdb.c index 27e1b3135..61265433d 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1871,7 +1871,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, robj *key) { } } else if (rdbtype == RDB_TYPE_MODULE || rdbtype == RDB_TYPE_MODULE_2) { uint64_t moduleid = rdbLoadLen(rdb,NULL); - if (rioGetReadError(rdb)) return NULL; + if (rioGetReadError(rdb)) { + rdbReportReadError("Short read module id"); + return NULL; + } moduleType *mt = moduleTypeLookupModuleByID(moduleid); char name[10]; From a1a25bc7f180c9a62cd69bf4a870314fe18ffa88 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Wed, 5 Feb 2020 18:15:38 +0200 Subject: [PATCH 11/21] update RM_SignalModifiedKey doc comment --- src/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/module.c b/src/module.c index 3516b8f13..b39383637 100644 --- a/src/module.c +++ b/src/module.c @@ -890,7 +890,8 @@ void RM_SetModuleOptions(RedisModuleCtx *ctx, int options) { ctx->module->options = options; } -/* Signals that the key is modified from user's perspective (i.e. invalidate WATCH). */ +/* Signals that the key is modified from user's perspective (i.e. invalidate WATCH + * and client side caching). */ int RM_SignalModifiedKey(RedisModuleCtx *ctx, RedisModuleString *keyname) { signalModifiedKey(ctx->client->db,keyname); return REDISMODULE_OK; From 98a59c540d752626ebcda4334f5f42e85e61ca7b Mon Sep 17 00:00:00 2001 From: Guy Benoish Date: Mon, 3 Feb 2020 15:19:44 +0530 Subject: [PATCH 12/21] Exclude "keymiss" notification from NOTIFY_ALL Because "keymiss" is "special" compared to the rest of the notifications (Trying not to break existing apps using the 'A' format for notifications) Also updated redis.conf and module.c docs --- redis.conf | 6 +++++- src/module.c | 3 ++- src/notify.c | 2 +- src/redismodule.h | 4 ++-- src/server.h | 4 ++-- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/redis.conf b/redis.conf index 3c7336747..c04880f32 100644 --- a/redis.conf +++ b/redis.conf @@ -1361,7 +1361,11 @@ latency-monitor-threshold 0 # z Sorted set commands # x Expired events (events generated every time a key expires) # e Evicted events (events generated when a key is evicted for maxmemory) -# A Alias for g$lshzxe, so that the "AKE" string means all the events. +# t Stream commands +# m Key-miss events (Note: It is not included in the 'A' class) +# A Alias for g$lshzxet, so that the "AKE" string means all the events +# (Except key-miss events which are excluded from 'A' due to their +# unique nature). # # The "notify-keyspace-events" takes as argument a string that is composed # of zero or multiple characters. The empty string means that notifications diff --git a/src/module.c b/src/module.c index b39383637..ed79ee226 100644 --- a/src/module.c +++ b/src/module.c @@ -4807,7 +4807,8 @@ void moduleReleaseGIL(void) { * - REDISMODULE_NOTIFY_EXPIRED: Expiration events * - REDISMODULE_NOTIFY_EVICTED: Eviction events * - REDISMODULE_NOTIFY_STREAM: Stream events - * - REDISMODULE_NOTIFY_ALL: All events + * - REDISMODULE_NOTIFY_KEYMISS: Key-miss events + * - REDISMODULE_NOTIFY_ALL: All events (Excluding REDISMODULE_NOTIFY_KEYMISS) * * We do not distinguish between key events and keyspace events, and it is up * to the module to filter the actions taken based on the key. diff --git a/src/notify.c b/src/notify.c index d6c3ad403..bb1055724 100644 --- a/src/notify.c +++ b/src/notify.c @@ -82,10 +82,10 @@ sds keyspaceEventsFlagsToString(int flags) { if (flags & NOTIFY_EXPIRED) res = sdscatlen(res,"x",1); if (flags & NOTIFY_EVICTED) res = sdscatlen(res,"e",1); if (flags & NOTIFY_STREAM) res = sdscatlen(res,"t",1); - if (flags & NOTIFY_KEY_MISS) res = sdscatlen(res,"m",1); } if (flags & NOTIFY_KEYSPACE) res = sdscatlen(res,"K",1); if (flags & NOTIFY_KEYEVENT) res = sdscatlen(res,"E",1); + if (flags & NOTIFY_KEY_MISS) res = sdscatlen(res,"m",1); return res; } diff --git a/src/redismodule.h b/src/redismodule.h index 637078f2b..e74611f13 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -127,8 +127,8 @@ #define REDISMODULE_NOTIFY_EXPIRED (1<<8) /* x */ #define REDISMODULE_NOTIFY_EVICTED (1<<9) /* e */ #define REDISMODULE_NOTIFY_STREAM (1<<10) /* t */ -#define REDISMODULE_NOTIFY_KEY_MISS (1<<11) /* m */ -#define REDISMODULE_NOTIFY_ALL (REDISMODULE_NOTIFY_GENERIC | REDISMODULE_NOTIFY_STRING | REDISMODULE_NOTIFY_LIST | REDISMODULE_NOTIFY_SET | REDISMODULE_NOTIFY_HASH | REDISMODULE_NOTIFY_ZSET | REDISMODULE_NOTIFY_EXPIRED | REDISMODULE_NOTIFY_EVICTED | REDISMODULE_NOTIFY_STREAM | REDISMODULE_NOTIFY_KEY_MISS) /* A */ +#define REDISMODULE_NOTIFY_KEY_MISS (1<<11) /* m (Note: This one is excluded from REDISMODULE_NOTIFY_ALL on purpose) */ +#define REDISMODULE_NOTIFY_ALL (REDISMODULE_NOTIFY_GENERIC | REDISMODULE_NOTIFY_STRING | REDISMODULE_NOTIFY_LIST | REDISMODULE_NOTIFY_SET | REDISMODULE_NOTIFY_HASH | REDISMODULE_NOTIFY_ZSET | REDISMODULE_NOTIFY_EXPIRED | REDISMODULE_NOTIFY_EVICTED | REDISMODULE_NOTIFY_STREAM) /* A */ /* A special pointer that we can use between the core and the module to signal diff --git a/src/server.h b/src/server.h index 6a081c338..cf423a664 100644 --- a/src/server.h +++ b/src/server.h @@ -413,8 +413,8 @@ typedef long long ustime_t; /* microsecond time type. */ #define NOTIFY_EXPIRED (1<<8) /* x */ #define NOTIFY_EVICTED (1<<9) /* e */ #define NOTIFY_STREAM (1<<10) /* t */ -#define NOTIFY_KEY_MISS (1<<11) /* m */ -#define NOTIFY_ALL (NOTIFY_GENERIC | NOTIFY_STRING | NOTIFY_LIST | NOTIFY_SET | NOTIFY_HASH | NOTIFY_ZSET | NOTIFY_EXPIRED | NOTIFY_EVICTED | NOTIFY_STREAM | NOTIFY_KEY_MISS) /* A flag */ +#define NOTIFY_KEY_MISS (1<<11) /* m (Note: This one is excluded from NOTIFY_ALL on purpose) */ +#define NOTIFY_ALL (NOTIFY_GENERIC | NOTIFY_STRING | NOTIFY_LIST | NOTIFY_SET | NOTIFY_HASH | NOTIFY_ZSET | NOTIFY_EXPIRED | NOTIFY_EVICTED | NOTIFY_STREAM) /* A flag */ /* Get the first bind addr or NULL */ #define NET_FIRST_BIND_ADDR (server.bindaddr_count ? server.bindaddr[0] : NULL) From af3ff9d7339fd16a460606a9cd9dddd26ba5f6b5 Mon Sep 17 00:00:00 2001 From: lifubang Date: Tue, 4 Feb 2020 17:32:30 +0800 Subject: [PATCH 13/21] fix ssl flag check for redis-cli Signed-off-by: lifubang --- src/redis-cli.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 1919829e1..c1bda0a00 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -1595,15 +1595,15 @@ static int parseOptions(int argc, char **argv) { #ifdef USE_OPENSSL } else if (!strcmp(argv[i],"--tls")) { config.tls = 1; - } else if (!strcmp(argv[i],"--sni")) { + } else if (!strcmp(argv[i],"--sni") && !lastarg) { config.sni = argv[++i]; - } else if (!strcmp(argv[i],"--cacertdir")) { + } else if (!strcmp(argv[i],"--cacertdir") && !lastarg) { config.cacertdir = argv[++i]; - } else if (!strcmp(argv[i],"--cacert")) { + } else if (!strcmp(argv[i],"--cacert") && !lastarg) { config.cacert = argv[++i]; - } else if (!strcmp(argv[i],"--cert")) { + } else if (!strcmp(argv[i],"--cert") && !lastarg) { config.cert = argv[++i]; - } else if (!strcmp(argv[i],"--key")) { + } else if (!strcmp(argv[i],"--key") && !lastarg) { config.key = argv[++i]; #endif } else if (!strcmp(argv[i],"-v") || !strcmp(argv[i], "--version")) { @@ -1701,12 +1701,13 @@ static void usage(void) { " -c Enable cluster mode (follow -ASK and -MOVED redirections).\n" #ifdef USE_OPENSSL " --tls Establish a secure TLS connection.\n" -" --cacert CA Certificate file to verify with.\n" -" --cacertdir Directory where trusted CA certificates are stored.\n" +" --sni Server name indication for TLS.\n" +" --cacert CA Certificate file to verify with.\n" +" --cacertdir Directory where trusted CA certificates are stored.\n" " If neither cacert nor cacertdir are specified, the default\n" " system-wide trusted root certs configuration will apply.\n" -" --cert Client certificate to authenticate with.\n" -" --key Private key file to authenticate with.\n" +" --cert Client certificate to authenticate with.\n" +" --key Private key file to authenticate with.\n" #endif " --raw Use raw formatting for replies (default when STDOUT is\n" " not a tty).\n" From 05b2e8c5c8d139be0e29302b7b80d684affde0b7 Mon Sep 17 00:00:00 2001 From: Guy Benoish Date: Mon, 3 Feb 2020 17:19:00 +0530 Subject: [PATCH 14/21] Diskless-load emptyDb-related fixes 1. Call emptyDb even in case of diskless-load: We want modules to get the same FLUSHDB event as disk-based replication. 2. Do not fire any module events when flushing the backups array. 3. Delete redundant call to signalFlushedDb (Called from emptyDb). --- src/db.c | 57 ++++++++++++++++++++++++++++------------------- src/replication.c | 14 +++++++----- src/server.h | 1 + 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/db.c b/src/db.c index 88342ac4d..c80524e94 100644 --- a/src/db.c +++ b/src/db.c @@ -347,7 +347,10 @@ robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o) { * DB number if we want to flush only a single Redis database number. * * Flags are be EMPTYDB_NO_FLAGS if no special flags are specified or - * EMPTYDB_ASYNC if we want the memory to be freed in a different thread + * 1. EMPTYDB_ASYNC if we want the memory to be freed in a different thread. + * 2. EMPTYDB_BACKUP if we want to empty the backup dictionaries created by + * disklessLoadMakeBackups. In that case we only free memory and avoid + * firing module events. * and the function to return ASAP. * * On success the fuction returns the number of keys removed from the @@ -355,6 +358,8 @@ robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o) { * DB number is out of range, and errno is set to EINVAL. */ long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)(void*)) { int async = (flags & EMPTYDB_ASYNC); + int backup = (flags & EMPTYDB_BACKUP); /* Just free the memory, nothing else */ + RedisModuleFlushInfoV1 fi = {REDISMODULE_FLUSHINFO_VERSION,!async,dbnum}; long long removed = 0; if (dbnum < -1 || dbnum >= server.dbnum) { @@ -362,16 +367,18 @@ long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)( return -1; } - /* Fire the flushdb modules event. */ - RedisModuleFlushInfoV1 fi = {REDISMODULE_FLUSHINFO_VERSION,!async,dbnum}; - moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, - REDISMODULE_SUBEVENT_FLUSHDB_START, - &fi); + /* Pre-flush actions */ + if (!backup) { + /* Fire the flushdb modules event. */ + moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, + REDISMODULE_SUBEVENT_FLUSHDB_START, + &fi); - /* Make sure the WATCHed keys are affected by the FLUSH* commands. - * Note that we need to call the function while the keys are still - * there. */ - signalFlushedDb(dbnum); + /* Make sure the WATCHed keys are affected by the FLUSH* commands. + * Note that we need to call the function while the keys are still + * there. */ + signalFlushedDb(dbnum); + } int startdb, enddb; if (dbnum == -1) { @@ -390,20 +397,24 @@ long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)( dictEmpty(dbarray[j].expires,callback); } } - if (server.cluster_enabled) { - if (async) { - slotToKeyFlushAsync(); - } else { - slotToKeyFlush(); - } - } - if (dbnum == -1) flushSlaveKeysWithExpireList(); - /* Also fire the end event. Note that this event will fire almost - * immediately after the start event if the flush is asynchronous. */ - moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, - REDISMODULE_SUBEVENT_FLUSHDB_END, - &fi); + /* Post-flush actions */ + if (!backup) { + if (server.cluster_enabled) { + if (async) { + slotToKeyFlushAsync(); + } else { + slotToKeyFlush(); + } + } + if (dbnum == -1) flushSlaveKeysWithExpireList(); + + /* Also fire the end event. Note that this event will fire almost + * immediately after the start event if the flush is asynchronous. */ + moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, + REDISMODULE_SUBEVENT_FLUSHDB_END, + &fi); + } return removed; } diff --git a/src/replication.c b/src/replication.c index a006d0ad1..4843f97d5 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1339,8 +1339,8 @@ void disklessLoadRestoreBackups(redisDb *backup, int restore, int empty_db_flags server.db[i] = backup[i]; } } else { - /* Delete. */ - emptyDbGeneric(backup,-1,empty_db_flags,replicationEmptyDbCallback); + /* Delete (Pass EMPTYDB_BACKUP in order to avoid firing module events) . */ + emptyDbGeneric(backup,-1,empty_db_flags|EMPTYDB_BACKUP,replicationEmptyDbCallback); for (int i=0; i Date: Mon, 10 Feb 2020 16:32:46 +0900 Subject: [PATCH 15/21] [FIX] revisit CVE-2015-8080 vulnerability --- deps/lua/src/lua_struct.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/deps/lua/src/lua_struct.c b/deps/lua/src/lua_struct.c index 4d5f027b8..c58c8e72b 100644 --- a/deps/lua/src/lua_struct.c +++ b/deps/lua/src/lua_struct.c @@ -89,12 +89,14 @@ typedef struct Header { } Header; -static int getnum (const char **fmt, int df) { +static int getnum (lua_State *L, const char **fmt, int df) { if (!isdigit(**fmt)) /* no number? */ return df; /* return default value */ else { int a = 0; do { + if (a > (INT_MAX / 10) || a * 10 > (INT_MAX - (**fmt - '0'))) + luaL_error(L, "integral size overflow"); a = a*10 + *((*fmt)++) - '0'; } while (isdigit(**fmt)); return a; @@ -115,9 +117,9 @@ static size_t optsize (lua_State *L, char opt, const char **fmt) { case 'f': return sizeof(float); case 'd': return sizeof(double); case 'x': return 1; - case 'c': return getnum(fmt, 1); + case 'c': return getnum(L, fmt, 1); case 'i': case 'I': { - int sz = getnum(fmt, sizeof(int)); + int sz = getnum(L, fmt, sizeof(int)); if (sz > MAXINTSIZE) luaL_error(L, "integral size %d is larger than limit of %d", sz, MAXINTSIZE); @@ -150,7 +152,7 @@ static void controloptions (lua_State *L, int opt, const char **fmt, case '>': h->endian = BIG; return; case '<': h->endian = LITTLE; return; case '!': { - int a = getnum(fmt, MAXALIGN); + int a = getnum(L, fmt, MAXALIGN); if (!isp2(a)) luaL_error(L, "alignment %d is not a power of 2", a); h->align = a; From 5b3a6b5e80655a6801857dc866349ca7a976df51 Mon Sep 17 00:00:00 2001 From: lifubang Date: Wed, 12 Feb 2020 16:34:22 +0800 Subject: [PATCH 16/21] correct help info for --user and --pass Signed-off-by: lifubang --- src/redis-cli.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index c1bda0a00..1d79e0db0 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -1688,8 +1688,8 @@ static void usage(void) { " You can also use the " REDIS_CLI_AUTH_ENV " environment\n" " variable to pass this password more safely\n" " (if both are used, this argument takes predecence).\n" -" -user Used to send ACL style 'AUTH username pass'. Needs -a.\n" -" -pass Alias of -a for consistency with the new --user option.\n" +" --user Used to send ACL style 'AUTH username pass'. Needs -a.\n" +" --pass Alias of -a for consistency with the new --user option.\n" " -u Server URI.\n" " -r Execute specified command N times.\n" " -i When -r is used, waits seconds per command.\n" From 0545a70d52df76c5e47f9d18cff4eef2e18b6228 Mon Sep 17 00:00:00 2001 From: Khem Raj Date: Sat, 21 Dec 2019 11:17:50 -0800 Subject: [PATCH 17/21] Mark extern definition of SDS_NOINIT in sds.h This helps in avoiding multiple definition of this variable, its also defined globally in sds.c Signed-off-by: Khem Raj --- src/sds.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sds.h b/src/sds.h index 1bdb60dec..adcc12c0a 100644 --- a/src/sds.h +++ b/src/sds.h @@ -34,7 +34,7 @@ #define __SDS_H #define SDS_MAX_PREALLOC (1024*1024) -const char *SDS_NOINIT; +extern const char *SDS_NOINIT; #include #include From d733b3ba170533068da0c1bc3016f43f44a7f9aa Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 6 Feb 2020 15:06:33 +0200 Subject: [PATCH 18/21] add no-slowlog option to RM_CreateCommand --- src/module.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/module.c b/src/module.c index ed79ee226..aae15d74b 100644 --- a/src/module.c +++ b/src/module.c @@ -730,6 +730,7 @@ int64_t commandFlagsFromString(char *s) { else if (!strcasecmp(t,"random")) flags |= CMD_RANDOM; else if (!strcasecmp(t,"allow-stale")) flags |= CMD_STALE; else if (!strcasecmp(t,"no-monitor")) flags |= CMD_SKIP_MONITOR; + else if (!strcasecmp(t,"no-slowlog")) flags |= CMD_SKIP_SLOWLOG; else if (!strcasecmp(t,"fast")) flags |= CMD_FAST; else if (!strcasecmp(t,"no-auth")) flags |= CMD_NO_AUTH; else if (!strcasecmp(t,"getkeys-api")) flags |= CMD_MODULE_GETKEYS; @@ -781,6 +782,8 @@ int64_t commandFlagsFromString(char *s) { * this means. * * **"no-monitor"**: Don't propagate the command on monitor. Use this if * the command has sensible data among the arguments. + * * **"no-slowlog"**: Don't log this command in the slowlog. Use this if + * the command has sensible data among the arguments. * * **"fast"**: The command time complexity is not greater * than O(log(N)) where N is the size of the collection or * anything else representing the normal scalability From fce22c6a42942c03e47d938ec0e23eac850e3018 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 7 Feb 2020 14:03:43 +0100 Subject: [PATCH 19/21] Tracking: first conversion from hashing to key names. --- src/server.c | 4 +- src/server.h | 2 +- src/tracking.c | 206 +++++++++++++++++++++---------------------------- 3 files changed, 91 insertions(+), 121 deletions(-) diff --git a/src/server.c b/src/server.c index f87dfba46..df6a8c1f4 100644 --- a/src/server.c +++ b/src/server.c @@ -4221,7 +4221,7 @@ sds genRedisInfoString(const char *section) { "active_defrag_misses:%lld\r\n" "active_defrag_key_hits:%lld\r\n" "active_defrag_key_misses:%lld\r\n" - "tracking_used_slots:%lld\r\n", + "tracking_tracked_keys:%lld\r\n", server.stat_numconnections, server.stat_numcommands, getInstantaneousMetric(STATS_METRIC_COMMAND), @@ -4249,7 +4249,7 @@ sds genRedisInfoString(const char *section) { server.stat_active_defrag_misses, server.stat_active_defrag_key_hits, server.stat_active_defrag_key_misses, - trackingGetUsedSlots()); + (unsigned long long) trackingGetTotalItems()); } /* Replication */ diff --git a/src/server.h b/src/server.h index dfbc15ac3..a30db6b57 100644 --- a/src/server.h +++ b/src/server.h @@ -1654,7 +1654,7 @@ void trackingRememberKeys(client *c); void trackingInvalidateKey(robj *keyobj); void trackingInvalidateKeysOnFlush(int dbid); void trackingLimitUsedSlots(void); -unsigned long long trackingGetUsedSlots(void); +uint64_t trackingGetTotalItems(void); /* List data type */ void listTypeTryConversion(robj *subject, robj *value); diff --git a/src/tracking.c b/src/tracking.c index acb97800a..ecb7fbdcc 100644 --- a/src/tracking.c +++ b/src/tracking.c @@ -30,37 +30,22 @@ #include "server.h" -/* The tracking table is constituted by 2^24 radix trees (each tree, and the - * table itself, are allocated in a lazy way only when needed) tracking - * clients that may have certain keys in their local, client side, cache. - * - * Keys are grouped into 2^24 slots, in a way similar to Redis Cluster hash - * slots, however here the function we use is crc64, taking the least - * significant 24 bits of the output. +/* The tracking table is constituted by a radix tree of keys, each pointing + * to a radix tree of client IDs, used to track the clients that may have + * certain keys in their local, client side, cache. * * When a client enables tracking with "CLIENT TRACKING on", each key served to - * the client is hashed to one of such slots, and Redis will remember what - * client may have keys about such slot. Later, when a key in a given slot is - * modified, all the clients that may have local copies of keys in that slot - * will receive an invalidation message. There is no distinction of database - * number: a single table is used. + * the client is remembered in the table mapping the keys to the client IDs. + * Later, when a key is modified, all the clients that may have local copy + * of such key will receive an invalidation message. * * Clients will normally take frequently requested objects in memory, removing - * them when invalidation messages are received. A strategy clients may use is - * to just cache objects in a dictionary, associating to each cached object - * some incremental epoch, or just a timestamp. When invalidation messages are - * received clients may store, in a different table, the timestamp (or epoch) - * of the invalidation of such given slot: later when accessing objects, the - * eviction of stale objects may be performed in a lazy way by checking if the - * cached object timestamp is older than the invalidation timestamp for such - * objects. - * - * The output of the 24 bit hash function is very large (more than 16 million - * possible slots), so clients that may want to use less resources may only - * use the most significant bits instead of the full 24 bits. */ -#define TRACKING_TABLE_SIZE (1<<24) -rax **TrackingTable = NULL; -unsigned long TrackingTableUsedSlots = 0; + * them when invalidation messages are received. */ +rax *TrackingTable = NULL; +uint64_t TrackingTableTotalItems = 0; /* Total number of IDs stored across + the whole tracking table. This givesn + an hint about the total memory we + are using server side for CSC. */ robj *TrackingChannelName; /* Remove the tracking state from the client 'c'. Note that there is not much @@ -90,7 +75,7 @@ void enableTracking(client *c, uint64_t redirect_to) { c->client_tracking_redirection = redirect_to; server.tracking_clients++; if (TrackingTable == NULL) { - TrackingTable = zcalloc(sizeof(rax*) * TRACKING_TABLE_SIZE); + TrackingTable = raxNew(); TrackingChannelName = createStringObject("__redis__:invalidate",20); } } @@ -108,19 +93,20 @@ void trackingRememberKeys(client *c) { for(int j = 0; j < numkeys; j++) { int idx = keys[j]; sds sdskey = c->argv[idx]->ptr; - uint64_t hash = crc64(0, - (unsigned char*)sdskey,sdslen(sdskey))&(TRACKING_TABLE_SIZE-1); - if (TrackingTable[hash] == NULL) { - TrackingTable[hash] = raxNew(); - TrackingTableUsedSlots++; + rax *ids = raxFind(TrackingTable,(unsigned char*)sdskey,sdslen(sdskey)); + if (ids == raxNotFound) { + ids = raxNew(); + int inserted = raxTryInsert(TrackingTable,(unsigned char*)sdskey, + sdslen(sdskey),ids, NULL); + serverAssert(inserted == 1); } - raxTryInsert(TrackingTable[hash], - (unsigned char*)&c->id,sizeof(c->id),NULL,NULL); + if (raxTryInsert(ids,(unsigned char*)&c->id,sizeof(c->id),NULL,NULL)) + TrackingTableTotalItems++; } getKeysFreeResult(keys); } -void sendTrackingMessage(client *c, long long hash) { +void sendTrackingMessage(client *c, char *keyname, size_t keylen) { int using_redirection = 0; if (c->client_tracking_redirection) { client *redir = lookupClientByID(c->client_tracking_redirection); @@ -146,49 +132,44 @@ void sendTrackingMessage(client *c, long long hash) { if (c->resp > 2) { addReplyPushLen(c,2); addReplyBulkCBuffer(c,"invalidate",10); - addReplyLongLong(c,hash); + addReplyBulkCBuffer(c,keyname,keylen); } else if (using_redirection && c->flags & CLIENT_PUBSUB) { - robj *msg = createStringObjectFromLongLong(hash); - addReplyPubsubMessage(c,TrackingChannelName,msg); - decrRefCount(msg); + /* We use a static object to speedup things, however we assume + * that addReplyPubsubMessage() will not take a reference. */ + robj keyobj; + initStaticStringObject(keyobj,keyname); + addReplyPubsubMessage(c,TrackingChannelName,&keyobj); + serverAssert(keyobj.refcount == 1); } } -/* Invalidates a caching slot: this is actually the low level implementation - * of the API that Redis calls externally, that is trackingInvalidateKey(). */ -void trackingInvalidateSlot(uint64_t slot) { - if (TrackingTable == NULL || TrackingTable[slot] == NULL) return; - - raxIterator ri; - raxStart(&ri,TrackingTable[slot]); - raxSeek(&ri,"^",NULL,0); - while(raxNext(&ri)) { - uint64_t id; - memcpy(&id,ri.key,sizeof(id)); - client *c = lookupClientByID(id); - if (c == NULL || !(c->flags & CLIENT_TRACKING)) continue; - sendTrackingMessage(c,slot); - } - raxStop(&ri); - - /* Free the tracking table: we'll create the radix tree and populate it - * again if more keys will be modified in this caching slot. */ - raxFree(TrackingTable[slot]); - TrackingTable[slot] = NULL; - TrackingTableUsedSlots--; -} - /* This function is called from signalModifiedKey() or other places in Redis * when a key changes value. In the context of keys tracking, our task here is * to send a notification to every client that may have keys about such caching * slot. */ void trackingInvalidateKey(robj *keyobj) { - if (TrackingTable == NULL || TrackingTableUsedSlots == 0) return; - + if (TrackingTable == NULL) return; sds sdskey = keyobj->ptr; - uint64_t hash = crc64(0, - (unsigned char*)sdskey,sdslen(sdskey))&(TRACKING_TABLE_SIZE-1); - trackingInvalidateSlot(hash); + rax *ids = raxFind(TrackingTable,(unsigned char*)sdskey,sdslen(sdskey)); + if (ids == raxNotFound) return;; + + raxIterator ri; + raxStart(&ri,ids); + raxSeek(&ri,"^",NULL,0); + while(raxNext(&ri)) { + uint64_t id; + memcpy(&id,ri.key,sizeof(id)); + client *c = lookupClientByID(id); + if (c == NULL || !(c->flags & CLIENT_TRACKING)) continue; + sendTrackingMessage(c,sdskey,sdslen(sdskey)); + } + raxStop(&ri); + + /* Free the tracking table: we'll create the radix tree and populate it + * again if more keys will be modified in this caching slot. */ + TrackingTableTotalItems -= raxSize(ids); + raxFree(ids); + raxRemove(TrackingTable,(unsigned char*)sdskey,sdslen(sdskey),NULL); } /* This function is called when one or all the Redis databases are flushed @@ -205,6 +186,10 @@ void trackingInvalidateKey(robj *keyobj) { * we just send the invalidation message to all the clients, but don't * flush the table: it will slowly get garbage collected as more keys * are modified in the used caching slots. */ +void freeTrackingRadixTree(void *rt) { + raxFree(rt); +} + void trackingInvalidateKeysOnFlush(int dbid) { if (server.tracking_clients) { listNode *ln; @@ -213,84 +198,69 @@ void trackingInvalidateKeysOnFlush(int dbid) { while ((ln = listNext(&li)) != NULL) { client *c = listNodeValue(ln); if (c->flags & CLIENT_TRACKING) { - sendTrackingMessage(c,-1); + sendTrackingMessage(c,"",1); } } } /* In case of FLUSHALL, reclaim all the memory used by tracking. */ if (dbid == -1 && TrackingTable) { - for (int j = 0; j < TRACKING_TABLE_SIZE && TrackingTableUsedSlots > 0; j++) { - if (TrackingTable[j] != NULL) { - raxFree(TrackingTable[j]); - TrackingTable[j] = NULL; - TrackingTableUsedSlots--; - } - } - - /* If there are no clients with tracking enabled, we can even - * reclaim the memory used by the table itself. The code assumes - * the table is allocated only if there is at least one client alive - * with tracking enabled. */ - if (server.tracking_clients == 0) { - zfree(TrackingTable); - TrackingTable = NULL; - } + raxFreeWithCallback(TrackingTable,freeTrackingRadixTree); + TrackingTableTotalItems = 0; } } /* Tracking forces Redis to remember information about which client may have - * keys about certian caching slots. In workloads where there are a lot of - * reads, but keys are hardly modified, the amount of information we have - * to remember server side could be a lot: for each 16 millions of caching - * slots we may end with a radix tree containing many entries. + * certain keys. In workloads where there are a lot of reads, but keys are + * hardly modified, the amount of information we have to remember server side + * could be a lot, with the number of keys being totally not bound. * - * So Redis allows the user to configure a maximum fill rate for the + * So Redis allows the user to configure a maximum number of keys for the * invalidation table. This function makes sure that we don't go over the * specified fill rate: if we are over, we can just evict informations about - * random caching slots, and send invalidation messages to clients like if - * the key was modified. */ + * a random key, and send invalidation messages to clients like if the key was + * modified. */ void trackingLimitUsedSlots(void) { static unsigned int timeout_counter = 0; - + if (TrackingTable == NULL) return; if (server.tracking_table_max_fill == 0) return; /* No limits set. */ - unsigned int max_slots = - (TRACKING_TABLE_SIZE/100) * server.tracking_table_max_fill; - if (TrackingTableUsedSlots <= max_slots) { + size_t max_keys = server.tracking_table_max_fill; + if (raxSize(TrackingTable) <= max_keys) { timeout_counter = 0; return; /* Limit not reached. */ } - /* We have to invalidate a few slots to reach the limit again. The effort + /* We have to invalidate a few keys to reach the limit again. The effort * we do here is proportional to the number of times we entered this * function and found that we are still over the limit. */ int effort = 100 * (timeout_counter+1); - /* Let's start at a random position, and perform linear probing, in order - * to improve cache locality. However once we are able to find an used - * slot, jump again randomly, in order to avoid creating big holes in the - * table (that will make this funciton use more resourced later). */ + /* We just remove one key after another by using a random walk. */ + raxIterator ri; + raxStart(&ri,TrackingTable); while(effort > 0) { - unsigned int idx = rand() % TRACKING_TABLE_SIZE; - do { - effort--; - idx = (idx+1) % TRACKING_TABLE_SIZE; - if (TrackingTable[idx] != NULL) { - trackingInvalidateSlot(idx); - if (TrackingTableUsedSlots <= max_slots) { - timeout_counter = 0; - return; /* Return ASAP: we are again under the limit. */ - } else { - break; /* Jump to next random position. */ - } - } - } while(effort > 0); + effort--; + raxSeek(&ri,"^",NULL,0); + raxRandomWalk(&ri,0); + rax *ids = ri.data; + TrackingTableTotalItems -= raxSize(ids); + raxFree(ids); + raxRemove(TrackingTable,ri.key,ri.key_len,NULL); + if (raxSize(TrackingTable) <= max_keys) { + timeout_counter = 0; + raxStop(&ri); + return; /* Return ASAP: we are again under the limit. */ + } } + + /* If we reach this point, we were not able to go under the configured + * limit using the maximum effort we had for this run. */ + raxStop(&ri); timeout_counter++; } /* This is just used in order to access the amount of used slots in the * tracking table. */ -unsigned long long trackingGetUsedSlots(void) { - return TrackingTableUsedSlots; +uint64_t trackingGetTotalItems(void) { + return TrackingTableTotalItems; } From e264fa89176bfacf78501b58d5b52714d54621df Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 7 Feb 2020 17:19:11 +0100 Subject: [PATCH 20/21] Tracking: rename INFO field with total items. --- src/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.c b/src/server.c index df6a8c1f4..2cf8dbf19 100644 --- a/src/server.c +++ b/src/server.c @@ -4221,7 +4221,7 @@ sds genRedisInfoString(const char *section) { "active_defrag_misses:%lld\r\n" "active_defrag_key_hits:%lld\r\n" "active_defrag_key_misses:%lld\r\n" - "tracking_tracked_keys:%lld\r\n", + "tracking_total_items:%lld\r\n", server.stat_numconnections, server.stat_numcommands, getInstantaneousMetric(STATS_METRIC_COMMAND), From 13fbdf970660b15011c4312f31137e58bbda5b2c Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 7 Feb 2020 18:12:10 +0100 Subject: [PATCH 21/21] Rax.c: populate data field after random walk. --- src/rax.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rax.c b/src/rax.c index 29b74ae90..a560dde02 100644 --- a/src/rax.c +++ b/src/rax.c @@ -1766,6 +1766,7 @@ int raxRandomWalk(raxIterator *it, size_t steps) { if (n->iskey) steps--; } it->node = n; + it->data = raxGetData(it->node); return 1; }