monit http thread responsiveness patch

[Martin Pala]

Hi,

here is the patch which improves the responsiveness of the monit http 
thread (described at http://www.tildeslash.com/monit/doc/next.php#23).

Description of the patch:

When some action is requested via http, the http thread just sets the 
"doaction" task on the given service thread and wakes up the main 
thread. The main thread then performs normal testing cycle and when it 
founds the doaction flag on any service, it performs the requested 
action. Thus, the http thread is not blocked by driving the service 
stop/restart, etc. and returns almost immediately.

The patch also simplifies the code and is probably safer then the 
previous behavior (just one thread is responsible for service management).


What do you think? May i send it to cvs?

Martin


Index: control.c
===================================================================
RCS file: /sources/monit/monit/control.c,v
retrieving revision 1.103
diff -u -r1.103 control.c
--- control.c   27 Apr 2006 20:16:03 -0000      1.103
+++ control.c   27 Jun 2006 11:38:58 -0000
@@ -128,23 +128,40 @@
 /**
  * Check to see if we should try to start/stop service
  * @param S A service name as stated in the config file
- * @param action A string describing the action to execute
- * @return TRUE if the service was handled successfully otherwise FALSE
+ * @param A A string describing the action to execute
  */
-void control_service(const char *S, const char *action) {
+void control_service_string(const char *S, const char *A) {
+
+  int a;
+
+  ASSERT(S);
+  ASSERT(A);
+
+  if((a = Util_getAction(A)) == ACTION_IGNORE) {
+    LogError("%s: service '%s' -- invalid action %s\n", prog, S, A);
+    return;
+  }
+  control_service(S, a);
+}
+
+
+/**
+ * Check to see if we should try to start/stop service
+ * @param S A service name as stated in the config file
+ * @param A An action id describing the action to execute
+ */
+void control_service(const char *S, int A) {
 
   Service_T s = NULL;
 
   ASSERT(S);
-  ASSERT(action);
 
   if(!(s= Util_getService(S))) {
-    LogError("%s: Cannot %s service '%s' -- not found in %s\n",
-        prog, action, S, Run.controlfile);
+    LogError("%s: service '%s' -- doesn't exist\n", prog, S);
     return;
   }
 
-  switch(Util_getAction(action)) {
+  switch(A) {
 
     case ACTION_START:
       if(s->type==TYPE_PROCESS) {
@@ -212,8 +229,7 @@
       break;
 
     default:
-      LogError("%s: Cannot %s service '%s' -- invalid action %s\n",
-          prog, action, S, action);
+      LogError("%s: service '%s' -- invalid action %s\n", prog, S, A);
       break;
   }
 }
Index: event.c
===================================================================
RCS file: /sources/monit/monit/event.c,v
retrieving revision 1.61
diff -u -r1.61 event.c
--- event.c     27 Apr 2006 20:16:03 -0000      1.61
+++ event.c     27 Jun 2006 11:38:58 -0000
@@ -88,7 +88,7 @@
 
 
 static void handle_event(Event_T);
-static int  handle_action(Event_T, Action_T);
+static void handle_action(Event_T, Action_T);
 static void Event_queue_add(Event_T);
 
 
@@ -748,7 +748,7 @@
 }
 
 
-static int handle_action(Event_T E, Action_T A) {
+static void handle_action(Event_T E, Action_T A) {
 
   Service_T s;
 
@@ -759,7 +759,7 @@
 
   if(A->id == ACTION_IGNORE)
   {
-    return TRUE;
+    return;
   }
 
   /* Alert and collector event notification are common actions */
@@ -780,60 +780,40 @@
     }
   }
 
-  s = Event_get_source(E);
-  if(!s)
+  if(!(s = Event_get_source(E)))
   {
     LogError("Event action handling aborted\n");
-    return FALSE;
+    return;
   }
 
-  switch(A->id)
+  if(A->id == ACTION_ALERT)
   {
-  case ACTION_ALERT:
-      /* Already handled. */
-      break;
-
-  case ACTION_EXEC:
-      spawn(s, A->exec, Event_get_description(E));
-      break;
-
-  case ACTION_RESTART:
-      if(s->def_timeout)
-        s->nstart++;
-      if((s->mode != MODE_PASSIVE))
-      {
-        control_service(s->name, "restart");
-      }
-      return FALSE;
-
-  case ACTION_START:
-      if(s->def_timeout)
-        s->nstart++;
-      if((s->mode != MODE_PASSIVE))
-      {
-        control_service(s->name, "start");
-      } 
-      return FALSE;
+    return;     /* Already handled */
+  }
+  else if(A->id == ACTION_EXEC)
+  {
+    spawn(s, A->exec, Event_get_description(E));
+    return;
+  }
+  else 
+  {
+    if(s->def_timeout &&
+         (A->id == ACTION_START ||
+          A->id == ACTION_RESTART))
+    {
+      s->nstart++;
+    }
 
-  case ACTION_STOP:
-      if((s->mode != MODE_PASSIVE))
-      {
-        control_service(s->name, "stop");
-      } 
-      return FALSE;
-
-  case ACTION_UNMONITOR:
-      control_service(s->name, "unmonitor");
-      return FALSE;
-
-  default:
-      LogError("'%s' error -- unknown failure action: [%d]\n", s->name,
-        A->id);
-      break;
+    if(s->mode == MODE_PASSIVE &&
+         (A->id == ACTION_START ||
+          A->id == ACTION_STOP  ||
+          A->id == ACTION_RESTART))
+    {
+      return;
+    }
 
+    control_service(s->name, A->id);
   }
-
-  return TRUE;
 }
 
 
Index: monitor.c
===================================================================
RCS file: /sources/monit/monit/monitor.c,v
retrieving revision 1.129
diff -u -r1.129 monitor.c
--- monitor.c   27 Apr 2006 20:16:03 -0000      1.129
+++ monitor.c   27 Jun 2006 11:38:58 -0000
@@ -361,7 +361,7 @@
     if(service) {
 
       void (*_control_service)(const char *, const char *)=
-        exist_daemon()?control_service_daemon:control_service;
+        exist_daemon()?control_service_daemon:control_service_string;
 
       if(IS(service, "all")) {
 
Index: monitor.h
===================================================================
RCS file: /sources/monit/monit/monitor.h,v
retrieving revision 1.186
diff -u -r1.186 monitor.h
--- monitor.h   27 Apr 2006 20:56:41 -0000      1.186
+++ monitor.h   27 Jun 2006 11:38:58 -0000
@@ -665,6 +665,7 @@
   unsigned long long error;                          /**< Error flags bitmap */
   Info_T             inf;                          /**< Service check result */
   time_t             collected;                /**< When were data collected */
+  int                doaction;          /**< Action scheduled by http thread */
 
   /** Events */
   struct myevent {
@@ -788,7 +789,8 @@
 /* FIXME: move remaining prototypes into seperate header-files */
 
 int   parse(char *);
-void  control_service(const char *, const char *);
+void  control_service(const char *, int);
+void  control_service_string(const char *, const char *);
 void  control_service_daemon(const char *, const char *);
 void  setup_dependants();
 void  reset_depend();
Index: validate.c
===================================================================
RCS file: /sources/monit/monit/validate.c,v
retrieving revision 1.153
diff -u -r1.153 validate.c
--- validate.c  23 Jun 2006 21:51:25 -0000      1.153
+++ validate.c  27 Jun 2006 11:38:58 -0000
@@ -148,7 +148,10 @@
   for(s= servicelist; s; s= s->next) {
     LOCK(s->mutex)
       set_signal_block(&ns, &os);
-      if(s->monitor && !check_skip(s) && !check_timeout(s)) {
+      if(s->doaction != ACTION_IGNORE) {
+        control_service(s->name, s->doaction);
+        s->doaction = ACTION_IGNORE;
+      } else if(s->monitor && !check_skip(s) && !check_timeout(s)) {
         s->check(s);
         s->monitor= MONITOR_YES;
       }
Index: http/cervlet.c
===================================================================
RCS file: /sources/monit/monit/http/cervlet.c,v
retrieving revision 1.203
diff -u -r1.203 cervlet.c
--- http/cervlet.c      23 Jun 2006 20:47:39 -0000      1.203
+++ http/cervlet.c      27 Jun 2006 11:38:58 -0000
@@ -593,104 +593,31 @@
   
   if(Util_existService(++name)) {
 
-    Service_T s= Util_getService(name);
+    Service_T s;
 
-    if(s == NULL) {
+    if(!(s = Util_getService(name))) {
       send_error(res, SC_BAD_REQUEST, "There is no service by that name");
       return;
     }
-
     if(!action) {
-
       do_service(req, res, s);
-
     } else {
-      
       if(is_readonly(req)) {
        send_error(res, SC_FORBIDDEN,
                   "You do not have sufficent privilegs to access this page");
        return;
       }
-
-      if(IS(action, "start")) {
-       if(s->start) {
-         LOCK(s->mutex)
-             control_service(name, action);
-             if(s->type==TYPE_PROCESS) {
-               /* Wait for the service to start (or fail) */
-               int max_tries= Run.polltime;
-               while(max_tries-- && !Run.stopped) {
-                 if(Util_isProcessRunning(s))
-                     break;
-                 sleep(1);
-               }
-             }
-         END_LOCK;
-         if(s->type==TYPE_PROCESS && !Util_isProcessRunning(s)) {
-           send_error(res, SC_INTERNAL_SERVER_ERROR,
-                      "Could not start the service");
-           goto quit;
-         }
-       } else {
-          send_error(res, SC_BAD_REQUEST,
-                    "Start method not defined for the service");
-         goto quit;
-       }
-      } else if(IS(action, "stop")) {
-       if(s->stop) {
-         LOCK(s->mutex)
-             control_service(name, action);
-         END_LOCK;
-       } else {
-          send_error(res, SC_BAD_REQUEST,
-                    "Stop method not defined for the service");
-         goto quit;
-       }
-      } else if(IS(action, "restart")) {
-       if(s->start && s->stop) {
-         LOCK(s->mutex)
-             control_service(name, action);
-             if(s->type==TYPE_PROCESS) {
-               /* Wait for the service to restart (or fail) */
-               int max_tries= Run.polltime;
-               while(max_tries-- && !Run.stopped) {
-                 if(Util_isProcessRunning(s))
-                     break;
-                 sleep(1);
-               }
-             }
-         END_LOCK;
-         if(s->type==TYPE_PROCESS && !Util_isProcessRunning(s)) {
-           send_error(res, SC_INTERNAL_SERVER_ERROR,
-                      "Could not restart the service");
-           goto quit;
-         }
-       } else {
-          send_error(res, SC_BAD_REQUEST,
-                    "Start or stop method not defined for the service");
-         goto quit;
-       }
-      } else if(IS(action, "monitor")) {
-         LOCK(s->mutex)
-           control_service(name, action);
-         END_LOCK;
-      } else if(IS(action, "unmonitor")) {
-         LOCK(s->mutex)
-           control_service(name, action);
-         END_LOCK;
+      if((s->doaction = Util_getAction(action)) == ACTION_IGNORE) {
+        send_error(res, SC_BAD_REQUEST, "Invalid action");
+        return;
       }
-
+      do_wakeupcall();
       send_redirect(res, req->url);
-
     }
-
   } else {
-
     not_found(req, res);
-
   }
 
-  quit:
   reset_depend();
 
 }