From 4d74a7b2e9a9e34f2fce7b5a49eb6ddc2b081ee8 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@newdream.net>
Date: Tue, 27 Mar 2012 15:12:07 -0700
Subject: [PATCH] osd: fix handling of recovery sources when osds go down

If a source osd goes down, we need to

 - reset any pulls (already did that before)
 - remove peer from missing_loc so that we know what is now unfound
 - restart recovery/discover_all_missing in case new stuff is now unfound

This fixes a bug like so:

 - we peer
 - we find an object we need to recover on a stray osd
 - that osd goes down
 - recover_primary() thinks unfound=0 but it really is 1

... recover_primary 3270c60b/mds0_sessionmap/head 4'1 (missing) (missing head)
... pull 3270c60b/mds0_sessionmap/head v 4'1 but it is unfound

Signed-off-by: Sage Weil <sage@newdream.net>
---
 src/osd/PG.cc           |  8 +++--
 src/osd/PG.h            |  2 +-
 src/osd/ReplicatedPG.cc | 73 +++++++++++++++++++++++++++++++----------
 src/osd/ReplicatedPG.h  |  2 +-
 4 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index f50960f09942d..a2b5763fae726 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -4050,8 +4050,12 @@ boost::statechart::result PG::RecoveryState::Active::react(const ActMap&)
   assert(pg->is_active());
   assert(pg->is_primary());
 
-  pg->check_recovery_op_pulls(pg->get_osdmap());
-	
+  if (pg->check_recovery_sources(pg->get_osdmap()) &&
+      pg->have_unfound()) {
+    // object may have become unfound
+    pg->discover_all_missing(*context< RecoveryMachine >().get_query_map());
+  }
+
   if (g_conf->osd_check_for_log_corruption)
     pg->check_log_for_corruption(pg->osd->store);
 
diff --git a/src/osd/PG.h b/src/osd/PG.h
index 9f50ed3136606..1b9161dff17bf 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -768,7 +768,7 @@ public:
   void clear_recovery_state();
   virtual void _clear_recovery_state() = 0;
   void defer_recovery();
-  virtual void check_recovery_op_pulls(const OSDMapRef newmap) = 0;
+  virtual bool check_recovery_sources(const OSDMapRef newmap) = 0;
   void start_recovery_op(const hobject_t& soid);
   void finish_recovery_op(const hobject_t& soid, bool dequeue=false);
 
diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 4f641fd3e68a6..a08c0ed634ac5 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -5637,28 +5637,65 @@ void ReplicatedPG::_clear_recovery_state()
   pull_from_peer.clear();
 }
 
-void ReplicatedPG::check_recovery_op_pulls(const OSDMapRef osdmap)
-{
-  for (map<int, set<hobject_t> >::iterator j = pull_from_peer.begin();
-       j != pull_from_peer.end();
-       ) {
-    if (osdmap->is_up(j->first)) {
-      ++j;
+bool ReplicatedPG::check_recovery_sources(const OSDMapRef osdmap)
+{
+  /*
+   * check that any peers we are planning to (or currently) pulling
+   * objects from are dealt with.
+   */
+  set<int> now_down;
+  for (set<int>::iterator p = missing_loc_sources.begin();
+       p != missing_loc_sources.end();
+       ++p) {
+    if (osdmap->is_up(*p)) {
+      p++;
       continue;
     }
-    dout(10) << "Reseting pulls from osd." << j->first
-	     << ", osdmap has it marked down" << dendl;
-    
-    for (set<hobject_t>::iterator i = j->second.begin();
-	 i != j->second.end();
-	 ++i) {
-      assert(pulling.count(*i) == 1);
-      pulling.erase(*i);
-      finish_recovery_op(*i);
+    dout(10) << "check_recovery_sources source osd." << *p << " now down" << dendl;
+    now_down.insert(*p);
+
+    // reset pulls?
+    map<int, set<hobject_t> >::iterator j = pull_from_peer.find(*p);
+    if (j != pull_from_peer.end()) {
+      dout(10) << "check_recovery_sources resetting pulls from osd." << *p
+	       << ", osdmap has it marked down" << dendl;
+      for (set<hobject_t>::iterator i = j->second.begin();
+	   i != j->second.end();
+	   ++i) {
+	assert(pulling.count(*i) == 1);
+	pulling.erase(*i);
+	finish_recovery_op(*i);
+      }
+      log.last_requested = 0;
+      pull_from_peer.erase(j++);
     }
-    log.last_requested = 0;
-    pull_from_peer.erase(j++);
+
+    // remove from missing_loc_sources
+    missing_loc_sources.erase(p++);
+  }
+  if (now_down.empty()) {
+    dout(10) << "check_recovery_sources source osds (" << missing_loc_sources << ") went down" << dendl;
+    return false;
+  }
+  dout(10) << "check_recovery_sources sources osds " << now_down << " now down, remaining sources are "
+	   << missing_loc_sources << dendl;
+
+  // filter missing_loc
+  map<hobject_t, set<int> >::iterator p = missing_loc.begin();
+  while (p != missing_loc.end()) {
+    set<int>::iterator q = p->second.begin();
+    while (q != p->second.end())
+      if (now_down.count(*q))
+	p->second.erase(q++);
+      else
+	q++;
+    if (p->second.empty())
+      missing_loc.erase(p++);
+    else
+      p++;
   }
+
+  return true;
 }
   
 
diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index b8b09fcf4d84a..f535e976a4f4b 100644
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -639,7 +639,7 @@ protected:
   void finish_degraded_object(const hobject_t& oid);
 
   // Cancels/resets pulls from peer
-  void check_recovery_op_pulls(const OSDMapRef map);
+  bool check_recovery_sources(const OSDMapRef map);
   int pull(const hobject_t& oid, eversion_t v);
 
   // low level ops
-- 
2.39.5