osd/scrub: scrubbing schedule - minor related cleanups

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

.gitignore

@@ -82,3 +82,4 @@ GTAGS
 
 # Python building things where it shouldn't
 /src/python-common/build/
+.cache

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());

src/mon/PGMap.h

@@ -460,10 +460,10 @@ class PGMap : public PGMapDigest {
   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;

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

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,

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);

src/osd/PrimaryLogPG.h

@@ -196,7 +196,7 @@ class PrimaryLogPG : public PG, public PGBackend::Listener {
   friend struct CopyFromFinisher;
   friend class PromoteCallback;
   friend struct PromoteFinisher;
-  friend class C_gather;
+  friend struct C_gather;
 
   struct ProxyReadOp {
     OpRequestRef op;

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);

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;

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

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();
 }
 

src/osd/scrubber/pg_scrubber.h

@@ -671,8 +671,8 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener {
    */
   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 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener {
    */
   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; }