crimson/os/seastore: handle duplicate keys in LogNode::remove_entry

Previously, LogNode::remove_entry returned early when a log_key was
found, assuming uniqueness. However, duplicate keys can exist in the
node if an older entry was previously removed.
This commit also adds a unit test to verify this scenario.

Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>

diff --git a/src/crimson/os/seastore/omap_manager/log/log_node.cc b/src/crimson/os/seastore/omap_manager/log/log_node.cc
index c8c01b7af9a8a4512f5bc16fad6aaa20adecd784..e87a3997b3fe5808f3ab9f65513b74bc37b3fd2b 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/log/log_node.cc
+++ b/src/crimson/os/seastore/omap_manager/log/log_node.cc
@@ -244,11 +244,7 @@ bool LogNode::remove_entry(const std::string key)
   while(iter != iter_end()) {
     if (iter->get_key() == key) {
       set_cur_bitmap(index, index);
-      /* If key is time-series log,
-       * duplicate key does not exist. In this case, return true */
-      if (is_log_key(key)) {
-       return true;
-      }
+      // Duplicate keys may exist if the old entry was removed.
       removed = true;
     }
     index++;

diff --git a/src/test/crimson/seastore/test_seastore.cc b/src/test/crimson/seastore/test_seastore.cc
index b251b603e7985e0b1bb31af0dbd05828c82718b4..166ad7b3de3e8abddbe62cbe29eab69f99d9ea2f 100644 (file)
--- a/src/test/crimson/seastore/test_seastore.cc
+++ b/src/test/crimson/seastore/test_seastore.cc
@@ -1879,6 +1879,43 @@ TEST_P(seastore_test_t, pgmeta_io)
         p);
     }
 
+    auto &test_obj7 = get_object(make_oid(600));
+    test_obj7.touch(*sharded_seastore);
+    test_obj7.set_log_object(*sharded_seastore);
+    epoch += 1;
+    {
+      std::string start = generate_key(epoch, version);
+      {
+       CTransaction t;
+       for (int i = 0; i < 20; i++) {
+         std::string key = generate_key(epoch, version);
+         char c_array[240] = {(char)((i % 10) + '0')};
+         std::string ss(&c_array[0], sizeof(c_array));
+         bufferlist l;
+         encode(ss, l);
+         std::map<string, bufferlist> kvs;
+         kvs[key] = l;
+         test_obj7.set_omaps(t, kvs);
+         version += i;
+       }
+       do_transaction(std::move(t));
+      }
+      bufferlist l;
+      test_obj7.set_omap(*sharded_seastore, "_info", l);
+      test_obj7.rm_omap_range(*sharded_seastore, eversion_t().get_key_name(), 
+       eversion_t::max().get_key_name());
+      std::map<string, bufferlist> kvs;
+      kvs[start] = l;
+      test_obj7.set_omap(*sharded_seastore, start, l);
+      std::set<std::string> keys;
+      keys.insert(start);
+      keys.insert(generate_key(epoch, version));
+      test_obj7.rm_omaps(*sharded_seastore, keys);
+      kvs = test_obj7.get_omaps(*sharded_seastore, start);
+      // test_obj7 should have only _info at this point
+      EXPECT_EQ(kvs.size(), 1);
+    }
+
     auto kvs = test_obj.get_omaps(*sharded_seastore, std::string());
     EXPECT_EQ(kvs.size(), test_obj.omap.size());