common/scrub,osd/scrub: minor cleanups to ScrubStore

Including:
- introducing 'no out param' encode() for the inconsistent wrappers;
- renaming the ambiguous 'empty()' to 'is_empty()';
- removing unused code;
- a few other minor cleanups.

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

diff --git a/src/common/scrub_types.cc b/src/common/scrub_types.cc
index b03a3cab70c8427a3ee4de8e5233dd0852232a40..4b4d191e09c390392407ee214f3f8efdcc765354 100644 (file)
--- a/src/common/scrub_types.cc
+++ b/src/common/scrub_types.cc
@@ -161,6 +161,13 @@ void inconsistent_obj_wrapper::encode(bufferlist& bl) const
   ENCODE_FINISH(bl);
 }
 
+bufferlist inconsistent_obj_wrapper::encode() const
+{
+  bufferlist bl;
+  encode(bl);
+  return bl;
+}
+
 void inconsistent_obj_wrapper::decode(bufferlist::const_iterator& bp)
 {
   DECODE_START(2, bp);
@@ -240,6 +247,13 @@ void inconsistent_snapset_wrapper::encode(bufferlist& bl) const
   ENCODE_FINISH(bl);
 }
 
+bufferlist inconsistent_snapset_wrapper::encode() const
+{
+  bufferlist bl;
+  encode(bl);
+  return bl;
+}
+
 void inconsistent_snapset_wrapper::decode(bufferlist::const_iterator& bp)
 {
   DECODE_START(2, bp);

diff --git a/src/common/scrub_types.h b/src/common/scrub_types.h
index dd206f56f60353366c1ab5db1c5e31642f51d9d2..d86fc12b6c8cff90998fb209629757eab2fb5a49 100644 (file)
--- a/src/common/scrub_types.h
+++ b/src/common/scrub_types.h
@@ -152,6 +152,7 @@ struct inconsistent_obj_wrapper : librados::inconsistent_obj_t {
                        const pg_shard_t &primary);
   void set_version(uint64_t ver) { version = ver; }
   void encode(ceph::buffer::list& bl) const;
+  ceph::buffer::list encode() const;
   void decode(ceph::buffer::list::const_iterator& bp);
 };
 
@@ -181,6 +182,7 @@ struct inconsistent_snapset_wrapper : public librados::inconsistent_snapset_t {
   void set_size_mismatch();
 
   void encode(ceph::buffer::list& bl) const;
+  ceph::buffer::list encode() const;
   void decode(ceph::buffer::list::const_iterator& bp);
 };
 

diff --git a/src/osd/scrubber/ScrubStore.cc b/src/osd/scrubber/ScrubStore.cc
index dd141d1c38ca42a1d6aa27d032dfaa20a7ebe90b..033ea6b24dfd48c332f4d3d96a327e27f4328093 100644 (file)
--- a/src/osd/scrubber/ScrubStore.cc
+++ b/src/osd/scrubber/ScrubStore.cc
@@ -15,21 +15,9 @@ using std::vector;
 using ceph::bufferlist;
 
 namespace {
-ghobject_t make_scrub_object(const spg_t& pgid)
-{
-  ostringstream ss;
-  ss << "scrub_" << pgid;
-  return pgid.make_temp_ghobject(ss.str());
-}
-
 string first_object_key(int64_t pool)
 {
-  auto hoid = hobject_t(object_t(),
-                       "",
-                       0,
-                       0x00000000,
-                       pool,
-                       "");
+  auto hoid = hobject_t(object_t(), "", CEPH_NOSNAP, 0x00000000, pool, "");
   hoid.build_hash_cache();
   return "SCRUB_OBJ_" + hoid.to_str();
 }
@@ -49,12 +37,7 @@ string to_object_key(int64_t pool, const librados::object_id_t& oid)
 
 string last_object_key(int64_t pool)
 {
-  auto hoid = hobject_t(object_t(),
-                       "",
-                       0,
-                       0xffffffff,
-                       pool,
-                       "");
+  auto hoid = hobject_t(object_t(), "", CEPH_NOSNAP, 0xffffffff, pool, "");
   hoid.build_hash_cache();
   return "SCRUB_OBJ_" + hoid.to_str();
 }
@@ -62,14 +45,9 @@ string last_object_key(int64_t pool)
 string first_snap_key(int64_t pool)
 {
   // scrub object is per spg_t object, so we can misuse the hash (pg.seed) for
-  // the representing the minimal and maximum keys. and this relies on how
+  // representing the minimal and maximum keys. and this relies on how
   // hobject_t::to_str() works: hex(pool).hex(revhash).
-  auto hoid = hobject_t(object_t(),
-                       "",
-                       0,
-                       0x00000000,
-                       pool,
-                       "");
+  auto hoid = hobject_t(object_t(), "", 0, 0x00000000, pool, "");
   hoid.build_hash_cache();
   return "SCRUB_SS_" + hoid.to_str();
 }
@@ -88,12 +66,7 @@ string to_snap_key(int64_t pool, const librados::object_id_t& oid)
 
 string last_snap_key(int64_t pool)
 {
-  auto hoid = hobject_t(object_t(),
-                       "",
-                       0,
-                       0xffffffff,
-                       pool,
-                       "");
+  auto hoid = hobject_t(object_t(), "", 0, 0xffffffff, pool, "");
   hoid.build_hash_cache();
   return "SCRUB_SS_" + hoid.to_str();
 }
@@ -168,22 +141,23 @@ void Store::add_error(int64_t pool, const inconsistent_snapset_wrapper& e)
   add_snap_error(pool, e);
 }
 
+
 void Store::add_snap_error(int64_t pool, const inconsistent_snapset_wrapper& e)
 {
-  bufferlist bl;
-  e.encode(bl);
-  errors_db->results[to_snap_key(pool, e.object)] = bl;
+  errors_db->results[to_snap_key(pool, e.object)] = e.encode();
 }
 
-bool Store::empty() const
+
+bool Store::is_empty() const
 {
-  return errors_db->results.empty();
+  return !errors_db || errors_db->results.empty();
 }
 
+
 void Store::flush(ObjectStore::Transaction* t)
 {
   if (t) {
-    OSDriver::OSTransaction txn = errors_db->driver.get_transaction(t);
+    auto txn = errors_db->driver.get_transaction(t);
     errors_db->backend.set_keys(errors_db->results, &txn);
   }
   errors_db->results.clear();
@@ -234,10 +208,11 @@ void Store::cleanup(ObjectStore::Transaction* t)
     t->remove(coll, errors_db->errors_hoid);
 }
 
-std::vector<bufferlist>
-Store::get_snap_errors(int64_t pool,
-                      const librados::object_id_t& start,
-                      uint64_t max_return) const
+
+std::vector<bufferlist> Store::get_snap_errors(
+    int64_t pool,
+    const librados::object_id_t& start,
+    uint64_t max_return) const
 {
   const string begin = (start.name.empty() ?
                        first_snap_key(pool) : to_snap_key(pool, start));
@@ -272,6 +247,8 @@ Store::get_errors(const string& begin,
     errors.push_back(next.second);
     max_return--;
   }
+
+  dout(10) << fmt::format("{} errors reported", errors.size()) << dendl;
   return errors;
 }
 

diff --git a/src/osd/scrubber/ScrubStore.h b/src/osd/scrubber/ScrubStore.h
index 7d590d2d1915e6ed56fd0a240b3f38c0376d727d..9eb77ab667db709ae7e57f430471c696223fe8b8 100644 (file)
--- a/src/osd/scrubber/ScrubStore.h
+++ b/src/osd/scrubber/ScrubStore.h
@@ -1,8 +1,6 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
-
-#ifndef CEPH_SCRUB_RESULT_H
-#define CEPH_SCRUB_RESULT_H
+#pragma once
 
 #include "common/map_cacher.hpp"
 #include "osd/osd_types_fmt.h"
@@ -39,7 +37,7 @@ class Store {
   void add_error(int64_t pool, const inconsistent_obj_wrapper& e);
   void add_error(int64_t pool, const inconsistent_snapset_wrapper& e);
 
-  bool empty() const;
+  [[nodiscard]] bool is_empty() const;
   void flush(ObjectStore::Transaction*);
 
   /// remove both shallow and deep errors DBs. Called on interval.
@@ -101,11 +99,6 @@ class Store {
       MapCacher::MapCacher<std::string, ceph::buffer::list>::PosAndData;
   using ExpCacherPosData = tl::expected<CacherPosData, int>;
 
-
-  std::vector<ceph::buffer::list> get_errors(const std::string& start,
-                                            const std::string& end,
-                                            uint64_t max_return) const;
-
   /// access to the owning Scrubber object, for logging mostly
   PgScrubber& m_scrubber;
 
@@ -124,6 +117,11 @@ class Store {
   mutable std::optional<at_level_t> errors_db;
   // not yet: mutable std::optional<at_level_t> deep_db;
 
+  std::vector<ceph::buffer::list> get_errors(
+      const std::string& start,
+      const std::string& end,
+      uint64_t max_return) const;
+
   /**
    * Clear the DB of errors at a specific scrub level by performing an
    * omap_clear() on the DB object, and resetting the MapCacher.
@@ -135,5 +133,3 @@ class Store {
 
 };
 }  // namespace Scrub
-
-#endif // CEPH_SCRUB_RESULT_H