From: antirez Date: Tue, 7 Feb 2012 16:41:31 +0000 (+0100) Subject: Fixes to c->reply_bytes computation, and debug messages to closely study the behavior... X-Git-Url: https://git.saurik.com/redis.git/commitdiff_plain/609baba8a2d115f05e8fbc0db742ca60848e3c80 Fixes to c->reply_bytes computation, and debug messages to closely study the behavior of memory pressure + slaves + maxmemory + blocked slaves. --- diff --git a/src/networking.c b/src/networking.c index 99a381ce..dab6bfd9 100644 --- a/src/networking.c +++ b/src/networking.c @@ -3,6 +3,14 @@ static void setProtocolError(redisClient *c, int pos); +/* To evaluate the output buffer size of a client we need to get size of + * allocated objects, however we can't used zmalloc_size() directly on sds + * strings because of the trick they use to work (the header is before the + * returned pointer), so we use this helper function. */ +size_t zmalloc_size_sds(sds s) { + return zmalloc_size(s-sizeof(struct sdshdr)); +} + void *dupClientReplyValue(void *o) { incrRefCount((robj*)o); return o; @@ -137,6 +145,7 @@ void _addReplyObjectToList(redisClient *c, robj *o) { if (listLength(c->reply) == 0) { incrRefCount(o); listAddNodeTail(c->reply,o); + c->reply_bytes += zmalloc_size_sds(o->ptr); } else { tail = listNodeValue(listLast(c->reply)); @@ -144,14 +153,16 @@ void _addReplyObjectToList(redisClient *c, robj *o) { if (tail->ptr != NULL && sdslen(tail->ptr)+sdslen(o->ptr) <= REDIS_REPLY_CHUNK_BYTES) { + c->reply_bytes -= zmalloc_size_sds(tail->ptr); tail = dupLastObjectIfNeeded(c->reply); tail->ptr = sdscatlen(tail->ptr,o->ptr,sdslen(o->ptr)); + c->reply_bytes += zmalloc_size_sds(tail->ptr); } else { incrRefCount(o); listAddNodeTail(c->reply,o); + c->reply_bytes += zmalloc_size_sds(o->ptr); } } - c->reply_bytes += zmalloc_size(o->ptr); asyncCloseClientOnOutputBufferLimitReached(c); } @@ -165,9 +176,9 @@ void _addReplySdsToList(redisClient *c, sds s) { return; } - c->reply_bytes += zmalloc_size(s); if (listLength(c->reply) == 0) { listAddNodeTail(c->reply,createObject(REDIS_STRING,s)); + c->reply_bytes += zmalloc_size_sds(s); } else { tail = listNodeValue(listLast(c->reply)); @@ -175,11 +186,14 @@ void _addReplySdsToList(redisClient *c, sds s) { if (tail->ptr != NULL && sdslen(tail->ptr)+sdslen(s) <= REDIS_REPLY_CHUNK_BYTES) { + c->reply_bytes -= zmalloc_size_sds(tail->ptr); tail = dupLastObjectIfNeeded(c->reply); tail->ptr = sdscatlen(tail->ptr,s,sdslen(s)); + c->reply_bytes += zmalloc_size_sds(tail->ptr); sdsfree(s); } else { listAddNodeTail(c->reply,createObject(REDIS_STRING,s)); + c->reply_bytes += zmalloc_size_sds(s); } } asyncCloseClientOnOutputBufferLimitReached(c); @@ -194,7 +208,7 @@ void _addReplyStringToList(redisClient *c, char *s, size_t len) { robj *o = createStringObject(s,len); listAddNodeTail(c->reply,o); - c->reply_bytes += zmalloc_size(o->ptr); + c->reply_bytes += zmalloc_size_sds(o->ptr); } else { tail = listNodeValue(listLast(c->reply)); @@ -202,15 +216,15 @@ void _addReplyStringToList(redisClient *c, char *s, size_t len) { if (tail->ptr != NULL && sdslen(tail->ptr)+len <= REDIS_REPLY_CHUNK_BYTES) { - c->reply_bytes -= zmalloc_size(tail->ptr); + c->reply_bytes -= zmalloc_size_sds(tail->ptr); tail = dupLastObjectIfNeeded(c->reply); tail->ptr = sdscatlen(tail->ptr,s,len); - c->reply_bytes += zmalloc_size(tail->ptr); + c->reply_bytes += zmalloc_size_sds(tail->ptr); } else { robj *o = createStringObject(s,len); listAddNodeTail(c->reply,o); - c->reply_bytes += zmalloc_size(o->ptr); + c->reply_bytes += zmalloc_size_sds(o->ptr); } } asyncCloseClientOnOutputBufferLimitReached(c); @@ -343,7 +357,7 @@ void setDeferredMultiBulkLength(redisClient *c, void *node, long length) { len = listNodeValue(ln); len->ptr = sdscatprintf(sdsempty(),"*%ld\r\n",length); - c->reply_bytes += zmalloc_size(len->ptr); + c->reply_bytes += zmalloc_size_sds(len->ptr); if (ln->next != NULL) { next = listNodeValue(ln->next); @@ -671,7 +685,7 @@ void sendReplyToClient(aeEventLoop *el, int fd, void *privdata, int mask) { } else { o = listNodeValue(listFirst(c->reply)); objlen = sdslen(o->ptr); - objmem = zmalloc_size(o->ptr); + objmem = zmalloc_size_sds(o->ptr); if (objlen == 0) { listDelNode(c->reply,listFirst(c->reply)); diff --git a/src/redis.c b/src/redis.c index b4acd6ea..bd5d77ce 100644 --- a/src/redis.c +++ b/src/redis.c @@ -1801,10 +1801,18 @@ void monitorCommand(redisClient *c) { int freeMemoryIfNeeded(void) { size_t mem_used, mem_tofree, mem_freed; int slaves = listLength(server.slaves); + static time_t xt; + int debug = 0; + + if (xt != time(NULL)) { + debug = 1; + xt = time(NULL); + } /* Remove the size of slaves output buffers and AOF buffer from the * count of used memory. */ mem_used = zmalloc_used_memory(); + if (debug) printf("used_full: %zu\n", mem_used); if (slaves) { listIter li; listNode *ln; @@ -1819,6 +1827,7 @@ int freeMemoryIfNeeded(void) { mem_used -= obuf_bytes; } } + if (debug) printf("used_nosl: %zu\n", mem_used); if (server.aof_state != REDIS_AOF_OFF) { mem_used -= sdslen(server.aof_buf); mem_used -= sdslen(server.aof_rewrite_buf); @@ -1833,6 +1842,7 @@ int freeMemoryIfNeeded(void) { /* Compute how much memory we need to free. */ mem_tofree = mem_used - server.maxmemory; mem_freed = 0; + if (debug) printf("tofree: %zu\n", mem_tofree); while (mem_freed < mem_tofree) { int j, k, keys_freed = 0; @@ -1934,8 +1944,12 @@ int freeMemoryIfNeeded(void) { if (slaves) flushSlavesOutputBuffers(); } } - if (!keys_freed) return REDIS_ERR; /* nothing to free... */ + if (!keys_freed) { + if (debug) printf("-freed: %zu\n\n", mem_freed); + return REDIS_ERR; /* nothing to free... */ + } } + if (debug) printf("+freed: %zu\n\n", mem_freed); return REDIS_OK; }