Browse Source

Check restart limits on stop, rather than initiating restart first

With this change we can avoid a service going into "starting" state when
we really don't want to restart it.
Davin McCall 1 year ago
parent
commit
3303f6f3a6
5 changed files with 39 additions and 19 deletions
  1. 1 17
      src/baseproc-service.cc
  2. 25 0
      src/includes/proc-service.h
  3. 6 0
      src/includes/service.h
  4. 2 2
      src/proc-service.cc
  5. 5 0
      src/service.cc

+ 1 - 17
src/baseproc-service.cc

@@ -22,7 +22,7 @@
 
 void base_process_service::do_smooth_recovery() noexcept
 {
-    if (! restart_ps_process()) {
+    if (!restart_ps_process()) {
         unrecoverable_stop();
         services->process_queues();
     }
@@ -270,7 +270,6 @@ void base_process_service::do_restart() noexcept
     // be a regular restart.
 
     waiting_restart_timer = false;
-    restart_interval_count++;
     auto service_state = get_state();
 
     if (!start_ps_process(exec_arg_parts, have_console || onstart_flags.shares_console)) {
@@ -299,21 +298,6 @@ bool base_process_service::restart_ps_process() noexcept
     time_val current_time;
     event_loop.get_time(current_time, clock_type::MONOTONIC);
 
-    if (max_restart_interval_count != 0) {
-        // Check whether we're still in the most recent restart check interval:
-        time_val int_diff = current_time - restart_interval_time;
-        if (int_diff < restart_interval) {
-            if (restart_interval_count >= max_restart_interval_count) {
-                log(loglevel_t::ERROR, "Service ", get_name(), " restarting too quickly; stopping.");
-                return false;
-            }
-        }
-        else {
-            restart_interval_time = current_time;
-            restart_interval_count = 0;
-        }
-    }
-
     // Check if enough time has lapsed since the previous restart. If not, start a timer:
     time_val tdiff = current_time - last_start_time;
     if (restart_delay <= tdiff) {

+ 25 - 0
src/includes/proc-service.h

@@ -496,6 +496,31 @@ class process_service : public base_process_service
         }
     }
 
+    virtual bool check_restart() noexcept
+    {
+        if (max_restart_interval_count != 0) {
+            // Check whether we're still in the most recent restart check interval:
+            time_val current_time;
+            event_loop.get_time(current_time, clock_type::MONOTONIC);
+            time_val int_diff = current_time - restart_interval_time;
+            if (int_diff < restart_interval) {
+                // Within the restart limiting interval; check number of restarts
+                if (restart_interval_count >= max_restart_interval_count) {
+                    log(loglevel_t::ERROR, "Service ", get_name(), " restarting too quickly; stopping.");
+                    return false;
+                }
+                ++restart_interval_count;
+            }
+            else {
+                // Not within the last limiting interval; start a new interval
+                restart_interval_time = current_time;
+                restart_interval_count = 1;
+            }
+        }
+
+        return true;
+    }
+
     process_service(service_set *sset, const string &name, service_type_t s_type, ha_string &&command,
             std::list<std::pair<unsigned,unsigned>> &command_offsets,
             const std::list<prelim_dep> &depends_p)

+ 6 - 0
src/includes/service.h

@@ -437,6 +437,12 @@ class service_record
     // any appropriate cleanup.
     virtual void becoming_inactive() noexcept { }
 
+    // Check whether the service should automatically restart (assuming auto_restart is true)
+    virtual bool check_restart() noexcept
+    {
+        return true;
+    }
+
     public:
 
     service_record(service_set *set, const string &name)

+ 2 - 2
src/proc-service.cc

@@ -314,7 +314,7 @@ void process_service::handle_exit_status(bp_sys::exit_status exit_status) noexce
             initiate_start();
         }
     }
-    else if (smooth_recovery && service_state == service_state_t::STARTED) {
+    else if (smooth_recovery && service_state == service_state_t::STARTED && check_restart()) {
         // unexpected termination, with smooth recovery
         doing_smooth_recovery = true;
         do_smooth_recovery();
@@ -494,7 +494,7 @@ void bgproc_service::handle_exit_status(bp_sys::exit_status exit_status) noexcep
     }
     else {
         // we must be STARTED
-        if (smooth_recovery && get_target_state() == service_state_t::STARTED) {
+        if (smooth_recovery && get_target_state() == service_state_t::STARTED && check_restart()) {
             doing_smooth_recovery = true;
             do_smooth_recovery();
             if (get_state() != service_state_t::STARTED) {

+ 5 - 0
src/service.cc

@@ -605,6 +605,11 @@ void service_record::do_stop(bool with_restart) noexcept
     bool for_restart = with_restart || (auto_restart && desired_state == service_state_t::STARTED);
     bool restart_deps = with_restart;
 
+    if (!with_restart && for_restart) {
+        // auto restart - check for restarting too often
+        for_restart = check_restart();
+    }
+
     // If we won't restart, release explicit activation:
     if (!for_restart) {
         if (start_explicit) {