From 5ec8f6863bc804a6bce2cf16e819b3fa452a8e4c Mon Sep 17 00:00:00 2001
From: "sjust@redhat.com" <sjust@redhat.com>
Date: Wed, 27 Mar 2019 16:13:23 -0700
Subject: [PATCH] osd/: rephrase [un]reg_next_scrub as on_info_history_change

As PeeringState won't be as careful about usage of
on_info_history_change, add a scrub_registered flag
to protect against double-(un)registers.

Signed-off-by: sjust@redhat.com <sjust@redhat.com>
---
 src/osd/PG.cc           | 19 ++++++++++++++++++-
 src/osd/PG.h            |  8 ++++++--
 src/osd/PeeringState.cc | 22 ++++++++--------------
 src/osd/PeeringState.h  |  4 ++--
 4 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 98d6a546e7a..f6f46361035 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -4028,16 +4028,33 @@ void PG::reg_next_scrub()
 					       must);
   dout(10) << __func__ << " pg " << pg_id << " register next scrub, scrub time "
       << scrubber.scrub_reg_stamp << ", must = " << (int)must << dendl;
+  scrub_registered = true;
 }
 
 void PG::unreg_next_scrub()
 {
-  if (is_primary()) {
+  if (scrub_registered) {
     osd->unreg_pg_scrub(info.pgid, scrubber.scrub_reg_stamp);
     scrubber.scrub_reg_stamp = utime_t();
+    scrub_registered = false;
   }
 }
 
+void PG::on_info_history_change()
+{
+  unreg_next_scrub();
+  reg_next_scrub();
+}
+
+void PG::scrub_requested(bool deep, bool repair)
+{
+  unreg_next_scrub();
+  scrubber.must_scrub = true;
+  scrubber.must_deep_scrub = deep || repair;
+  scrubber.must_repair = repair;
+  reg_next_scrub();
+}
+
 void PG::clear_ready_to_merge() {
   osd->clear_ready_to_merge(this);
 }
diff --git a/src/osd/PG.h b/src/osd/PG.h
index 003050f0731..e5853066a35 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -403,8 +403,8 @@ public:
 
   void scrub(epoch_t queued, ThreadPool::TPHandle &handle);
 
-  void reg_next_scrub() override;
-  void unreg_next_scrub() override;
+  void reg_next_scrub();
+  void unreg_next_scrub();
 
   void clear_ready_to_merge() override;
 
@@ -420,6 +420,9 @@ public:
   void on_pool_change() override;
   virtual void plpg_on_pool_change() = 0;
 
+  void on_info_history_change() override;
+
+  void scrub_requested(bool deep, bool repair) override;
 
   void clear_publish_stats() override;
   void clear_primary_state() override;
@@ -617,6 +620,7 @@ protected:
   /* You should not use these items without taking their respective queue locks
    * (if they have one) */
   xlist<PG*>::item stat_queue_item;
+  bool scrub_registered = false;
   bool scrub_queued;
   bool recovery_queued;
 
diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index 0f035a40139..2f4f0b21bac 100644
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -168,7 +168,6 @@ void PeeringState::check_recovery_sources(const OSDMapRef& osdmap)
 
 void PeeringState::update_history(const pg_history_t& new_history)
 {
-  pl->unreg_next_scrub();
   if (info.history.merge(new_history)) {
     psdout(20) << __func__ << " advanced history from " << new_history << dendl;
     dirty_info = true;
@@ -178,7 +177,7 @@ void PeeringState::update_history(const pg_history_t& new_history)
       dirty_big_info = true;
     }
   }
-  pl->reg_next_scrub();
+  pl->on_info_history_change();
 }
 
 void PeeringState::purge_strays()
@@ -470,7 +469,6 @@ void PeeringState::start_peering_interval(
   vector<int> oldacting, oldup;
   int oldrole = get_role();
 
-  pl->unreg_next_scrub();
   if (is_primary()) {
     pl->clear_ready_to_merge();
   }
@@ -573,7 +571,7 @@ void PeeringState::start_peering_interval(
   }
 
   pl->on_new_interval();
-  pl->reg_next_scrub();
+  pl->on_info_history_change();
 
   psdout(1) << __func__ << " up " << oldup << " -> " << up
 	    << ", acting " << oldacting << " -> " << acting
@@ -1082,14 +1080,11 @@ boost::statechart::result PeeringState::Primary::react(
 boost::statechart::result PeeringState::Primary::react(
   const RequestScrub& evt)
 {
-  PG *pg = context< PeeringMachine >().pg;
-  if (pg->is_primary()) {
-    pg->unreg_next_scrub();
-    pg->scrubber.must_scrub = true;
-    pg->scrubber.must_deep_scrub = evt.deep || evt.repair;
-    pg->scrubber.must_repair = evt.repair;
-    pg->reg_next_scrub();
-    ldout(pg->cct,10) << "marking for scrub" << dendl;
+  PeeringListener *pl = context< PeeringMachine >().pl;
+  PeeringState *ps = context< PeeringMachine >().state;
+  if (ps->is_primary()) {
+    pl->scrub_requested(evt.deep, evt.repair);
+    psdout(10) << "marking for scrub" << dendl;
   }
   return discard_event();
 }
@@ -2685,9 +2680,8 @@ boost::statechart::result PeeringState::Stray::react(const MLogRec& logevt)
   ObjectStore::Transaction* t = context<PeeringMachine>().get_cur_transaction();
   if (msg->info.last_backfill == hobject_t()) {
     // restart backfill
-    pg->unreg_next_scrub();
     pg->info = msg->info;
-    pg->reg_next_scrub();
+    pg->on_info_history_change();
     pg->dirty_info = true;
     pg->dirty_big_info = true;  // maybe.
 
diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
index 249ac29553b..7c78db7aab3 100644
--- a/src/osd/PeeringState.h
+++ b/src/osd/PeeringState.h
@@ -66,8 +66,8 @@ public:
       ObjectStore::Transaction &t) = 0;
     virtual void update_heartbeat_peers(set<int> peers) = 0;
 
-    virtual void reg_next_scrub() = 0;
-    virtual void unreg_next_scrub() = 0;
+    virtual void on_info_history_change() = 0;
+    virtual void scrub_requested(bool deep, bool repair) = 0;
 
     virtual void send_cluster_message(int osd, Message *m, epoch_t epoch) = 0;
 
-- 
2.39.5