osd: PG::prior_set_affected: fix lost OSD detection

When looking for newly-lost OSDs, we should check prior_set_lost rather
than prior_set. Down OSDs often are in PG::prior_set_down and NOT in
PG::prior_set.

Also update comment for prior_set_down. Sometimes OSDs are in both
PG::prior_set and PG::prior_set_down.

Signed-off-by: Colin McCabe <colinm@hq.newdream.net>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 53a74a4b56671eb94d21114e519d1c57f643ca84..661a46231938ce781da9de3bcfe95e964b3f0e24 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -784,23 +784,29 @@ bool PG::prior_set_affected(OSDMap *osdmap)
       return true;
     }
 
-    // did someone in the prior set get lost?
-    const osd_info_t& pinfo(osdmap->get_info(o));
-    if (pinfo.lost_at > pinfo.up_from) {
-      set<int>::const_iterator pl = prior_set_lost.find(o);
-      if (pl == prior_set_lost.end())
-       return true;
-    }
+    // If someone in the prior set is marked as lost, it would also have to be
+    // marked as down. So don't check for newly lost osds here.
   }
 
   // did someone in the prior down set go up?
   for (set<int>::iterator p = prior_set_down.begin();
        p != prior_set_down.end();
        p++)
-    if (osdmap->is_up(*p)) {
+  {
+    int o = *p;
+
+    if (osdmap->is_up(o)) {
       dout(10) << "prior_set_affected: osd" << *p << " now up" << dendl;
       return true;
     }
+    // did someone in the prior set get lost?
+    const osd_info_t& pinfo(osdmap->get_info(o));
+    if (pinfo.lost_at > pinfo.up_from) {
+      set<int>::const_iterator pl = prior_set_lost.find(o);
+      if (pl == prior_set_lost.end())
+       return true;
+    }
+  }
   
   // did a significant osd's up_thru change?
   for (map<int,epoch_t>::iterator p = prior_set_up_thru.begin();

diff --git a/src/osd/PG.h b/src/osd/PG.h
index 9800de244e979e52899798a4473ae4ee173b5d22..d7eeeef97d3c0ad589699ff7547c7dde32c29389 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -782,7 +782,7 @@ public:
   bool        have_master_log;
  protected:
   set<int>    prior_set;   // current+prior OSDs, as defined by info.history.last_epoch_started.
-  set<int>    prior_set_down;          // down osds exluded from prior_set
+  set<int>    prior_set_down;     // down osds normally exluded from prior_set
   map<int,epoch_t> prior_set_up_thru;  // osds whose up_thru we care about
   set<int>    prior_set_lost;  // osds in the prior set which are lost
   bool        need_up_thru;

diff --git a/src/test/test_lost.sh b/src/test/test_lost.sh
index 1f76f2a2f4607edc36805aa62f1e492d65e7095e..f66344b9192c89899bfe1e1b71fcac3f8e48e144 100755 (executable)
--- a/src/test/test_lost.sh
+++ b/src/test/test_lost.sh
@@ -5,7 +5,7 @@
 #
 
 # Constants
-MAX_OBJS=100
+MAX_OBJS=50
 OBJ_SIZE=1000000
 TEMPDIR=`mktemp -d`
 SDIR=`dirname $0`/..
@@ -65,13 +65,13 @@ setup() {
 
 do_test() {
         # Write lots and lots of objects
-        write_objects 1 3
+        write_objects 1 2
 
         # Take down osd1
         stop_osd 1
 
         # Continue writing a lot of objects
-        write_objects 4 6
+        write_objects 3 4
 
         # Bring up osd1
         restart_osd 1