diff --git a/src/aof.c b/src/aof.c index c51fdbeff..b1c7595b5 100644 --- a/src/aof.c +++ b/src/aof.c @@ -669,6 +669,7 @@ struct client *createAOFClient(void) { c->querybuf_peak = 0; c->argc = 0; c->argv = NULL; + c->argv_len_sum = 0; c->bufpos = 0; c->flags = 0; c->btype = BLOCKED_NONE; @@ -694,6 +695,7 @@ void freeFakeClientArgv(struct client *c) { for (j = 0; j < c->argc; j++) decrRefCount(c->argv[j]); zfree(c->argv); + c->argv_len_sum = 0; } void freeFakeClient(struct client *c) { diff --git a/src/networking.c b/src/networking.c index bfb65f5a1..fcb612e0b 100644 --- a/src/networking.c +++ b/src/networking.c @@ -48,7 +48,7 @@ size_t sdsZmallocSize(sds s) { } /* Return the amount of memory used by the sds string at object->ptr - * for a string object. */ + * for a string object. This includes internal fragmentation. */ size_t getStringObjectSdsUsedMemory(robj *o) { serverAssertWithInfo(NULL,o,o->type == OBJ_STRING); switch(o->encoding) { @@ -58,6 +58,17 @@ size_t getStringObjectSdsUsedMemory(robj *o) { } } +/* Return the length of a string object. + * This does NOT includes internal fragmentation or sds unused space. */ +size_t getStringObjectLen(robj *o) { + serverAssertWithInfo(NULL,o,o->type == OBJ_STRING); + switch(o->encoding) { + case OBJ_ENCODING_RAW: return sdslen(o->ptr); + case OBJ_ENCODING_EMBSTR: return sdslen(o->ptr); + default: return 0; /* Just integer encoding for now. */ + } +} + /* Client.reply list dup and free methods. */ void *dupClientReplyValue(void *o) { clientReplyBlock *old = o; @@ -117,6 +128,7 @@ client *createClient(connection *conn) { c->reqtype = 0; c->argc = 0; c->argv = NULL; + c->argv_len_sum = 0; c->cmd = c->lastcmd = NULL; c->user = DefaultUser; c->multibulklen = 0; @@ -1052,6 +1064,7 @@ static void freeClientArgv(client *c) { decrRefCount(c->argv[j]); c->argc = 0; c->cmd = NULL; + c->argv_len_sum = 0; } /* Close all the slaves connections. This is useful in chained replication @@ -1284,6 +1297,7 @@ void freeClient(client *c) { * and finally release the client structure itself. */ if (c->name) decrRefCount(c->name); zfree(c->argv); + c->argv_len_sum = 0; freeClientMultiState(c); sdsfree(c->peerid); zfree(c); @@ -1630,12 +1644,14 @@ int processInlineBuffer(client *c) { if (argc) { if (c->argv) zfree(c->argv); c->argv = zmalloc(sizeof(robj*)*argc); + c->argv_len_sum = 0; } /* Create redis objects for all arguments. */ for (c->argc = 0, j = 0; j < argc; j++) { c->argv[c->argc] = createObject(OBJ_STRING,argv[j]); c->argc++; + c->argv_len_sum += sdslen(argv[j]); } zfree(argv); return C_OK; @@ -1727,6 +1743,7 @@ int processMultibulkBuffer(client *c) { /* Setup argv array on client structure */ if (c->argv) zfree(c->argv); c->argv = zmalloc(sizeof(robj*)*c->multibulklen); + c->argv_len_sum = 0; } serverAssertWithInfo(c,NULL,c->multibulklen > 0); @@ -1799,6 +1816,7 @@ int processMultibulkBuffer(client *c) { sdslen(c->querybuf) == (size_t)(c->bulklen+2)) { c->argv[c->argc++] = createObject(OBJ_STRING,c->querybuf); + c->argv_len_sum += c->bulklen; sdsIncrLen(c->querybuf,-2); /* remove CRLF */ /* Assume that if we saw a fat argument we'll see another one * likely... */ @@ -1807,6 +1825,7 @@ int processMultibulkBuffer(client *c) { } else { c->argv[c->argc++] = createStringObject(c->querybuf+c->qb_pos,c->bulklen); + c->argv_len_sum += c->bulklen; c->qb_pos += c->bulklen+2; } c->bulklen = -1; @@ -2129,8 +2148,21 @@ sds catClientInfoString(sds s, client *client) { if (connHasWriteHandler(client->conn)) *p++ = 'w'; } *p = '\0'; + + /* Compute the total memory consumed by this client. */ + size_t obufmem = getClientOutputBufferMemoryUsage(client); + size_t total_mem = obufmem; + total_mem += zmalloc_size(client); /* includes client->buf */ + total_mem += sdsZmallocSize(client->querybuf); + /* For efficiency (less work keeping track of the argv memory), it doesn't include the used memory + * i.e. unused sds space and internal fragmentation, just the string length. but this is enough to + * spot problematic clients. */ + total_mem += client->argv_len_sum; + if (client->argv) + total_mem += zmalloc_size(client->argv); + return sdscatfmt(s, - "id=%U addr=%s %s 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 user=%s", + "id=%U addr=%s %s name=%s age=%I idle=%I flags=%s db=%i sub=%i psub=%i multi=%i qbuf=%U qbuf-free=%U argv-mem=%U obl=%U oll=%U omem=%U tot-mem=%U events=%s cmd=%s user=%s", (unsigned long long) client->id, getClientPeerId(client), connGetInfo(client->conn, conninfo, sizeof(conninfo)), @@ -2144,9 +2176,11 @@ sds catClientInfoString(sds s, client *client) { (client->flags & CLIENT_MULTI) ? client->mstate.count : -1, (unsigned long long) sdslen(client->querybuf), (unsigned long long) sdsavail(client->querybuf), + (unsigned long long) client->argv_len_sum, (unsigned long long) client->bufpos, (unsigned long long) listLength(client->reply), - (unsigned long long) getClientOutputBufferMemoryUsage(client), + (unsigned long long) obufmem, /* should not include client->buf since we want to see 0 for static clients. */ + (unsigned long long) total_mem, events, client->lastcmd ? client->lastcmd->name : "NULL", client->user ? client->user->name : "(superuser)"); @@ -2684,6 +2718,10 @@ void rewriteClientCommandVector(client *c, int argc, ...) { /* Replace argv and argc with our new versions. */ c->argv = argv; c->argc = argc; + c->argv_len_sum = 0; + for (j = 0; j < c->argc; j++) + if (c->argv[j]) + c->argv_len_sum += getStringObjectLen(c->argv[j]); c->cmd = lookupCommandOrOriginal(c->argv[0]->ptr); serverAssertWithInfo(c,NULL,c->cmd != NULL); va_end(ap); @@ -2691,10 +2729,15 @@ void rewriteClientCommandVector(client *c, int argc, ...) { /* Completely replace the client command vector with the provided one. */ void replaceClientCommandVector(client *c, int argc, robj **argv) { + int j; freeClientArgv(c); zfree(c->argv); c->argv = argv; c->argc = argc; + c->argv_len_sum = 0; + for (j = 0; j < c->argc; j++) + if (c->argv[j]) + c->argv_len_sum += getStringObjectLen(c->argv[j]); c->cmd = lookupCommandOrOriginal(c->argv[0]->ptr); serverAssertWithInfo(c,NULL,c->cmd != NULL); } @@ -2719,6 +2762,8 @@ void rewriteClientCommandArgument(client *c, int i, robj *newval) { c->argv[i] = NULL; } oldval = c->argv[i]; + if (oldval) c->argv_len_sum -= getStringObjectLen(oldval); + if (newval) c->argv_len_sum += getStringObjectLen(newval); c->argv[i] = newval; incrRefCount(newval); if (oldval) decrRefCount(oldval); diff --git a/src/server.c b/src/server.c index 2f7aa8713..d8aaef06d 100644 --- a/src/server.c +++ b/src/server.c @@ -1627,7 +1627,9 @@ int clientsCronTrackClientsMemUsage(client *c) { int type = getClientType(c); mem += getClientOutputBufferMemoryUsage(c); mem += sdsZmallocSize(c->querybuf); - mem += sizeof(client); + mem += zmalloc_size(c); + mem += c->argv_len_sum; + if (c->argv) mem += zmalloc_size(c->argv); /* Now that we have the memory used by the client, remove the old * value from the old category, and add it back. */ server.stat_clients_type_memory[c->client_cron_last_memory_type] -= diff --git a/src/server.h b/src/server.h index b920e1f4c..114034962 100644 --- a/src/server.h +++ b/src/server.h @@ -802,6 +802,7 @@ typedef struct client { size_t querybuf_peak; /* Recent (100ms or more) peak of querybuf size. */ int argc; /* Num of arguments of current command. */ robj **argv; /* Arguments of current command. */ + size_t argv_len_sum; /* Sum of lengths of objects in argv list. */ struct redisCommand *cmd, *lastcmd; /* Last command executed. */ user *user; /* User associated with this connection. If the user is set to NULL the connection can do diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index c8bc976cf..2d50b417b 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -1,7 +1,7 @@ start_server {tags {"introspection"}} { test {CLIENT LIST} { r client list - } {*addr=*:* fd=* age=* idle=* flags=N db=9 sub=0 psub=0 multi=-1 qbuf=26 qbuf-free=* obl=0 oll=0 omem=0 events=r cmd=client*} + } {*addr=*:* fd=* age=* idle=* flags=N db=9 sub=0 psub=0 multi=-1 qbuf=26 qbuf-free=* argv-mem=* obl=0 oll=0 omem=0 tot-mem=* events=r cmd=client*} test {MONITOR can log executed commands} { set rd [redis_deferring_client]