On Sun, Mar 29, 2009 at 10:25 PM, Mladen Turk <[email protected]> wrote:
> Paul Querna wrote:
>>
>> On Sun, Mar 29, 2009 at 9:17 PM,  <[email protected]> wrote:
>>>
>>> Author: mturk
>>> Date: Sun Mar 29 19:17:30 2009
>>> New Revision: 759751
>>>
>>> URL: http://svn.apache.org/viewvc?rev=759751&view=rev
>>> Log:
>>> Use child singleton watchdog for running the heartbeat module
>>>
>>> Modified:
>>>   httpd/httpd/trunk/modules/cluster/mod_heartbeat.c
>>
>> I'm looking at the API exposed by mod_watchdog.h, and its not quite
>> feeling right.
>>
>> If we want this to be an API used easily, I think we should just make
>> them proper timers (ie, run function X in singleton in 10 seconds),
>> and when the timer finishes, it can re-register -- this is far more
>> flexiable, and in the long term the API could be taken over by MPMs
>> that can better support it.
>>
>> WDYT?
>>
>
> There are actually two API's
> The one I implemented for heartbeat is the simple one,
> cause the heartbeat functionality is simple.

And actually the 'simple' API is the one I don't like.

Specifically, the watchdog_need hook. Using the parent/singleton
parameters and toggling them back and forth is what I don't like.

I think the whole hook section could be replaced with a single
provider, with a structure like this:

typedef struct ap_watchdog_provider_t ap_watchdog_provider_t;
struct ap_watchdog_provider_t {
    const char *name;
    apr_time_t interval;
    apr_uint32_t flags;
    void *baton;

    apr_status_t (*watchdog_init)(void *baton, apr_pool_t *pool);
    apr_status_t (*watchdog_step)(void *baton, apr_pool_t *pool);
    apr_status_t (*watchdog_exit)(void *baton, apr_pool_t *pool);
};

This would expose the ability to set it as parent or singleton via
flags, and as long as the interval was re-inspected after every step
call, you could change the interval.

With something like this, I'm not even sure you need the other API --
i'd prefer to provide one good API that works easily for everyone,
than 2 or more.

> The second one is where you create a watchdog instance
> with some interval and then it calls the provided
> callback. If the callback return != 0 it's not
> called any more. If you need a different interval,
> change it by API call.

hm.  I'm think I'd like to restructure the internals so that it could
be powered by an MPM (ie Simple or Event), or if the MPM isn't
available it would fallback to the modules functions.

> IMO having separate watchdogs is much better then
> having one that will call multiple callbacks with
> different timeouts because if one job blocks, others
> will not execute.
>
> BTW, we had this discussion few months back.

Yes, but I suck and completely failed to emailize my thoughts, sorry :)

Paul

Reply via email to