ping? On Aug 23, 2013, at 2:29 PM, Jim Jagielski <[email protected]> wrote:
> Anyone else think this isn't worthwhile? I'll revert. > > No sense in adding stuff if not "useful". > > On Aug 23, 2013, at 2:20 PM, Jim Jagielski <[email protected]> wrote: > >> I explained the rationale. >> >> Thx for the ping_timeout_set catch, fwiw >> >> On Aug 23, 2013, at 2:08 PM, Ruediger Pluem <[email protected]> wrote: >> >>> >>> >>> [email protected] 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 >>> >> >
