httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r759862 - /httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
Date Mon, 30 Mar 2009 18:39:15 GMT


On 03/30/2009 10:08 AM, mturk@apache.org wrote:
> Author: mturk
> Date: Mon Mar 30 08:07:59 2009
> New Revision: 759862
> 
> URL: http://svn.apache.org/viewvc?rev=759862&view=rev
> Log:
> Use named watchdog for heartmonitor.
> The watchdog has zero interval, leaving to the callback to determine the running loop.
> 
> Modified:
>     httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
> 
> Modified: httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c?rev=759862&r1=759861&r2=759862&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c (original)
> +++ httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c Mon Mar 30 08:07:59 2009

> @@ -299,194 +302,114 @@
>  #define apr_time_from_msec(x) (x * 1000)
>  #endif
>  
> -static void* APR_THREAD_FUNC hm_worker(apr_thread_t *thd, void *data)
> -{
> -    apr_time_t last;
> -    hm_ctx_t *ctx = (hm_ctx_t *) data;
> -    apr_status_t rv;
> -
> -    ctx->p = apr_thread_pool_get(thd);
> -    ctx->status = APR_SUCCESS;
> -    ctx->keep_running = 1;
> -    apr_thread_mutex_unlock(ctx->start_mtx);
> -
> -    while (ctx->keep_running) {
> -        rv = apr_proc_mutex_trylock(ctx->mutex);
> -        if (rv == APR_SUCCESS) {
> -            break;
> -        }
> -        apr_sleep(apr_time_from_msec(200));
> -    }
> -
> -    rv = hm_listen(ctx);
> -
> -    if (rv) {
> -        ctx->status = rv;
> -        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, NULL,
> -                     "Heartmonitor: Unable to listen for connections!");
> -        apr_proc_mutex_unlock(ctx->mutex);
> -        apr_thread_exit(ctx->thread, rv);
> -        return NULL;
> -    }
> -
> -
> -    last = apr_time_now();
> -    while (ctx->keep_running) {
> -        int n;
> -        apr_pool_t *p;
> -        apr_pollfd_t pfd;
> -        apr_interval_time_t timeout;
> -        apr_time_t now;
> -        apr_pool_create(&p, ctx->p);
> -
> -        now = apr_time_now();
> -
> -        if (apr_time_sec((now - last)) > HN_UPDATE_SEC) {
> -            hm_update_stats(ctx, p);
> -            apr_pool_clear(p);
> -            last = now;
> -        }
> -
> -        pfd.desc_type = APR_POLL_SOCKET;
> -        pfd.desc.s = ctx->sock;
> -        pfd.p = p;
> -        pfd.reqevents = APR_POLLIN;
> -
> -        timeout = apr_time_from_sec(1);
> -
> -        rv = apr_poll(&pfd, 1, &n, timeout);
> -
> -        if (!ctx->keep_running) {
> -            break;
> -        }
> -
> -        if (rv) {
> -            apr_pool_destroy(p);
> -            continue;
> -        }
> -
> -        if (pfd.rtnevents & APR_POLLIN) {
> -            hm_recv(ctx, p);
> -        }
> -
> -        apr_pool_destroy(p);
> -    }
> -
> -    apr_proc_mutex_unlock(ctx->mutex);
> -    apr_thread_exit(ctx->thread, APR_SUCCESS);
> -
> -    return NULL;
> -}
> -
> -static apr_status_t hm_pool_cleanup(void *baton)
> -{
> -    apr_status_t rv;
> -    hm_ctx_t *ctx = (hm_ctx_t *) baton;
> -
> -    ctx->keep_running = 0;
>  
> -    apr_thread_join(&rv, ctx->thread);
> -
> -    return rv;
> -}
> -
> -static void start_hm_worker(apr_pool_t *p, hm_ctx_t *ctx)
> +static apr_status_t hm_watchdog_callback(int state, void *data,
> +                                         apr_pool_t *pool)
>  {
> -    apr_status_t rv;
> -
> -    rv = apr_thread_mutex_create(&ctx->start_mtx, APR_THREAD_MUTEX_UNNESTED,
> -                                 p);
> -
> -    if (rv) {
> -        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, NULL,
> -                     "Heartmonitor: apr_thread_mutex_create failed");
> -        ctx->status = rv;
> -        return;
> -    }
> -
> -    /* This mutex fixes problems with a fast start/fast end, where the pool 
> -     * cleanup was being invoked before the thread completely spawned. 
> -     */
> -    apr_thread_mutex_lock(ctx->start_mtx);
> -
> -    apr_pool_cleanup_register(p, ctx, hm_pool_cleanup, apr_pool_cleanup_null);
> -
> -    rv = apr_thread_create(&ctx->thread, NULL, hm_worker, ctx, p);
> -    if (rv) {
> -        apr_pool_cleanup_kill(p, ctx, hm_pool_cleanup);
> -        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, NULL,
> -                     "Heartmonitor: apr_thread_create failed");
> -        ctx->status = rv;
> -    }
> -
> -    apr_thread_mutex_lock(ctx->start_mtx);
> -    apr_thread_mutex_unlock(ctx->start_mtx);
> -    apr_thread_mutex_destroy(ctx->start_mtx);
> -}
> -
> -static void hm_child_init(apr_pool_t *p, server_rec *s)
> -{
> -    hm_ctx_t *ctx =
> -        ap_get_module_config(s->module_config, &heartmonitor_module);
> +    apr_status_t rv = APR_SUCCESS;
> +    apr_time_t cur, now;
> +    hm_ctx_t *ctx = (hm_ctx_t *)data;
>  
>      if (!ctx->active) {
> -        return;
> +        return rv;
>      }
>  
> -    apr_proc_mutex_child_init(&ctx->mutex, ctx->mutex_path, p);
> -
> -    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s,
> -                 "Heartmonitor: Starting Listener Thread. mcast=%pI",
> -                 ctx->mcast_addr);
> -
> -    ctx->status = APR_EGENERAL;
> -
> -    start_hm_worker(p, ctx);
> -
> -    if (ctx->status) {
> -        ap_log_error(APLOG_MARK, APLOG_CRIT, ctx->status, s,
> -                     "Heartmonitor: Failed to start listener thread.");
> -        return;
> +    switch (state) {
> +        case AP_WATCHDOG_STATE_STARTING:
> +            rv = hm_listen(ctx);
> +            if (rv) {
> +                ctx->status = rv;
> +                ap_log_error(APLOG_MARK, APLOG_CRIT, rv, NULL,
> +                             "Heartmonitor: Unable to listen for connections!");
> +            }
> +            else {
> +                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ctx->s,
> +                             "Heartmonitor: %s listener started.",
> +                             HM_WATHCHDOG_NAME);
> +            }
> +        break;
> +        case AP_WATCHDOG_STATE_RUNNING:
> +            hm_update_stats(ctx, pool);
> +            cur = now = apr_time_sec(apr_time_now());
> +            /* TODO: Insted HN_UPDATE_SEC use
> +             * the ctx->interval
> +             */
> +            while ((now - cur) < apr_time_sec(ctx->interval)) {
> +                int n;
> +                apr_status_t rc;
> +                apr_pool_t *p;
> +                apr_pollfd_t pfd;
> +                apr_interval_time_t timeout;
> +
> +                apr_pool_create(&p, pool);
> +
> +                pfd.desc_type = APR_POLL_SOCKET;
> +                pfd.desc.s = ctx->sock;
> +                pfd.p = p;
> +                pfd.reqevents = APR_POLLIN;
> +
> +                timeout = apr_time_from_sec(1);
> +
> +                rc = apr_poll(&pfd, 1, &n, timeout);
> +
> +                if (!ctx->keep_running) {
> +                    break;
> +                }
> +                if (rc == APR_SUCCESS && (pfd.rtnevents & APR_POLLIN)) {
> +                    hm_recv(ctx, p);
> +                }
> +                now = apr_time_sec(apr_time_now());
> +                apr_pool_destroy(p);
> +            }

IMHO we should do apr_pool_create *once * before the loop and
apr_pool_destroy *after* the lookp. In the loop we should only use
apr_pool_clear. apr_pool_create and apr_pool_destroy require locking
which seems to be an unneeded overhead here.

Regards

RĂ¼diger

Mime
View raw message