From: Ronen Friedman <rfriedma@redhat.com>
Date: Sat, 13 Nov 2021 14:12:57 +0000 (+0000)
Subject: osd/scrub: removing some safeguards against out-of-order scrub calls
X-Git-Tag: v17.1.0~412^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=1a528a1222265b337ec53053d4a6f5d5d917fd7d;p=ceph.git

osd/scrub: removing some safeguards against out-of-order scrub calls

as m_scrubber is created in the PG ctor, and only deleted in the dtor,
we should not encounter a scrub event dispatched to the PG without
having a valid scrubber sub-object.

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

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index af667a091302f..d3a17f5ce3b7e 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -814,13 +814,12 @@ void PG::publish_stats_to_osd()
   if (!is_primary())
     return;
 
-  if (m_scrubber) {
-    recovery_state.update_stats_wo_resched(
-      [scrubber = m_scrubber.get()](pg_history_t& hist,
-                                    pg_stat_t& info) mutable -> void {
-        info.scrub_sched_status = scrubber->get_schedule();
-      });
-  }
+  ceph_assert(m_scrubber);
+  recovery_state.update_stats_wo_resched(
+    [scrubber = m_scrubber.get()](pg_history_t& hist,
+                                  pg_stat_t& info) mutable -> void {
+      info.scrub_sched_status = scrubber->get_schedule();
+    });
 
   std::lock_guard l{pg_stats_publish_lock};
   auto stats =
@@ -1333,6 +1332,7 @@ Scrub::schedule_result_t PG::sched_scrub()
 	  << (is_active() ? ") <active>" : ") <not-active>")
 	  << (is_clean() ? " <clean>" : " <not-clean>") << dendl;
   ceph_assert(ceph_mutex_is_locked(_lock));
+  ceph_assert(m_scrubber);
 
   if (is_scrub_queued_or_active()) {
     return Scrub::schedule_result_t::already_started;
@@ -1546,9 +1546,8 @@ void PG::on_info_history_change()
 {
   dout(20) << __func__ << " for a " << (is_primary() ? "Primary" : "non-primary") <<dendl;
 
-  if (m_scrubber) {
-    m_scrubber->on_maybe_registration_change(m_planned_scrub);
-  }
+  ceph_assert(m_scrubber);
+  m_scrubber->on_maybe_registration_change(m_planned_scrub);
 }
 
 void PG::reschedule_scrub()
@@ -1556,7 +1555,8 @@ void PG::reschedule_scrub()
   dout(20) << __func__ << " for a " << (is_primary() ? "Primary" : "non-primary") <<dendl;
 
   // we are assuming no change in primary status
-  if (is_primary() && m_scrubber) {
+  if (is_primary()) {
+    ceph_assert(m_scrubber);
     m_scrubber->update_scrub_job(m_planned_scrub);
   }
 }
@@ -1564,18 +1564,17 @@ void PG::reschedule_scrub()
 void PG::on_primary_status_change(bool was_primary, bool now_primary)
 {
   // make sure we have a working scrubber when becoming a primary
-  ceph_assert(m_scrubber || !now_primary);
 
-  if ((was_primary != now_primary) && m_scrubber) {
+  if (was_primary != now_primary) {
+    ceph_assert(m_scrubber);
     m_scrubber->on_primary_change(m_planned_scrub);
   }
 }
 
 void PG::scrub_requested(scrub_level_t scrub_level, scrub_type_t scrub_type)
 {
-  if (m_scrubber) {
-    m_scrubber->scrub_requested(scrub_level, scrub_type, m_planned_scrub);
-  }
+  ceph_assert(m_scrubber);
+  m_scrubber->scrub_requested(scrub_level, scrub_type, m_planned_scrub);
 }
 
 void PG::clear_ready_to_merge() {
diff --git a/src/osd/PG.h b/src/osd/PG.h
index 9d4872d926e9a..6bcd0dca13194 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -181,6 +181,8 @@ public:
   const pg_shard_t pg_whoami;
   const spg_t pg_id;
 
+  /// the 'scrubber'. Will be allocated in the derivative (PrimaryLogPG) ctor,
+  /// and be removed only in the PrimaryLogPG destructor.
   std::unique_ptr<ScrubPgIF> m_scrubber;
 
   /// flags detailing scheduling/operation characteristics of the next scrub 
diff --git a/src/osd/scrubber/PrimaryLogScrub.cc b/src/osd/scrubber/PrimaryLogScrub.cc
index 50d62338ba803..b3d2f098ce6a5 100644
--- a/src/osd/scrubber/PrimaryLogScrub.cc
+++ b/src/osd/scrubber/PrimaryLogScrub.cc
@@ -549,7 +549,7 @@ void PrimaryLogScrub::scrub_snapshot_metadata(ScrubMap& scrubmap,
     ++num_digest_updates_pending;
     ctx->register_on_success([this]() {
       dout(20) << "updating scrub digest " << num_digest_updates_pending << dendl;
-      if (--num_digest_updates_pending <= 0) {
+      if ((num_digest_updates_pending >= 1) && (--num_digest_updates_pending == 0)) {
 	m_osds->queue_scrub_digest_update(m_pl_pg, m_pl_pg->is_scrub_blocking_ops());
       }
     });