diff --git a/src/redis.c b/src/redis.c index 61d646363..782f9f53f 100644 --- a/src/redis.c +++ b/src/redis.c @@ -912,9 +912,12 @@ long long getInstantaneousMetric(int metric) { return sum / REDIS_METRIC_SAMPLES; } -/* Check for timeouts. Returns non-zero if the client was terminated */ -int clientsCronHandleTimeout(redisClient *c) { - time_t now = server.unixtime; +/* Check for timeouts. Returns non-zero if the client was terminated. + * The function gets the current time in milliseconds as argument since + * it gets called multiple times in a loop, so calling gettimeofday() for + * each iteration would be costly without any actual gain. */ +int clientsCronHandleTimeout(redisClient *c, mstime_t now_ms) { + time_t now = now_ms/1000; if (server.maxidletime && !(c->flags & REDIS_SLAVE) && /* no timeout for slaves */ @@ -930,7 +933,6 @@ int clientsCronHandleTimeout(redisClient *c) { /* Blocked OPS timeout is handled with milliseconds resolution. * However note that the actual resolution is limited by * server.hz. */ - mstime_t now_ms = mstime(); if (c->bpop.timeout != 0 && c->bpop.timeout < now_ms) { /* Handle blocking operation specific timeout. */ @@ -972,17 +974,23 @@ int clientsCronResizeQueryBuffer(redisClient *c) { return 0; } +#define CLIENTS_CRON_MIN_ITERATIONS 5 void clientsCron(void) { - /* Make sure to process at least numclients/(server.hz*10) of clients + /* Make sure to process at least numclients/server.hz of clients * per call. Since this function is called server.hz times per second - * we are sure that in the worst case we process all the clients in 10 - * seconds. In normal conditions (a reasonable number of clients) we - * process all the clients in a shorter time. */ + * we are sure that in the worst case we process all the clients in 1 + * second. */ int numclients = listLength(server.clients); - int iterations = numclients/(server.hz*10); + int iterations = numclients/server.hz; + mstime_t now = mstime(); + + /* Process at least a few clients while we are at it, even if we need + * to process less than CLIENTS_CRON_MIN_ITERATIONS to meet our contract + * of processing each client once per second. */ + if (iterations < CLIENTS_CRON_MIN_ITERATIONS) + iterations = (numclients < CLIENTS_CRON_MIN_ITERATIONS) ? + numclients : CLIENTS_CRON_MIN_ITERATIONS; - if (iterations < 50) - iterations = (numclients < 50) ? numclients : 50; while(listLength(server.clients) && iterations--) { redisClient *c; listNode *head; @@ -996,7 +1004,7 @@ void clientsCron(void) { /* The following functions do different service checks on the client. * The protocol is that they return non-zero if the client was * terminated. */ - if (clientsCronHandleTimeout(c)) continue; + if (clientsCronHandleTimeout(c,now)) continue; if (clientsCronResizeQueryBuffer(c)) continue; } }