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 >
