From d8d415e717415130b02ebe8fb07743a8ab69f3e8 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 28 Apr 2014 17:36:57 +0200 Subject: [PATCH] CLIENT LIST speedup via peerid caching + smart allocation. This commit adds peer ID caching in the client structure plus an API change and the use of sdsMakeRoomFor() in order to improve the reallocation pattern to generate the CLIENT LIST output. Both the changes account for a very significant speedup. --- src/aof.c | 1 + src/debug.c | 2 +- src/networking.c | 50 +++++++++++++++++++++++++++++------------------ src/redis.h | 5 +++-- src/replication.c | 10 +++++++--- 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/aof.c b/src/aof.c index 567d7ffc..9409c49b 100644 --- a/src/aof.c +++ b/src/aof.c @@ -506,6 +506,7 @@ struct redisClient *createFakeClient(void) { c->reply_bytes = 0; c->obuf_soft_limit_reached_time = 0; c->watched_keys = listCreate(); + c->peerid = NULL; listSetFreeMethod(c->reply,decrRefCountVoid); listSetDupMethod(c->reply,dupClientReplyValue); initClientMultiState(c); diff --git a/src/debug.c b/src/debug.c index 10ef2af6..b94959b7 100644 --- a/src/debug.c +++ b/src/debug.c @@ -692,7 +692,7 @@ void logCurrentClient(void) { int j; redisLog(REDIS_WARNING, "--- CURRENT CLIENT INFO"); - client = getClientInfoString(cc); + client = catClientInfoString(sdsempty(),cc); redisLog(REDIS_WARNING,"client: %s", client); sdsfree(client); for (j = 0; j < cc->argc; j++) { diff --git a/src/networking.c b/src/networking.c index 95922dde..d0b0ce0e 100644 --- a/src/networking.c +++ b/src/networking.c @@ -103,6 +103,7 @@ redisClient *createClient(int fd) { c->watched_keys = listCreate(); c->pubsub_channels = dictCreate(&setDictType,NULL); c->pubsub_patterns = listCreate(); + c->peerid = NULL; listSetFreeMethod(c->pubsub_patterns,decrRefCountVoid); listSetMatchMethod(c->pubsub_patterns,listMatchObjects); if (fd != -1) listAddNodeTail(server.clients,c); @@ -745,6 +746,7 @@ void freeClient(redisClient *c) { if (c->name) decrRefCount(c->name); zfree(c->argv); freeClientMultiState(c); + sdsfree(c->peerid); zfree(c); } @@ -923,7 +925,7 @@ int processInlineBuffer(redisClient *c) { * multi bulk requests idempotent. */ static void setProtocolError(redisClient *c, int pos) { if (server.verbosity >= REDIS_VERBOSE) { - sds client = getClientInfoString(c); + sds client = catClientInfoString(sdsempty(),c); redisLog(REDIS_VERBOSE, "Protocol error from client: %s", client); sdsfree(client); @@ -1158,7 +1160,7 @@ void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { return; } if (sdslen(c->querybuf) > server.client_max_querybuf_len) { - sds ci = getClientInfoString(c), bytes = sdsempty(); + sds ci = catClientInfoString(sdsempty(),c), bytes = sdsempty(); bytes = sdscatrepr(bytes,c->querybuf,64); redisLog(REDIS_WARNING,"Closing client that reached max query buffer length: %s (qbuf initial bytes: %s)", ci, bytes); @@ -1189,7 +1191,7 @@ void getClientsMaxBuffers(unsigned long *longest_output_list, *biggest_input_buffer = bib; } -/* This is a helper function for getClientPeerId(). +/* This is a helper function for genClientPeerId(). * It writes the specified ip/port to "peerid" as a null termiated string * in the form ip:port if ip does not contain ":" itself, otherwise * [ip]:port format is used (for IPv6 addresses basically). */ @@ -1213,7 +1215,7 @@ void formatPeerId(char *peerid, size_t peerid_len, char *ip, int port) { * On failure the function still populates 'peerid' with the "?:0" string * in case you want to relax error checking or need to display something * anyway (see anetPeerToString implementation for more info). */ -int getClientPeerId(redisClient *client, char *peerid, size_t peerid_len) { +int genClientPeerId(redisClient *client, char *peerid, size_t peerid_len) { char ip[REDIS_IP_STR_LEN]; int port; @@ -1229,12 +1231,26 @@ int getClientPeerId(redisClient *client, char *peerid, size_t peerid_len) { } } -/* Turn a Redis client into an sds string representing its state. */ -sds getClientInfoString(redisClient *client) { - char peerid[REDIS_PEER_ID_LEN], flags[16], events[3], *p; +/* This function returns the client peer id, by creating and caching it + * if client->perrid is NULL, otherwise returning the cached value. + * The Peer ID never changes during the life of the client, however it + * is expensive to compute. */ +char *getClientPeerId(redisClient *c) { + char peerid[REDIS_PEER_ID_LEN]; + + if (c->peerid == NULL) { + genClientPeerId(c,peerid,sizeof(peerid)); + c->peerid = sdsnew(peerid); + } + return c->peerid; +} + +/* Concatenate a string representing the state of a client in an human + * readable format, into the sds string 's'. */ +sds catClientInfoString(sds s, redisClient *client) { + char flags[16], events[3], *p; int emask; - getClientPeerId(client,peerid,sizeof(peerid)); p = flags; if (client->flags & REDIS_SLAVE) { if (client->flags & REDIS_MONITOR) @@ -1258,9 +1274,9 @@ sds getClientInfoString(redisClient *client) { if (emask & AE_READABLE) *p++ = 'r'; if (emask & AE_WRITABLE) *p++ = 'w'; *p = '\0'; - return sdscatfmt(sdsempty(), + return sdscatfmt(s, "addr=%s fd=%i name=%s age=%I idle=%I flags=%s db=%i sub=%i psub=%i multi=%i qbuf=%U qbuf-free=%U obl=%U oll=%U omem=%U events=%s cmd=%s", - peerid, + getClientPeerId(client), client->fd, client->name ? (char*)client->name->ptr : "", (long long)(server.unixtime - client->ctime), @@ -1285,14 +1301,11 @@ sds getAllClientsInfoString(void) { redisClient *client; sds o = sdsempty(); + o = sdsMakeRoomFor(o,200*listLength(server.clients)); listRewind(server.clients,&li); while ((ln = listNext(&li)) != NULL) { - sds cs; - client = listNodeValue(ln); - cs = getClientInfoString(client); - o = sdscatsds(o,cs); - sdsfree(cs); + o = catClientInfoString(o,client); o = sdscatlen(o,"\n",1); } return o; @@ -1310,11 +1323,10 @@ void clientCommand(redisClient *c) { } else if (!strcasecmp(c->argv[1]->ptr,"kill") && c->argc == 3) { listRewind(server.clients,&li); while ((ln = listNext(&li)) != NULL) { - char peerid[REDIS_PEER_ID_LEN]; + char *peerid; client = listNodeValue(ln); - if (getClientPeerId(client,peerid,sizeof(peerid)) == REDIS_ERR) - continue; + peerid = getClientPeerId(client); if (strcmp(peerid,c->argv[2]->ptr) == 0) { addReply(c,shared.ok); if (c == client) { @@ -1513,7 +1525,7 @@ void asyncCloseClientOnOutputBufferLimitReached(redisClient *c) { redisAssert(c->reply_bytes < ULONG_MAX-(1024*64)); if (c->reply_bytes == 0 || c->flags & REDIS_CLOSE_ASAP) return; if (checkClientOutputBufferLimits(c)) { - sds client = getClientInfoString(c); + sds client = catClientInfoString(sdsempty(),c); freeClientAsync(c); redisLog(REDIS_WARNING,"Client %s scheduled to be closed ASAP for overcoming of output buffer limits.", client); diff --git a/src/redis.h b/src/redis.h index f6deb797..085638a4 100644 --- a/src/redis.h +++ b/src/redis.h @@ -486,6 +486,7 @@ typedef struct redisClient { list *watched_keys; /* Keys WATCHED for MULTI/EXEC CAS */ dict *pubsub_channels; /* channels a client is interested in (SUBSCRIBE) */ list *pubsub_patterns; /* patterns a client is interested in (SUBSCRIBE) */ + sds peerid; /* Cached peer ID. */ /* Response buffer */ int bufpos; @@ -928,8 +929,8 @@ void *dupClientReplyValue(void *o); void getClientsMaxBuffers(unsigned long *longest_output_list, unsigned long *biggest_input_buffer); void formatPeerId(char *peerid, size_t peerid_len, char *ip, int port); -int getClientPeerId(redisClient *client, char *peerid, size_t peerid_len); -sds getClientInfoString(redisClient *client); +char *getClientPeerId(redisClient *client); +sds catClientInfoString(sds s, redisClient *client); sds getAllClientsInfoString(void); void rewriteClientCommandVector(redisClient *c, int argc, ...); void rewriteClientCommandArgument(redisClient *c, int i, robj *newval); diff --git a/src/replication.c b/src/replication.c index 439dbba2..ae128b0d 100644 --- a/src/replication.c +++ b/src/replication.c @@ -238,7 +238,6 @@ void replicationFeedMonitors(redisClient *c, list *monitors, int dictid, robj ** int j; sds cmdrepr = sdsnew("+"); robj *cmdobj; - char peerid[REDIS_PEER_ID_LEN]; struct timeval tv; gettimeofday(&tv,NULL); @@ -248,8 +247,7 @@ void replicationFeedMonitors(redisClient *c, list *monitors, int dictid, robj ** } else if (c->flags & REDIS_UNIX_SOCKET) { cmdrepr = sdscatprintf(cmdrepr,"[%d unix:%s] ",dictid,server.unixsocket); } else { - getClientPeerId(c,peerid,sizeof(peerid)); - cmdrepr = sdscatprintf(cmdrepr,"[%d %s] ",dictid,peerid); + cmdrepr = sdscatprintf(cmdrepr,"[%d %s] ",dictid,getClientPeerId(c)); } for (j = 0; j < argc; j++) { @@ -1363,6 +1361,12 @@ void replicationCacheMaster(redisClient *c) { /* Set fd to -1 so that we can safely call freeClient(c) later. */ c->fd = -1; + /* Invalidate the Peer ID cache. */ + if (c->peerid) { + sdsfree(c->peerid); + c->peerid = NULL; + } + /* Caching the master happens instead of the actual freeClient() call, * so make sure to adjust the replication state. This function will * also set server.master to NULL. */ -- GitLab