Merge 30a1332d0d into 804110a487

src/networking.c

@@ -75,6 +75,7 @@ void linkClient(client *c) {
     c->client_list_node = listLast(server.clients);
     uint64_t id = htonu64(c->id);
     raxInsert(server.clients_index,(unsigned char*)&id,sizeof(id),c,NULL);
+    updateClientMemUsageAndBucket(c);
 }
 
 /* Initialize client authentication state.
@@ -192,8 +193,8 @@ client *createClient(connection *conn) {
     listInitNode(&c->clients_pending_write_node, c);
     c->mem_usage_bucket = NULL;
     c->mem_usage_bucket_node = NULL;
-    if (conn) linkClient(c);
     initClientMultiState(c);
+    if (conn) linkClient(c);
     return c;
 }
 
@@ -1459,6 +1460,15 @@ void unlinkClient(client *c) {
                 }
             }
         }
+
+        /* Remove the contribution that this client gave to the type of memory statistics. */
+        server.stat_clients_type_memory[c->last_memory_type] -= c->last_memory_usage;
+        /* Remove client from memory usage buckets */
+        if (c->mem_usage_bucket) {
+            c->mem_usage_bucket->mem_usage_sum -= c->last_memory_usage;
+            listDelNode(c->mem_usage_bucket->clients, c->mem_usage_bucket_node);
+        }
+
         /* Only use shutdown when the fork is active and we are the parent. */
         if (server.child_type) connShutdown(c->conn);
         connClose(c->conn);
@@ -1629,12 +1639,6 @@ void freeClient(client *c) {
     reqresReset(c, 1);
 #endif
 
-    /* Remove the contribution that this client gave to our
-     * incrementally computed memory usage. */
-    if (c->conn)
-        server.stat_clients_type_memory[c->last_memory_type] -=
-            c->last_memory_usage;
-
     /* Unlink the client: this will close the socket, remove the I/O
      * handlers, and remove references of the client from different
      * places where active clients may be referenced. */
@@ -1683,12 +1687,6 @@ void freeClient(client *c) {
      * we lost the connection with the master. */
     if (c->flags & CLIENT_MASTER) replicationHandleMasterDisconnection();
 
-    /* Remove client from memory usage buckets */
-    if (c->mem_usage_bucket) {
-        c->mem_usage_bucket->mem_usage_sum -= c->last_memory_usage;
-        listDelNode(c->mem_usage_bucket->clients, c->mem_usage_bucket_node);
-    }
-
     /* Release other dynamically allocated client structure fields,
      * and finally release the client structure itself. */
     if (c->name) decrRefCount(c->name);

src/server.c

@@ -850,10 +850,6 @@ static inline clientMemUsageBucket *getMemUsageBucket(size_t mem) {
  * This method updates the client memory usage and update the
  * server stats for client type.
  *
- * This method is called from the clientsCron to have updated
- * stats for non CLIENT_TYPE_NORMAL/PUBSUB clients to accurately
- * provide information around clients memory usage.
- *
  * It is also used in updateClientMemUsageAndBucket to have latest
  * client memory usage information to place it into appropriate client memory
  * usage bucket.
@@ -904,25 +900,20 @@ void removeClientFromMemUsageBucket(client *c, int allow_eviction) {
  * all clients with roughly the same amount of memory. This way we group
  * together clients consuming about the same amount of memory and can quickly
  * free them in case we reach maxmemory-clients (client eviction).
- *
- * Note: This function filters clients of type no-evict, master or replica regardless
- * of whether the eviction is enabled or not, so the memory usage we get from these
- * types of clients via the INFO command may be out of date.
- *
- * returns 1 if client eviction for this client is allowed, 0 otherwise.
  */
-int updateClientMemUsageAndBucket(client *c) {
+void updateClientMemUsageAndBucket(client *c) {
     serverAssert(io_threads_op == IO_THREADS_OP_IDLE && c->conn);
+
+    /* Update client memory usage. */
+    updateClientMemoryUsage(c);
+
     int allow_eviction = clientEvictionAllowed(c);
     removeClientFromMemUsageBucket(c, allow_eviction);
 
     if (!allow_eviction) {
-        return 0;
+        return;
     }
 
-    /* Update client memory usage. */
-    updateClientMemoryUsage(c);
-
     /* Update the client in the mem usage buckets */
     clientMemUsageBucket *bucket = getMemUsageBucket(c->last_memory_usage);
     bucket->mem_usage_sum += c->last_memory_usage;
@@ -934,7 +925,6 @@ int updateClientMemUsageAndBucket(client *c) {
         listAddNodeTail(bucket->clients, c);
         c->mem_usage_bucket_node = listLast(bucket->clients);
     }
-    return 1;
 }
 
 /* Return the max samples in the memory usage of clients tracked by
@@ -1014,18 +1004,7 @@ void clientsCron(void) {
         if (clientsCronHandleTimeout(c,now)) continue;
         if (clientsCronResizeQueryBuffer(c)) continue;
         if (clientsCronResizeOutputBuffer(c,now)) continue;
-
         if (clientsCronTrackExpansiveClients(c, curr_peak_mem_usage_slot)) continue;
-
-        /* Iterating all the clients in getMemoryOverheadData() is too slow and
-         * in turn would make the INFO command too slow. So we perform this
-         * computation incrementally and track the (not instantaneous but updated
-         * to the second) total memory used by clients using clientsCron() in
-         * a more incremental way (depending on server.hz).
-         * If client eviction is enabled, update the bucket as well. */
-        if (!updateClientMemUsageAndBucket(c))
-            updateClientMemoryUsage(c);
-
         if (closeClientOnOutputBufferLimitReached(c, 0)) continue;
     }
 }

src/server.h

@@ -2654,7 +2654,7 @@ int handleClientsWithPendingWritesUsingThreads(void);
 int handleClientsWithPendingReadsUsingThreads(void);
 int stopThreadedIOIfNeeded(void);
 int clientHasPendingReplies(client *c);
-int updateClientMemUsageAndBucket(client *c);
+void updateClientMemUsageAndBucket(client *c);
 void removeClientFromMemUsageBucket(client *c, int allow_eviction);
 void unlinkClient(client *c);
 int writeToClient(client *c, int handler_installed);