Objecter: respect higher epoch subscription in tick

The OSD and Objecter share the same MonClient. During preboot, a potential
race condition exists where the OSD subscribes to osdmap epoch X, while
the Objecter subscribes to epoch X - 1.

The Objecter's subscription overrides the OSD's subscription. Consequently,
the monitor ignores the request (as it believes the OSD already has the
older map), causing the OSD to hang during preboot.

To fix this, check if a higher epoch is already subscribed before calling
_maybe_request_map during Objecter::tick. If a higher epoch is found,
maintain the existing subscription.

Fixes: https://tracker.ceph.com/issues/71931
Signed-off-by: Nitzan Mordechai <nmordech@ibm.com>

diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h
index e259a261525a8a514f13d0ea2aec9108d51d145c..3b78ea62ae272a7a00683b1033a9f604e2f45720 100644 (file)
--- a/src/mon/MonClient.h
+++ b/src/mon/MonClient.h
@@ -454,6 +454,10 @@ public:
     std::lock_guard l(monc_lock);
     _renew_subs();
   }
+  version_t get_start(const std::string& what) const {
+    std::lock_guard l(monc_lock);
+    return sub.get_start(what);
+  }
   bool sub_want(std::string what, version_t start, unsigned flags) {
     std::lock_guard l(monc_lock);
     return sub.want(what, start, flags);

diff --git a/src/mon/MonSub.h b/src/mon/MonSub.h
index 9938ce39763fa64aa2f445ceae8af500ee147a1d..2153c4a665712deae07b8883ae95d36b384ced69 100644 (file)
--- a/src/mon/MonSub.h
+++ b/src/mon/MonSub.h
@@ -19,6 +19,19 @@ public:
   auto get_subs() const {
     return sub_new;
   }
+  // get the requested start epoch for a subscription
+  // search first in "new" subs - subs that have not yet been sent
+  // but requested and going to be sent, then in "sent" subs.
+  // if not found, return 0
+  version_t get_start(const std::string& what) const {
+    if (auto i = sub_new.find(what); i != sub_new.end()) {
+      return i->second.start;
+    }
+    if (auto i = sub_sent.find(what); i != sub_sent.end()) {
+      return i->second.start;
+    }
+    return 0;
+  }
   bool need_renew() const;
   // change the status of "new" subscriptions to "sent"
   void renewed();

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 392ef0f2863940e910e78e31148793fb11a2430b..5d001084efcd3eaf918e3a0e33c90cdc37a5bfa2 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -1321,6 +1321,7 @@ void Objecter::handle_osd_map(MOSDMap *m)
        ldout(cct, 3) << "handle_osd_map hmm, i want a full map, requesting"
                      << dendl;
        monc->sub_want("osdmap", 0, CEPH_SUBSCRIBE_ONETIME);
+    last_osdmap_request_time = ceph::coarse_mono_clock::now();
        monc->renew_subs();
       }
     }
@@ -2056,7 +2057,6 @@ void Objecter::maybe_request_map()
 
 void Objecter::_maybe_request_map()
 {
-  last_osdmap_request_time = ceph::coarse_mono_clock::now();
   // rwlock is locked
   int flag = 0;
   if (_osdmap_full_flag()
@@ -2069,6 +2069,7 @@ void Objecter::_maybe_request_map()
       << "_maybe_request_map subscribing (onetime) to next osd map" << dendl;
     flag = CEPH_SUBSCRIBE_ONETIME;
   }
+  last_osdmap_request_time = ceph::coarse_mono_clock::now();
   epoch_t epoch = osdmap->get_epoch() ? osdmap->get_epoch()+1 : 0;
   if (monc->sub_want("osdmap", epoch, flag)) {
     monc->renew_subs();
@@ -2247,13 +2248,27 @@ void Objecter::tick()
   if (num_homeless_ops || !toping.empty()) {
     _maybe_request_map();
   } else if (last_osdmap_request_time != ceph::coarse_mono_clock::time_point()) {
+    const epoch_t epoch = osdmap->get_epoch() ? osdmap->get_epoch() + 1 : 0;
     auto now = ceph::coarse_mono_clock::now();
     auto elapsed = now - last_osdmap_request_time;
     auto stale_window = ceph::make_timespan(cct->_conf->objecter_tick_interval) * 2;
-    if (elapsed > stale_window) {
+    auto exist_sub = monc->get_start("osdmap");
+    ldout(cct, 20) << __func__ << ": elapsed since last osdmap request "
+      << std::chrono::duration<double>(elapsed).count() << "s"
+      << ", stale_window "
+      << std::chrono::duration<double>(stale_window).count() << "s"
+      << ", epoch " << epoch
+      << ", monc->get_start(osdmap) " << exist_sub
+      << dendl;
+    // check that:
+    //  1) we passed stale_window since last request AND
+    //  2) we don't have any osdmap subscription in flight OR
+    //     we have subscription - but the epoch we need is newer than the one we subscribed for
+    if (elapsed > stale_window &&
+       ((exist_sub == 0) || (epoch > exist_sub))) {
       double elapsed_s = std::chrono::duration<double>(elapsed).count();
       double thresh_s  = std::chrono::duration<double>(stale_window).count();
-      ldout(cct, 10) << __func__ << ": osdmap stale: " << elapsed_s 
+      ldout(cct, 20) << __func__ << ": osdmap stale: " << elapsed_s
         << "s > " << thresh_s << "s, maybe requesting map" << dendl;
       _maybe_request_map();
     }