You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2009/03/30 20:39:15 UTC
Re: svn commit: r759862 - /httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
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
Re: svn commit: r759862 - /httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Mar 30, 2009, at 2:39 PM, Ruediger Pluem wrote:
>
> 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.
>
+1