mds: use HeartbeatMap to control beacons

...so that if something hogs mds_lock without
pinging the heartbeat map, we will stop sending
beacons to the mon and it will have the chance
to recognise that we are laggy.

Signed-off-by: John Spray <john.spray@redhat.com>

diff --git a/src/mds/Beacon.cc b/src/mds/Beacon.cc
index 667a7d7f2f5ee432fd1149fe27550ba27124311d..b4a54e1862b8831d30447dec3c6bff7cf6148487 100644 (file)
--- a/src/mds/Beacon.cc
+++ b/src/mds/Beacon.cc
@@ -14,6 +14,8 @@
 
 
 #include "common/dout.h"
+#include "common/HeartbeatMap.h"
+
 #include "messages/MMDSBeacon.h"
 #include "mon/MonClient.h"
 
@@ -141,6 +143,13 @@ void Beacon::_send()
   sender = new C_MDS_BeaconSender(this);
   timer.add_event_after(g_conf->mds_beacon_interval, sender);
 
+  if (!cct->get_heartbeat_map()->is_healthy()) {
+    /* If anything isn't progressing, let avoid sending a beacon so that
+     * the MDS will consider us laggy */
+    dout(1) << __func__ << " skipping beacon, heartbeat map not healthy" << dendl;
+    return;
+  }
+
   ++last_seq;
   dout(10) << __func__ << " " << ceph_mds_state_name(want_state)
           << " seq " << last_seq

diff --git a/src/mds/MDS.cc b/src/mds/MDS.cc
index 3d832b8151d9a3c107587f139ac4302f770edeb9..37210f945d5aba517aabf407e50fdc65bd1295f7 100644 (file)
--- a/src/mds/MDS.cc
+++ b/src/mds/MDS.cc
@@ -23,7 +23,6 @@
 #include "common/ceph_argparse.h"
 #include "common/errno.h"
 
-
 #include "msg/Messenger.h"
 #include "mon/MonClient.h"
 
@@ -46,6 +45,8 @@
 
 #include "InoTable.h"
 
+#include "common/HeartbeatMap.h"
+
 #include "common/perf_counters.h"
 
 #include "common/Timer.h"
@@ -83,6 +84,7 @@ MDS::MDS(const std::string &n, Messenger *m, MonClient *mc) :
   Dispatcher(m->cct),
   mds_lock("MDS::mds_lock"),
   timer(m->cct, mds_lock),
+  hb(NULL),
   beacon(m->cct, mc, n),
   authorize_handler_cluster_registry(new AuthAuthorizeHandlerRegistry(m->cct,
                                                                      m->cct->_conf->auth_supported.length() ?
@@ -104,6 +106,8 @@ MDS::MDS(const std::string &n, Messenger *m, MonClient *mc) :
   finisher(cct),
   sessionmap(this), asok_hook(NULL) {
 
+  hb = cct->get_heartbeat_map()->add_worker("MDS");
+
   orig_argc = 0;
   orig_argv = NULL;
 
@@ -184,6 +188,10 @@ MDS::~MDS() {
   
   if (messenger)
     delete messenger;
+
+  if (hb) {
+    cct->get_heartbeat_map()->remove_worker(hb);
+  }
 }
 
 class MDSSocketHook : public AdminSocketHook {
@@ -221,6 +229,8 @@ bool MDS::asok_command(string command, cmdmap_t& cmdmap, string format,
   } else if (command == "session ls") {
     mds_lock.Lock();
 
+    heartbeat_reset();
+
     // Dump sessions, decorated with recovery/replay status
     f->open_array_section("sessions");
     const ceph::unordered_map<entity_name_t, Session*> session_map = sessionmap.get_sessions();
@@ -694,6 +704,8 @@ void MDS::reset_tick()
 
 void MDS::tick()
 {
+  heartbeat_reset();
+
   tick_event = 0;
 
   // reschedule
@@ -1743,6 +1755,14 @@ void MDS::suicide()
 
   // shut down messenger
   messenger->shutdown();
+
+  // Workaround unclean shutdown: HeartbeatMap will assert if
+  // worker is not removed (as we do in ~MDS), but ~MDS is not
+  // always called after suicide.
+  if (hb) {
+    cct->get_heartbeat_map()->remove_worker(hb);
+    hb = NULL;
+  }
 }
 
 void MDS::respawn()
@@ -1792,6 +1812,9 @@ bool MDS::ms_dispatch(Message *m)
 {
   bool ret;
   mds_lock.Lock();
+
+  heartbeat_reset();
+
   if (want_state == CEPH_MDS_STATE_DNE) {
     dout(10) << " stopping, discarding " << *m << dendl;
     m->put();
@@ -2016,6 +2039,8 @@ bool MDS::_dispatch(Message *m)
       dout(1) << "          " << this->mds_lock.is_locked_by_me() << dendl;
       ls.front()->complete(0);
       ls.pop_front();
+
+      heartbeat_reset();
     }
   }
 
@@ -2034,6 +2059,8 @@ bool MDS::_dispatch(Message *m)
       dout(7) << " processing laggy deferred " << *old << dendl;
       handle_deferrable_message(old);
     }
+
+    heartbeat_reset();
   }
 
   // done with all client replayed requests?
@@ -2314,3 +2341,17 @@ void MDS::set_want_state(MDSMap::DaemonState newstate)
     beacon.notify_want_state(newstate);
   }
 }
+
+/**
+ * Call this when you take mds_lock, or periodically if you're going to
+ * hold the lock for a long time (e.g. iterating over clients/inodes)
+ */
+void MDS::heartbeat_reset()
+{
+  assert(hb != NULL);
+  // NB not enabling suicide grace, because the mon takes care of killing us
+  // (by blacklisting us) when we fail to send beacons, and it's simpler to
+  // only have one way of dying.
+  cct->get_heartbeat_map()->reset_timeout(hb, g_conf->mds_beacon_grace, 0);
+}
+

diff --git a/src/mds/MDS.h b/src/mds/MDS.h
index a69885d47433e1c67f1e9a8db0e9e79086c4c807..6a8bd795bce31ccdb01695a9b0031841339d5658 100644 (file)
--- a/src/mds/MDS.h
+++ b/src/mds/MDS.h
@@ -104,6 +104,9 @@ enum {
 
 
 
+namespace ceph {
+  struct heartbeat_handle_d;
+}
 class filepath;
 
 class MonClient;
@@ -145,6 +148,8 @@ class MDS : public Dispatcher, public md_config_obs_t {
   SafeTimer    timer;
 
  private:
+  ceph::heartbeat_handle_d *hb;  // Heartbeat for threads using mds_lock
+  void heartbeat_reset();
   Beacon  beacon;
   void set_want_state(MDSMap::DaemonState newstate);
  public: