osd/scrub: scrubbing schedule - minor related cleanups

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

diff --git a/.gitignore b/.gitignore
index 4c9ff3b26deb60b1af0cd962379bcb1f3d12e12c..b01aef839bef6902a88811f19a56e5f10753f82d 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -82,3 +82,4 @@ GTAGS
 
 # Python building things where it shouldn't
 /src/python-common/build/
+.cache

diff --git a/src/mon/PGMap.cc b/src/mon/PGMap.cc
index d773c2fd3861fecbbdb13ca9b95345dd0c487ea6..ea90270a9ef8fc82bf880bdef7600f9303136ff4 100644 (file)
--- a/src/mon/PGMap.cc
+++ b/src/mon/PGMap.cc
@@ -1608,7 +1608,7 @@ void PGMap::dump_osd_stats(ceph::Formatter *f, bool with_net) const
 void PGMap::dump_osd_ping_times(ceph::Formatter *f) const
 {
   f->open_array_section("osd_ping_times");
-  for (auto& [osd, stat] : osd_stat) {
+  for (const auto& [osd, stat] : osd_stat) {
     f->open_object_section("osd_ping_time");
     f->dump_int("osd", osd);
     stat.dump_ping_time(f);
@@ -1617,10 +1617,11 @@ void PGMap::dump_osd_ping_times(ceph::Formatter *f) const
   f->close_section();
 }
 
+// note: dump_pg_stats_plain() is static
 void PGMap::dump_pg_stats_plain(
   ostream& ss,
   const mempool::pgmap::unordered_map<pg_t, pg_stat_t>& pg_stats,
-  bool brief) const
+  bool brief)
 {
   TextTable tab;
 
@@ -1661,11 +1662,9 @@ void PGMap::dump_pg_stats_plain(
     tab.define_column("SCRUB_SCHEDULING", TextTable::LEFT, TextTable::LEFT);
   }
 
-  for (auto i = pg_stats.begin();
-       i != pg_stats.end(); ++i) {
-    const pg_stat_t &st(i->second);
+  for (const auto& [pg, st] : pg_stats) {
     if (brief) {
-      tab << i->first
+      tab << pg
           << pg_state_string(st.state)
           << st.up
           << st.up_primary
@@ -1676,7 +1675,7 @@ void PGMap::dump_pg_stats_plain(
       ostringstream reported;
       reported << st.reported_epoch << ":" << st.reported_seq;
 
-      tab << i->first
+      tab << pg
           << st.stats.sum.num_objects
           << st.stats.sum.num_objects_missing_on_primary
           << st.stats.sum.num_objects_degraded
@@ -1700,7 +1699,8 @@ void PGMap::dump_pg_stats_plain(
           << st.last_deep_scrub
           << st.last_deep_scrub_stamp
           << st.snaptrimq_len
-          << st.scrub_duration
+          << st.last_scrub_duration
+          << st.dump_scrub_schedule()
           << TextTable::endrow;
     }
   }
@@ -2271,7 +2271,7 @@ void PGMap::dump_filtered_pg_stats(ostream& ss, set<pg_t>& pgs) const
 void PGMap::dump_pool_stats_and_io_rate(int64_t poolid, const OSDMap &osd_map,
                                         ceph::Formatter *f,
                                         stringstream *rs) const {
-  string pool_name = osd_map.get_pool_name(poolid);
+  const string& pool_name = osd_map.get_pool_name(poolid);
   if (f) {
     f->open_object_section("pool");
     f->dump_string("pool_name", pool_name.c_str());

diff --git a/src/mon/PGMap.h b/src/mon/PGMap.h
index c592f338ecb33cb6bb5b5462304ab015110f994a..d55d2397c1b9885fc97a1654b69975cf9f9b1bdb 100644 (file)
--- a/src/mon/PGMap.h
+++ b/src/mon/PGMap.h
@@ -460,10 +460,10 @@ public:
   void dump_pool_stats_and_io_rate(int64_t poolid, const OSDMap &osd_map, ceph::Formatter *f,
                                   std::stringstream *ss) const;
 
-  void dump_pg_stats_plain(
+  static void dump_pg_stats_plain(
     std::ostream& ss,
     const mempool::pgmap::unordered_map<pg_t, pg_stat_t>& pg_stats,
-    bool brief) const;
+    bool brief);
   void get_stuck_stats(
     int types, const utime_t cutoff,
     mempool::pgmap::unordered_map<pg_t, pg_stat_t>& stuck_pgs) const;

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 67664389c879b266b7b776cba3dbb384d8ba1e01..ca1dc472b2197a53d96a9cabaa76c519eda3dc8d 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7592,6 +7592,7 @@ void OSD::resched_all_scrubs()
 
 MPGStats* OSD::collect_pg_stats()
 {
+  dout(15) << __func__ << dendl;
   // This implementation unconditionally sends every is_primary PG's
   // stats every time we're called.  This has equivalent cost to the
   // previous implementation's worst case where all PGs are busy and

diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index ae9344ec4ab49f2b13e99f70e2e50bb68d5d9634..b5e6b69573e4ca2827028eecee85fad2db1da472 100644 (file)
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -3810,7 +3810,7 @@ std::optional<pg_stat_t> PeeringState::prepare_stats_for_publish(
   if (info.stats.state != state) {
     info.stats.last_change = now;
     // Optimistic estimation, if we just find out an inactive PG,
-    // assumt it is active till now.
+    // assume it is active till now.
     if (!(state & PG_STATE_ACTIVE) &&
        (info.stats.state & PG_STATE_ACTIVE))
       info.stats.last_active = now;
@@ -3990,7 +3990,6 @@ void PeeringState::update_stats_wo_resched(
   f(info.history, info.stats);
 }
 
-
 bool PeeringState::append_log_entries_update_missing(
   const mempool::osd_pglog::list<pg_log_entry_t> &entries,
   ObjectStore::Transaction &t, std::optional<eversion_t> trim_to,

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index 6c2343ba34f4fe17099cc3cae1ca0fa66c6d0dd9..298716a1a0c7137762be8afe8ae873c1e4209749 100644 (file)
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -947,7 +947,7 @@ PrimaryLogPG::get_pgls_filter(bufferlist::const_iterator& iter)
   if (type.compare("plain") == 0) {
     filter = std::make_unique<PGLSPlainFilter>();
   } else {
-    std::size_t dot = type.find(".");
+    std::size_t dot = type.find('.');
     if (dot == std::string::npos || dot == 0 || dot == type.size() - 1) {
       return { -EINVAL, nullptr };
     }
@@ -1179,7 +1179,7 @@ void PrimaryLogPG::do_command(
       if (deep) {
         set_last_deep_scrub_stamp(stamp);
       } else {
-        set_last_scrub_stamp(stamp);
+        set_last_scrub_stamp(stamp); // also for 'deep', as we use this value to order scrubs
       }
       f->open_object_section("result");
       f->dump_bool("deep", deep);

diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h
index 23ecdffda6396f7aa1df9d26dd32ff1aa5bcfa72..b92c46bf4a9265d11af87de36426a5d84fa1b1ab 100644 (file)
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -196,7 +196,7 @@ public:
   friend struct CopyFromFinisher;
   friend class PromoteCallback;
   friend struct PromoteFinisher;
-  friend class C_gather;
+  friend struct C_gather;
   
   struct ProxyReadOp {
     OpRequestRef op;

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index 9e260de9426ea439c5044c5bb2506fb51df745b9..d439cdb2e8d71d1709abbd7e3f9bd2ff8c660785 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -6397,8 +6397,9 @@ void object_info_t::dump(Formatter *f) const
   f->dump_unsigned("lost", (int)is_lost());
   vector<string> sv = get_flag_vector(flags);
   f->open_array_section("flags");
-  for (auto str: sv)
+  for (const auto& str: sv) {
     f->dump_string("flags", str);
+  }
   f->close_section();
   f->dump_unsigned("truncate_seq", truncate_seq);
   f->dump_unsigned("truncate_size", truncate_size);

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index 1bb384548d6d6d129b08686cb110b601eda2f73d..65c0cf1f4109d456df3213f6ac04d97aa0c6f8b9 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -2259,7 +2259,7 @@ struct pg_stat_t {
   int32_t acting_primary;
 
   // snaptrimq.size() is 64bit, but let's be serious - anything over 50k is
-  // absurd already, so cap it to 2^32 and save 4 bytes at  the same time
+  // absurd already, so cap it to 2^32 and save 4 bytes at the same time
   uint32_t snaptrimq_len;
 
   pg_scrubbing_status_t scrub_sched_status;

diff --git a/src/osd/scrubber/PrimaryLogScrub.cc b/src/osd/scrubber/PrimaryLogScrub.cc
index 2be7b56f61b03faf6115068a09331f0f0c7a49fe..06dce06f5457f5ba170c1f7a86e72dc44fd36620 100644 (file)
--- a/src/osd/scrubber/PrimaryLogScrub.cc
+++ b/src/osd/scrubber/PrimaryLogScrub.cc
@@ -23,7 +23,6 @@ template <class T> static ostream& _prefix(std::ostream* _dout, T* t)
 }
 
 using namespace Scrub;
-using Scrub::ScrubMachine;
 
 bool PrimaryLogScrub::get_store_errors(const scrub_ls_arg_t& arg,
                                       scrub_ls_result_t& res_inout) const

diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index 9e3256163e90186d300c91f1e1cdf1f2596c0a6e..038d9392c627147ce5f04a09d428c8975fade30e 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.cc
+++ b/src/osd/scrubber/pg_scrubber.cc
@@ -3,6 +3,7 @@
 
 #include "./pg_scrubber.h"  // the '.' notation used to affect clang-format order
 
+#include <cmath>
 #include <iostream>
 #include <vector>
 
@@ -520,7 +521,7 @@ void PgScrubber::update_scrub_job(const requested_scrub_t& request_flags)
 }
 
 ScrubQueue::sched_params_t
-PgScrubber::determine_scrub_time(const requested_scrub_t& request_flags)
+PgScrubber::determine_scrub_time(const requested_scrub_t& request_flags) const
 {
   ScrubQueue::sched_params_t res;
 
@@ -795,7 +796,7 @@ void PgScrubber::add_delayed_scheduling()
   if (m_needs_sleep) {
     double scrub_sleep =
       1000.0 * m_osds->get_scrub_services().scrub_sleep_time(m_flags.required);
-    sleep_time = milliseconds{long(scrub_sleep)};
+    sleep_time = milliseconds{int64_t(scrub_sleep)};
   }
   dout(15) << __func__ << " sleep: " << sleep_time.count() << "ms. needed? "
           << m_needs_sleep << dendl;
@@ -1348,7 +1349,7 @@ void PgScrubber::set_op_parameters(requested_scrub_t& request)
     // not calling update_op_mode_text() yet, as m_is_deep not set yet
   }
 
-  // the publishing here seems to be required for tests synchronization
+  // the publishing here is required for tests synchronization
   m_pg->publish_stats_to_osd();
   m_flags.deep_scrub_on_error = request.deep_scrub_on_error;
 }
@@ -1919,7 +1920,6 @@ void PgScrubber::on_digest_updates()
   }
 }
 
-
 /*
  * note that the flags-set fetched from the PG (m_pg->m_planned_scrub)
  * is cleared once scrubbing starts; Some of the values dumped here are
@@ -2049,7 +2049,7 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const
 
 void PgScrubber::handle_query_state(ceph::Formatter* f)
 {
-  dout(10) << __func__ << dendl;
+  dout(15) << __func__ << dendl;
 
   f->open_object_section("scrub");
   f->dump_stream("scrubber.epoch_start") << m_interval_start;
@@ -2095,7 +2095,8 @@ PgScrubber::PgScrubber(PG* pg)
                                                     m_osds->get_nodeid());
 }
 
-void PgScrubber::set_scrub_begin_time() {
+void PgScrubber::set_scrub_begin_time()
+{
   scrub_begin_stamp = ceph_clock_now();
 }
 

diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h
index 895ff1232daa8ef99c5d59b80407dd420b9bc1ad..8da41a0235373747f3916fdd1835faa725b41158 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.h
+++ b/src/osd/scrubber/pg_scrubber.h
@@ -671,8 +671,8 @@ private:
    */
   void request_rescrubbing(requested_scrub_t& req_flags);
 
-  ScrubQueue::sched_params_t
-  determine_scrub_time(const requested_scrub_t& request_flags);
+  ScrubQueue::sched_params_t determine_scrub_time(
+    const requested_scrub_t& request_flags) const;
 
   void unregister_from_osd();
 
@@ -745,7 +745,7 @@ private:
    */
   class preemption_data_t : public Scrub::preemption_t {
    public:
-    preemption_data_t(PG* pg); // the PG access is used for conf access (and logs)
+    explicit preemption_data_t(PG* pg);        // the PG access is used for conf access (and logs)
 
     [[nodiscard]] bool is_preemptable() const final { return m_preemptable; }