On 5/21/07, Jim Jagielski <[EMAIL PROTECTED]> wrote:
On May 19, 2007, at 3:22 PM, Ruediger Pluem wrote:
>
>
> On 05/19/2007 04:07 PM, Eric Covener wrote:
>> On 5/18/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
>>
>>> Currently ProxyTimeout does not work as documented as the default
>>> value is not
>>> 300 secs, but the Timeout setting of the server. The question to
>>> me is
>>> now:
>>> What should be fixed?
>>>
>>> - Documentation (such that it matches the code)
>>> - Code (such that it matches the documentation)
>>
>>
>> Acting like a connection timeout only for me proxying HTTP on
>> 2.2.4. I
>> think I've read about similiar befuddlements on assorted PRs.
>>
>> It is raised a secondary issue here:
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=11540
>
> I know :-). This is the next issue I want to address once the issue
> above is solved.
> In 2.2.x / trunk ProxyTimeout is ignored almost completely (it is
> only used for CONNECT).
> Workers either use their own timeout set via the worker timeout
> parameter or
> they use the server timeout as default, if no worker timeout is
> set. Although this (nearly)
> works as documented I plan to change this to let the workers use
> the ProxyTimeout setting
> as a default value in the case that they do not have their own
> timeout set via a parameter.
> This sounds a lot more sane to me instead of using the server
> timeout here as a default value.
>
The logic should be:
1. If a per-worker value is set, use that.
2. If not, then if a ProxyTimeout value is set, use that.
3. Otherwise, use Timeout
+1 on fixing that :)
OK, I did something similar already. I attached a 'diff' that really
is not valid ;) I had a whole bunch of other changes in there and did
not have time to get a proper diff and just removed my extra stuff
that did not pertain to timeouts.
Basically, I created a ap_proxy_do_connect_backend as a replacement
for ap_proxy_connect_backend that adds a request_rec *r parameter and
put the timeout stuff in
ap_proxy_do_connect_backend.
I needed the request_rec in there for some other work I was doing so
it turned out to work nicely. Read the large comment block in the
'diff' for an explanation.
Comments on the idea of this?
And again note that this is not a proper diff, but just some text to
give you an idea of what I have done.
thanks,
-B
--- ./modules/proxy/proxy_util.c 2006-07-11 23:38:44.000000000 -0400
+++ ../../trunk/./modules/proxy/proxy_util.c 2007-03-16 13:26:05.000000000 -0400
+PROXY_DECLARE(int) ap_proxy_do_connect_backend(const char *proxy_function,
+ proxy_conn_rec *conn,
+ proxy_worker *worker,
+ server_rec *s,
+ request_rec *r)
@@ -2029,9 +2120,15 @@
/* Set a timeout on the socket */
if (worker->timeout_set == 1) {
apr_socket_timeout_set(newsock, worker->timeout);
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+ "proxy: using worker socket timeout=%"
+ APR_INT64_T_FMT, worker->timeout);
}
else {
- apr_socket_timeout_set(newsock, s->timeout);
+ apr_socket_timeout_set(newsock, s->timeout);
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+ "proxy: using server socket timeout=%"
+ APR_INT64_T_FMT, s->timeout);
}
/* Set a keepalive option */
if (worker->keepalive) {
@@ -2076,23 +2212,36 @@
worker->s->status |= PROXY_WORKER_IN_ERROR;
worker->s->error_time = apr_time_now();
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
- "ap_proxy_connect_backend disabling worker for (%s)",
+ "ap_proxy_do_connect_backend disabling worker for (%s)",
worker->hostname);
}
else {
worker->s->error_time = 0;
worker->s->retries = 0;
}
+
return connected ? OK : DECLINED;
}
+/* Just wrap ap_proxy_do_connect_backend */
+PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function,
+ proxy_conn_rec *conn,
+ proxy_worker *worker,
+ server_rec *s)
+{
+ return ap_proxy_do_connect_backend(proxy_function, conn, worker, s, NULL);
+}
+
@@ -2152,6 +2301,42 @@
return rc;
}
+ /* Set a timeout on the socket for the request/response to the backend
+ *
+ * This overloads ProxyTimeout to be used as both the connection
+ * timeout and the request/response timeout. You can use the
+ * timeout= option to further override the connection timeout for even
+ * finer grained control.
+ *
+ * ProxyTimeout 300
+ * ProxyPass / http:/foo:8000/
+ *
+ * Both backend connection and request/response timeout is 300s
+ *
+ * ProxyTimeout 300
+ * ProxyPass / http:/foo:8000/ timeout=5
+ *
+ * Here the connection timeout is 5s and the req/res timeout is 300s
+ *
+ * If ProxyTimeout is not set Timeout is used.
+ *
+ * TODO make this another parameter, maybe rtimeout (req/res timeout)?
+ */
+ if (conf) {
+ apr_interval_time_t prev_timeout;
+ apr_socket_timeout_get(conn->sock, &prev_timeout);
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+ "proxy: timeout sock=%"APR_INT64_T_FMT" conf=%"APR_INT64_T_FMT"",
+ prev_timeout, conf->timeout);
+
+ if (conf->timeout_set == 1) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+ "proxy: setting socket timeout %"APR_INT64_T_FMT" -> %"APR_INT64_T_FMT"",
+ prev_timeout, conf->timeout);
+ apr_socket_timeout_set(conn->sock, conf->timeout);
+ }
+ }
+
return OK;
}
--- ./modules/proxy/mod_proxy_http.c 2006-07-11 23:38:44.000000000 -0400
+++ ../../trunk/./modules/proxy/mod_proxy_http.c 2007-03-15 08:21:54.000000000 -0400
@@ -1687,7 +1691,7 @@
goto cleanup;
/* Step Two: Make the Connection */
- if (ap_proxy_connect_backend(proxy_function, backend, worker, r->server)) {
+ if (ap_proxy_do_connect_backend(proxy_function, backend, worker, r->server, r)) {
if (r->proxyreq == PROXYREQ_PROXY)
status = HTTP_NOT_FOUND;
else
--- ./modules/proxy/mod_proxy.h 2006-07-11 23:38:44.000000000 -0400
+++ ../../trunk/./modules/proxy/mod_proxy.h 2007-03-16 13:26:52.000000000 -0400
@@ -660,6 +663,22 @@
proxy_conn_rec *conn,
server_rec *s);
/**
+ * Make a connection to the backend with the optional request
+ * @param proxy_function calling proxy scheme (http, ajp, ...)
+ * @param conn acquired connection
+ * @param worker connection worker
+ * @param s current server record
+ * @param r current request record (if available)
+ * @return OK or HTTP_XXX error
+ * @note In case the socket already exists for conn, just check the link
+ * status.
+ */
+PROXY_DECLARE(int) ap_proxy_do_connect_backend(const char *proxy_function,
+ proxy_conn_rec *conn,
+ proxy_worker *worker,
+ server_rec *s,
+ request_rec *r);
+/**
* Make a connection to the backend
* @param proxy_function calling proxy scheme (http, ajp, ...)
* @param conn acquired connection
--- ./modules/proxy/mod_proxy_ftp.c 2006-07-11 23:38:44.000000000 -0400
+++ ../../trunk/./modules/proxy/mod_proxy_ftp.c 2007-03-15 08:21:40.000000000 -0400
@@ -958,7 +960,7 @@
*/
- if (ap_proxy_connect_backend("FTP", backend, worker, r->server)) {
+ if (ap_proxy_do_connect_backend("FTP", backend, worker, r->server, r)) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"proxy: FTP: an error occurred creating a new connection to %pI (%s)",
connect_addr, connectname);
--- ./modules/proxy/mod_proxy_ajp.c 2006-07-11 23:38:44.000000000 -0400
+++ ../../trunk/./modules/proxy/mod_proxy_ajp.c 2007-03-15 08:21:30.000000000 -0400
@@ -522,7 +524,7 @@
goto cleanup;
/* Step Two: Make the Connection */
- if (ap_proxy_connect_backend(scheme, backend, worker, r->server)) {
+ if (ap_proxy_do_connect_backend(scheme, backend, worker, r->server, r)) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
"proxy: AJP: failed to make connection to backend: %s",
backend->hostname);
--- ./modules/proxy/mod_proxy.c 2006-07-11 23:38:44.000000000 -0400
+++ ../../trunk/./modules/proxy/mod_proxy.c 2007-03-07 13:28:01.000000000 -0500
@@ -873,15 +876,25 @@
ps->domain = (overrides->domain == NULL) ? base->domain : overrides->domain;
ps->viaopt = (overrides->viaopt_set == 0) ? base->viaopt : overrides->viaopt;
+ ps->viaopt_set = overrides->viaopt_set || base->viaopt_set;
ps->req = (overrides->req_set == 0) ? base->req : overrides->req;
+ ps->req_set = overrides->req_set || base->req_set;
ps->recv_buffer_size = (overrides->recv_buffer_size_set == 0) ? base->recv_buffer_size : overrides->recv_buffer_size;
+ ps->recv_buffer_size_set = overrides->recv_buffer_size_set || base->recv_buffer_size_set;
ps->io_buffer_size = (overrides->io_buffer_size_set == 0) ? base->io_buffer_size : overrides->io_buffer_size;
+ ps->io_buffer_size_set = overrides->io_buffer_size_set || base->io_buffer_size_set;
ps->maxfwd = (overrides->maxfwd_set == 0) ? base->maxfwd : overrides->maxfwd;
+ ps->maxfwd_set = overrides->maxfwd_set || base->maxfwd_set;
ps->error_override = (overrides->error_override_set == 0) ? base->error_override : overrides->error_override;
+ ps->error_override_set = overrides->error_override_set || base->error_override_set;
ps->preserve_host = (overrides->preserve_host_set == 0) ? base->preserve_host : overrides->preserve_host;
+ ps->preserve_host_set = overrides->preserve_host_set || base->preserve_host_set;
ps->timeout= (overrides->timeout_set == 0) ? base->timeout : overrides->timeout;
+ ps->timeout_set = overrides->timeout_set || base->timeout_set;
ps->badopt = (overrides->badopt_set == 0) ? base->badopt : overrides->badopt;
+ ps->badopt_set = overrides->badopt_set || base->badopt_set;
ps->proxy_status = (overrides->proxy_status_set == 0) ? base->proxy_status : overrides->proxy_status;
+ ps->proxy_status_set = overrides->proxy_status_set || base->proxy_status_set;
ps->pool = p;
return ps;
}
