[SSSD] [PATCH] Restart services with a delay in case they are restarted too often

Jakub Hrozek jhrozek at redhat.com
Sun Nov 18 17:09:02 UTC 2012


On Fri, Nov 16, 2012 at 11:41:50AM +0100, Pavel Březina wrote:
> On 11/15/2012 07:51 PM, Jakub Hrozek wrote:
> >In case a service is restarted while the DP is not ready yet, it gets
> >restarted again immediatelly, which means the DP might still not be
> >ready. The allowed number of restarts is then depleted quickly.
> >
> >This patch changes the restart mechanism such that the first restart
> >happens immediatelly, the second is scheduled after 2 second, then 4
> >etc..
> >
> >https://fedorahosted.org/sssd/ticket/1528
> 
> Hi,
> 
> >+    /* restarts are schedule after 0, 2, 4, ...seconds */
> >+    tv = tevent_timeval_current_ofs((svc->restarts << 1), 0);
> >+    te = tevent_add_timer(svc->mt_ctx->ev, svc, tv, mt_svc_restart, svc);
> 
> I would consider setting 4 seconds as a maximum delay instead of
> going to infinity. Any opinions?

I agree. I wanted to make the delays self-configurable but setting an
upper limit is the right thing to do.

> 
> >+static void mt_svc_restart(struct tevent_context *ev,
> >+                           struct tevent_timer *te,
> >+                           struct timeval t, void *ptr)
> >+{
> >+    struct mt_svc *svc;
> >+
> >+    svc = talloc_get_type(ptr, struct mt_svc);
> >+    if (svc == NULL) {
> >+        return;
> >+    }
> >+
> >+    DEBUG(SSSDBG_TRACE_FUNC,
> >+          ("Scheduling a service for restart %d\n", svc->restarts));
> 
> It should say svc->restarts + 1. Can you also add a service name to the
> message?

Done and done. Thank you for the review.
-------------- next part --------------
>From df88d01e110eda0229874f33c39be7b032ff5469 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 15 Nov 2012 19:26:18 +0100
Subject: [PATCH] Restart services with a delay in case they are restarted too
 often

In case a service is restarted while the DP is not ready yet, it gets
restarted again immediatelly, which means the DP might still not be
ready. The allowed number of restarts is then depleted quickly.

This patch changes the restart mechanism such that the first restart
happens immediatelly, the second is scheduled after 2 second, then 4
etc..

https://fedorahosted.org/sssd/ticket/1528
---
 src/monitor/monitor.c | 75 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 15 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index caa157138c19d18b61a77b1f9d4b29ac9e1fb64e..7d7099df3887e28fd6fec4e7320e428c19b90e00 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -64,6 +64,19 @@
  * doesn't shutdown on receiving SIGTERM */
 #define MONITOR_DEF_FORCE_TIME 60
 
+/* TODO: get the restart related values from config */
+#define MONITOR_RESTART_CNT_RESET   30
+/* maximum allowed number of service restarts if the restarts
+ * were less than MONITOR_RESTART_CNT_RESET apart, which would
+ * indicate a crash after startup or after every request */
+#define MONITOR_MAX_SVC_RESTARTS    2
+/* The services are restarted with a delay in case the restart was
+ * hitting a race condition where the DP is not ready yet either.
+ * The MONITOR_MAX_RESTART_DELAY defines the maximum delay between
+ * restarts.
+ */
+#define MONITOR_MAX_RESTART_DELAY   4
+
 /* name of the monitor server instance */
 #define MONITOR_NAME        "sssd"
 #define SSSD_PIDFILE_PATH   PID_PATH"/"MONITOR_NAME".pid"
@@ -2506,11 +2519,44 @@ static void service_startup_handler(struct tevent_context *ev,
     _exit(1);
 }
 
+static void mt_svc_restart(struct tevent_context *ev,
+                           struct tevent_timer *te,
+                           struct timeval t, void *ptr)
+{
+    struct mt_svc *svc;
+
+    svc = talloc_get_type(ptr, struct mt_svc);
+    if (svc == NULL) {
+        return;
+    }
+
+    DEBUG(SSSDBG_TRACE_FUNC, ("Scheduling service %s for restart %d\n",
+                              svc->name, svc->restarts+1));
+
+    if (svc->type == MT_SVC_SERVICE) {
+        add_new_service(svc->mt_ctx, svc->name, svc->restarts + 1);
+    } else if (svc->type == MT_SVC_PROVIDER) {
+        add_new_provider(svc->mt_ctx, svc->name, svc->restarts + 1);
+    } else {
+        /* Invalid type? */
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("BUG: Invalid child process type [%d]\n", svc->type));
+    }
+
+    /* Free the old service (which will also remove it
+     * from the child list)
+     */
+    talloc_free(svc);
+}
+
 static void mt_svc_exit_handler(int pid, int wait_status, void *pvt)
 {
     struct mt_svc *svc = talloc_get_type(pvt, struct mt_svc);
     struct mt_ctx *mt_ctx = svc->mt_ctx;
     time_t now = time(NULL);
+    struct tevent_timer *te;
+    struct timeval tv;
+    int restart_delay;
 
     if WIFEXITED(wait_status) {
         DEBUG(SSSDBG_OP_FAILURE,
@@ -2531,12 +2577,12 @@ static void mt_svc_exit_handler(int pid, int wait_status, void *pvt)
         return;
     }
 
-    if ((now - svc->last_restart) > 30) { /* TODO: get val from config */
+    if ((now - svc->last_restart) > MONITOR_RESTART_CNT_RESET) {
         svc->restarts = 0;
     }
 
     /* Restart the service */
-    if (svc->restarts > 2) { /* TODO: get val from config */
+    if (svc->restarts > MONITOR_MAX_SVC_RESTARTS) {
         DEBUG(SSSDBG_FATAL_FAILURE,
               ("Process [%s], definitely stopped!\n", svc->name));
         talloc_free(svc);
@@ -2546,20 +2592,19 @@ static void mt_svc_exit_handler(int pid, int wait_status, void *pvt)
         return;
     }
 
-    if (svc->type == MT_SVC_SERVICE) {
-        add_new_service(svc->mt_ctx, svc->name, svc->restarts + 1);
-    } else if (svc->type == MT_SVC_PROVIDER) {
-        add_new_provider(svc->mt_ctx, svc->name, svc->restarts + 1);
-    } else {
-        /* Invalid type? */
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              ("BUG: Invalid child process type [%d]\n", svc->type));
-    }
+    /* restarts are schedule after 0, 2, 4 seconds */
+    restart_delay = svc->restarts << 1;
+    restart_delay = (restart_delay > MONITOR_MAX_RESTART_DELAY) ? \
+                    MONITOR_MAX_RESTART_DELAY : \
+                    restart_delay;
 
-    /* Free the old service (which will also remove it
-     * from the child list)
-     */
-    talloc_free(svc);
+    tv = tevent_timeval_current_ofs(restart_delay, 0);
+    te = tevent_add_timer(svc->mt_ctx->ev, svc, tv, mt_svc_restart, svc);
+    if (!te) {
+        /* Nothing much we can do */
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory?!\n"));
+        return;
+    }
 }
 
 int main(int argc, const char *argv[])
-- 
1.8.0



More information about the sssd-devel mailing list