From 2482b4cc91579b4ef5748097f1adacaad91510b1 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Wed, 13 Sep 2017 18:09:35 -0400
Subject: [PATCH] osd/PG: clean up set_force_{recovery,backfill} interface

- update state under pg lock
- clean up PG interface
- log only when we adjust the state

Signed-off-by: Sage Weil <sage@redhat.com>
---
 src/osd/OSD.cc | 43 +++++++------------------------------------
 src/osd/PG.cc  | 45 ++++++++++++++++++++++++++++++++++++++-------
 src/osd/PG.h   |  6 +++---
 3 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 691df0da9436a..f339b6f558f23 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -9049,45 +9049,16 @@ bool OSDService::_recover_now(uint64_t *available_pushes)
 
 void OSDService::adjust_pg_priorities(const vector<PGRef>& pgs, int newflags)
 {
-  if (!pgs.size() || !(newflags & (OFR_BACKFILL | OFR_RECOVERY)))
+  if (!pgs.size() || !(newflags & (OFR_BACKFILL | OFR_RECOVERY))) {
     return;
-  int newstate = 0;
-
-  if (newflags & OFR_BACKFILL) {
-    newstate = PG_STATE_FORCED_BACKFILL;
-  } else if (newflags & OFR_RECOVERY) {
-    newstate = PG_STATE_FORCED_RECOVERY;
   }
-
-  // debug output here may get large, don't generate it if debug level is below
-  // 10 and use abbreviated pg ids otherwise
-  if ((cct)->_conf->subsys.should_gather(ceph_subsys_osd, 10)) {
-    stringstream ss;
-
-    for (auto& i : pgs) {
-      ss << i->get_pgid() << " ";
-    }
-
-    dout(10) << __func__ << " working on " << ss.str() << dendl;
-  }
-
-  if (newflags & OFR_CANCEL) {
-    for (auto& i : pgs) {
-      i->lock();
-      i->_change_recovery_force_mode(newstate, true);
-      i->unlock();
+  if (newflags & OFR_BACKFILL) {
+    for (auto& pg : pgs) {
+      pg->set_force_backfill(!(newflags & OFR_CANCEL));
     }
-  } else {
-    for (auto& i : pgs) {
-      // make sure the PG is in correct state before forcing backfill or recovery, or
-      // else we'll make PG keeping FORCE_* flag forever, requiring osds restart
-      // or forcing somehow recovery/backfill.
-      i->lock();
-      int pgstate = i->get_state();
-      if ( ((newstate == PG_STATE_FORCED_RECOVERY) && (pgstate & (PG_STATE_DEGRADED | PG_STATE_RECOVERY_WAIT | PG_STATE_RECOVERING))) ||
-	    ((newstate == PG_STATE_FORCED_BACKFILL) && (pgstate & (PG_STATE_DEGRADED | PG_STATE_BACKFILL_WAIT | PG_STATE_BACKFILLING))) )
-        i->_change_recovery_force_mode(newstate, false);
-      i->unlock();
+  } else if (newflags & OFR_RECOVERY) {
+    for (auto& pg : pgs) {
+      pg->set_force_recovery(!(newflags & OFR_CANCEL));
     }
   }
 }
diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 2e7c9d32eddd1..fb18d50c59721 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -2022,17 +2022,48 @@ void PG::mark_clean()
   kick_snap_trim();
 }
 
-void PG::_change_recovery_force_mode(int new_mode, bool clear)
+void PG::set_force_recovery(bool b)
 {
+  lock();
   if (!deleting) {
-    // we can't and shouldn't do anything if the PG is being deleted locally
-    if (clear) {
-      state_clear(new_mode);
-    } else {
-      state_set(new_mode);
+    if (b) {
+      if (!(state & PG_STATE_FORCED_RECOVERY) &&
+	  (state & (PG_STATE_DEGRADED |
+		    PG_STATE_RECOVERY_WAIT |
+		    PG_STATE_RECOVERING))) {
+	dout(20) << __func__ << " set" << dendl;
+	state_set(PG_STATE_FORCED_RECOVERY);
+	publish_stats_to_osd();
+      }
+    } else if (state & PG_STATE_FORCED_RECOVERY) {
+      dout(20) << __func__ << " clear" << dendl;
+      state_clear(PG_STATE_FORCED_RECOVERY);
+      publish_stats_to_osd();
+    }
+  }
+  unlock();
+}
+
+void PG::set_force_backfill(bool b)
+{
+  lock();
+  if (!deleting) {
+    if (b) {
+      if (!(state & PG_STATE_FORCED_RECOVERY) &&
+	  (state & (PG_STATE_DEGRADED |
+		    PG_STATE_BACKFILL_WAIT |
+		    PG_STATE_BACKFILLING))) {
+	dout(10) << __func__ << " set" << dendl;
+	state_set(PG_STATE_FORCED_RECOVERY);
+	publish_stats_to_osd();
+      }
+    } else if (state & PG_STATE_FORCED_RECOVERY) {
+      dout(10) << __func__ << " clear" << dendl;
+      state_clear(PG_STATE_FORCED_RECOVERY);
+      publish_stats_to_osd();
     }
-    publish_stats_to_osd();
   }
+  unlock();
 }
 
 inline int PG::clamp_recovery_priority(int priority)
diff --git a/src/osd/PG.h b/src/osd/PG.h
index 1841b16a6e270..071a5ea02f71b 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -286,6 +286,9 @@ public:
     return acting;
   }
 
+  void set_force_recovery(bool b);
+  void set_force_backfill(bool b);
+
 protected:
   OSDService *osd;
   CephContext *cct;
@@ -1003,9 +1006,6 @@ protected:
   unsigned get_backfill_priority();
 
   void mark_clean();  ///< mark an active pg clean
-public:
-  void _change_recovery_force_mode(int new_mode, bool clear);
-protected:
 
   /// return [start,end) bounds for required past_intervals
   static pair<epoch_t, epoch_t> get_required_past_interval_bounds(
-- 
2.39.5