httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: error_time reset in proxy_util.c
Date Fri, 06 Mar 2015 16:01:35 GMT
On Fri, Mar 6, 2015 at 4:25 PM, Ruediger Pluem <rpluem@apache.org> wrote:
>
> Index: proxy_util.c
> ===================================================================
> --- proxy_util.c        (revision 1664261)
> +++ proxy_util.c        (working copy)
> @@ -2887,14 +2887,15 @@
>
>          connected    = 1;
>      }
> +    if (PROXY_WORKER_IS_USABLE(worker)) {
>      /*
>       * Put the entire worker to error state if
>       * the PROXY_WORKER_IGNORE_ERRORS flag is not set.
>       * Altrough some connections may be alive
>       * no further connections to the worker could be made
>       */
> -    if (!connected && PROXY_WORKER_IS_USABLE(worker) &&
> -        !(worker->s->status & PROXY_WORKER_IGNORE_ERRORS)) {
> +        if (!connected) {
> +            if (!(worker->s->status & PROXY_WORKER_IGNORE_ERRORS)) {
>          worker->s->error_time = apr_time_now();
>          worker->s->status |= PROXY_WORKER_IN_ERROR;
>          ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00959)
> @@ -2902,6 +2903,7 @@
>              APR_TIME_T_FMT "s",
>              worker->s->hostname, apr_time_sec(worker->s->retry));
>      }
> +        }
>      else {
>          if (worker->s->retries) {
>              /*
> @@ -2915,6 +2917,19 @@
>      }
>      return connected ? OK : DECLINED;
>  }
> +    else {
> +        /*
> +         * The worker is in error likely done by a different thread / process
> +         * e.g. for a timeout or bad status. We should respect this and should
> +         * not continue with a connection via this worker even if we got one.
> +         */
> +        if (connected) {
> +            apr_socket_close(conn->sock );
> +            conn->sock = NULL;
> +        }
> +        return DECLINED;

If the worker is *not a BalancerMember*, couldn't we simply mark the
connection as close = 1 here, and continue with it until the end of
this request?
This is to avoid responding 503 whereas it succeed for this particular
request (since this is a race, the worker was put in error state a
very little time ago, not at ap_proxy_acquire_connection() time for
this request at least).

Otherwise, I agree with the move of :
+            worker->s->error_time = 0;
+            worker->s->retries = 0;
in the correct block.

Regards,
Yann.

Mime
View raw message