osd/scrub: verify SnapMapper consistency

Whenever the scrubber access the SnapMapper for the snaps of a specific
clone, the mapper will now verify that the snaps have the required
mapping DB entries (the 'SNA_' keys).

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
(cherry picked from commit dc254da593956eb05f585a7ff30614d3cc694504)

Conflicts:
- changes moved from scrub_backend to the pg_scrubber

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h
index 400b00347198d2466f067af8c31973b91d497372..5e2c6f43499569812aa6a415009889ea041009dd 100644 (file)
--- a/src/osd/PGBackend.h
+++ b/src/osd/PGBackend.h
@@ -298,7 +298,6 @@ typedef std::shared_ptr<const OSDMap> OSDMapRef;
 
      virtual bool check_failsafe_full() = 0;
 
-     virtual bool pg_is_repair() = 0;
      virtual void inc_osd_stat_repaired() = 0;
      virtual bool pg_is_remote_backfilling() = 0;
      virtual void pg_add_local_num_bytes(int64_t num_bytes) = 0;

diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h
index f676c3202942608522c88477588b0bb2d092b0a1..3a8ef27d1b9a76f32eaa450b46f2615ac8131ab1 100644 (file)
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -441,9 +441,6 @@ public:
     release_object_locks(manager);
   }
 
-  bool pg_is_repair() override {
-    return is_repair();
-  }
   void inc_osd_stat_repaired() override {
     osd->inc_osd_stat_repaired();
   }

diff --git a/src/osd/SnapMapper.h b/src/osd/SnapMapper.h
index 36eed57810c30584b658882073dbd175416a5706..45d03298b2fb5415f10f3ee62be9b46aff00b487 100644 (file)
--- a/src/osd/SnapMapper.h
+++ b/src/osd/SnapMapper.h
@@ -106,7 +106,8 @@ public:
  * particular snap will group under up to 8 prefixes.
  */
 class SnapMapper : public Scrub::SnapMapReaderI {
-  friend class MapperVerifier;
+  friend class MapperVerifier; // unit-test support
+  friend class DirectMapper; // unit-test support
 public:
   CephContext* cct;
   struct object_snaps {

diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h
index 2279d5c64f7d25e0c50844d1ddcff0ce4acd9002..b2f7b619662f66f08aedb230ca923ac75d4be60d 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.h
+++ b/src/osd/scrubber/pg_scrubber.h
@@ -38,10 +38,8 @@ struct BuildMap;
  *  sorting) is better than boost::small_vec. And for std::vector: no need to pre-reserve.
  */
 class ReplicaReservations {
-  using OrigSet = decltype(std::declval<PG>().get_actingset());
-
   PG* m_pg;
-  OrigSet m_acting_set;
+  std::set<pg_shard_t> m_acting_set;
   OSDService* m_osds;
   std::vector<pg_shard_t> m_waited_for_peers;
   std::vector<pg_shard_t> m_reserved_peers;

diff --git a/src/test/test_snap_mapper.cc b/src/test/test_snap_mapper.cc
index 77e31ec0eb730eae0c51593728fb48afceeedab8..154d28d71dac956f735ea62c1707d9bfed57fe16 100644 (file)
--- a/src/test/test_snap_mapper.cc
+++ b/src/test/test_snap_mapper.cc
@@ -8,6 +8,7 @@
 
 #include "include/buffer.h"
 #include "common/map_cacher.hpp"
+#include "osd/osd_types_fmt.h"
 #include "osd/SnapMapper.h"
 #include "common/Cond.h"
 
@@ -683,3 +684,162 @@ TEST_F(SnapMapperTest, LegacyKeyConvertion) {
     ASSERT_EQ(converted_key, new_key);
 }
 
+/**
+ * 'DirectMapper' provides simple, controlled, interface to the underlying
+ * SnapMapper.
+ */
+class DirectMapper {
+public:
+  std::unique_ptr<PausyAsyncMap> driver{make_unique<PausyAsyncMap>()};
+  std::unique_ptr<SnapMapper> mapper;
+  uint32_t mask;
+  uint32_t bits;
+  ceph::mutex lock = ceph::make_mutex("lock");
+
+  DirectMapper(
+    uint32_t mask,
+    uint32_t bits)
+   : mapper(new SnapMapper(g_ceph_context, driver.get(), mask, bits, 0, shard_id_t(1))), 
+             mask(mask), bits(bits) {}
+
+  hobject_t random_hobject() {
+    return hobject_t(
+      random_string(1+(rand() % 16)),
+      random_string(1+(rand() % 16)),
+      snapid_t(rand() % 1000),
+      (rand() & ((~0)<<bits)) | (mask & ~((~0)<<bits)),
+      0, random_string(rand() % 16));
+  }
+
+  void create_object(const hobject_t& obj, const set<snapid_t> &snaps) {
+    std::lock_guard l{lock};
+      PausyAsyncMap::Transaction t;
+      mapper->add_oid(obj, snaps, &t);
+      driver->submit(&t);
+  }
+
+  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 shorten_mapping_key(snapid_t snap, const hobject_t &clone)
+  {
+    // calculate the relevant key
+    std::string k = mapper->to_raw_key(snap, clone);
+
+    // find the value for this key
+    map<string, bufferlist> kvmap;
+    auto r = mapper->backend.get_keys(set{k}, &kvmap);
+    ASSERT_GE(r, 0);
+
+    // replace the key with its shortened version
+    PausyAsyncMap::Transaction t;
+    mapper->backend.remove_keys(set{k}, &t);
+    auto short_k = k.substr(0, 10);
+    mapper->backend.set_keys(map<string, bufferlist>{{short_k, kvmap[k]}}, &t);
+    driver->submit(&t);
+    driver->flush();
+  }
+};
+
+class DirectMapperTest : public ::testing::Test {
+ public:
+  // ctor & initialization
+  DirectMapperTest() = default;
+  ~DirectMapperTest() = default;
+  void SetUp() override;
+  void TearDown() override;
+
+ protected:
+  std::unique_ptr<DirectMapper> direct;
+};
+
+void DirectMapperTest::SetUp()
+{
+  direct = std::make_unique<DirectMapper>(0, 0);
+}
+
+void DirectMapperTest::TearDown()
+{
+  direct->driver->stop();
+  direct->mapper.reset();
+  direct->driver.reset();
+}
+
+
+TEST_F(DirectMapperTest, BasciObject)
+{
+  auto obj = direct->random_hobject();
+  set<snapid_t> snaps{100, 200};
+  direct->create_object(obj, snaps);
+
+  // verify that the OBJ_ & SNA_ entries are there
+  auto osn1 = direct->mapper->get_snaps(obj);
+  ASSERT_EQ(snaps, osn1);
+  auto vsn1 = direct->mapper->get_snaps_check_consistency(obj);
+  ASSERT_EQ(snaps, vsn1);
+}
+
+TEST_F(DirectMapperTest, CorruptedSnaRecord)
+{
+  object_t base_name{"obj"};
+  std::string key{"key"};
+
+  hobject_t head{base_name, key, CEPH_NOSNAP, 0x17, 0, ""};
+  hobject_t cln1{base_name, key, 10, 0x17, 0, ""};
+  hobject_t cln2{base_name, key, 20, 0x17, 0, ""}; // the oldest version
+  set<snapid_t> head_snaps{400, 500};
+  set<snapid_t> cln1_snaps{300};
+  set<snapid_t> cln2_snaps{100, 200};
+
+  PausyAsyncMap::Transaction t;
+  direct->mapper->add_oid(head, head_snaps, &t);
+  direct->mapper->add_oid(cln1, cln1_snaps, &t);
+  direct->mapper->add_oid(cln2, cln2_snaps, &t);
+  direct->driver->submit(&t);
+  direct->driver->flush();
+
+  // verify that the OBJ_ & SNA_ entries are there
+  {
+    auto osn1 = direct->mapper->get_snaps(cln1);
+    EXPECT_EQ(cln1_snaps, osn1);
+    auto osn2 = direct->mapper->get_snaps(cln2);
+    EXPECT_EQ(cln2_snaps, osn2);
+    auto osnh = direct->mapper->get_snaps(head);
+    EXPECT_EQ(head_snaps, osnh);
+  }
+  {
+    auto vsn1 = direct->mapper->get_snaps_check_consistency(cln1);
+    EXPECT_EQ(cln1_snaps, vsn1);
+    auto vsn2 = direct->mapper->get_snaps_check_consistency(cln2);
+    EXPECT_EQ(cln2_snaps, vsn2);
+    auto vsnh = direct->mapper->get_snaps_check_consistency(head);
+    EXPECT_EQ(head_snaps, vsnh);
+  }
+
+  // corrupt the SNA_ entry for cln1
+  direct->shorten_mapping_key(300, cln1);
+  {
+    auto vsnh = direct->mapper->get_snaps_check_consistency(head);
+    EXPECT_EQ(head_snaps, vsnh);
+    auto vsn1 = direct->mapper->get_snaps(cln1);
+    EXPECT_EQ(cln1_snaps, vsn1);
+    auto osn1 = direct->mapper->get_snaps_check_consistency(cln1);
+    EXPECT_NE(cln1_snaps, osn1);
+    auto vsn2 = direct->mapper->get_snaps_check_consistency(cln2);
+    EXPECT_EQ(cln2_snaps, vsn2);
+  }
+}
+
+///\todo test the case of a corrupted OBJ_ entry