[RocksDB] Fix LRUCache Eviction problem

Summary:
1. The stock LRUCache nukes itself whenever the working set (the total number of entries not released by client at a certain time) is bigger than the cache capacity.
See https://our.dev.facebook.com/intern/tasks/?t=2252281
2. There's a bug in shard calculation leading to segmentation fault when only one shard is needed.

Test Plan: make check

Reviewers: dhruba, heyongqiang

Reviewed By: heyongqiang

CC: leveldb, zshao, sheki

Differential Revision: https://reviews.facebook.net/D9927

util/cache.cc

@@ -189,7 +189,7 @@ void LRUCache::Unref(LRUHandle* e) {
   assert(e->refs > 0);
   e->refs--;
   if (e->refs <= 0) {
-    usage_ -= e->charge;
+
     (*e->deleter)(e->key(), e->value);
     free(e);
   }
@@ -198,6 +198,7 @@ void LRUCache::Unref(LRUHandle* e) {
 void LRUCache::LRU_Remove(LRUHandle* e) {
   e->next->prev = e->prev;
   e->prev->next = e->next;
+  usage_ -= e->charge;
 }
 
 void LRUCache::LRU_Append(LRUHandle* e) {
@@ -206,6 +207,7 @@ void LRUCache::LRU_Append(LRUHandle* e) {
   e->prev = lru_.prev;
   e->prev->next = e;
   e->next->prev = e;
+  usage_ += e->charge;
 }
 
 Cache::Handle* LRUCache::Lookup(const Slice& key, uint32_t hash) {
@@ -239,7 +241,6 @@ Cache::Handle* LRUCache::Insert(
   e->refs = 2;  // One from LRUCache, one for the returned handle
   memcpy(e->key_data, key.data(), key.size());
   LRU_Append(e);
-  usage_ += charge;
 
   LRUHandle* old = table_.Insert(e);
   if (old != nullptr) {
@@ -281,7 +282,8 @@ class ShardedLRUCache : public Cache {
   }
 
   uint32_t Shard(uint32_t hash) {
-    return hash >> (32 - numShardBits);
+    // Note, hash >> 32 yields hash in gcc, not the zero we expect!
+    return (numShardBits > 0) ? (hash >> (32 - numShardBits)) : 0;
   }
 
   void init(size_t capacity, int numbits) {

util/cache_test.cc

@@ -5,6 +5,8 @@
 #include "leveldb/cache.h"
 
 #include <vector>
+#include <string>
+#include <iostream>
 #include "util/coding.h"
 #include "util/testharness.h"
 
@@ -178,6 +180,51 @@ TEST(CacheTest, NewId) {
   ASSERT_NE(a, b);
 }
 
+
+class Value {
+ private:
+  int v_;
+ public:
+  Value(int v) : v_(v) { }
+
+  ~Value() { std::cout << v_ << " is destructed\n"; }
+};
+
+void deleter(const Slice& key, void* value) {
+  delete (Value *)value;
+}
+
+
+TEST(CacheTest, BadEviction) {
+  int n = 10;
+
+  // a LRUCache with n entries and one shard only
+  std::shared_ptr<Cache> cache = NewLRUCache(n, 0);
+
+  std::vector<Cache::Handle*> handles(n+1);
+
+  // Insert n+1 entries, but not releasing.
+  for (int i = 0; i < n+1; i++) {
+    std::string key = std::to_string(i+1);
+    handles[i] = cache->Insert(key, new Value(i+1), 1, &deleter);
+  }
+
+  // Guess what's in the cache now?
+  for (int i = 0; i < n+1; i++) {
+    std::string key = std::to_string(i+1);
+    auto h = cache->Lookup(key);
+    std::cout << key << (h?" found\n":" not found\n");
+    // Only the first entry should be missing
+    ASSERT_TRUE(h || i == 0);
+    if (h) cache->Release(h);
+  }
+
+  for (int i = 0; i < n+1; i++) {
+    cache->Release(handles[i]);
+  }
+  std::cout << "Poor entries\n";
+}
+
 }  // namespace leveldb
 
 int main(int argc, char** argv) {