From: Sage Weil <sage@redhat.com>
Date: Sat, 20 May 2017 21:25:11 +0000 (-0400)
Subject: mon/PGMap: update osd_epoch in synchrony with osd_stat_updates
X-Git-Tag: ses5-milestone6~8^2~19^2~38
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f0eff9223a923ba97de8be3e2201e0a2f297636f;p=ceph.git

mon/PGMap: update osd_epoch in synchrony with osd_stat_updates

I'm not sure why this didn't bite us earlier, but there is an assert
in apply_incremental (not used in preluminous mon) and an implicit
dereference in PGMonitor::encode_pending (maybe didn't cause crash?)
that will trigger if we have an osd_stat_updates record without a matching
osd_epochs update.  Maybe there is some subtle reason why the osd_epochs
update happens elsewhere in master (it doesn't on the mgr), but my guess
is we were silently dereferencing the invalid iterator and not noticing.

Anyway, it's easy to fix.  We use the epoch from the previous PGMap.

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/mon/PGMap.cc b/src/mon/PGMap.cc
index 9b0d6d2c7dd..da0b5157d07 100644
--- a/src/mon/PGMap.cc
+++ b/src/mon/PGMap.cc
@@ -3235,7 +3235,9 @@ void PGMapUpdater::check_osd_map(const OSDMap::Incremental &osd_inc,
   for (const auto &p : osd_inc.new_weight) {
     if (p.second == CEPH_OSD_OUT) {
       dout(10) << __func__ << "  osd." << p.first << " went OUT" << dendl;
-      pending_inc->stat_osd_out(p.first, osd_inc.epoch);
+      auto j = pg_map->osd_epochs.find(p.first);
+      if (j != pg_map->osd_epochs.end())
+	pending_inc->stat_osd_out(p.first, j->second);
     }
   }
 
@@ -3285,7 +3287,10 @@ void PGMapUpdater::check_osd_map(
     } else if (osdmap.is_out(p.first)) {
       // zero osd_stat
       if (p.second.kb != 0) {
-	pending_inc->stat_osd_out(p.first, osdmap.get_epoch());
+	auto j = pgmap.osd_epochs.find(p.first);
+	if (j != pgmap.osd_epochs.end()) {
+	  pending_inc->stat_osd_out(p.first, j->second);
+	}
       }
     } else if (!osdmap.is_up(p.first)) {
       // zero the op_queue_age_hist
diff --git a/src/mon/PGMap.h b/src/mon/PGMap.h
index 28c3b4ede11..178e427dd8f 100644
--- a/src/mon/PGMap.h
+++ b/src/mon/PGMap.h
@@ -221,7 +221,8 @@ public:
     mempool::pgmap::map<int32_t,osd_stat_t> osd_stat_updates;
     mempool::pgmap::set<int32_t> osd_stat_rm;
 
-    // mapping of osd to most recently reported osdmap epoch
+    // mapping of osd to most recently reported osdmap epoch.
+    // 1:1 with osd_stat_updates.
     mempool::pgmap::map<int32_t,epoch_t> osd_epochs;
   public:
 
@@ -244,7 +245,13 @@ public:
     void stat_osd_out(int32_t osd, epoch_t epoch) {
       // 0 the stats for the osd
       osd_stat_updates[osd] = osd_stat_t();
-      osd_epochs[osd] = epoch;
+      // only fill in the epoch if the osd didn't already report htis
+      // epoch.  that way we zero the stat but still preserve a reported
+      // new epoch...
+      if (!osd_epochs.count(osd))
+	osd_epochs[osd] = epoch;
+      // ...and maintain our invariant.
+      assert(osd_epochs.size() == osd_stat_updates.size());
     }
     void stat_osd_down_up(int32_t osd, epoch_t epoch, const PGMap& pg_map) {
       // 0 the op_queue_age_hist for this osd