From abd5156f259ace9dd5923d51da4646ba39c1e29f Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Wed, 17 Jul 2019 11:00:51 +0800 Subject: [PATCH 1/6] lazyfree: add a new configuration lazyfree-lazy-user-del Delete keys in async way when executing DEL command, if lazyfree-lazy-user-del is yes. --- redis.conf | 5 ++++- src/config.c | 1 + src/db.c | 2 +- src/server.h | 1 + 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/redis.conf b/redis.conf index 7c55a3ab0..14388e320 100644 --- a/redis.conf +++ b/redis.conf @@ -936,7 +936,9 @@ replica-priority 100 # or SORT with STORE option may delete existing keys. The SET command # itself removes any old content of the specified key in order to replace # it with the specified string. -# 4) During replication, when a replica performs a full resynchronization with +# 4) The DEL command itself, and normally it's not easy to replace DEL with +# UNLINK in user's codes. +# 5) During replication, when a replica performs a full resynchronization with # its master, the content of the whole database is removed in order to # load the RDB file just transferred. # @@ -948,6 +950,7 @@ replica-priority 100 lazyfree-lazy-eviction no lazyfree-lazy-expire no lazyfree-lazy-server-del no +lazyfree-lazy-user-del no replica-lazy-flush no ################################ THREADED I/O ################################# diff --git a/src/config.c b/src/config.c index 7c87ebe6e..e0cbcc281 100644 --- a/src/config.c +++ b/src/config.c @@ -2099,6 +2099,7 @@ standardConfig configs[] = { createBoolConfig("lazyfree-lazy-eviction", NULL, MODIFIABLE_CONFIG, server.lazyfree_lazy_eviction, 0, NULL, NULL), createBoolConfig("lazyfree-lazy-expire", NULL, MODIFIABLE_CONFIG, server.lazyfree_lazy_expire, 0, NULL, NULL), createBoolConfig("lazyfree-lazy-server-del", NULL, MODIFIABLE_CONFIG, server.lazyfree_lazy_server_del, 0, NULL, NULL), + createBoolConfig("lazyfree-lazy-user-del", NULL, MODIFIABLE_CONFIG, server.lazyfree_lazy_user_del , 0, NULL, NULL), createBoolConfig("repl-disable-tcp-nodelay", NULL, MODIFIABLE_CONFIG, server.repl_disable_tcp_nodelay, 0, NULL, NULL), createBoolConfig("repl-diskless-sync", NULL, MODIFIABLE_CONFIG, server.repl_diskless_sync, 0, NULL, NULL), createBoolConfig("gopher-enabled", NULL, MODIFIABLE_CONFIG, server.gopher_enabled, 0, NULL, NULL), diff --git a/src/db.c b/src/db.c index 6e5a8bf3a..9b5d62f29 100644 --- a/src/db.c +++ b/src/db.c @@ -556,7 +556,7 @@ void delGenericCommand(client *c, int lazy) { } void delCommand(client *c) { - delGenericCommand(c,0); + delGenericCommand(c,server.lazyfree_lazy_user_del); } void unlinkCommand(client *c) { diff --git a/src/server.h b/src/server.h index b17995948..9b77f55ac 100644 --- a/src/server.h +++ b/src/server.h @@ -1397,6 +1397,7 @@ struct redisServer { int lazyfree_lazy_eviction; int lazyfree_lazy_expire; int lazyfree_lazy_server_del; + int lazyfree_lazy_user_del; /* Latency monitor */ long long latency_monitor_threshold; dict *latency_events; From c8dbcff9dbe724dfa0c369a289e6f2f6dda13cab Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 6 Apr 2020 17:32:04 +0200 Subject: [PATCH 2/6] Clarify redis.conf comment about lazyfree-lazy-user-del. --- redis.conf | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/redis.conf b/redis.conf index 14388e320..5baeae65f 100644 --- a/redis.conf +++ b/redis.conf @@ -936,23 +936,27 @@ replica-priority 100 # or SORT with STORE option may delete existing keys. The SET command # itself removes any old content of the specified key in order to replace # it with the specified string. -# 4) The DEL command itself, and normally it's not easy to replace DEL with -# UNLINK in user's codes. -# 5) During replication, when a replica performs a full resynchronization with +# 4) During replication, when a replica performs a full resynchronization with # its master, the content of the whole database is removed in order to # load the RDB file just transferred. # # In all the above cases the default is to delete objects in a blocking way, # like if DEL was called. However you can configure each case specifically # in order to instead release memory in a non-blocking way like if UNLINK -# was called, using the following configuration directives: +# was called, using the following configuration directives. lazyfree-lazy-eviction no lazyfree-lazy-expire no lazyfree-lazy-server-del no -lazyfree-lazy-user-del no replica-lazy-flush no +# It is also possible, for the case when to replace the user code DEL calls +# with UNLINK calls is not easy, to modify the default behavior of the DEL +# command to act exactly like UNLINK, using the following configuration +# directive: + +lazyfree-lazy-user-del no + ################################ THREADED I/O ################################# # Redis is mostly single threaded, however there are certain threaded From c3ac717487099ef8b3a9a47a2f8f147b3230b3ba Mon Sep 17 00:00:00 2001 From: qetu3790 Date: Mon, 6 Apr 2020 20:52:32 +0800 Subject: [PATCH 3/6] fix comments about RESIZE DB opcode in rdb.c fix comments about RESIZE DB opcode in rdb.c --- src/rdb.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index fc8911979..78ec83cce 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1175,10 +1175,7 @@ int rdbSaveRio(rio *rdb, int *error, int rdbflags, rdbSaveInfo *rsi) { if (rdbSaveType(rdb,RDB_OPCODE_SELECTDB) == -1) goto werr; if (rdbSaveLen(rdb,j) == -1) goto werr; - /* Write the RESIZE DB opcode. We trim the size to UINT32_MAX, which - * is currently the largest type we are able to represent in RDB sizes. - * However this does not limit the actual size of the DB to load since - * these sizes are just hints to resize the hash tables. */ + /* Write the RESIZE DB opcode. */ uint64_t db_size, expires_size; db_size = dictSize(db->dict); expires_size = dictSize(db->expires); From 35c64b898c1e4a5c2809888671730156731367a2 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 7 Apr 2020 12:07:09 +0200 Subject: [PATCH 4/6] Speedup INFO by counting client memory incrementally. Related to #5145. Design note: clients may change type when they turn into replicas or are moved into the Pub/Sub category and so forth. Moreover the recomputation of the bytes used is problematic for obvious reasons: it changes continuously, so as a conservative way to avoid accumulating errors, each client remembers the contribution it gave to the sum, and removes it when it is freed or before updating it with the new memory usage. --- src/networking.c | 7 +++++++ src/object.c | 33 +++++++++------------------------ src/server.c | 25 +++++++++++++++++++++++++ src/server.h | 13 +++++++++++-- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/networking.c b/src/networking.c index 85c640e34..654fda517 100644 --- a/src/networking.c +++ b/src/networking.c @@ -157,6 +157,8 @@ client *createClient(connection *conn) { c->client_list_node = NULL; c->client_tracking_redirection = 0; c->client_tracking_prefixes = NULL; + c->client_cron_last_memory_usage = 0; + c->client_cron_last_memory_type = CLIENT_TYPE_NORMAL; c->auth_callback = NULL; c->auth_callback_privdata = NULL; c->auth_module = NULL; @@ -1160,6 +1162,11 @@ void freeClient(client *c) { listDelNode(server.clients_to_close,ln); } + /* Remove the contribution that this client gave to our + * incrementally computed memory usage. */ + server.stat_clients_type_memory[c->client_cron_last_memory_type] -= + c->client_cron_last_memory_usage; + /* Release other dynamically allocated client structure fields, * and finally release the client structure itself. */ if (c->name) decrRefCount(c->name); diff --git a/src/object.c b/src/object.c index 11e335afc..52d5b11f5 100644 --- a/src/object.c +++ b/src/object.c @@ -974,30 +974,15 @@ struct redisMemOverhead *getMemoryOverheadData(void) { mh->repl_backlog = mem; mem_total += mem; - mem = 0; - if (listLength(server.clients)) { - listIter li; - listNode *ln; - size_t mem_normal = 0, mem_slaves = 0; - - listRewind(server.clients,&li); - while((ln = listNext(&li))) { - size_t mem_curr = 0; - client *c = listNodeValue(ln); - int type = getClientType(c); - mem_curr += getClientOutputBufferMemoryUsage(c); - mem_curr += sdsAllocSize(c->querybuf); - mem_curr += sizeof(client); - if (type == CLIENT_TYPE_SLAVE) - mem_slaves += mem_curr; - else - mem_normal += mem_curr; - } - mh->clients_slaves = mem_slaves; - mh->clients_normal = mem_normal; - mem = mem_slaves + mem_normal; - } - mem_total+=mem; + /* Computing the memory used by the clients would be O(N) if done + * here online. We use our values computed incrementally by + * clientsCronTrackClientsMemUsage(). */ + mh->clients_slaves = server.stat_clients_type_memory[CLIENT_TYPE_SLAVE]; + mh->clients_normal = server.stat_clients_type_memory[CLIENT_TYPE_MASTER]+ + server.stat_clients_type_memory[CLIENT_TYPE_PUBSUB]+ + server.stat_clients_type_memory[CLIENT_TYPE_NORMAL]; + mem_total += mh->clients_slaves; + mem_total += mh->clients_normal; mem = 0; if (server.aof_state != AOF_OFF) { diff --git a/src/server.c b/src/server.c index 56feb09a3..996e0f5d2 100644 --- a/src/server.c +++ b/src/server.c @@ -1593,6 +1593,28 @@ int clientsCronTrackExpansiveClients(client *c) { return 0; /* This function never terminates the client. */ } +/* Iterating all the clients in getMemoryOverheadData() is too slow and + * in turn would make the INFO command too slow. So we perform this + * computation incrementally and track the (not instantaneous but updated + * to the second) total memory used by clients using clinetsCron() in + * a more incremental way (depending on server.hz). */ +int clientsCronTrackClientsMemUsage(client *c) { + size_t mem = 0; + int type = getClientType(c); + mem += getClientOutputBufferMemoryUsage(c); + mem += sdsAllocSize(c->querybuf); + mem += sizeof(client); + /* Now that we have the memory used by the client, remove the old + * value from the old categoty, and add it back. */ + server.stat_clients_type_memory[c->client_cron_last_memory_type] -= + c->client_cron_last_memory_usage; + server.stat_clients_type_memory[type] += mem; + /* Remember what we added and where, to remove it next time. */ + c->client_cron_last_memory_usage = mem; + c->client_cron_last_memory_type = type; + return 0; +} + /* Return the max samples in the memory usage of clients tracked by * the function clientsCronTrackExpansiveClients(). */ void getExpansiveClientsInfo(size_t *in_usage, size_t *out_usage) { @@ -1653,6 +1675,7 @@ void clientsCron(void) { if (clientsCronHandleTimeout(c,now)) continue; if (clientsCronResizeQueryBuffer(c)) continue; if (clientsCronTrackExpansiveClients(c)) continue; + if (clientsCronTrackClientsMemUsage(c)) continue; } } @@ -2792,6 +2815,8 @@ void initServer(void) { server.stat_rdb_cow_bytes = 0; server.stat_aof_cow_bytes = 0; server.stat_module_cow_bytes = 0; + for (int j = 0; j < CLIENT_TYPE_COUNT; j++) + server.stat_clients_type_memory[j] = 0; server.cron_malloc_stats.zmalloc_used = 0; server.cron_malloc_stats.process_rss = 0; server.cron_malloc_stats.allocator_allocated = 0; diff --git a/src/server.h b/src/server.h index 9b77f55ac..cf4c285f8 100644 --- a/src/server.h +++ b/src/server.h @@ -274,6 +274,7 @@ typedef long long ustime_t; /* microsecond time type. */ #define CLIENT_TYPE_SLAVE 1 /* Slaves. */ #define CLIENT_TYPE_PUBSUB 2 /* Clients subscribed to PubSub channels. */ #define CLIENT_TYPE_MASTER 3 /* Master. */ +#define CLIENT_TYPE_COUNT 4 /* Total number of client types. */ #define CLIENT_TYPE_OBUF_COUNT 3 /* Number of clients to expose to output buffer configuration. Just the first three: normal, slave, pubsub. */ @@ -820,10 +821,10 @@ typedef struct client { * when the authenticated user * changes. */ void *auth_callback_privdata; /* Private data that is passed when the auth - * changed callback is executed. Opaque for + * changed callback is executed. Opaque for * Redis Core. */ void *auth_module; /* The module that owns the callback, which is used - * to disconnect the client if the module is + * to disconnect the client if the module is * unloaded for cleanup. Opaque for Redis Core.*/ /* If this client is in tracking mode and this field is non zero, @@ -833,6 +834,13 @@ typedef struct client { rax *client_tracking_prefixes; /* A dictionary of prefixes we are already subscribed to in BCAST mode, in the context of client side caching. */ + /* In clientsCronTrackClientsMemUsage() we track the memory usage of + * each client and add it to the sum of all the clients of a given type, + * however we need to remember what was the old contribution of each + * client, and in which categoty the client was, in order to remove it + * before adding it the new value. */ + uint64_t client_cron_last_memory_usage; + int client_cron_last_memory_type; /* Response buffer */ int bufpos; char buf[PROTO_REPLY_CHUNK_BYTES]; @@ -1129,6 +1137,7 @@ struct redisServer { size_t stat_rdb_cow_bytes; /* Copy on write bytes during RDB saving. */ size_t stat_aof_cow_bytes; /* Copy on write bytes during AOF rewrite. */ size_t stat_module_cow_bytes; /* Copy on write bytes during module fork. */ + uint64_t stat_clients_type_memory[CLIENT_TYPE_COUNT];/* Mem usage by type */ long long stat_unexpected_error_replies; /* Number of unexpected (aof-loading, replica to master, etc.) error replies */ /* The following two are used to track instantaneous metrics, like * number of operations per second, network traffic. */ From 2437455f2769be96e948c8b0ffed10ef684f160d Mon Sep 17 00:00:00 2001 From: mymilkbottles Date: Mon, 6 Apr 2020 19:27:06 +0800 Subject: [PATCH 5/6] Judge the log level in advance --- src/scripting.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scripting.c b/src/scripting.c index 32a511e13..bccbcf637 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -959,6 +959,7 @@ int luaLogCommand(lua_State *lua) { lua_pushstring(lua, "Invalid debug level."); return lua_error(lua); } + if (level < server.verbosity) return 0; /* Glue together all the arguments */ log = sdsempty(); From cbcd07777dc569618a34f59e5fd0de53178f4f1d Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 8 Apr 2020 10:54:18 +0200 Subject: [PATCH 6/6] Fix ACL HELP table missing comma. --- src/acl.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/acl.c b/src/acl.c index 27f4bdb84..733988013 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1847,18 +1847,18 @@ void aclCommand(client *c) { } } else if (!strcasecmp(sub,"help")) { const char *help[] = { -"LOAD -- Reload users from the ACL file.", -"SAVE -- Save the current config to the ACL file." -"LIST -- Show user details in config file format.", -"USERS -- List all the registered usernames.", -"SETUSER [attribs ...] -- Create or modify a user.", -"GETUSER -- Get the user details.", -"DELUSER [...] -- Delete a list of users.", -"CAT -- List available categories.", -"CAT -- List commands inside category.", -"GENPASS -- Generate a secure user password.", -"WHOAMI -- Return the current connection username.", -"LOG [ | RESET] -- Show the ACL log entries.", +"LOAD -- Reload users from the ACL file.", +"SAVE -- Save the current config to the ACL file.", +"LIST -- Show user details in config file format.", +"USERS -- List all the registered usernames.", +"SETUSER [attribs ...] -- Create or modify a user.", +"GETUSER -- Get the user details.", +"DELUSER [...] -- Delete a list of users.", +"CAT -- List available categories.", +"CAT -- List commands inside category.", +"GENPASS -- Generate a secure user password.", +"WHOAMI -- Return the current connection username.", +"LOG [ | RESET] -- Show the ACL log entries.", NULL }; addReplyHelp(c,help);