KeyValueStore: Fix incorrect rm_keys behaviors

GenericObjectMap will copyup parents keys when rm keys, but if
caller(KeyValueStore) already buffer "set keys", it shouldn't
copyup reset keys.

Signed-off-by: Haomai Wang <haomaiwang@gmail.com>

diff --git a/src/os/GenericObjectMap.cc b/src/os/GenericObjectMap.cc
index 241ea5e68c663b4425cc1f47675792a840038135..c0ecd17209655030b296fc0ab8273594d31ddbb6 100644 (file)
--- a/src/os/GenericObjectMap.cc
+++ b/src/os/GenericObjectMap.cc
@@ -534,6 +534,7 @@ int GenericObjectMap::clear(const Header header,
 
 int GenericObjectMap::rm_keys(const Header header,
                               const string &prefix,
+                              const set<string> &buffered_keys,
                               const set<string> &to_clear,
                               KeyValueDB::Transaction t)
 {
@@ -563,7 +564,7 @@ int GenericObjectMap::rm_keys(const Header header,
         begin = new_complete.rbegin()->first;
       }
       while (iter->valid() && copied < 20) {
-        if (!to_clear.count(iter->key()))
+        if (!to_clear.count(iter->key()) && !buffered_keys.count(iter->key()))
           to_write[iter->key()].append(iter->value());
         if (i != to_clear.end() && *i <= iter->key()) {
           ++i;
@@ -692,6 +693,7 @@ void GenericObjectMap::clone(const Header parent, const coll_t &cid,
   // to find parent header. So it will let lookup_parent fail when "clone" and
   // "rm_keys" in one transaction. Here have to sync transaction to make
   // visiable for lookup_parent
+  // FIXME: Clear transaction operations here
   int r = submit_transaction_sync(t);
   assert(r == 0);
 }

diff --git a/src/os/GenericObjectMap.h b/src/os/GenericObjectMap.h
index d5979e2456943604cc8ffb9def2095652e5d5c6a..e947bb4f31119a2d9edf463bbb1efb605d086b92 100644 (file)
--- a/src/os/GenericObjectMap.h
+++ b/src/os/GenericObjectMap.h
@@ -240,6 +240,7 @@ class GenericObjectMap {
   int rm_keys(
     const Header header,
     const string &prefix,
+    const set<string> &buffered_keys,
     const set<string> &to_clear,
     KeyValueDB::Transaction t
     );

diff --git a/src/os/KeyValueStore.cc b/src/os/KeyValueStore.cc
index 390113470eacce786ff47bcff2f4032a25cc5632..e13469f097beb58781f9772b6da2b2234429a184 100644 (file)
--- a/src/os/KeyValueStore.cc
+++ b/src/os/KeyValueStore.cc
@@ -389,13 +389,22 @@ int KeyValueStore::BufferTransaction::remove_buffer_keys(
 {
   uniq_id uid = make_pair(strip_header->cid, strip_header->oid);
   map< uniq_id, map<pair<string, string>, bufferlist> >::iterator obj_it = buffers.find(uid);
+  set<string> buffered_keys;
   if ( obj_it != buffers.end() ) {
+    // TODO: Avoid use empty bufferlist to indicate the key is removed
     for (set<string>::iterator iter = keys.begin(); iter != keys.end(); ++iter) {
       obj_it->second[make_pair(prefix, *iter)] = bufferlist();
     }
+    // TODO: Avoid collect all buffered keys when remove keys
+    if (strip_header->header->parent) {
+      for (map<pair<string, string>, bufferlist>::iterator iter = obj_it->second.begin();
+           iter != obj_it->second.end(); ++iter) {
+        buffered_keys.insert(iter->first.second);
+      }
+    }
   }
 
-  return store->backend->rm_keys(strip_header->header, prefix, keys, t);
+  return store->backend->rm_keys(strip_header->header, prefix, buffered_keys, keys, t);
 }
 
 void KeyValueStore::BufferTransaction::clear_buffer_keys(