osd/SnapMapper: fix pacific legacy key conversion and introduce test

Octopus modified the SnapMapper key format from

  <LEGACY_MAPPING_PREFIX><snapid>_<shardid>_<hobject_t::to_str()>

to

  <MAPPING_PREFIX><pool>_<snapid>_<shardid>_<hobject_t::to_str()>

When this change was introduced, 94ebe0ea also introduced a conversion
with a crucial bug which essentially destroyed legacy keys by mapping them
to

  <MAPPING_PREFIX><poolid>_<snapid>_

without the object-unique suffix.  This commit fixes this conversion going
forward, but a fix for existing clusters still needs to be developed.

Fixes: https://tracker.ceph.com/issues/56147
Signed-off-by: Manuel Lausch <manuel.lausch@1und1.de>
Signed-off-by: Matan Breizman <mbreizma@redhat.com>

diff --git a/src/osd/SnapMapper.cc b/src/osd/SnapMapper.cc
index 07c6f57c78ae4ac0efbdc01a4f947b520a3feb71..804213b1f264461d6830a7f0ed2681868dc3a579 100644 (file)
--- a/src/osd/SnapMapper.cc
+++ b/src/osd/SnapMapper.cc
@@ -660,6 +660,38 @@ bool SnapMapper::is_legacy_mapping(const string &to_test)
     LEGACY_MAPPING_PREFIX;
 }
 
+/* Octopus modified the SnapMapper key format from
+ *
+ *  <LEGACY_MAPPING_PREFIX><snapid>_<shardid>_<hobject_t::to_str()>
+ *
+ * to
+ *
+ *  <MAPPING_PREFIX><pool>_<snapid>_<shardid>_<hobject_t::to_str()>
+ *
+ * We can't reconstruct the new key format just from the value since the
+ * Mapping object contains an hobject rather than a ghobject. Instead,
+ * we exploit the fact that the new format is identical starting at <snapid>.
+ *
+ * Note that the original version of this conversion introduced in 94ebe0ea
+ * had a crucial bug which essentially destroyed legacy keys by mapping
+ * them to
+ *
+ *  <MAPPING_PREFIX><poolid>_<snapid>_
+ *
+ * without the object-unique suffix.
+ * See https://tracker.ceph.com/issues/56147
+ */
+std::string SnapMapper::convert_legacy_key(
+  const std::string& old_key,
+  const bufferlist& value)
+{
+  auto old = from_raw(make_pair(old_key, value));
+  std::string object_suffix = old_key.substr(
+    SnapMapper::LEGACY_MAPPING_PREFIX.length());
+  return SnapMapper::MAPPING_PREFIX + std::to_string(old.second.pool)
+    + "_" + object_suffix;
+}
+
 int SnapMapper::convert_legacy(
   CephContext *cct,
   ObjectStore *store,
@@ -681,13 +713,9 @@ int SnapMapper::convert_legacy(
   while (iter->valid()) {
     bool valid = SnapMapper::is_legacy_mapping(iter->key());
     if (valid) {
-      SnapMapper::Mapping m;
-      bufferlist bl(iter->value());
-      auto bp = bl.cbegin();
-      decode(m, bp);
       to_set.emplace(
-       SnapMapper::get_prefix(m.hoid.pool, m.snap),
-       bl);
+       convert_legacy_key(iter->key(), iter->value()),
+       iter->value());
       ++n;
       iter->next();
     }

diff --git a/src/osd/SnapMapper.h b/src/osd/SnapMapper.h
index f8c2aff1b20f420826ac6b0e0593fd06f6444bfb..90b0c7c8d82e1b4c5141b8918e275f1ffa4a2e28 100644 (file)
--- a/src/osd/SnapMapper.h
+++ b/src/osd/SnapMapper.h
@@ -100,6 +100,7 @@ public:
  * particular snap will group under up to 8 prefixes.
  */
 class SnapMapper {
+  friend class MapperVerifier;
 public:
   CephContext* cct;
   struct object_snaps {
@@ -174,6 +175,10 @@ public:
     void run();
   };
 
+  static std::string convert_legacy_key(
+    const std::string& old_key,
+    const bufferlist& value);
+
   static int convert_legacy(
     CephContext *cct,
     ObjectStore *store,

diff --git a/src/test/test_snap_mapper.cc b/src/test/test_snap_mapper.cc
index 50730080d8dc7387dc52755cef827f3c5c9c05af..77e31ec0eb730eae0c51593728fb48afceeedab8 100644 (file)
--- a/src/test/test_snap_mapper.cc
+++ b/src/test/test_snap_mapper.cc
@@ -496,6 +496,21 @@ public:
     }
   }
 
+  std::pair<std::string, ceph::buffer::list> to_raw(
+    const std::pair<snapid_t, hobject_t> &to_map) {
+    return mapper->to_raw(to_map);
+  }
+
+  std::string to_legacy_raw_key(
+    const std::pair<snapid_t, hobject_t> &to_map) {
+    return mapper->to_legacy_raw_key(to_map);
+  }
+
+  std::string to_raw_key(
+    const std::pair<snapid_t, hobject_t> &to_map) {
+    return mapper->to_raw_key(to_map);
+  }
+
   void trim_snap() {
     std::lock_guard l{lock};
     if (snap_to_hobject.empty())
@@ -652,3 +667,19 @@ TEST_F(SnapMapperTest, MultiPG) {
   init(50);
   run();
 }
+
+TEST_F(SnapMapperTest, LegacyKeyConvertion) {
+    init(1);
+    auto obj = get_tester().random_hobject();
+    snapid_t snapid = random() % 10;
+    auto snap_obj = make_pair(snapid, obj);
+    auto raw = get_tester().to_raw(snap_obj);
+    std::string old_key = get_tester().to_legacy_raw_key(snap_obj);
+    std::string converted_key =
+      SnapMapper::convert_legacy_key(old_key, raw.second);
+    std::string new_key = get_tester().to_raw_key(snap_obj);
+    std::cout << "Converted: " << old_key << "\nTo:        " << converted_key
+             << "\nNew key:   " << new_key << std::endl;
+    ASSERT_EQ(converted_key, new_key);
+}
+