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: r1516930 - in /httpd/httpd/trunk: docs/manual/mod/mod_proxy.xml modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_ajp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
Date Fri, 23 Aug 2013 18:08:14 GMT


jim@apache.org wrote:
> Author: jim
> Date: Fri Aug 23 16:48:42 2013
> New Revision: 1516930
> 
> URL: http://svn.apache.org/r1516930
> Log:
> Allow for a simple socket check in addition to the
> higher level protocol-level checks for backends...
> 
> Not sure if it makes sense to do both or not... Comments?
> 
> Modified:
>     httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
>     httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
>     httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>     httpd/httpd/trunk/modules/proxy/proxy_util.c
> 
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Fri Aug 23 16:48:42 2013
> @@ -759,22 +759,35 @@ static int proxy_ajp_handler(request_rec
>  
>          /* Handle CPING/CPONG */
>          if (worker->s->ping_timeout_set) {
> -            status = ajp_handle_cping_cpong(backend->sock, r,
> -                                            worker->s->ping_timeout);
> -            /*
> -             * In case the CPING / CPONG failed for the first time we might be
> -             * just out of luck and got a faulty backend connection, but the
> -             * backend might be healthy nevertheless. So ensure that the backend
> -             * TCP connection gets closed and try it once again.
> -             */
> -            if (status != APR_SUCCESS) {
> -                backend->close = 1;
> -                ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
> -                              "cping/cpong failed to %pI (%s)",
> -                              worker->cp->addr, worker->s->hostname);
> -                status = HTTP_SERVICE_UNAVAILABLE;
> -                retry++;
> -                continue;
> +            if (worker->s->ping_timeout_set < 0) {

How can this < 0? It is either 0 or 1.
I guess it should be ping_timeout.

> +                if (!ap_proxy_is_socket_connected(backend->sock)) {
> +                    backend->close = 1;
> +                    ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO()
> +                                  "socket check failed to %pI (%s)",
> +                                  worker->cp->addr, worker->s->hostname);
> +                    status = HTTP_SERVICE_UNAVAILABLE;
> +                    retry++;
> +                    continue;
> +                }
> +            }
> +            else {
> +                status = ajp_handle_cping_cpong(backend->sock, r,
> +                                                worker->s->ping_timeout);
> +                /*
> +                 * In case the CPING / CPONG failed for the first time we might be
> +                 * just out of luck and got a faulty backend connection, but the
> +                 * backend might be healthy nevertheless. So ensure that the backend
> +                 * TCP connection gets closed and try it once again.
> +                 */
> +                if (status != APR_SUCCESS) {
> +                    backend->close = 1;
> +                    ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
> +                                  "cping/cpong failed to %pI (%s)",
> +                                  worker->cp->addr, worker->s->hostname);
> +                    status = HTTP_SERVICE_UNAVAILABLE;
> +                    retry++;
> +                    continue;
> +                }
>              }
>          }
>          /* Step Three: Process the Request */


To be honest. I think this additional check is a waste of cycles as we already know that the
socket is connected at this
point of time and even if it just disconnected it could happen also a few moments later when
we passed this check. So I
see no gain for the race condition we face regarding the connectivity state of the socket.
But as it is only done with
negative ping timeouts I am -0.

Regards

RĂ¼diger

Mime
View raw message