From cec404f099e3a1a3ce6e94c01ce45d851bd3e843 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 11 Jun 2018 17:12:28 +0200 Subject: [PATCH] Use a less aggressive query buffer resize policy. A user with many connections (10 thousand) on a single Redis server reports in issue #4983 that sometimes Redis is idle becuase at the same time many clients need to resize their query buffer according to the old policy. It looks like this was created by the fact that we allow the query buffer to grow without problems to a size up to PROTO_MBULK_BIG_ARG normally, but when the client is idle we immediately are more strict, and a query buffer greater than 1024 bytes is already enough to trigger the resize. So for instance if most of the clients stop at the same time this issue should be easily triggered. This behavior actually looks odd, and there should be only a clear limit after we say, let's look at this query buffer to check if it's time to resize it. This commit puts the limit at PROTO_MBULK_BIG_ARG, and the check is performed both if compared to the peak usage the current usage is too big, or if the client is idle. Then when the check is performed, to waste just a few kbytes is considered enough to proceed with the resize. This should fix the issue. --- src/server.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/server.c b/src/server.c index 375c6477..16c807f6 100644 --- a/src/server.c +++ b/src/server.c @@ -845,13 +845,14 @@ int clientsCronResizeQueryBuffer(client *c) { /* There are two conditions to resize the query buffer: * 1) Query buffer is > BIG_ARG and too big for latest peak. - * 2) Client is inactive and the buffer is bigger than 1k. */ - if (((querybuf_size > PROTO_MBULK_BIG_ARG) && - (querybuf_size/(c->querybuf_peak+1)) > 2) || - (querybuf_size > 1024 && idletime > 2)) + * 2) Query buffer is > BIG_ARG and client is idle. */ + if (querybuf_size > PROTO_MBULK_BIG_ARG && + ((querybuf_size/(c->querybuf_peak+1)) > 2 || + idletime > 2)) { - /* Only resize the query buffer if it is actually wasting space. */ - if (sdsavail(c->querybuf) > 1024) { + /* Only resize the query buffer if it is actually wasting + * at least a few kbytes. */ + if (sdsavail(c->querybuf) > 1024*4) { c->querybuf = sdsRemoveFreeSpace(c->querybuf); } }