From ba289244185fd717c4ebc52ae6ab02d9b7bcdd25 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 22e45d46fe8aeb1fa385fdca5db80eac6776a290 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 919fbf421d0a1aed5648d6df82ccca99105e6855 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 a678390e52cf0db89486fcecc47e8114449bb5dd 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 8e7282eb3ec17052cb55b597036a02d0c4200cdf 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 1333a46b7e45395cb47ddde760ab8de3833d9f73 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 c9577941442618e257c891bb1c382d2612d88c4f 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 4440133e9ac043e9f1db14d8630dfb08eaade9b2 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 9baaf858f940fbb5b5a8e90f37d5b6ae15ceb9ee 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 3067352a8d8883b459e3029c4edb9e536b207d48 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 36caf2e42b2ad4174af0760a597bc34d6e2569bd 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 dcbe8bfad143b32230125710756285be5179722b 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 5e042dbc05f8c92ff8f4210a39f2be43ae4f0e6e 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 dd34f703687ff3ce416fea604b44df1f733dc9ac 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 54f5499aee18f9cc3901b0bd4699f29d49bb9dce 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 5e762d847cc046e830b899b4d885d971d11df42b 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 3b4f14777569d1f35d20b9b5a7949ecc1803e2d2 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 3c16d6b32d0acb31f5ded65df2a12378979acba4 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 0517da3618be3af60d5e928f983a437d3172ff82 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 df83892760d19b1b7e92d76e72daf4834ad2df6c 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; }