Re: environment variables

[Jan-Henrik Haukeland]

Martin Pala <address@hidden> writes:

>>* Event?  It is the "method" like "start", "stop"...?
>>
> We discussed the general event driven model, where all events were
> mapped by default to method names, but later we "slowed down". 

Hehe, yes, we got carried away there for a while; me suggesting to
rewrite everything and you planning a fancy new event model. Lucily we
"slowed down" :-) and looking at the monit code base now I agree with
Christian that the code is not half that bad and we should absolutely
be able to build on it and do more fine work with it in the future.

The key here is REFACTORING and for instance with regards to an event
model [PLEASE READ]:

You may think of validate.c as an event generator. This function
validate the system and if something is wrong it emits events via an
alert function. For instance alert_checksum. The event is filtered in
the local do_alert function in alert.c and dispatched to the
apropriate method. Since the only appropriate method now is to send a
mail this is what do_alert does - it calls sendmail. (As I was working
with the exec statement I planned for do_alert to also handle exec
events.)

So we do have the building blocks for an event driven model. In a way
we have a similar model to the the event model used in Java 1.0. This
model handled all sorts of events but it was to slow for handling GUI
events because every and I mean every GUI event went through this one
method (bottle-neck) which had a switch block for dispatching the
events (see code below). If you think of how many MOUSE_MOVED events
are generated by flipping the mouse you may understand why this model
was to slow and why the event model later was reimplemented in java
1.1. The new java 1.1 event model which is still in use, is based on
that you register a kind of callback for only those events you are
interested in. Thus if you're not interested in MOUSE_MOVED events
they will not be sent to your program which again makes your program
much faster.

Anyway, the first event model described (the one used by java 1.0) is
more than good enough for us. monit will only handle events when an
error occurs and certainly not any MOUSE_MOVED events :-) so our event
model does not have to be very fast.

To be very concrete, here is the GUI event dispatching method used in
java 1.0 found in the class java.awt.Component. (taken from the java
source code):

    /**
     * @deprecated As of JDK version 1.1
     * replaced by processEvent(AWTEvent).
     */
    public boolean handleEvent(Event evt) {
        switch (evt.id) {
          case Event.MOUSE_ENTER:
            return mouseEnter(evt, evt.x, evt.y);

          case Event.MOUSE_EXIT:
            return mouseExit(evt, evt.x, evt.y);

          case Event.MOUSE_MOVE:
            return mouseMove(evt, evt.x, evt.y);

          case Event.MOUSE_DOWN:
            return mouseDown(evt, evt.x, evt.y);

          case Event.MOUSE_DRAG:
            return mouseDrag(evt, evt.x, evt.y);

          case Event.MOUSE_UP:
            return mouseUp(evt, evt.x, evt.y);

          case Event.KEY_PRESS:
          case Event.KEY_ACTION:
            return keyDown(evt, evt.key);

          case Event.KEY_RELEASE:
          case Event.KEY_ACTION_RELEASE:
            return keyUp(evt, evt.key);

          case Event.ACTION_EVENT:
            return action(evt, evt.arg);
          case Event.GOT_FOCUS:
            return gotFocus(evt, evt.arg);
          case Event.LOST_FOCUS:
            return lostFocus(evt, evt.arg);
        }
        return false;
    }

[The Component class is the root class for every widget class in the
awt package and interfaces native GUI widgets in the local OS]

A bit simplified; The programmer would for instance use a sub-class of
Component (e.g. java.awt.Window) and implement the method
mouseEnter(evt, evt.x, evt.y) and when the mouse entered a window the
method would be called via the handleEvent method shown above (the
handleEvent method would be called by the OS/Window manager through
many layers in the JVM).

If you extrapolate you could say that validate.c which is our event
generator calls handleEvent (in our case handleEvent is do_alert in
alert.c and also in our case the calls goes not directly to do_alert,
but via the various alert_xxx methods. Optionally we could dispose of
the alert_xxx functions and instead call do_alert directly from
validate.c with the event as in do_alert(service, DO_TIMESTAMP, ..))

BTW, when I write this a couple of things occurs to me to fine tune
stuff.

 1) The Alert_T object should be renamed to Event_T
 2) The alert.c and alert_xxx methods should be renamed to event.c 
    and fire_xxx_event. For instance fire_restart_event(). 
 3) do_alert should be renamed to handle_event.

If you look in the current alert.c:do_alert you can see that it's kind
of a "switch" statement. The first (and only for the moment) check is
this:

   if(s->maillist)

which conseptually means; if a maillist is present the user has
registered interest for an alert event via the ALERT MAILADDR
statement in the control file. Likewise, for the exec statement I
planed to simply add

   if(s->execlist) { .. }

which would have meant that the user registered interest for an error
event that would call an external program. For instance in the control
file: if timestamp /foo/bar was changed exec "/bar/baz"


BTW, maybe instead we should call the s->maillist s->mail_event_list
to make it clear that the list is there for mail alert events? 


THE BOTTOM LINE: To create a clear-cut event model we should really
refactor validate.c

For instance we have code like this in validate.c:


     case TASK_PROCESS:

        ........

        for(pr= s->resourcelist; pr; pr= pr->next) {
      
          if(!check_resources(s, pr, report)) {
        
            switch(pr->action) {
            case ACTION_ALERT:
              if(s->def_timeout) s->nstart++;
              alert_resource(s, "Reason: %s\n", report);
              /* We are also interested in other alerts/stops/restarts! */
              pr->cycle=0;
              break;

            case ACTION_STOP:
              do_stop(s, "Reason: %s\n", report);
              reset_resource_counter(s);
              goto reinstall;

            case ACTION_RESTART:
              do_restart(s, "Reason: %s\n", report);
              reset_resource_counter(s);
              goto reinstall;

            default:
              log("'%s' Unknow resource failure action.\n", s->name);
              break;
            }

          }

It would look much nicer and cleaner if the code above was hidden in a
function and validate simply called:

  check_processes(..) 

where check_processes() contained all the code above and if an event
occured for instance if the process should be stopped
check_processes() would internally call handle_event() with a DO_STOP
event. If we do this, do_validate could simply look like:

static void do_validate(..) {

  check_processes(); // Including mem/cpu/children/ and if process is running
  check_device()
  check_file();
  check_directory();
  
}

every check_xxx function would generate an event in case of errors and
send the event, e.g DO_RESTART, DO_STOP to handle_event which would
again call control.c:do_start and so on.

This would require a rewrite of validate.c, not a minor task but the
result would be a nice modular design and a clearer event model. And
you have to agree that the different
   case TASK_PROCESS
   case TASK_DEVICE
   case TASK_FILE
   case TASK_DIRECTORY
in validate.c screams out to be refactored and put in it's own module
or function :-)


> Present event is subset of proposed model and it is choice of
> (alert.c):
>
> #define DO_TIMEOUT   0
> #define DO_RESTART   1
> #define DO_CHECKSUM  2
> #define DO_RESOURCE  3
> #define DO_STOP      4
> #define DO_FAILED    5
> #define DO_TIMESTAMP 6
>
> It could be, that the implementation will be close to the original
> proposal (which wasn't very realistic because of development
> resources) and events will comprise all "events" (like 'start', etc.)
> - we will see :)

Don't we already have it? I mean we have DO_STOP and DO_RESTART covers
start since it's really DO_(RE)START?

>>* What about MONIT_REPORT... the message generated for the mail?
>>
> I'm more inclined to give the application in environment only
> informations which will be needed. Application then could create the
> same message itself (monit's report contains just compilation of the
> same information with lot of 'user-friendly' words - i think it is not
> needed to give complete form of the message to method).

In principle I agree with Martin on this, but on the other hand
MONIT_REPORT could contain some info that is not easily deductible
from a script, for instance: 

  do_restart(s, "Reason: unable to read device state\n");

-- 
Jan-Henrik Haukeland