IMHO the current behaviour of mod_proxy_ajp is not correct if a CPING/CPONG fails on a backend connection. In this case the status is set to HTTP_SERVICE_UNAVAILABLE and the scheme handler returns. In the case of an unbalanced backend this results in a HTTP_SERVICE_UNAVAILABLE (503) to be returned to the client. But a failing CPING/CPONG can be caused by a faulty AJP connection that was closed by the backend or a race condition in sending the CPING and closing the connection on the backend. So after the first failed CPING/CPONG we should try again with a new TCP connection and should only return HTTP_SERVICE_UNAVAILABLE if this fails as well. The following (and attached patch) does exactly that. Thoughs to the above and/or to the patch before I commit?
Index: modules/proxy/mod_proxy_ajp.c
===================================================================
--- modules/proxy/mod_proxy_ajp.c (revision 692409)
+++ modules/proxy/mod_proxy_ajp.c (working copy)
@@ -554,6 +554,7 @@
conn_rec *origin = NULL;
proxy_conn_rec *backend = NULL;
const char *scheme = "AJP";
+ int retry;
proxy_dir_conf *dconf = ap_get_module_config(r->per_dir_config,
&proxy_module);
@@ -597,43 +598,53 @@
backend->is_ssl = 0;
backend->close = 0;
- /* Step One: Determine Who To Connect To */
- status = ap_proxy_determine_connection(p, r, conf, worker, backend,
- uri, &url, proxyname, proxyport,
- server_portstr,
- sizeof(server_portstr));
+ retry = 0;
+ while (retry < 2) {
+ /* Step One: Determine Who To Connect To */
+ status = ap_proxy_determine_connection(p, r, conf, worker, backend,
+ uri, &url, proxyname, proxyport,
+ server_portstr,
+ sizeof(server_portstr));
- if (status != OK)
- goto cleanup;
+ if (status != OK)
+ break;
- /* Step Two: Make the Connection */
- if (ap_proxy_connect_backend(scheme, backend, worker, r->server)) {
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
- "proxy: AJP: failed to make connection to backend: %s",
- backend->hostname);
- status = HTTP_SERVICE_UNAVAILABLE;
- goto cleanup;
- }
-
- /* Handle CPING/CPONG */
- if (worker->ping_timeout_set) {
- status = ajp_handle_cping_cpong(backend->sock, r,
- worker->ping_timeout);
- if (status != APR_SUCCESS) {
- backend->close++;
- ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
- "proxy: AJP: cping/cpong failed to %pI (%s)",
- worker->cp->addr,
- worker->hostname);
+ /* Step Two: Make the Connection */
+ if (ap_proxy_connect_backend(scheme, backend, worker, r->server)) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
+ "proxy: AJP: failed to make connection to backend:
%s",
+ backend->hostname);
status = HTTP_SERVICE_UNAVAILABLE;
- goto cleanup;
+ break;
}
+
+ /* Handle CPING/CPONG */
+ if (worker->ping_timeout_set) {
+ status = ajp_handle_cping_cpong(backend->sock, r,
+ worker->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++;
+ ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
+ "proxy: AJP: cping/cpong failed to %pI (%s)",
+ worker->cp->addr,
+ worker->hostname);
+ status = HTTP_SERVICE_UNAVAILABLE;
+ retry++;
+ continue;
+ }
+ }
+ /* Step Three: Process the Request */
+ status = ap_proxy_ajp_request(p, r, backend, origin, dconf, uri, url,
+ server_portstr);
+ break;
}
- /* Step Three: Process the Request */
- status = ap_proxy_ajp_request(p, r, backend, origin, dconf, uri, url,
- server_portstr);
-cleanup:
/* Do not close the socket */
ap_proxy_release_connection(scheme, backend, r->server);
return status;
Regards
Rüdiger
cping_cpong_retry.diff
Description: cping_cpong_retry.diff
