fix zero hash items eviction

If all hash values of five tail items are zero on the specified slab
class, expire check is unintentionally skipped and items stay without
being evicted. Consequently, new item allocation consume memory space
every time an item is set, that leads to slab OOM errors.

4 files changed, 16 insertions, 17 deletions

diff --git a/items.c b/items.c
index 21b9279..7cee415 100644
--- a/items.c
+++ b/items.c
@@ -90,7 +90,7 @@ void item_stats_reset(void) {
 }
 
 static int lru_pull_tail(const int orig_id, const int cur_lru,
-        const uint64_t total_bytes, const bool do_evict, const uint32_t cur_hv);
+        const uint64_t total_bytes, const bool do_evict);
 static int lru_crawler_start(uint32_t id, uint32_t remaining);
 
 /* Get the next CAS id for a new item. */
@@ -157,8 +157,7 @@ static size_t item_make_header(const uint8_t nkey, const unsigned int flags, con
 }
 
 item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags,
-                    const rel_time_t exptime, const int nbytes,
-                    const uint32_t cur_hv) {
+                    const rel_time_t exptime, const int nbytes) {
     int i;
     uint8_t nsuffix;
     item *it = NULL;
@@ -190,7 +189,7 @@ item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags,
         uint64_t total_bytes;
         /* Try to reclaim memory first */
         if (!settings.lru_maintainer_thread) {
-            lru_pull_tail(id, COLD_LRU, 0, false, cur_hv);
+            lru_pull_tail(id, COLD_LRU, 0, false);
         }
         it = slabs_alloc(ntotal, id, &total_bytes, 0);
 
@@ -199,12 +198,12 @@ item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags,
 
         if (it == NULL) {
             if (settings.lru_maintainer_thread) {
-                lru_pull_tail(id, HOT_LRU, total_bytes, false, cur_hv);
-                lru_pull_tail(id, WARM_LRU, total_bytes, false, cur_hv);
-                if (lru_pull_tail(id, COLD_LRU, total_bytes, true, cur_hv) <= 0)
+                lru_pull_tail(id, HOT_LRU, total_bytes, false);
+                lru_pull_tail(id, WARM_LRU, total_bytes, false);
+                if (lru_pull_tail(id, COLD_LRU, total_bytes, true) <= 0)
                     break;
             } else {
-                if (lru_pull_tail(id, COLD_LRU, 0, true, cur_hv) <= 0)
+                if (lru_pull_tail(id, COLD_LRU, 0, true) <= 0)
                     break;
             }
         } else {
@@ -841,7 +840,7 @@ item *do_item_touch(const char *key, size_t nkey, uint32_t exptime,
 /* Returns number of items remove, expired, or evicted.
  * Callable from worker threads or the LRU maintainer thread */
 static int lru_pull_tail(const int orig_id, const int cur_lru,
-        const uint64_t total_bytes, const bool do_evict, const uint32_t cur_hv) {
+        const uint64_t total_bytes, const bool do_evict) {
     item *it = NULL;
     int id = orig_id;
     int removed = 0;
@@ -870,7 +869,7 @@ static int lru_pull_tail(const int orig_id, const int cur_lru,
         uint32_t hv = hash(ITEM_key(search), search->nkey);
         /* Attempt to hash item lock the "search" item. If locked, no
          * other callers can incr the refcount. Also skip ourselves. */
-        if (hv == cur_hv || (hold_lock = item_trylock(hv)) == NULL)
+        if ((hold_lock = item_trylock(hv)) == NULL)
             continue;
         /* Now see if the item is refcount locked */
         if (refcount_incr(&search->refcount) != 2) {
@@ -1017,11 +1016,11 @@ static int lru_maintainer_juggle(const int slabs_clsid) {
     /* Juggle HOT/WARM up to N times */
     for (i = 0; i < 1000; i++) {
         int do_more = 0;
-        if (lru_pull_tail(slabs_clsid, HOT_LRU, total_bytes, false, 0) ||
-            lru_pull_tail(slabs_clsid, WARM_LRU, total_bytes, false, 0)) {
+        if (lru_pull_tail(slabs_clsid, HOT_LRU, total_bytes, false) ||
+            lru_pull_tail(slabs_clsid, WARM_LRU, total_bytes, false)) {
             do_more++;
         }
-        do_more += lru_pull_tail(slabs_clsid, COLD_LRU, total_bytes, false, 0);
+        do_more += lru_pull_tail(slabs_clsid, COLD_LRU, total_bytes, false);
         if (do_more == 0)
             break;
         did_moves++;
diff --git a/items.h b/items.h
index d235f87..32aefb8 100644
--- a/items.h
+++ b/items.h
@@ -2,7 +2,7 @@
 uint64_t get_cas_id(void);
 
 /*@null@*/
-item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags, const rel_time_t exptime, const int nbytes, const uint32_t cur_hv);
+item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags, const rel_time_t exptime, const int nbytes);
 void item_free(item *it);
 bool item_size_ok(const size_t nkey, const int flags, const int nbytes);
 
diff --git a/memcached.c b/memcached.c
index 14a0d27..09bfe0f 100644
--- a/memcached.c
+++ b/memcached.c
@@ -2625,7 +2625,7 @@ enum store_item_type do_store_item(item *it, int comm, conn *c, const uint32_t h
 
                 flags = (uint32_t) strtoul(ITEM_suffix(old_it), (char **) NULL, 10);
 
-                new_it = do_item_alloc(key, it->nkey, flags, old_it->exptime, it->nbytes + old_it->nbytes - 2 /* CRLF */, hv);
+                new_it = do_item_alloc(key, it->nkey, flags, old_it->exptime, it->nbytes + old_it->nbytes - 2 /* CRLF */);
 
                 if (new_it == NULL) {
                     failed_alloc = 1;
@@ -3600,7 +3600,7 @@ enum delta_result_type do_add_delta(conn *c, const char *key, const size_t nkey,
     } else if (it->refcount > 1) {
         item *new_it;
         uint32_t flags = (uint32_t) strtoul(ITEM_suffix(it)+1, (char **) NULL, 10);
-        new_it = do_item_alloc(ITEM_key(it), it->nkey, flags, it->exptime, res + 2, hv);
+        new_it = do_item_alloc(ITEM_key(it), it->nkey, flags, it->exptime, res + 2);
         if (new_it == 0) {
             do_item_remove(it);
             return EOM;
diff --git a/thread.c b/thread.c
index 4e431b5..65fbd2b 100644
--- a/thread.c
+++ b/thread.c
@@ -537,7 +537,7 @@ int is_listen_thread() {
 item *item_alloc(char *key, size_t nkey, int flags, rel_time_t exptime, int nbytes) {
     item *it;
     /* do_item_alloc handles its own locks */
-    it = do_item_alloc(key, nkey, flags, exptime, nbytes, 0);
+    it = do_item_alloc(key, nkey, flags, exptime, nbytes);
     return it;
 }