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>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index f50960f09942d8bc9d2e3cb14c2d8809104ed84c..a2b5763fae72662341a098fc28b6dee56a8276b7 100644 (file)
--- 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 9f50ed31366063002e11410e30c5b540f70fbf82..1b9161dff17bf7399d2c5a85a7b135f98b88a78f 100644 (file)
--- 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 4f641fd3e68a672e39d62cebecbb9dd768b067db..a08c0ed634ac5562716c7453d85414ba72ba8d4e 100644 (file)
--- 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 b8b09fcf4d84a9ecded171c0f018341da0c45151..f535e976a4f4b5a4af6c02700a732f433d370bbc 100644 (file)
--- 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