You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2013/08/23 18:48:42 UTC
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
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/docs/manual/mod/mod_proxy.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23 16:48:42 2013
@@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
<tr><td>ping</td>
<td>0</td>
<td>Ping property tells the webserver to "test" the connection to
- the backend before forwarding the request. For AJP, it causes
+ the backend before forwarding the request. For negative values
+ the test is a simple socket check, for positive values it's
+ a more functional check, dependent upon the protocol. For AJP, it causes
<module>mod_proxy_ajp</module>to send a <code>CPING</code>
request on the ajp13 connection (implemented on Tomcat 3.3.2+, 4.1.28+
and 5.0.13+). For HTTP, it causes <module>mod_proxy_http</module>
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42 2013
@@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
*/
if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
return "Ping/Pong timeout has wrong format";
- if (timeout < 1000)
+ if (timeout < 1000 && timeout >= 0)
return "Ping/Pong timeout must be at least one millisecond";
worker->s->ping_timeout = timeout;
worker->s->ping_timeout_set = 1;
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42 2013
@@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
(request_rec *r, apr_table_t *headers));
+
+/**
+ * @param socket socket to test
+ * @return TRUE if socket is connected/active
+ */
+PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
+
#define PROXY_LBMETHOD "proxylbmethod"
/* The number of dynamic workers that can be added when reconfiguring.
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) {
+ 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 */
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42 2013
@@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
}
}
+ /* Step Three-and-a-Half: See if the socket is still connected (if desired) */
+ if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
+ !ap_proxy_is_socket_connected(backend->sock)) {
+ backend->close = 1;
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
+ "socket check failed to %pI (%s)",
+ worker->cp->addr, worker->s->hostname);
+ retry++;
+ continue;
+ }
+
/* Step Four: Send the Request
* On the off-chance that we forced a 100-Continue as a
* kinda HTTP ping test, allow for retries
*/
if ((status = ap_proxy_http_request(p, r, backend, worker,
conf, uri, locurl, server_portstr)) != OK) {
- if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set) {
+ if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
+ worker->s->ping_timeout > 0) {
backend->close = 1;
ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
"HTTP: 100-Continue failed to %pI (%s)",
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42 2013
@@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
#endif
#if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
-static int is_socket_connected(apr_socket_t *socket)
+PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
{
apr_pollfd_t pfds[1];
apr_status_t status;
@@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
}
#else
-static int is_socket_connected(apr_socket_t *sock)
+PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
{
apr_size_t buffer_len = 1;
@@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
(proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
if (conn->sock) {
- if (!(connected = is_socket_connected(conn->sock))) {
+ if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
socket_cleanup(conn);
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
"%s: backend socket is disconnected.",
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
Committed revision 1530603.
thx again!
On Oct 8, 2013, at 4:24 PM, Yann Ylavic <yl...@gmail.com> wrote:
> Sure, here it is.
>
> Please note 2 chances compared to the previous patch (pasted) :
> - the slow path ap_request_has_body used last to compute do_100_continue,
> - step Three-and-a-Half moved into step 3, with the associated comment updated.
>
> For the latter change, it avoids the systematic double-check (needlessly, IMHO) when light ping is configured and the connection is reused.
>
> Thank you for taking that into account.
>
>
>
> On Tue, Oct 8, 2013 at 8:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> OK, I gotcha now...
>
> Do you have a patch file? tia!
>
> On Oct 8, 2013, at 12:56 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> > On Tue, Oct 8, 2013 at 6:26 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> > Does it matter whether or not it's a heavy ping or not?
> > It doesn't matter what sort of test was used, the socket is down.
> >
> > Yes it is down, but for the ajp case for example, that determines the return value GATEWAY_TIMEOUT vs INTERNAL_SERVER_ERROR, and according to the comments, the former is when cping/cpong was used and that will not "affect the whole worker", does it make sense when the light ping (socket health-check) only was done before forwarding?
> >
> > For the ap_proxy_http_request error case, should mod_proxy retry a new request when the heavy ping is off (light ping on)?
> >
> > For ap_proxy_create_hdrbrgd and ap_proxy_process_response, the "100 continue" things have nothing to do with light ping only, have they?
> >
> > Regards.
> >
> >
> > On Oct 8, 2013, at 11:53 AM, Yann Ylavic <yl...@gmail.com> wrote:
> > > Hi,
> > >
> > > Some code in trunk still only check for ping_timeout_set without ping_timeout positive value to handle the "heavy" ping (CPING/Expect: 100-continue), see patch below.
> > >
> > > Regards,
> > > Yann.
> > >
> > > Index: modules/proxy/mod_proxy_ajp.c
> > > ===================================================================
> > > --- modules/proxy/mod_proxy_ajp.c (revision 1530243)
> > > +++ modules/proxy/mod_proxy_ajp.c (working copy)
> > > @@ -341,7 +341,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > > * we assume it is a request that cause a back-end timeout,
> > > * but doesn't affect the whole worker.
> > > */
> > > - if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set) {
> > > + if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set
> > > + && conn->worker->s->ping_timeout >= 0) {
> > > return HTTP_GATEWAY_TIME_OUT;
> > > }
> > >
> > > @@ -661,7 +662,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > > * we assume it is a request that cause a back-end timeout,
> > > * but doesn't affect the whole worker.
> > > */
> > > - if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set) {
> > > + if (APR_STATUS_IS_TIMEUP(status) &&
> > > + conn->worker->s->ping_timeout_set &&
> > > + conn->worker->s->ping_timeout >= 0) {
> > > apr_table_set(r->notes, "proxy_timedout", "1");
> > > rv = HTTP_GATEWAY_TIME_OUT;
> > > }
> > > Index: modules/proxy/proxy_util.c
> > > ===================================================================
> > > --- modules/proxy/proxy_util.c (revision 1530243)
> > > +++ modules/proxy/proxy_util.c (working copy)
> > > @@ -3073,6 +3073,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
> > > * We also make sure we won't be talking HTTP/1.0 as well.
> > > */
> > > do_100_continue = (worker->s->ping_timeout_set
> > > + && (worker->s->ping_timeout >= 0)
> > > && ap_request_has_body(r)
> > > && (PROXYREQ_REVERSE == r->proxyreq)
> > > && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> > > Index: modules/proxy/mod_proxy_http.c
> > > ===================================================================
> > > --- modules/proxy/mod_proxy_http.c (revision 1530243)
> > > +++ modules/proxy/mod_proxy_http.c (working copy)
> > > @@ -1241,6 +1241,7 @@ int ap_proxy_http_process_response(apr_pool_t * p,
> > > dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
> > >
> > > do_100_continue = (worker->s->ping_timeout_set
> > > + && (worker->s->ping_timeout >= 0)
> > > && ap_request_has_body(r)
> > > && (PROXYREQ_REVERSE == r->proxyreq)
> > > && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> > > @@ -1992,8 +1993,9 @@ static int proxy_http_handler(request_rec *r, prox
> > > */
> > > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > > conf, uri, locurl, server_portstr)) != OK) {
> > > - if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
> > > - worker->s->ping_timeout > 0) {
> > > + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> > > + worker->s->ping_timeout_set &&
> > > + worker->s->ping_timeout >= 0) {
> > > backend->close = 1;
> > > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
> > > "HTTP: 100-Continue failed to %pI (%s)",
> > >
> > >
> > >
> > > On Fri, Aug 23, 2013 at 6:48 PM, <ji...@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/docs/manual/mod/mod_proxy.xml
> > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
> > > ==============================================================================
> > > --- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
> > > +++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23 16:48:42 2013
> > > @@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
> > > <tr><td>ping</td>
> > > <td>0</td>
> > > <td>Ping property tells the webserver to "test" the connection to
> > > - the backend before forwarding the request. For AJP, it causes
> > > + the backend before forwarding the request. For negative values
> > > + the test is a simple socket check, for positive values it's
> > > + a more functional check, dependent upon the protocol. For AJP, it causes
> > > <module>mod_proxy_ajp</module>to send a <code>CPING</code>
> > > request on the ajp13 connection (implemented on Tomcat 3.3.2+, 4.1.28+
> > > and 5.0.13+). For HTTP, it causes <module>mod_proxy_http</module>
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > > ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> > > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42 2013
> > > @@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
> > > */
> > > if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
> > > return "Ping/Pong timeout has wrong format";
> > > - if (timeout < 1000)
> > > + if (timeout < 1000 && timeout >= 0)
> > > return "Ping/Pong timeout must be at least one millisecond";
> > > worker->s->ping_timeout = timeout;
> > > worker->s->ping_timeout_set = 1;
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
> > > ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> > > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42 2013
> > > @@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
> > > APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
> > > (request_rec *r, apr_table_t *headers));
> > >
> > > +
> > > +/**
> > > + * @param socket socket to test
> > > + * @return TRUE if socket is connected/active
> > > + */
> > > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
> > > +
> > > #define PROXY_LBMETHOD "proxylbmethod"
> > >
> > > /* The number of dynamic workers that can be added when reconfiguring.
> > >
> > > 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) {
> > > + 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 */
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > > ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> > > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42 2013
> > > @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> > > }
> > > }
> > >
> > > + /* Step Three-and-a-Half: See if the socket is still connected (if desired) */
> > > + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
> > > + !ap_proxy_is_socket_connected(backend->sock)) {
> > > + backend->close = 1;
> > > + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> > > + "socket check failed to %pI (%s)",
> > > + worker->cp->addr, worker->s->hostname);
> > > + retry++;
> > > + continue;
> > > + }
> > > +
> > > /* Step Four: Send the Request
> > > * On the off-chance that we forced a 100-Continue as a
> > > * kinda HTTP ping test, allow for retries
> > > */
> > > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > > conf, uri, locurl, server_portstr)) != OK) {
> > > - if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set) {
> > > + if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
> > > + worker->s->ping_timeout > 0) {
> > > backend->close = 1;
> > > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
> > > "HTTP: 100-Continue failed to %pI (%s)",
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > > ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42 2013
> > > @@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
> > > #endif
> > >
> > > #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> > > -static int is_socket_connected(apr_socket_t *socket)
> > > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> > > {
> > > apr_pollfd_t pfds[1];
> > > apr_status_t status;
> > > @@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
> > >
> > > }
> > > #else
> > > -static int is_socket_connected(apr_socket_t *sock)
> > > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> > >
> > > {
> > > apr_size_t buffer_len = 1;
> > > @@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> > > (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
> > >
> > > if (conn->sock) {
> > > - if (!(connected = is_socket_connected(conn->sock))) {
> > > + if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> > > socket_cleanup(conn);
> > > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> > > "%s: backend socket is disconnected.",
> > >
> > >
> > >
> >
> >
>
>
> <httpd-trunk-mod_proxy_ping_timeout.patch>
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
Posted by Yann Ylavic <yl...@gmail.com>.
Sure, here it is.
Please note 2 chances compared to the previous patch (pasted) :
- the slow path ap_request_has_body used last to compute do_100_continue,
- step Three-and-a-Half moved into step 3, with the associated comment
updated.
For the latter change, it avoids the systematic double-check (needlessly,
IMHO) when light ping is configured and the connection is reused.
Thank you for taking that into account.
On Tue, Oct 8, 2013 at 8:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> OK, I gotcha now...
>
> Do you have a patch file? tia!
>
> On Oct 8, 2013, at 12:56 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> > On Tue, Oct 8, 2013 at 6:26 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> > Does it matter whether or not it's a heavy ping or not?
> > It doesn't matter what sort of test was used, the socket is down.
> >
> > Yes it is down, but for the ajp case for example, that determines the
> return value GATEWAY_TIMEOUT vs INTERNAL_SERVER_ERROR, and according to the
> comments, the former is when cping/cpong was used and that will not "affect
> the whole worker", does it make sense when the light ping (socket
> health-check) only was done before forwarding?
> >
> > For the ap_proxy_http_request error case, should mod_proxy retry a new
> request when the heavy ping is off (light ping on)?
> >
> > For ap_proxy_create_hdrbrgd and ap_proxy_process_response, the "100
> continue" things have nothing to do with light ping only, have they?
> >
> > Regards.
> >
> >
> > On Oct 8, 2013, at 11:53 AM, Yann Ylavic <yl...@gmail.com> wrote:
> > > Hi,
> > >
> > > Some code in trunk still only check for ping_timeout_set without
> ping_timeout positive value to handle the "heavy" ping (CPING/Expect:
> 100-continue), see patch below.
> > >
> > > Regards,
> > > Yann.
> > >
> > > Index: modules/proxy/mod_proxy_ajp.c
> > > ===================================================================
> > > --- modules/proxy/mod_proxy_ajp.c (revision 1530243)
> > > +++ modules/proxy/mod_proxy_ajp.c (working copy)
> > > @@ -341,7 +341,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > > * we assume it is a request that cause a back-end timeout,
> > > * but doesn't affect the whole worker.
> > > */
> > > - if (APR_STATUS_IS_TIMEUP(status) &&
> conn->worker->s->ping_timeout_set) {
> > > + if (APR_STATUS_IS_TIMEUP(status) &&
> conn->worker->s->ping_timeout_set
> > > + && conn->worker->s->ping_timeout >= 0) {
> > > return HTTP_GATEWAY_TIME_OUT;
> > > }
> > >
> > > @@ -661,7 +662,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > > * we assume it is a request that cause a back-end
> timeout,
> > > * but doesn't affect the whole worker.
> > > */
> > > - if (APR_STATUS_IS_TIMEUP(status) &&
> conn->worker->s->ping_timeout_set) {
> > > + if (APR_STATUS_IS_TIMEUP(status) &&
> > > + conn->worker->s->ping_timeout_set &&
> > > + conn->worker->s->ping_timeout >= 0) {
> > > apr_table_set(r->notes, "proxy_timedout", "1");
> > > rv = HTTP_GATEWAY_TIME_OUT;
> > > }
> > > Index: modules/proxy/proxy_util.c
> > > ===================================================================
> > > --- modules/proxy/proxy_util.c (revision 1530243)
> > > +++ modules/proxy/proxy_util.c (working copy)
> > > @@ -3073,6 +3073,7 @@ PROXY_DECLARE(int)
> ap_proxy_create_hdrbrgd(apr_poo
> > > * We also make sure we won't be talking HTTP/1.0 as well.
> > > */
> > > do_100_continue = (worker->s->ping_timeout_set
> > > + && (worker->s->ping_timeout >= 0)
> > > && ap_request_has_body(r)
> > > && (PROXYREQ_REVERSE == r->proxyreq)
> > > && !(apr_table_get(r->subprocess_env,
> "force-proxy-request-1.0")));
> > > Index: modules/proxy/mod_proxy_http.c
> > > ===================================================================
> > > --- modules/proxy/mod_proxy_http.c (revision 1530243)
> > > +++ modules/proxy/mod_proxy_http.c (working copy)
> > > @@ -1241,6 +1241,7 @@ int ap_proxy_http_process_response(apr_pool_t *
> p,
> > > dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
> > >
> > > do_100_continue = (worker->s->ping_timeout_set
> > > + && (worker->s->ping_timeout >= 0)
> > > && ap_request_has_body(r)
> > > && (PROXYREQ_REVERSE == r->proxyreq)
> > > && !(apr_table_get(r->subprocess_env,
> "force-proxy-request-1.0")));
> > > @@ -1992,8 +1993,9 @@ static int proxy_http_handler(request_rec *r,
> prox
> > > */
> > > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > > conf, uri, locurl,
> server_portstr)) != OK) {
> > > - if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set &&
> > > - worker->s->ping_timeout > 0) {
> > > + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> > > + worker->s->ping_timeout_set &&
> > > + worker->s->ping_timeout >= 0) {
> > > backend->close = 1;
> > > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r,
> APLOGNO(01115)
> > > "HTTP: 100-Continue failed to %pI (%s)",
> > >
> > >
> > >
> > > On Fri, Aug 23, 2013 at 6:48 PM, <ji...@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/docs/manual/mod/mod_proxy.xml
> > > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
> > >
> ==============================================================================
> > > --- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
> > > +++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23
> 16:48:42 2013
> > > @@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
> > > <tr><td>ping</td>
> > > <td>0</td>
> > > <td>Ping property tells the webserver to "test" the
> connection to
> > > - the backend before forwarding the request. For AJP, it causes
> > > + the backend before forwarding the request. For negative values
> > > + the test is a simple socket check, for positive values it's
> > > + a more functional check, dependent upon the protocol. For
> AJP, it causes
> > > <module>mod_proxy_ajp</module>to send a <code>CPING</code>
> > > request on the ajp13 connection (implemented on Tomcat
> 3.3.2+, 4.1.28+
> > > and 5.0.13+). For HTTP, it causes
> <module>mod_proxy_http</module>
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> > > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > >
> ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> > > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42
> 2013
> > > @@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
> > > */
> > > if (ap_timeout_parameter_parse(val, &timeout, "s") !=
> APR_SUCCESS)
> > > return "Ping/Pong timeout has wrong format";
> > > - if (timeout < 1000)
> > > + if (timeout < 1000 && timeout >= 0)
> > > return "Ping/Pong timeout must be at least one
> millisecond";
> > > worker->s->ping_timeout = timeout;
> > > worker->s->ping_timeout_set = 1;
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> > > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
> > >
> ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> > > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42
> 2013
> > > @@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
> > > APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
> > > (request_rec *r, apr_table_t *headers));
> > >
> > > +
> > > +/**
> > > + * @param socket socket to test
> > > + * @return TRUE if socket is connected/active
> > > + */
> > > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
> > > +
> > > #define PROXY_LBMETHOD "proxylbmethod"
> > >
> > > /* The number of dynamic workers that can be added when reconfiguring.
> > >
> > > 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) {
> > > + 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 */
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> > > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > >
> ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> > > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23
> 16:48:42 2013
> > > @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> > > }
> > > }
> > >
> > > + /* Step Three-and-a-Half: See if the socket is still
> connected (if desired) */
> > > + if (worker->s->ping_timeout_set && worker->s->ping_timeout <
> 0 &&
> > > + !ap_proxy_is_socket_connected(backend->sock)) {
> > > + backend->close = 1;
> > > + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> > > + "socket check failed to %pI (%s)",
> > > + worker->cp->addr, worker->s->hostname);
> > > + retry++;
> > > + continue;
> > > + }
> > > +
> > > /* Step Four: Send the Request
> > > * On the off-chance that we forced a 100-Continue as a
> > > * kinda HTTP ping test, allow for retries
> > > */
> > > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > > conf, uri, locurl,
> server_portstr)) != OK) {
> > > - if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set) {
> > > + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set &&
> > > + worker->s->ping_timeout > 0) {
> > > backend->close = 1;
> > > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r,
> APLOGNO(01115)
> > > "HTTP: 100-Continue failed to %pI (%s)",
> > >
> > > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > >
> ==============================================================================
> > > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42
> 2013
> > > @@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
> > > #endif
> > >
> > > #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> > > -static int is_socket_connected(apr_socket_t *socket)
> > > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> > > {
> > > apr_pollfd_t pfds[1];
> > > apr_status_t status;
> > > @@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
> > >
> > > }
> > > #else
> > > -static int is_socket_connected(apr_socket_t *sock)
> > > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> > >
> > > {
> > > apr_size_t buffer_len = 1;
> > > @@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> > > (proxy_server_conf *) ap_get_module_config(sconf,
> &proxy_module);
> > >
> > > if (conn->sock) {
> > > - if (!(connected = is_socket_connected(conn->sock))) {
> > > + if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> > > socket_cleanup(conn);
> > > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> > > "%s: backend socket is disconnected.",
> > >
> > >
> > >
> >
> >
>
>
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
OK, I gotcha now...
Do you have a patch file? tia!
On Oct 8, 2013, at 12:56 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Oct 8, 2013 at 6:26 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> Does it matter whether or not it's a heavy ping or not?
> It doesn't matter what sort of test was used, the socket is down.
>
> Yes it is down, but for the ajp case for example, that determines the return value GATEWAY_TIMEOUT vs INTERNAL_SERVER_ERROR, and according to the comments, the former is when cping/cpong was used and that will not "affect the whole worker", does it make sense when the light ping (socket health-check) only was done before forwarding?
>
> For the ap_proxy_http_request error case, should mod_proxy retry a new request when the heavy ping is off (light ping on)?
>
> For ap_proxy_create_hdrbrgd and ap_proxy_process_response, the "100 continue" things have nothing to do with light ping only, have they?
>
> Regards.
>
>
> On Oct 8, 2013, at 11:53 AM, Yann Ylavic <yl...@gmail.com> wrote:
> > Hi,
> >
> > Some code in trunk still only check for ping_timeout_set without ping_timeout positive value to handle the "heavy" ping (CPING/Expect: 100-continue), see patch below.
> >
> > Regards,
> > Yann.
> >
> > Index: modules/proxy/mod_proxy_ajp.c
> > ===================================================================
> > --- modules/proxy/mod_proxy_ajp.c (revision 1530243)
> > +++ modules/proxy/mod_proxy_ajp.c (working copy)
> > @@ -341,7 +341,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > * we assume it is a request that cause a back-end timeout,
> > * but doesn't affect the whole worker.
> > */
> > - if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set) {
> > + if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set
> > + && conn->worker->s->ping_timeout >= 0) {
> > return HTTP_GATEWAY_TIME_OUT;
> > }
> >
> > @@ -661,7 +662,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > * we assume it is a request that cause a back-end timeout,
> > * but doesn't affect the whole worker.
> > */
> > - if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set) {
> > + if (APR_STATUS_IS_TIMEUP(status) &&
> > + conn->worker->s->ping_timeout_set &&
> > + conn->worker->s->ping_timeout >= 0) {
> > apr_table_set(r->notes, "proxy_timedout", "1");
> > rv = HTTP_GATEWAY_TIME_OUT;
> > }
> > Index: modules/proxy/proxy_util.c
> > ===================================================================
> > --- modules/proxy/proxy_util.c (revision 1530243)
> > +++ modules/proxy/proxy_util.c (working copy)
> > @@ -3073,6 +3073,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
> > * We also make sure we won't be talking HTTP/1.0 as well.
> > */
> > do_100_continue = (worker->s->ping_timeout_set
> > + && (worker->s->ping_timeout >= 0)
> > && ap_request_has_body(r)
> > && (PROXYREQ_REVERSE == r->proxyreq)
> > && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> > Index: modules/proxy/mod_proxy_http.c
> > ===================================================================
> > --- modules/proxy/mod_proxy_http.c (revision 1530243)
> > +++ modules/proxy/mod_proxy_http.c (working copy)
> > @@ -1241,6 +1241,7 @@ int ap_proxy_http_process_response(apr_pool_t * p,
> > dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
> >
> > do_100_continue = (worker->s->ping_timeout_set
> > + && (worker->s->ping_timeout >= 0)
> > && ap_request_has_body(r)
> > && (PROXYREQ_REVERSE == r->proxyreq)
> > && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> > @@ -1992,8 +1993,9 @@ static int proxy_http_handler(request_rec *r, prox
> > */
> > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > conf, uri, locurl, server_portstr)) != OK) {
> > - if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
> > - worker->s->ping_timeout > 0) {
> > + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> > + worker->s->ping_timeout_set &&
> > + worker->s->ping_timeout >= 0) {
> > backend->close = 1;
> > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
> > "HTTP: 100-Continue failed to %pI (%s)",
> >
> >
> >
> > On Fri, Aug 23, 2013 at 6:48 PM, <ji...@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/docs/manual/mod/mod_proxy.xml
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
> > +++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23 16:48:42 2013
> > @@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
> > <tr><td>ping</td>
> > <td>0</td>
> > <td>Ping property tells the webserver to "test" the connection to
> > - the backend before forwarding the request. For AJP, it causes
> > + the backend before forwarding the request. For negative values
> > + the test is a simple socket check, for positive values it's
> > + a more functional check, dependent upon the protocol. For AJP, it causes
> > <module>mod_proxy_ajp</module>to send a <code>CPING</code>
> > request on the ajp13 connection (implemented on Tomcat 3.3.2+, 4.1.28+
> > and 5.0.13+). For HTTP, it causes <module>mod_proxy_http</module>
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42 2013
> > @@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
> > */
> > if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
> > return "Ping/Pong timeout has wrong format";
> > - if (timeout < 1000)
> > + if (timeout < 1000 && timeout >= 0)
> > return "Ping/Pong timeout must be at least one millisecond";
> > worker->s->ping_timeout = timeout;
> > worker->s->ping_timeout_set = 1;
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42 2013
> > @@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
> > APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
> > (request_rec *r, apr_table_t *headers));
> >
> > +
> > +/**
> > + * @param socket socket to test
> > + * @return TRUE if socket is connected/active
> > + */
> > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
> > +
> > #define PROXY_LBMETHOD "proxylbmethod"
> >
> > /* The number of dynamic workers that can be added when reconfiguring.
> >
> > 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) {
> > + 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 */
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42 2013
> > @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> > }
> > }
> >
> > + /* Step Three-and-a-Half: See if the socket is still connected (if desired) */
> > + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
> > + !ap_proxy_is_socket_connected(backend->sock)) {
> > + backend->close = 1;
> > + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> > + "socket check failed to %pI (%s)",
> > + worker->cp->addr, worker->s->hostname);
> > + retry++;
> > + continue;
> > + }
> > +
> > /* Step Four: Send the Request
> > * On the off-chance that we forced a 100-Continue as a
> > * kinda HTTP ping test, allow for retries
> > */
> > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > conf, uri, locurl, server_portstr)) != OK) {
> > - if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set) {
> > + if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
> > + worker->s->ping_timeout > 0) {
> > backend->close = 1;
> > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
> > "HTTP: 100-Continue failed to %pI (%s)",
> >
> > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42 2013
> > @@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
> > #endif
> >
> > #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> > -static int is_socket_connected(apr_socket_t *socket)
> > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> > {
> > apr_pollfd_t pfds[1];
> > apr_status_t status;
> > @@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
> >
> > }
> > #else
> > -static int is_socket_connected(apr_socket_t *sock)
> > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> >
> > {
> > apr_size_t buffer_len = 1;
> > @@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> > (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
> >
> > if (conn->sock) {
> > - if (!(connected = is_socket_connected(conn->sock))) {
> > + if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> > socket_cleanup(conn);
> > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> > "%s: backend socket is disconnected.",
> >
> >
> >
>
>
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
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 8, 2013 at 6:26 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> Does it matter whether or not it's a heavy ping or not?
> It doesn't matter what sort of test was used, the socket is down.
>
Yes it is down, but for the ajp case for example, that determines the
return value GATEWAY_TIMEOUT vs INTERNAL_SERVER_ERROR, and according to the
comments, the former is when cping/cpong was used and that will not "affect
the whole worker", does it make sense when the light ping (socket
health-check) only was done before forwarding?
For the ap_proxy_http_request error case, should mod_proxy retry a new
request when the heavy ping is off (light ping on)?
For ap_proxy_create_hdrbrgd and ap_proxy_process_response, the "100
continue" things have nothing to do with light ping only, have they?
Regards.
> On Oct 8, 2013, at 11:53 AM, Yann Ylavic <yl...@gmail.com> wrote:
> > Hi,
> >
> > Some code in trunk still only check for ping_timeout_set without
> ping_timeout positive value to handle the "heavy" ping (CPING/Expect:
> 100-continue), see patch below.
> >
> > Regards,
> > Yann.
> >
> > Index: modules/proxy/mod_proxy_ajp.c
> > ===================================================================
> > --- modules/proxy/mod_proxy_ajp.c (revision 1530243)
> > +++ modules/proxy/mod_proxy_ajp.c (working copy)
> > @@ -341,7 +341,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > * we assume it is a request that cause a back-end timeout,
> > * but doesn't affect the whole worker.
> > */
> > - if (APR_STATUS_IS_TIMEUP(status) &&
> conn->worker->s->ping_timeout_set) {
> > + if (APR_STATUS_IS_TIMEUP(status) &&
> conn->worker->s->ping_timeout_set
> > + && conn->worker->s->ping_timeout >= 0) {
> > return HTTP_GATEWAY_TIME_OUT;
> > }
> >
> > @@ -661,7 +662,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> > * we assume it is a request that cause a back-end timeout,
> > * but doesn't affect the whole worker.
> > */
> > - if (APR_STATUS_IS_TIMEUP(status) &&
> conn->worker->s->ping_timeout_set) {
> > + if (APR_STATUS_IS_TIMEUP(status) &&
> > + conn->worker->s->ping_timeout_set &&
> > + conn->worker->s->ping_timeout >= 0) {
> > apr_table_set(r->notes, "proxy_timedout", "1");
> > rv = HTTP_GATEWAY_TIME_OUT;
> > }
> > Index: modules/proxy/proxy_util.c
> > ===================================================================
> > --- modules/proxy/proxy_util.c (revision 1530243)
> > +++ modules/proxy/proxy_util.c (working copy)
> > @@ -3073,6 +3073,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
> > * We also make sure we won't be talking HTTP/1.0 as well.
> > */
> > do_100_continue = (worker->s->ping_timeout_set
> > + && (worker->s->ping_timeout >= 0)
> > && ap_request_has_body(r)
> > && (PROXYREQ_REVERSE == r->proxyreq)
> > && !(apr_table_get(r->subprocess_env,
> "force-proxy-request-1.0")));
> > Index: modules/proxy/mod_proxy_http.c
> > ===================================================================
> > --- modules/proxy/mod_proxy_http.c (revision 1530243)
> > +++ modules/proxy/mod_proxy_http.c (working copy)
> > @@ -1241,6 +1241,7 @@ int ap_proxy_http_process_response(apr_pool_t * p,
> > dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
> >
> > do_100_continue = (worker->s->ping_timeout_set
> > + && (worker->s->ping_timeout >= 0)
> > && ap_request_has_body(r)
> > && (PROXYREQ_REVERSE == r->proxyreq)
> > && !(apr_table_get(r->subprocess_env,
> "force-proxy-request-1.0")));
> > @@ -1992,8 +1993,9 @@ static int proxy_http_handler(request_rec *r, prox
> > */
> > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > conf, uri, locurl,
> server_portstr)) != OK) {
> > - if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set &&
> > - worker->s->ping_timeout > 0) {
> > + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> > + worker->s->ping_timeout_set &&
> > + worker->s->ping_timeout >= 0) {
> > backend->close = 1;
> > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r,
> APLOGNO(01115)
> > "HTTP: 100-Continue failed to %pI (%s)",
> >
> >
> >
> > On Fri, Aug 23, 2013 at 6:48 PM, <ji...@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/docs/manual/mod/mod_proxy.xml
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
> > +++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23 16:48:42
> 2013
> > @@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
> > <tr><td>ping</td>
> > <td>0</td>
> > <td>Ping property tells the webserver to "test" the connection
> to
> > - the backend before forwarding the request. For AJP, it causes
> > + the backend before forwarding the request. For negative values
> > + the test is a simple socket check, for positive values it's
> > + a more functional check, dependent upon the protocol. For AJP,
> it causes
> > <module>mod_proxy_ajp</module>to send a <code>CPING</code>
> > request on the ajp13 connection (implemented on Tomcat 3.3.2+,
> 4.1.28+
> > and 5.0.13+). For HTTP, it causes
> <module>mod_proxy_http</module>
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42 2013
> > @@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
> > */
> > if (ap_timeout_parameter_parse(val, &timeout, "s") !=
> APR_SUCCESS)
> > return "Ping/Pong timeout has wrong format";
> > - if (timeout < 1000)
> > + if (timeout < 1000 && timeout >= 0)
> > return "Ping/Pong timeout must be at least one millisecond";
> > worker->s->ping_timeout = timeout;
> > worker->s->ping_timeout_set = 1;
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42 2013
> > @@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
> > APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
> > (request_rec *r, apr_table_t *headers));
> >
> > +
> > +/**
> > + * @param socket socket to test
> > + * @return TRUE if socket is connected/active
> > + */
> > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
> > +
> > #define PROXY_LBMETHOD "proxylbmethod"
> >
> > /* The number of dynamic workers that can be added when reconfiguring.
> >
> > 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) {
> > + 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 */
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42
> 2013
> > @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> > }
> > }
> >
> > + /* Step Three-and-a-Half: See if the socket is still connected
> (if desired) */
> > + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0
> &&
> > + !ap_proxy_is_socket_connected(backend->sock)) {
> > + backend->close = 1;
> > + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> > + "socket check failed to %pI (%s)",
> > + worker->cp->addr, worker->s->hostname);
> > + retry++;
> > + continue;
> > + }
> > +
> > /* Step Four: Send the Request
> > * On the off-chance that we forced a 100-Continue as a
> > * kinda HTTP ping test, allow for retries
> > */
> > if ((status = ap_proxy_http_request(p, r, backend, worker,
> > conf, uri, locurl,
> server_portstr)) != OK) {
> > - if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set) {
> > + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set &&
> > + worker->s->ping_timeout > 0) {
> > backend->close = 1;
> > ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r,
> APLOGNO(01115)
> > "HTTP: 100-Continue failed to %pI (%s)",
> >
> > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42 2013
> > @@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
> > #endif
> >
> > #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> > -static int is_socket_connected(apr_socket_t *socket)
> > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> > {
> > apr_pollfd_t pfds[1];
> > apr_status_t status;
> > @@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
> >
> > }
> > #else
> > -static int is_socket_connected(apr_socket_t *sock)
> > +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> >
> > {
> > apr_size_t buffer_len = 1;
> > @@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> > (proxy_server_conf *) ap_get_module_config(sconf,
> &proxy_module);
> >
> > if (conn->sock) {
> > - if (!(connected = is_socket_connected(conn->sock))) {
> > + if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> > socket_cleanup(conn);
> > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> > "%s: backend socket is disconnected.",
> >
> >
> >
>
>
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
Does it matter whether or not it's a heavy ping or not?
It doesn't matter what sort of test was used, the socket is down.
On Oct 8, 2013, at 11:53 AM, Yann Ylavic <yl...@gmail.com> wrote:
> Hi,
>
> Some code in trunk still only check for ping_timeout_set without ping_timeout positive value to handle the "heavy" ping (CPING/Expect: 100-continue), see patch below.
>
> Regards,
> Yann.
>
> Index: modules/proxy/mod_proxy_ajp.c
> ===================================================================
> --- modules/proxy/mod_proxy_ajp.c (revision 1530243)
> +++ modules/proxy/mod_proxy_ajp.c (working copy)
> @@ -341,7 +341,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> * we assume it is a request that cause a back-end timeout,
> * but doesn't affect the whole worker.
> */
> - if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set) {
> + if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set
> + && conn->worker->s->ping_timeout >= 0) {
> return HTTP_GATEWAY_TIME_OUT;
> }
>
> @@ -661,7 +662,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
> * we assume it is a request that cause a back-end timeout,
> * but doesn't affect the whole worker.
> */
> - if (APR_STATUS_IS_TIMEUP(status) && conn->worker->s->ping_timeout_set) {
> + if (APR_STATUS_IS_TIMEUP(status) &&
> + conn->worker->s->ping_timeout_set &&
> + conn->worker->s->ping_timeout >= 0) {
> apr_table_set(r->notes, "proxy_timedout", "1");
> rv = HTTP_GATEWAY_TIME_OUT;
> }
> Index: modules/proxy/proxy_util.c
> ===================================================================
> --- modules/proxy/proxy_util.c (revision 1530243)
> +++ modules/proxy/proxy_util.c (working copy)
> @@ -3073,6 +3073,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
> * We also make sure we won't be talking HTTP/1.0 as well.
> */
> do_100_continue = (worker->s->ping_timeout_set
> + && (worker->s->ping_timeout >= 0)
> && ap_request_has_body(r)
> && (PROXYREQ_REVERSE == r->proxyreq)
> && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> Index: modules/proxy/mod_proxy_http.c
> ===================================================================
> --- modules/proxy/mod_proxy_http.c (revision 1530243)
> +++ modules/proxy/mod_proxy_http.c (working copy)
> @@ -1241,6 +1241,7 @@ int ap_proxy_http_process_response(apr_pool_t * p,
> dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
>
> do_100_continue = (worker->s->ping_timeout_set
> + && (worker->s->ping_timeout >= 0)
> && ap_request_has_body(r)
> && (PROXYREQ_REVERSE == r->proxyreq)
> && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> @@ -1992,8 +1993,9 @@ static int proxy_http_handler(request_rec *r, prox
> */
> if ((status = ap_proxy_http_request(p, r, backend, worker,
> conf, uri, locurl, server_portstr)) != OK) {
> - if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
> - worker->s->ping_timeout > 0) {
> + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> + worker->s->ping_timeout_set &&
> + worker->s->ping_timeout >= 0) {
> backend->close = 1;
> ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
> "HTTP: 100-Continue failed to %pI (%s)",
>
>
>
> On Fri, Aug 23, 2013 at 6:48 PM, <ji...@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/docs/manual/mod/mod_proxy.xml
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23 16:48:42 2013
> @@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
> <tr><td>ping</td>
> <td>0</td>
> <td>Ping property tells the webserver to "test" the connection to
> - the backend before forwarding the request. For AJP, it causes
> + the backend before forwarding the request. For negative values
> + the test is a simple socket check, for positive values it's
> + a more functional check, dependent upon the protocol. For AJP, it causes
> <module>mod_proxy_ajp</module>to send a <code>CPING</code>
> request on the ajp13 connection (implemented on Tomcat 3.3.2+, 4.1.28+
> and 5.0.13+). For HTTP, it causes <module>mod_proxy_http</module>
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42 2013
> @@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
> */
> if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
> return "Ping/Pong timeout has wrong format";
> - if (timeout < 1000)
> + if (timeout < 1000 && timeout >= 0)
> return "Ping/Pong timeout must be at least one millisecond";
> worker->s->ping_timeout = timeout;
> worker->s->ping_timeout_set = 1;
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42 2013
> @@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
> APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
> (request_rec *r, apr_table_t *headers));
>
> +
> +/**
> + * @param socket socket to test
> + * @return TRUE if socket is connected/active
> + */
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
> +
> #define PROXY_LBMETHOD "proxylbmethod"
>
> /* The number of dynamic workers that can be added when reconfiguring.
>
> 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) {
> + 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 */
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42 2013
> @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> }
> }
>
> + /* Step Three-and-a-Half: See if the socket is still connected (if desired) */
> + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
> + !ap_proxy_is_socket_connected(backend->sock)) {
> + backend->close = 1;
> + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> + "socket check failed to %pI (%s)",
> + worker->cp->addr, worker->s->hostname);
> + retry++;
> + continue;
> + }
> +
> /* Step Four: Send the Request
> * On the off-chance that we forced a 100-Continue as a
> * kinda HTTP ping test, allow for retries
> */
> if ((status = ap_proxy_http_request(p, r, backend, worker,
> conf, uri, locurl, server_portstr)) != OK) {
> - if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set) {
> + if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set &&
> + worker->s->ping_timeout > 0) {
> backend->close = 1;
> ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
> "HTTP: 100-Continue failed to %pI (%s)",
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42 2013
> @@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
> #endif
>
> #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> -static int is_socket_connected(apr_socket_t *socket)
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> {
> apr_pollfd_t pfds[1];
> apr_status_t status;
> @@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
>
> }
> #else
> -static int is_socket_connected(apr_socket_t *sock)
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
>
> {
> apr_size_t buffer_len = 1;
> @@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
>
> if (conn->sock) {
> - if (!(connected = is_socket_connected(conn->sock))) {
> + if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> socket_cleanup(conn);
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> "%s: backend socket is disconnected.",
>
>
>
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
Posted by Yann Ylavic <yl...@gmail.com>.
Hi,
Some code in trunk still only check for ping_timeout_set without
ping_timeout positive value to handle the "heavy" ping (CPING/Expect:
100-continue), see patch below.
Regards,
Yann.
Index: modules/proxy/mod_proxy_ajp.c
===================================================================
--- modules/proxy/mod_proxy_ajp.c (revision 1530243)
+++ modules/proxy/mod_proxy_ajp.c (working copy)
@@ -341,7 +341,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
* we assume it is a request that cause a back-end timeout,
* but doesn't affect the whole worker.
*/
- if (APR_STATUS_IS_TIMEUP(status) &&
conn->worker->s->ping_timeout_set) {
+ if (APR_STATUS_IS_TIMEUP(status) &&
conn->worker->s->ping_timeout_set
+ && conn->worker->s->ping_timeout >= 0) {
return HTTP_GATEWAY_TIME_OUT;
}
@@ -661,7 +662,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
* we assume it is a request that cause a back-end timeout,
* but doesn't affect the whole worker.
*/
- if (APR_STATUS_IS_TIMEUP(status) &&
conn->worker->s->ping_timeout_set) {
+ if (APR_STATUS_IS_TIMEUP(status) &&
+ conn->worker->s->ping_timeout_set &&
+ conn->worker->s->ping_timeout >= 0) {
apr_table_set(r->notes, "proxy_timedout", "1");
rv = HTTP_GATEWAY_TIME_OUT;
}
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 1530243)
+++ modules/proxy/proxy_util.c (working copy)
@@ -3073,6 +3073,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
* We also make sure we won't be talking HTTP/1.0 as well.
*/
do_100_continue = (worker->s->ping_timeout_set
+ && (worker->s->ping_timeout >= 0)
&& ap_request_has_body(r)
&& (PROXYREQ_REVERSE == r->proxyreq)
&& !(apr_table_get(r->subprocess_env,
"force-proxy-request-1.0")));
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 1530243)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -1241,6 +1241,7 @@ int ap_proxy_http_process_response(apr_pool_t * p,
dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
do_100_continue = (worker->s->ping_timeout_set
+ && (worker->s->ping_timeout >= 0)
&& ap_request_has_body(r)
&& (PROXYREQ_REVERSE == r->proxyreq)
&& !(apr_table_get(r->subprocess_env,
"force-proxy-request-1.0")));
@@ -1992,8 +1993,9 @@ static int proxy_http_handler(request_rec *r, prox
*/
if ((status = ap_proxy_http_request(p, r, backend, worker,
conf, uri, locurl,
server_portstr)) != OK) {
- if ((status == HTTP_SERVICE_UNAVAILABLE) &&
worker->s->ping_timeout_set &&
- worker->s->ping_timeout > 0) {
+ if ((status == HTTP_SERVICE_UNAVAILABLE) &&
+ worker->s->ping_timeout_set &&
+ worker->s->ping_timeout >= 0) {
backend->close = 1;
ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r,
APLOGNO(01115)
"HTTP: 100-Continue failed to %pI (%s)",
On Fri, Aug 23, 2013 at 6:48 PM, <ji...@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/docs/manual/mod/mod_proxy.xml
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1516930&r1=1516929&r2=1516930&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Fri Aug 23 16:48:42
> 2013
> @@ -1003,7 +1003,9 @@ ProxyPass /mirror/foo http://backend.exa
> <tr><td>ping</td>
> <td>0</td>
> <td>Ping property tells the webserver to "test" the connection to
> - the backend before forwarding the request. For AJP, it causes
> + the backend before forwarding the request. For negative values
> + the test is a simple socket check, for positive values it's
> + a more functional check, dependent upon the protocol. For AJP, it
> causes
> <module>mod_proxy_ajp</module>to send a <code>CPING</code>
> request on the ajp13 connection (implemented on Tomcat 3.3.2+,
> 4.1.28+
> and 5.0.13+). For HTTP, it causes <module>mod_proxy_http</module>
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1516930&r1=1516929&r2=1516930&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Aug 23 16:48:42 2013
> @@ -233,7 +233,7 @@ static const char *set_worker_param(apr_
> */
> if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
> return "Ping/Pong timeout has wrong format";
> - if (timeout < 1000)
> + if (timeout < 1000 && timeout >= 0)
> return "Ping/Pong timeout must be at least one millisecond";
> worker->s->ping_timeout = timeout;
> worker->s->ping_timeout_set = 1;
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1516930&r1=1516929&r2=1516930&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Aug 23 16:48:42 2013
> @@ -972,6 +972,13 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
> APR_DECLARE_OPTIONAL_FN(int, ap_proxy_clear_connection,
> (request_rec *r, apr_table_t *headers));
>
> +
> +/**
> + * @param socket socket to test
> + * @return TRUE if socket is connected/active
> + */
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket);
> +
> #define PROXY_LBMETHOD "proxylbmethod"
>
> /* The number of dynamic workers that can be added when reconfiguring.
>
> 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) {
> + 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 */
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42
> 2013
> @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> }
> }
>
> + /* Step Three-and-a-Half: See if the socket is still connected
> (if desired) */
> + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
> + !ap_proxy_is_socket_connected(backend->sock)) {
> + backend->close = 1;
> + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> + "socket check failed to %pI (%s)",
> + worker->cp->addr, worker->s->hostname);
> + retry++;
> + continue;
> + }
> +
> /* Step Four: Send the Request
> * On the off-chance that we forced a 100-Continue as a
> * kinda HTTP ping test, allow for retries
> */
> if ((status = ap_proxy_http_request(p, r, backend, worker,
> conf, uri, locurl,
> server_portstr)) != OK) {
> - if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set) {
> + if ((status == HTTP_SERVICE_UNAVAILABLE) &&
> worker->s->ping_timeout_set &&
> + worker->s->ping_timeout > 0) {
> backend->close = 1;
> ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r,
> APLOGNO(01115)
> "HTTP: 100-Continue failed to %pI (%s)",
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1516930&r1=1516929&r2=1516930&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Aug 23 16:48:42 2013
> @@ -2245,7 +2245,7 @@ ap_proxy_determine_connection(apr_pool_t
> #endif
>
> #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> -static int is_socket_connected(apr_socket_t *socket)
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> {
> apr_pollfd_t pfds[1];
> apr_status_t status;
> @@ -2283,7 +2283,7 @@ static int is_socket_connected(apr_socke
>
> }
> #else
> -static int is_socket_connected(apr_socket_t *sock)
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
>
> {
> apr_size_t buffer_len = 1;
> @@ -2466,7 +2466,7 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
>
> if (conn->sock) {
> - if (!(connected = is_socket_connected(conn->sock))) {
> + if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> socket_cleanup(conn);
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> "%s: backend socket is disconnected.",
>
>
>
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 23, 2013, at 12:59 PM, Eric Covener <co...@gmail.com> wrote:
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42 2013
>> @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
>> }
>> }
>>
>> + /* Step Three-and-a-Half: See if the socket is still connected (if desired) */
>> + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
>> + !ap_proxy_is_socket_connected(backend->sock)) {
>> + backend->close = 1;
>> + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
>> + "socket check failed to %pI (%s)",
>> + worker->cp->addr, worker->s->hostname);
>> + retry++;
>> + continue;
>> + }
>> +
>
> This is already done in step 2 -- Is this to just narrow the window
> between the check and writing the request, or is the previous check
> not always happening.
>
To narrow it. Esp if we just went through the whole process
of building one up.
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
Posted by Eric Covener <co...@gmail.com>.
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1516930&r1=1516929&r2=1516930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Aug 23 16:48:42 2013
> @@ -1975,13 +1975,25 @@ static int proxy_http_handler(request_re
> }
> }
>
> + /* Step Three-and-a-Half: See if the socket is still connected (if desired) */
> + if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
> + !ap_proxy_is_socket_connected(backend->sock)) {
> + backend->close = 1;
> + ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO()
> + "socket check failed to %pI (%s)",
> + worker->cp->addr, worker->s->hostname);
> + retry++;
> + continue;
> + }
> +
This is already done in step 2 -- Is this to just narrow the window
between the check and writing the request, or is the previous check
not always happening.
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
ping?
On Aug 23, 2013, at 2:29 PM, Jim Jagielski <ji...@jaguNET.com> 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 <ji...@jaguNET.com> wrote:
>
>> I explained the rationale.
>>
>> Thx for the ping_timeout_set catch, fwiw
>>
>> On Aug 23, 2013, at 2:08 PM, Ruediger Pluem <rp...@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
>>>
>>
>
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
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 <ji...@jaguNET.com> wrote:
> I explained the rationale.
>
> Thx for the ping_timeout_set catch, fwiw
>
> On Aug 23, 2013, at 2:08 PM, Ruediger Pluem <rp...@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
>>
>
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
Posted by Jim Jagielski <ji...@jaguNET.com>.
I explained the rationale.
Thx for the ping_timeout_set catch, fwiw
On Aug 23, 2013, at 2:08 PM, Ruediger Pluem <rp...@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
>
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
Posted by Ruediger Pluem <rp...@apache.org>.
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