httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
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:20:31 GMT
I explained the rationale.

Thx for the ping_timeout_set catch, fwiw

On Aug 23, 2013, at 2:08 PM, Ruediger Pluem <rpluem@apache.org> wrote:

> 
> 
> 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