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>
(cherry picked from commit 30b3baba4e0e8641a7dbd10b8df5f00acdee204d)

diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h
index 2beee15edae9efca25ea5c394e805928025e73ca..0db83a7f3af79f77c99d1c8756caa56be8e3c9f1 100644 (file)
--- a/src/mon/MonClient.h
+++ b/src/mon/MonClient.h
@@ -453,6 +453,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 8ff5a8f1872fd02d4f134fb5817a9ffecdf17a9a..c7e3e44dc8de0a1237f6ca0e149d2063d05a1b2e 100644 (file)
--- a/src/mon/MonSub.h
+++ b/src/mon/MonSub.h
@@ -18,6 +18,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 6ccaf9a76aa6a02aaa4bd42f5a405a5331c94b1f..e29fc95726e808965ec730fd2e2127eff16c2938 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -1320,6 +1320,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();
       }
     }
@@ -2055,7 +2056,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()
@@ -2068,6 +2068,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();
@@ -2246,13 +2247,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();
     }