osd/scrub: move initiate_a_scrub() to OsdScrub

Scrub initiation is now fully owned by OsdScrub.

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

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index efde588d939ebb06a059f3d40e86530382661f74..4ba3d7681194835e2ced856c39f6c7c392628e06 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -245,21 +245,6 @@ public:
  public:
   OsdScrub& get_scrub_services() { return m_osd_scrub; }
 
-  /**
-   * A callback used by the ScrubQueue object to initiate a scrub on a specific PG.
-   *
-   * The request might fail for multiple reasons, as ScrubQueue cannot by its own
-   * check some of the PG-specific preconditions and those are checked here. See
-   * attempt_t definition.
-   *
-   * @param pgid to scrub
-   * @param allow_requested_repair_only
-   * @return a Scrub::attempt_t detailing either a success, or the failure reason.
-   */
-  Scrub::schedule_result_t initiate_a_scrub(
-    spg_t pgid,
-    bool allow_requested_repair_only) final;
-
   /**
    * locks the named PG, returning an RAII wrapper that unlocks upon
    * destruction.

diff --git a/src/osd/scrubber/osd_scrub.cc b/src/osd/scrubber/osd_scrub.cc
index 6e1923b638f15f8bc9abc5f6b0139069d37c54ab..6c31dce802b4bbd7d88af0f62db59881ec3e8b1f 100644 (file)
--- a/src/osd/scrubber/osd_scrub.cc
+++ b/src/osd/scrubber/osd_scrub.cc
@@ -132,7 +132,7 @@ void OsdScrub::initiate_scrub(bool is_recovery_active)
     // we have a candidate to scrub. But we may fail when trying to initiate that
     // scrub. For some failures - we can continue with the next candidate. For
     // others - we should stop trying to scrub at this tick.
-    res = m_osd_svc.initiate_a_scrub(
+    res = initiate_a_scrub(
        candidate, env_restrictions->allow_requested_repair_only);
     switch (res) {
       case schedule_result_t::scrub_initiated:
@@ -224,53 +224,53 @@ std::optional<Scrub::OSDRestrictions> OsdScrub::restrictions_on_scrubbing(
 }
 
 
-// ////////////////////////////////////////////////////////////////////////// //
-// scrub initiation - OSD code temporarily moved here from OSD.cc
-
-// temporary dout() support for OSD members:
-static ostream& _prefix(std::ostream* _dout, int whoami, epoch_t epoch) {
-  return *_dout << "osd." << whoami << " " << epoch << " ";
-}
-#undef dout_prefix
-#define dout_prefix _prefix(_dout, whoami, get_osdmap_epoch())
-
-
-schedule_result_t OSDService::initiate_a_scrub(spg_t pgid,
-                                                     bool allow_requested_repair_only)
+Scrub::schedule_result_t OsdScrub::initiate_a_scrub(
+    spg_t pgid,
+    bool allow_requested_repair_only)
 {
-  dout(20) << __func__ << " trying " << pgid << dendl;
+  dout(20) << fmt::format("trying pg[{}]", pgid) << dendl;
 
-  // we have a candidate to scrub. We need some PG information to know if scrubbing is
-  // allowed
+  // we have a candidate to scrub. We need some PG information to
+  // know if scrubbing is allowed
 
-  PGRef pg = osd->lookup_lock_pg(pgid);
-  if (!pg) {
-    // the PG was dequeued in the short timespan between creating the candidates list
-    // (collect_ripe_jobs()) and here
-    dout(5) << __func__ << " pg  " << pgid << " not found" << dendl;
-    return schedule_result_t::no_such_pg;
+  auto locked_pg = m_osd_svc.get_locked_pg(pgid);
+  if (!locked_pg) {
+    // the PG was dequeued in the short timespan between creating the
+    // candidates list (ready_to_scrub()) and here
+    dout(5) << fmt::format("pg[{}] not found", pgid) << dendl;
+    return Scrub::schedule_result_t::no_such_pg;
   }
 
-  // This has already started, so go on to the next scrub job
-  if (pg->is_scrub_queued_or_active()) {
-    pg->unlock();
-    dout(20) << __func__ << ": already in progress pgid " << pgid << dendl;
-    return schedule_result_t::already_started;
+  // This one is already scrubbing, so go on to the next scrub job
+  if (locked_pg->pg()->is_scrub_queued_or_active()) {
+    dout(10) << fmt::format("pg[{}]: scrub already in progress", pgid) << dendl;
+    return Scrub::schedule_result_t::already_started;
   }
   // Skip other kinds of scrubbing if only explicitly requested repairing is allowed
-  if (allow_requested_repair_only && !pg->get_planned_scrub().must_repair) {
-    pg->unlock();
-    dout(10) << __func__ << " skip " << pgid
-            << " because repairing is not explicitly requested on it" << dendl;
-    return schedule_result_t::preconditions;
+  if (allow_requested_repair_only &&
+      !locked_pg->pg()->get_planned_scrub().must_repair) {
+    dout(10) << fmt::format(
+                   "skipping pg[{}] as repairing was not explicitly "
+                   "requested for that pg",
+                   pgid)
+            << dendl;
+    return Scrub::schedule_result_t::preconditions;
   }
 
-  auto scrub_attempt = pg->sched_scrub();
-  pg->unlock();
-  return scrub_attempt;
+  return locked_pg->pg()->sched_scrub();
 }
 
 
+// ////////////////////////////////////////////////////////////////////////// //
+// scrub initiation - OSD code temporarily moved here from OSD.cc
+
+// temporary dout() support for OSD members:
+static ostream& _prefix(std::ostream* _dout, int whoami, epoch_t epoch) {
+  return *_dout << "osd." << whoami << " " << epoch << " ";
+}
+#undef dout_prefix
+#define dout_prefix _prefix(_dout, whoami, get_osdmap_epoch())
+
 void OSD::resched_all_scrubs()
 {
   dout(10) << __func__ << ": start" << dendl;

diff --git a/src/osd/scrubber/osd_scrub.h b/src/osd/scrubber/osd_scrub.h
index 701a5d7f25f32bdfe9b7cbb20efc1f888065db68..75ad02fd60714938f0632fc36e020c0e43600450 100644 (file)
--- a/src/osd/scrubber/osd_scrub.h
+++ b/src/osd/scrubber/osd_scrub.h
@@ -178,6 +178,19 @@ class OsdScrub {
       bool is_recovery_active,
       utime_t scrub_clock_now) const;
 
+  /**
+   * initiate a scrub on a specific PG
+   * The PG is locked, enabling us to query its state. Specifically, we
+   * verify that the PG is not already scrubbing, and that
+   * a possible 'allow requested repair only' condition is not in conflict.
+   *
+   * \returns a schedule_result_t object, indicating whether the scrub was
+   *          initiated, and if not - why.
+   */
+  Scrub::schedule_result_t initiate_a_scrub(
+      spg_t pgid,
+      bool allow_requested_repair_only);
+
   /// resource reservation management
   Scrub::ScrubResources m_resource_bookkeeper;
 

diff --git a/src/osd/scrubber/osd_scrub_sched.h b/src/osd/scrubber/osd_scrub_sched.h
index e0904a653e9f5789811f1ccda7ce2dfc5ef27fb9..1c73fd6364a900481e4538b5196f92a42c68463c 100644 (file)
--- a/src/osd/scrubber/osd_scrub_sched.h
+++ b/src/osd/scrubber/osd_scrub_sched.h
@@ -6,9 +6,13 @@
 /*
 ┌───────────────────────┐
 │ OSD                   │
-│ OSDService           ─┼───┐
-│                       │   │
-│                       │   │
+│ OSDService            │
+│                       │
+│ ┌─────────────────────│
+│ │                     │
+│ │   OsdScrub          │
+│ │                    ─┼───┐
+│ │                     │   │
 └───────────────────────┘   │   Ownes & uses the following
                             │   ScrubQueue interfaces:
                             │
@@ -21,9 +25,6 @@
                             │
                             │
                             │
-                            │
-                            │
-                            │
  ScrubQueue                 │
 ┌───────────────────────────▼────────────┐
 │                                        │
@@ -139,21 +140,6 @@ class ScrubSchedListener {
    */
   virtual std::optional<PGLockWrapper> get_locked_pg(spg_t pgid) = 0;
 
-  /**
-   * A callback used by the ScrubQueue object to initiate a scrub on a specific
-   * PG.
-   *
-   * The request might fail for multiple reasons, as ScrubQueue cannot by its
-   * own check some of the PG-specific preconditions and those are checked here.
-   * See attempt_t definition.
-   *
-   * @return a Scrub::attempt_t detailing either a success, or the failure
-   * reason.
-   */
-  virtual schedule_result_t initiate_a_scrub(
-    spg_t pgid,
-    bool allow_requested_repair_only) = 0;
-
   virtual ~ScrubSchedListener() {}
 };
 
@@ -177,7 +163,6 @@ class ScrubQueue {
 
   friend class TestOSDScrub;
   friend class ScrubSchedTestWrapper; ///< unit-tests structure
-  friend class OsdScrub; ///< transitory - fixed in followup commits
   using sched_params_t = Scrub::sched_params_t;
 
   /**

diff --git a/src/test/osd/test_scrub_sched.cc b/src/test/osd/test_scrub_sched.cc
index 13c72ee6cf1bb5ddef8b91e2a7426228617d3a4a..da8fb3bb5e3ec80a52a3ade906d8f95ac67c5f78 100644 (file)
--- a/src/test/osd/test_scrub_sched.cc
+++ b/src/test/osd/test_scrub_sched.cc
@@ -99,17 +99,6 @@ class FakeOsd : public Scrub::ScrubSchedListener {
 
   int get_nodeid() const final { return m_osd_num; }
 
-  schedule_result_t initiate_a_scrub(spg_t pgid,
-                                    bool allow_requested_repair_only) final
-  {
-    std::ignore = allow_requested_repair_only;
-    auto res = m_next_response.find(pgid);
-    if (res == m_next_response.end()) {
-      return schedule_result_t::no_such_pg;
-    }
-    return m_next_response[pgid];
-  }
-
   void set_initiation_response(spg_t pgid, schedule_result_t result)
   {
     m_next_response[pgid] = result;