clean up global stats code a little.

tons of stats were left in the global stats structure that're no longer used,
and it looks like we kept accidentally adding new ones in there. There's also
an unused mutex.

Split global stats into `stats` and `stats_state`. initialize via memset,
reset only `stats` via memset, removing several places where stats values get
repeated. Looks much cleaner and should be less error prone.

Author: dormando

assoc.c

@@ -70,8 +70,8 @@ void assoc_init(const int hashtable_init) {
         exit(EXIT_FAILURE);
     }
     STATS_LOCK();
-    stats.hash_power_level = hashpower;
-    stats.hash_bytes = hashsize(hashpower) * sizeof(void *);
+    stats_state.hash_power_level = hashpower;
+    stats_state.hash_bytes = hashsize(hashpower) * sizeof(void *);
     STATS_UNLOCK();
 }
 
@@ -134,9 +134,9 @@ static void assoc_expand(void) {
         expanding = true;
         expand_bucket = 0;
         STATS_LOCK();
-        stats.hash_power_level = hashpower;
-        stats.hash_bytes += hashsize(hashpower) * sizeof(void *);
-        stats.hash_is_expanding = 1;
+        stats_state.hash_power_level = hashpower;
+        stats_state.hash_bytes += hashsize(hashpower) * sizeof(void *);
+        stats_state.hash_is_expanding = true;
         STATS_UNLOCK();
     } else {
         primary_hashtable = old_hashtable;
@@ -238,8 +238,8 @@ static void *assoc_maintenance_thread(void *arg) {
                         expanding = false;
                         free(old_hashtable);
                         STATS_LOCK();
-                        stats.hash_bytes -= hashsize(hashpower - 1) * sizeof(void *);
-                        stats.hash_is_expanding = 0;
+                        stats_state.hash_bytes -= hashsize(hashpower - 1) * sizeof(void *);
+                        stats_state.hash_is_expanding = false;
                         STATS_UNLOCK();
                         if (settings.verbose > 1)
                             fprintf(stderr, "Hash table expansion done\n");

globals.c

@@ -20,6 +20,7 @@ volatile rel_time_t current_time;
 
 /** exported globals **/
 struct stats stats;
+struct stats_state stats_state;
 struct settings settings;
 struct slab_rebalance slab_rebal;
 volatile int slab_rebalance_signal;

items.c

@@ -33,8 +33,8 @@ typedef struct {
     uint64_t reclaimed;
     uint64_t outofmemory;
     uint64_t tailrepairs;
-    uint64_t expired_unfetched;
-    uint64_t evicted_unfetched;
+    uint64_t expired_unfetched; /* items reclaimed but never touched */
+    uint64_t evicted_unfetched; /* items evicted but never touched */
     uint64_t crawler_reclaimed;
     uint64_t crawler_items_checked;
     uint64_t lrutail_reflocked;
@@ -332,8 +332,8 @@ int do_item_link(item *it, const uint32_t hv) {
     it->time = current_time;
 
     STATS_LOCK();
-    stats.curr_bytes += ITEM_ntotal(it);
-    stats.curr_items += 1;
+    stats_state.curr_bytes += ITEM_ntotal(it);
+    stats_state.curr_items += 1;
     stats.total_items += 1;
     STATS_UNLOCK();
 
@@ -352,8 +352,8 @@ void do_item_unlink(item *it, const uint32_t hv) {
     if ((it->it_flags & ITEM_LINKED) != 0) {
         it->it_flags &= ~ITEM_LINKED;
         STATS_LOCK();
-        stats.curr_bytes -= ITEM_ntotal(it);
-        stats.curr_items -= 1;
+        stats_state.curr_bytes -= ITEM_ntotal(it);
+        stats_state.curr_items -= 1;
         STATS_UNLOCK();
         item_stats_sizes_remove(it);
         assoc_delete(ITEM_key(it), it->nkey, hv);
@@ -368,8 +368,8 @@ void do_item_unlink_nolock(item *it, const uint32_t hv) {
     if ((it->it_flags & ITEM_LINKED) != 0) {
         it->it_flags &= ~ITEM_LINKED;
         STATS_LOCK();
-        stats.curr_bytes -= ITEM_ntotal(it);
-        stats.curr_items -= 1;
+        stats_state.curr_bytes -= ITEM_ntotal(it);
+        stats_state.curr_items -= 1;
         STATS_UNLOCK();
         item_stats_sizes_remove(it);
         assoc_delete(ITEM_key(it), it->nkey, hv);
@@ -1377,7 +1377,7 @@ static void *item_crawler_thread(void *arg) {
     if (settings.verbose > 2)
         fprintf(stderr, "LRU crawler thread sleeping\n");
     STATS_LOCK();
-    stats.lru_crawler_running = false;
+    stats_state.lru_crawler_running = false;
     STATS_UNLOCK();
     }
     pthread_mutex_unlock(&lru_crawler_lock);
@@ -1469,7 +1469,7 @@ static int do_lru_crawler_start(uint32_t id, uint32_t remaining) {
     }
     if (starts) {
         STATS_LOCK();
-        stats.lru_crawler_running = true;
+        stats_state.lru_crawler_running = true;
         stats.lru_crawler_starts++;
         STATS_UNLOCK();
         pthread_mutex_lock(&lru_crawler_stats_lock);

memcached.c

@@ -108,6 +108,7 @@ static void conn_free(conn *c);
 
 /** exported globals **/
 struct stats stats;
+struct stats_state stats_state;
 struct settings settings;
 time_t process_started;     /* when the process was started */
 conn **conns;
@@ -178,20 +179,9 @@ static rel_time_t realtime(const time_t exptime) {
 }
 
 static void stats_init(void) {
-    stats.curr_items = stats.total_items = stats.curr_conns = stats.total_conns = stats.conn_structs = 0;
-    stats.get_cmds = stats.set_cmds = stats.get_hits = stats.get_misses = stats.evictions = stats.reclaimed = 0;
-    stats.touch_cmds = stats.touch_misses = stats.touch_hits = stats.rejected_conns = 0;
-    stats.malloc_fails = 0;
-    stats.curr_bytes = stats.listen_disabled_num = 0;
-    stats.hash_power_level = stats.hash_bytes = stats.hash_is_expanding = 0;
-    stats.expired_unfetched = stats.evicted_unfetched = 0;
-    stats.slabs_moved = 0;
-    stats.lru_maintainer_juggles = 0;
-    stats.accepting_conns = true; /* assuming we start in this state. */
-    stats.slab_reassign_running = false;
-    stats.lru_crawler_running = false;
-    stats.lru_crawler_starts = 0;
-    stats.time_in_listen_disabled_us = 0;
+    memset(&stats, 0, sizeof(struct stats));
+    memset(&stats_state, 0, sizeof(struct stats_state));
+    stats_state.accepting_conns = true; /* assuming we start in this state. */
 
     /* make the time we started always be 2 seconds before we really
        did, so time(0) - time.started is never zero.  if so, things
@@ -203,12 +193,7 @@ static void stats_init(void) {
 
 static void stats_reset(void) {
     STATS_LOCK();
-    stats.total_items = stats.total_conns = 0;
-    stats.rejected_conns = 0;
-    stats.malloc_fails = 0;
-    stats.evictions = 0;
-    stats.reclaimed = 0;
-    stats.listen_disabled_num = 0;
+    memset(&stats, 0, sizeof(struct stats));
     stats_prefix_clear();
     STATS_UNLOCK();
     threadlocal_stats_reset();
@@ -513,7 +498,7 @@ conn *conn_new(const int sfd, enum conn_states init_state,
         }
 
         STATS_LOCK();
-        stats.conn_structs++;
+        stats_state.conn_structs++;
         STATS_UNLOCK();
 
         c->sfd = sfd;
@@ -593,7 +578,7 @@ conn *conn_new(const int sfd, enum conn_states init_state,
     }
 
     STATS_LOCK();
-    stats.curr_conns++;
+    stats_state.curr_conns++;
     stats.total_conns++;
     STATS_UNLOCK();
 
@@ -697,7 +682,7 @@ static void conn_close(conn *c) {
     pthread_mutex_unlock(&conn_lock);
 
     STATS_LOCK();
-    stats.curr_conns--;
+    stats_state.curr_conns--;
     STATS_UNLOCK();
 
     return;
@@ -2714,13 +2699,13 @@ static void server_stats(ADD_STAT add_stats, conn *c) {
                 (long)usage.ru_stime.tv_usec);
 #endif /* !WIN32 */
 
-    APPEND_STAT("curr_connections", "%llu", (unsigned long long)stats.curr_conns - 1);
+    APPEND_STAT("curr_connections", "%llu", (unsigned long long)stats_state.curr_conns - 1);
     APPEND_STAT("total_connections", "%llu", (unsigned long long)stats.total_conns);
     if (settings.maxconns_fast) {
         APPEND_STAT("rejected_connections", "%llu", (unsigned long long)stats.rejected_conns);
     }
-    APPEND_STAT("connection_structures", "%u", stats.conn_structs);
-    APPEND_STAT("reserved_fds", "%u", stats.reserved_fds);
+    APPEND_STAT("connection_structures", "%u", stats_state.conn_structs);
+    APPEND_STAT("reserved_fds", "%u", stats_state.reserved_fds);
     APPEND_STAT("cmd_get", "%llu", (unsigned long long)thread_stats.get_cmds);
     APPEND_STAT("cmd_set", "%llu", (unsigned long long)slab_stats.set_cmds);
     APPEND_STAT("cmd_flush", "%llu", (unsigned long long)thread_stats.flush_cmds);
@@ -2748,24 +2733,24 @@ static void server_stats(ADD_STAT add_stats, conn *c) {
     APPEND_STAT("bytes_read", "%llu", (unsigned long long)thread_stats.bytes_read);
     APPEND_STAT("bytes_written", "%llu", (unsigned long long)thread_stats.bytes_written);
     APPEND_STAT("limit_maxbytes", "%llu", (unsigned long long)settings.maxbytes);
-    APPEND_STAT("accepting_conns", "%u", stats.accepting_conns);
+    APPEND_STAT("accepting_conns", "%u", stats_state.accepting_conns);
     APPEND_STAT("listen_disabled_num", "%llu", (unsigned long long)stats.listen_disabled_num);
     APPEND_STAT("time_in_listen_disabled_us", "%llu", stats.time_in_listen_disabled_us);
     APPEND_STAT("threads", "%d", settings.num_threads);
     APPEND_STAT("conn_yields", "%llu", (unsigned long long)thread_stats.conn_yields);
-    APPEND_STAT("hash_power_level", "%u", stats.hash_power_level);
-    APPEND_STAT("hash_bytes", "%llu", (unsigned long long)stats.hash_bytes);
-    APPEND_STAT("hash_is_expanding", "%u", stats.hash_is_expanding);
+    APPEND_STAT("hash_power_level", "%u", stats_state.hash_power_level);
+    APPEND_STAT("hash_bytes", "%llu", (unsigned long long)stats_state.hash_bytes);
+    APPEND_STAT("hash_is_expanding", "%u", stats_state.hash_is_expanding);
     if (settings.slab_reassign) {
         APPEND_STAT("slab_reassign_rescues", "%llu", stats.slab_reassign_rescues);
         APPEND_STAT("slab_reassign_evictions_nomem", "%llu", stats.slab_reassign_evictions_nomem);
         APPEND_STAT("slab_reassign_inline_reclaim", "%llu", stats.slab_reassign_inline_reclaim);
         APPEND_STAT("slab_reassign_busy_items", "%llu", stats.slab_reassign_busy_items);
-        APPEND_STAT("slab_reassign_running", "%u", stats.slab_reassign_running);
+        APPEND_STAT("slab_reassign_running", "%u", stats_state.slab_reassign_running);
         APPEND_STAT("slabs_moved", "%llu", stats.slabs_moved);
     }
     if (settings.lru_crawler) {
-        APPEND_STAT("lru_crawler_running", "%u", stats.lru_crawler_running);
+        APPEND_STAT("lru_crawler_running", "%u", stats_state.lru_crawler_running);
         APPEND_STAT("lru_crawler_starts", "%u", stats.lru_crawler_starts);
     }
     if (settings.lru_maintainer_thread) {
@@ -4180,11 +4165,11 @@ void do_accept_new_conns(const bool do_accept) {
             (maxconns_exited.tv_sec - stats.maxconns_entered.tv_sec) * 1000000
             + (maxconns_exited.tv_usec - stats.maxconns_entered.tv_usec);
         stats.time_in_listen_disabled_us += elapsed_us;
-        stats.accepting_conns = true;
+        stats_state.accepting_conns = true;
         STATS_UNLOCK();
     } else {
         STATS_LOCK();
-        stats.accepting_conns = false;
+        stats_state.accepting_conns = false;
         gettimeofday(&stats.maxconns_entered,NULL);
         stats.listen_disabled_num++;
         STATS_UNLOCK();
@@ -4319,7 +4304,7 @@ static void drive_machine(conn *c) {
             }
 
             if (settings.maxconns_fast &&
-                stats.curr_conns + stats.reserved_fds >= settings.maxconns - 1) {
+                stats_state.curr_conns + stats_state.reserved_fds >= settings.maxconns - 1) {
                 str = "ERROR Too many open connections\r\n";
                 res = write(sfd, str, strlen(str));
                 close(sfd);
@@ -6093,7 +6078,7 @@ int main (int argc, char **argv) {
      * is only an advisory.
      */
     usleep(1000);
-    if (stats.curr_conns + stats.reserved_fds >= settings.maxconns - 1) {
+    if (stats_state.curr_conns + stats_state.reserved_fds >= settings.maxconns - 1) {
         fprintf(stderr, "Maxconns setting is too low, use -c to increase.\n");
         exit(EXIT_FAILURE);
     }

memcached.h

@@ -261,44 +261,20 @@ struct thread_stats {
 };
 
 /**
- * Global stats.
+ * Global stats. Only resettable stats should go into this structure.
  */
 struct stats {
-    pthread_mutex_t mutex;
-    uint64_t      curr_items;
     uint64_t      total_items;
-    uint64_t      curr_bytes;
-    uint64_t      curr_conns;
     uint64_t      total_conns;
     uint64_t      rejected_conns;
     uint64_t      malloc_fails;
-    unsigned int  reserved_fds;
-    unsigned int  conn_structs;
-    uint64_t      get_cmds;
-    uint64_t      set_cmds;
-    uint64_t      touch_cmds;
-    uint64_t      get_hits;
-    uint64_t      get_misses;
-    uint64_t      touch_hits;
-    uint64_t      touch_misses;
-    uint64_t      evictions;
-    uint64_t      reclaimed;
-    time_t        started;          /* when the process was started */
-    bool          accepting_conns;  /* whether we are currently accepting */
     uint64_t      listen_disabled_num;
-    unsigned int  hash_power_level; /* Better hope it's not over 9000 */
-    uint64_t      hash_bytes;       /* size used for hash tables */
-    bool          hash_is_expanding; /* If the hash table is being expanded */
-    uint64_t      expired_unfetched; /* items reclaimed but never touched */
-    uint64_t      evicted_unfetched; /* items evicted but never touched */
-    bool          slab_reassign_running; /* slab reassign in progress */
     uint64_t      slabs_moved;       /* times slabs were moved around */
     uint64_t      slab_reassign_rescues; /* items rescued during slab move */
     uint64_t      slab_reassign_evictions_nomem; /* valid items lost during slab move */
     uint64_t      slab_reassign_inline_reclaim; /* valid items lost during slab move */
     uint64_t      slab_reassign_busy_items; /* valid temporarily unmovable */
     uint64_t      lru_crawler_starts; /* Number of item crawlers kicked off */
-    bool          lru_crawler_running; /* crawl in progress */
     uint64_t      lru_maintainer_juggles; /* number of LRU bg pokes */
     uint64_t      time_in_listen_disabled_us;  /* elapsed time in microseconds while server unable to process new connections */
     uint64_t      log_worker_dropped; /* logs dropped by worker threads */
@@ -308,6 +284,24 @@ struct stats {
     struct timeval maxconns_entered;  /* last time maxconns entered */
 };
 
+/**
+ * Global "state" stats. Reflects state that shouldn't be wiped ever.
+ * Ordered for some cache line locality for commonly updated counters.
+ */
+struct stats_state {
+    uint64_t      curr_items;
+    uint64_t      curr_bytes;
+    uint64_t      curr_conns;
+    uint64_t      hash_bytes;       /* size used for hash tables */
+    unsigned int  conn_structs;
+    unsigned int  reserved_fds;
+    unsigned int  hash_power_level; /* Better hope it's not over 9000 */
+    bool          hash_is_expanding; /* If the hash table is being expanded */
+    bool          accepting_conns;  /* whether we are currently accepting */
+    bool          slab_reassign_running; /* slab reassign in progress */
+    bool          lru_crawler_running; /* crawl in progress */
+};
+
 #define MAX_VERBOSITY_LEVEL 2
 
 /* When adding a setting, be sure to update process_stat_settings */
@@ -361,6 +355,7 @@ struct settings {
 };
 
 extern struct stats stats;
+extern struct stats_state stats_state;
 extern time_t process_started;
 extern struct settings settings;
 

slabs.c

@@ -329,8 +329,8 @@ bool get_stats(const char *stat_type, int nkey, ADD_STAT add_stats, void *c) {
         if (!stat_type) {
             /* prepare general statistics for the engine */
             STATS_LOCK();
-            APPEND_STAT("bytes", "%llu", (unsigned long long)stats.curr_bytes);
-            APPEND_STAT("curr_items", "%llu", (unsigned long long)stats.curr_items);
+            APPEND_STAT("bytes", "%llu", (unsigned long long)stats_state.curr_bytes);
+            APPEND_STAT("curr_items", "%llu", (unsigned long long)stats_state.curr_items);
             APPEND_STAT("total_items", "%llu", (unsigned long long)stats.total_items);
             STATS_UNLOCK();
             if (settings.slab_automove > 0) {
@@ -588,7 +588,7 @@ static int slab_rebalance_start(void) {
     pthread_mutex_unlock(&slabs_lock);
 
     STATS_LOCK();
-    stats.slab_reassign_running = true;
+    stats_state.slab_reassign_running = true;
     STATS_UNLOCK();
 
     return 0;
@@ -870,11 +870,11 @@ static void slab_rebalance_finish(void) {
     pthread_mutex_unlock(&slabs_lock);
 
     STATS_LOCK();
-    stats.slab_reassign_running = false;
     stats.slabs_moved++;
     stats.slab_reassign_rescues += rescues;
     stats.slab_reassign_evictions_nomem += evictions_nomem;
     stats.slab_reassign_inline_reclaim += inline_reclaim;
+    stats_state.slab_reassign_running = false;
     STATS_UNLOCK();
 
     if (settings.verbose > 1) {

thread.c

@@ -516,7 +516,7 @@ void sidethread_conn_close(conn *c) {
     close(c->sfd);
 
     STATS_LOCK();
-    stats.curr_conns--;
+    stats_state.curr_conns--;
     STATS_UNLOCK();
 
     return;
@@ -851,7 +851,7 @@ void memcached_thread_init(int nthreads, struct event_base *main_base) {
 
         setup_thread(&threads[i]);
         /* Reserve three fds for the libevent base, and two for the pipe */
-        stats.reserved_fds += 5;
+        stats_state.reserved_fds += 5;
     }
 
     /* Create threads after we've done all the libevent setup. */