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 9b0d6d2c7dd4d049634b163050bee55e57b16316..da0b5157d075c840aa10a6504fb8a36f36c93e32 100644 (file)
--- 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 28c3b4ede11fdf5258698ad858b8061ffb343234..178e427dd8ff109a65a36bf3d7ee1c0612c7a940 100644 (file)
--- 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