.. is meant to do a simple TCP readability check on the connection,
using ap_proxy_is_socket_connected().
This is trunk only AFAICT.
However all our proxy modules handle this just after calling
ap_proxy_connect_backend() which already calls
ap_proxy_is_socket_connected() for the same purpose.
Couldn't we simply remove this ping_timeout < 0 code (eg. attached patch)?
Regards,
Yann.
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c (revision 1729388)
+++ modules/proxy/mod_proxy.c (working copy)
@@ -280,7 +280,7 @@ static const char *set_worker_param(apr_pool_t *p,
*/
if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
return "Ping/Pong timeout has wrong format";
- if (timeout < 1000 && timeout >= 0)
+ if (timeout < 1000)
return "Ping/Pong timeout must be at least one millisecond";
worker->s->ping_timeout = timeout;
worker->s->ping_timeout_set = 1;
Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h (revision 1729388)
+++ modules/proxy/mod_proxy.h (working copy)
@@ -365,7 +365,6 @@ do { \
#define PROXY_DO_100_CONTINUE(w, r) \
((w)->s->ping_timeout_set \
- && ((w)->s->ping_timeout >= 0) \
&& (PROXYREQ_REVERSE == (r)->proxyreq) \
&& !(apr_table_get((r)->subprocess_env, "force-proxy-request-1.0")) \
&& ap_request_has_body((r)))
Index: modules/proxy/mod_proxy_ajp.c
===================================================================
--- modules/proxy/mod_proxy_ajp.c (revision 1729388)
+++ modules/proxy/mod_proxy_ajp.c (working copy)
@@ -342,8 +342,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
* but doesn't affect the whole worker.
*/
if (APR_STATUS_IS_TIMEUP(status) &&
- conn->worker->s->ping_timeout_set &&
- conn->worker->s->ping_timeout >= 0) {
+ conn->worker->s->ping_timeout_set) {
return HTTP_GATEWAY_TIME_OUT;
}
@@ -680,8 +679,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
* but doesn't affect the whole worker.
*/
if (APR_STATUS_IS_TIMEUP(status) &&
- conn->worker->s->ping_timeout_set &&
- conn->worker->s->ping_timeout >= 0) {
+ conn->worker->s->ping_timeout_set) {
apr_table_setn(r->notes, "proxy_timedout", "1");
rv = HTTP_GATEWAY_TIME_OUT;
}
@@ -791,36 +789,23 @@ static int proxy_ajp_handler(request_rec *r, proxy
/* Handle CPING/CPONG */
if (worker->s->ping_timeout_set) {
- if (worker->s->ping_timeout < 0) {
- if (!ap_proxy_is_socket_connected(backend->sock)) {
- backend->close = 1;
- ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(02534)
- "socket check failed to %pI (%s)",
- worker->cp->addr, worker->s->hostname);
- status = HTTP_SERVICE_UNAVAILABLE;
- retry++;
- continue;
- }
+ 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;
}
- 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 */
status = ap_proxy_ajp_request(p, r, backend, origin, dconf, uri, locurl,
Index: modules/proxy/mod_proxy_hcheck.c
===================================================================
--- modules/proxy/mod_proxy_hcheck.c (revision 1729388)
+++ modules/proxy/mod_proxy_hcheck.c (working copy)
@@ -553,9 +553,6 @@ static apr_status_t hc_check_tcp(sctx_t *ctx, apr_
if (status == OK) {
backend->addr = hc->cp->addr;
status = ap_proxy_connect_backend("HCTCP", backend, hc, ctx->s);
- if (status == OK) {
- status = (ap_proxy_is_socket_connected(backend->sock) ? OK : !OK);
- }
}
return backend_cleanup("HCTCP", backend, ctx->s, status);
}
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 1729388)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -2097,21 +2092,6 @@ static int proxy_http_handler(request_rec *r, prox
"proxy-request-hostname",
backend->ssl_hostname);
}
-
- /* Step Three-and-a-Half: See if the socket is still connected (if
- * desired). Note: Since ap_proxy_connect_backend just above does
- * the same check (unconditionally), this step is not required when
- * backend's socket/connection is reused (ie. no Step Three).
- */
- 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(02535)
- "socket check failed to %pI (%s)",
- worker->cp->addr, worker->s->hostname);
- retry++;
- continue;
- }
}
/* Don't recycle the connection if prefetch (above) told not to do so */
@@ -2130,8 +2110,7 @@ static int proxy_http_handler(request_rec *r, prox
flushall)) != OK) {
proxy_run_detach_backend(r, backend);
if ((status == HTTP_SERVICE_UNAVAILABLE) &&
- worker->s->ping_timeout_set &&
- worker->s->ping_timeout >= 0) {
+ worker->s->ping_timeout_set) {
backend->close = 1;
ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
"HTTP: 100-Continue failed to %pI (%s)",