shared_cache: expose prior existence when inserting an element

The LRU now handles you attempting to insert multiple values for the
same key, by telling you that you've done so and returning the
existing value before it manages to muck up existing data.
The param 'existed' is not mandatory, default value is NULL.

Signed-off-by: Greg Farnum <greg@inktank.com>
Signed-off-by: Somnath Roy <somnath.roy@sandisk.com>

diff --git a/src/common/shared_cache.hpp b/src/common/shared_cache.hpp
index 2f5f05a0d644963d7e9665b21dc071574745ba0f..6e05a6880a30c7df9a3eaf4303c430c11f1e8608 100644 (file)
--- a/src/common/shared_cache.hpp
+++ b/src/common/shared_cache.hpp
@@ -169,12 +169,35 @@ public:
     return val;
   }
 
-  VPtr add(K key, V *value) {
+  /***
+   * Inserts a key if not present, or bumps it to the front of the LRU if
+   * it is, and then gives you a reference to the value. If the key already
+   * existed, you are responsible for deleting the new value you tried to
+   * insert.
+   *
+   * @param key The key to insert
+   * @param value The value that goes with the key
+   * @param existed Set to true if the value was already in the
+   * map, false otherwise
+   * @return A reference to the map's value for the given key
+   */
+  VPtr add(K key, V *value, bool *existed = NULL) {
     VPtr val(value, Cleanup(this, key));
     list<VPtr> to_release;
     {
       Mutex::Locker l(lock);
-      weak_refs.insert(make_pair(key, val));
+      typename map<K, WeakVPtr>::iterator actual = weak_refs.lower_bound(key);
+      if (actual != weak_refs.end() && actual->first == key) {
+        if (existed) 
+          *existed = true;
+
+        return actual->second.lock();
+      }
+
+      if (existed)      
+        *existed = false;
+
+      weak_refs.insert(actual, make_pair(key, val));
       lru_add(key, val, &to_release);
     }
     return val;

diff --git a/src/os/FDCache.h b/src/os/FDCache.h
index ba11e1207399451e2e11e9df6d4ffd51a0bcae30..712355b48192bc4d8c208c97932c1b42ccafed18 100644 (file)
--- a/src/os/FDCache.h
+++ b/src/os/FDCache.h
@@ -67,8 +67,8 @@ public:
     return registry.lookup(hoid);
   }
 
-  FDRef add(const ghobject_t &hoid, int fd) {
-    return registry.add(hoid, new FD(fd));
+  FDRef add(const ghobject_t &hoid, int fd, bool *existed) {
+    return registry.add(hoid, new FD(fd), existed);
   }
 
   /// clear cached fd for hoid, subsequent lookups will get an empty FD

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 165412dfab3629166330b87650d6c83b83111723..b3f06f20d6315d5709e68e5979ac1f976f94e521 100644 (file)
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -293,7 +293,9 @@ int FileStore::lfn_open(coll_t cid,
       VOID_TEMP_FAILURE_RETRY(::close(fd));
       return 0;
     } else {
-      *outfd = fdcache.add(oid, fd);
+      bool existed;
+      *outfd = fdcache.add(oid, fd, &existed);
+      assert(!existed);
     }
   } else {
     *outfd = FDRef(new FDCache::FD(fd));

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index a6568d0526e7223da54e345ec74df7525803e7a4..070c473da3a5555fcf1aae591cf20e9eccfce3ed 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -1154,7 +1154,11 @@ OSDMapRef OSDService::_add_map(OSDMap *o)
       OSDMap::dedup(for_dedup.get(), o);
     }
   }
-  OSDMapRef l = map_cache.add(e, o);
+  bool existed;
+  OSDMapRef l = map_cache.add(e, o, &existed);
+  if (existed) {
+    delete o;
+  }
   return l;
 }