]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mon: check is_shutdown() in timer callbacks 15084/head
authorKefu Chai <kchai@redhat.com>
Fri, 5 May 2017 04:02:05 +0000 (12:02 +0800)
committerAlexey Sheplyakov <asheplyakov@mirantis.com>
Tue, 20 Jun 2017 14:18:20 +0000 (18:18 +0400)
introduce a helper class: C_MonContext, and initialize all timer events
using it, to ensure that they do check is_shutdown() before doing their
work.

Fixes: http://tracker.ceph.com/issues/19825
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 561cbded0c7e28231b1c7ce18663b8d7d40aad6d)

src/mon/Elector.cc
src/mon/Elector.h
src/mon/Monitor.cc
src/mon/Monitor.h
src/mon/Paxos.cc
src/mon/PaxosService.cc
src/mon/PaxosService.h
src/mon/QuorumService.h

index c2c26b039b918a0175cc533ec78ce17b21314524..264de080ba702109fb72e09c50cf75836953064b 100644 (file)
@@ -129,6 +129,8 @@ void Elector::defer(int who)
 
 void Elector::reset_timer(double plus)
 {
+  // set the timer
+  cancel_timer();
   /**
    * This class is used as the callback when the expire_event timer fires up.
    *
@@ -142,17 +144,9 @@ void Elector::reset_timer(double plus)
    * as far as we know, we may even be dead); so, just propose ourselves as the
    * Leader.
    */
-  class C_ElectionExpire : public Context {
-    Elector *elector;
-  public:
-    explicit C_ElectionExpire(Elector *e) : elector(e) { }
-    void finish(int r) override {
-      elector->expire();
-    }
-  };
-  // set the timer
-  cancel_timer();
-  expire_event = new C_ElectionExpire(this);
+  expire_event = new C_MonContext(mon, [this](int) {
+      expire();
+    });
   mon->timer.add_event_after(g_conf->mon_election_timeout + plus,
                             expire_event);
 }
index 3ab347422ea39e5fceb2d9cbf5a793bac8d3ea45..a74433cfede17aba6d12c0732419683abeaa3053 100644 (file)
@@ -65,7 +65,7 @@ class Elector {
    * Event callback responsible for dealing with an expired election once a
    * timer runs out and fires up.
    */
-  Context *expire_event;
+  Context *expire_event = nullptr;
 
   /**
    * Resets the expire_event timer, by cancelling any existing one and
@@ -336,7 +336,6 @@ class Elector {
    * @param m A Monitor instance
    */
   explicit Elector(Monitor *m) : mon(m),
-                       expire_event(0),
                        epoch(0),
                        participating(true),
                        electing_me(false),
index e3200522141c5209a602045654b40b1a5f64b956..744817b256ffbfb76c9e26e2cae2e98741dc7202 100644 (file)
@@ -138,6 +138,12 @@ long parse_pos_long(const char *s, ostream *pss)
   return r;
 }
 
+void C_MonContext::finish(int r) {
+  if (mon->is_shutdown())
+    return;
+  FunctionContext::finish(r);
+}
+
 Monitor::Monitor(CephContext* cct_, string nm, MonitorDBStore *s,
                 Messenger *m, MonMap *map) :
   Dispatcher(cct_),
@@ -186,12 +192,8 @@ Monitor::Monitor(CephContext* cct_, string nm, MonitorDBStore *s,
   timecheck_rounds_since_clean(0),
   timecheck_event(NULL),
 
-  probe_timeout_event(NULL),
-
   paxos_service(PAXOS_NUM),
   admin_hook(NULL),
-  health_tick_event(NULL),
-  health_interval_event(NULL),
   routed_request_tid(0),
   op_tracker(cct, true, 1)
 {
@@ -1393,7 +1395,9 @@ void Monitor::sync_reset_timeout()
   dout(10) << __func__ << dendl;
   if (sync_timeout_event)
     timer.cancel_event(sync_timeout_event);
-  sync_timeout_event = new C_SyncTimeout(this);
+  sync_timeout_event = new C_MonContext(this, [this](int) {
+      sync_timeout();
+    });
   timer.add_event_after(g_conf->mon_sync_timeout, sync_timeout_event);
 }
 
@@ -1731,7 +1735,9 @@ void Monitor::cancel_probe_timeout()
 void Monitor::reset_probe_timeout()
 {
   cancel_probe_timeout();
-  probe_timeout_event = new C_ProbeTimeout(this);
+  probe_timeout_event = new C_MonContext(this, [this](int r) {
+      probe_timeout(r);
+    });
   double t = g_conf->mon_probe_timeout;
   timer.add_event_after(t, probe_timeout_event);
   dout(10) << "reset_probe_timeout " << probe_timeout_event << " after " << t << " seconds" << dendl;
@@ -2380,8 +2386,12 @@ void Monitor::health_tick_start()
   dout(15) << __func__ << dendl;
 
   health_tick_stop();
-  health_tick_event = new C_HealthToClogTick(this);
-
+  health_tick_event = new C_MonContext(this, [this](int r) {
+      if (r < 0)
+        return;
+      do_health_to_clog();
+      health_tick_start();
+    });
   timer.add_event_after(cct->_conf->mon_health_to_clog_tick_interval,
                         health_tick_event);
 }
@@ -2425,7 +2435,11 @@ void Monitor::health_interval_start()
 
   health_interval_stop();
   utime_t next = health_interval_calc_next_update();
-  health_interval_event = new C_HealthToClogInterval(this);
+  health_interval_event = new C_MonContext(this, [this](int r) {
+      if (r < 0)
+        return;
+      do_health_to_clog_interval();
+    });
   timer.add_event_at(next, health_interval_event);
 }
 
@@ -4182,7 +4196,9 @@ void Monitor::timecheck_reset_event()
            << " rounds_since_clean " << timecheck_rounds_since_clean
            << dendl;
 
-  timecheck_event = new C_TimeCheck(this);
+  timecheck_event = new C_MonContext(this, [this](int) {
+      timecheck_start_round();
+    });
   timer.add_event_after(delay, timecheck_event);
 }
 
@@ -5011,15 +5027,9 @@ void Monitor::scrub_event_start()
     return;
   }
 
-  struct C_Scrub : public Context {
-    Monitor *mon;
-    explicit C_Scrub(Monitor *m) : mon(m) { }
-    void finish(int r) override {
-      mon->scrub_start();
-    }
-  };
-
-  scrub_event = new C_Scrub(this);
+  scrub_event = new C_MonContext(this, [this](int) {
+      scrub_start();
+    });
   timer.add_event_after(cct->_conf->mon_scrub_interval, scrub_event);
 }
 
@@ -5045,33 +5055,18 @@ void Monitor::scrub_reset_timeout()
   dout(15) << __func__ << " reset timeout event" << dendl;
   scrub_cancel_timeout();
 
-  struct C_ScrubTimeout : public Context {
-    Monitor *mon;
-    explicit C_ScrubTimeout(Monitor *m) : mon(m) { }
-    void finish(int r) override {
-      mon->scrub_timeout();
-    }
-  };
-
-  scrub_timeout_event = new C_ScrubTimeout(this);
+  scrub_timeout_event = new C_MonContext(this, [this](int) {
+      scrub_timeout();
+    });
   timer.add_event_after(g_conf->mon_scrub_timeout, scrub_timeout_event);
 }
 
 /************ TICK ***************/
-
-class C_Mon_Tick : public Context {
-  Monitor *mon;
-public:
-  explicit C_Mon_Tick(Monitor *m) : mon(m) {}
-  void finish(int r) override {
-    mon->tick();
-  }
-};
-
 void Monitor::new_tick()
 {
-  C_Mon_Tick *ctx = new C_Mon_Tick(this);
-  timer.add_event_after(g_conf->mon_tick_interval, ctx);
+  timer.add_event_after(g_conf->mon_tick_interval, new C_MonContext(this, [this](int) {
+       tick();
+      }));
 }
 
 void Monitor::tick()
index f5cafb7fd2426214e2a2b3c4ee3df826e55bd73b..01c3fe2bd86d0ae74547d82150621b17252059ba 100644 (file)
@@ -111,6 +111,14 @@ struct MonCommand;
 
 #define COMPAT_SET_LOC "feature_set"
 
+class C_MonContext final : public FunctionContext {
+  const Monitor *mon;
+public:
+  explicit C_MonContext(Monitor *m, boost::function<void(int)>&& callback)
+    : FunctionContext(std::move(callback)), mon(m) {}
+  void finish(int r) override;
+};
+
 class Monitor : public Dispatcher,
                 public md_config_obs_t {
 public:
@@ -154,7 +162,6 @@ public:
 
 private:
   void new_tick();
-  friend class C_Mon_Tick;
 
   // -- local storage --
 public:
@@ -329,14 +336,6 @@ private:
    */
   version_t sync_last_committed_floor;
 
-  struct C_SyncTimeout : public Context {
-    Monitor *mon;
-    explicit C_SyncTimeout(Monitor *m) : mon(m) {}
-    void finish(int r) override {
-      mon->sync_timeout();
-    }
-  };
-
   /**
    * Obtain the synchronization target prefixes in set form.
    *
@@ -496,14 +495,6 @@ private:
    */
   Context *timecheck_event;
 
-  struct C_TimeCheck : public Context {
-    Monitor *mon;
-    explicit C_TimeCheck(Monitor *m) : mon(m) { }
-    void finish(int r) override {
-      mon->timecheck_start_round();
-    }
-  };
-
   void timecheck_start();
   void timecheck_finish();
   void timecheck_start_round();
@@ -539,15 +530,7 @@ private:
    */
   void handle_ping(MonOpRequestRef op);
 
-  Context *probe_timeout_event;  // for probing
-
-  struct C_ProbeTimeout : public Context {
-    Monitor *mon;
-    explicit C_ProbeTimeout(Monitor *m) : mon(m) {}
-    void finish(int r) override {
-      mon->probe_timeout(r);
-    }
-  };
+  Context *probe_timeout_event = nullptr;  // for probing
 
   void reset_probe_timeout();
   void cancel_probe_timeout();
@@ -706,29 +689,8 @@ public:
     }
   } health_status_cache;
 
-  struct C_HealthToClogTick : public Context {
-    Monitor *mon;
-    explicit C_HealthToClogTick(Monitor *m) : mon(m) { }
-    void finish(int r) override {
-      if (r < 0)
-        return;
-      mon->do_health_to_clog();
-      mon->health_tick_start();
-    }
-  };
-
-  struct C_HealthToClogInterval : public Context {
-    Monitor *mon;
-    explicit C_HealthToClogInterval(Monitor *m) : mon(m) { }
-    void finish(int r) override {
-      if (r < 0)
-        return;
-      mon->do_health_to_clog_interval();
-    }
-  };
-
-  Context *health_tick_event;
-  Context *health_interval_event;
+  Context *health_tick_event = nullptr;
+  Context *health_interval_event = nullptr;
 
   void health_tick_start();
   void health_tick_stop();
index 84e7327cf8b0dcbd614a0a7d718f2e8f363837d0..2ca66b24d58f0a9f210a9d43609dbef2b3fb5e72 100644 (file)
@@ -37,61 +37,6 @@ static ostream& _prefix(std::ostream *_dout, Monitor *mon, const string& name,
                << ") ";
 }
 
-class Paxos::C_CollectTimeout : public Context {
-  Paxos *paxos;
-public:
-  explicit C_CollectTimeout(Paxos *p) : paxos(p) {}
-  void finish(int r) override {
-    if (r == -ECANCELED)
-      return;
-    paxos->collect_timeout();
-  }
-};
-
-class Paxos::C_AcceptTimeout : public Context {
-  Paxos *paxos;
-public:
-  explicit C_AcceptTimeout(Paxos *p) : paxos(p) {}
-  void finish(int r) override {
-    if (r == -ECANCELED)
-      return;
-    paxos->accept_timeout();
-  }
-};
-
-class Paxos::C_LeaseAckTimeout : public Context {
-  Paxos *paxos;
-public:
-  explicit C_LeaseAckTimeout(Paxos *p) : paxos(p) {}
-  void finish(int r) override {
-    if (r == -ECANCELED)
-      return;
-    paxos->lease_ack_timeout();
-  }
-};
-
-class Paxos::C_LeaseTimeout : public Context {
-  Paxos *paxos;
-public:
-  explicit C_LeaseTimeout(Paxos *p) : paxos(p) {}
-  void finish(int r) override {
-    if (r == -ECANCELED)
-      return;
-    paxos->lease_timeout();
-  }
-};
-
-class Paxos::C_LeaseRenew : public Context {
-  Paxos *paxos;
-public:
-  explicit C_LeaseRenew(Paxos *p) : paxos(p) {}
-  void finish(int r) override {
-    if (r == -ECANCELED)
-      return;
-    paxos->lease_renew_timeout();
-  }
-};
-
 class Paxos::C_Trimmed : public Context {
   Paxos *paxos;
 public:
@@ -249,7 +194,11 @@ void Paxos::collect(version_t oldpn)
   }
 
   // set timeout event
-  collect_timeout_event = new C_CollectTimeout(this);
+  collect_timeout_event = new C_MonContext(mon, [this](int r) {
+       if (r == -ECANCELED)
+         return;
+       collect_timeout();
+    });
   mon->timer.add_event_after(g_conf->mon_accept_timeout_factor *
                             g_conf->mon_lease,
                             collect_timeout_event);
@@ -737,7 +686,11 @@ void Paxos::begin(bufferlist& v)
   }
 
   // set timeout event
-  accept_timeout_event = new C_AcceptTimeout(this);
+  accept_timeout_event = new C_MonContext(mon, [this](int r) {
+      if (r == -ECANCELED)
+       return;
+      accept_timeout();
+    });
   mon->timer.add_event_after(g_conf->mon_accept_timeout_factor *
                             g_conf->mon_lease,
                             accept_timeout_event);
@@ -1023,14 +976,22 @@ void Paxos::extend_lease()
   // set timeout event.
   //  if old timeout is still in place, leave it.
   if (!lease_ack_timeout_event) {
-    lease_ack_timeout_event = new C_LeaseAckTimeout(this);
+    lease_ack_timeout_event = new C_MonContext(mon, [this](int r) {
+       if (r == -ECANCELED)
+         return;
+       lease_ack_timeout();
+      });
     mon->timer.add_event_after(g_conf->mon_lease_ack_timeout_factor *
                               g_conf->mon_lease,
                               lease_ack_timeout_event);
   }
 
   // set renew event
-  lease_renew_event = new C_LeaseRenew(this);
+  lease_renew_event = new C_MonContext(mon, [this](int r) {
+      if (r == -ECANCELED)
+       return;
+      lease_renew_timeout();
+    });
   utime_t at = lease_expire;
   at -= g_conf->mon_lease;
   at += g_conf->mon_lease_renew_interval_factor * g_conf->mon_lease;
@@ -1213,7 +1174,11 @@ void Paxos::reset_lease_timeout()
   dout(20) << "reset_lease_timeout - setting timeout event" << dendl;
   if (lease_timeout_event)
     mon->timer.cancel_event(lease_timeout_event);
-  lease_timeout_event = new C_LeaseTimeout(this);
+  lease_timeout_event = new C_MonContext(mon, [this](int r) {
+      if (r == -ECANCELED)
+       return;
+      lease_timeout();
+    });
   mon->timer.add_event_after(g_conf->mon_lease_ack_timeout_factor *
                             g_conf->mon_lease,
                             lease_timeout_event);
index f666c8f98024da2bec5ded3b3229816d3fad3ffa..a91ea100b00c50e943d5d5fe25781f4c9d5b109d 100644 (file)
@@ -100,22 +100,15 @@ bool PaxosService::dispatch(MonOpRequestRef op)
           * Callback class used to propose the pending value once the proposal_timer
           * fires up.
           */
-         class C_Propose : public Context {
-           PaxosService *ps;
-         public:
-           explicit C_Propose(PaxosService *p) : ps(p) { }
-           void finish(int r) override {
-             ps->proposal_timer = 0;
+         proposal_timer = new C_MonContext(mon, [this](int r) {
+             proposal_timer = 0;
              if (r >= 0)
-               ps->propose_pending();
+               propose_pending();
              else if (r == -ECANCELED || r == -EAGAIN)
                return;
              else
-               assert(0 == "bad return value for C_Propose");
-           }
-         };
-
-         proposal_timer = new C_Propose(this);
+               assert(0 == "bad return value for proposal_timer");
+           });
          dout(10) << " setting proposal_timer " << proposal_timer << " with delay of " << delay << dendl;
          mon->timer.add_event_after(delay, proposal_timer);
        } else { 
index bb550d1ca07302cba3dc62a891e6037a86dd5265..0e5d26aeee02b7876cebf3a4b1ce1e17e280c805 100644 (file)
@@ -117,8 +117,6 @@ protected:
   /**
    * @}
    */
-  friend class C_Propose;
-  
 
 public:
   /**
index f1100520a0d0753132511d4d2ce227d67339c60d..b354c40a77f77d459a9640901c68264879e58a0e 100644 (file)
 
 class QuorumService
 {
-  Context *tick_event;
+  Context *tick_event = nullptr;
   double tick_period;
 
-  struct C_Tick : public Context {
-    QuorumService *s;
-    C_Tick(QuorumService *qs) : s(qs) { }
-    void finish(int r) override {
-      if (r < 0)
-        return;
-      s->tick();
-    }
-  };
-
 public:
   enum {
     SERVICE_HEALTH                   = 0x01,
@@ -50,7 +40,6 @@ protected:
   epoch_t epoch;
 
   QuorumService(Monitor *m) :
-    tick_event(NULL),
     tick_period(g_conf->mon_tick_interval),
     mon(m),
     epoch(0)
@@ -70,7 +59,11 @@ protected:
     if (tick_period <= 0)
       return;
 
-    tick_event = new C_Tick(this);
+    tick_event = new C_MonContext(mon, [this](int r) {
+       if (r < 0)
+         return;
+       tick();
+      });
     mon->timer.add_event_after(tick_period, tick_event);
   }