Re: ProxyTimeout does not work as documented
On Mon, 2007-06-18 at 13:57 +0200, Plüm, Rüdiger, VF-Group wrote:
-Ursprüngliche Nachricht-
Von: Jean-Frederic
Gesendet: Montag, 18. Juni 2007 12:06
An: dev@httpd.apache.org
Betreff: Re: ProxyTimeout does not work as documented
On Sat, 2007-06-16 at 17:40 +0200, Ruediger Pluem wrote:
Digging somewhat deeper to actually do this I found the following
obstacle (hopefully I am wrong):
1. The right place to fix this is in ap_proxy_connect_backend
(around line 2215 in proxy_util.c).
The Timeout is the Default value already See
c-base_server-timeout in
core_pre_connection().
This happens afterwards in ap_proxy_connection_create and not for
mod_proxy_ajp as
mod_proxy_ajp does not call ap_proxy_connection_create. This was fixed by
your patch in r546128.
2. We do not have a proxy_server_conf parameter in
ap_proxy_connect_backend
like in ap_proxy_connect_to_backend.
3. ap_proxy_connect_backend is part of a public API (mod_proxy.h).
4. Fixing the prototype of ap_proxy_connect_backend to add
a proxy_server_conf
parameter thus requires a major bump. Thus this would
not be backportable :-(.
I am not quite sure if we can do
void *sconf = s-module_config;
proxy_server_conf *conf = (proxy_server_conf *)
ap_get_module_config(sconf, proxy_module);
See modules/proxy/mod_proxy_balancer.c
Thanks for the pointer, but I stick to my opinion that this is ugly
there. But possibly we can go for the ugly way in 2.2.x and for the
API change in trunk.
The ugly hack works. Should I apply the attached patch?
Cheers
Jean-Frederic
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c (revision 546396)
+++ modules/proxy/proxy_util.c (working copy)
@@ -2167,6 +2167,9 @@
int loglevel;
apr_sockaddr_t *backend_addr = conn-addr;
apr_socket_t *newsock;
+void *sconf = s-module_config;
+proxy_server_conf *conf =
+(proxy_server_conf *) ap_get_module_config(sconf, proxy_module);
if (conn-sock) {
/*
@@ -2215,6 +2218,9 @@
if (worker-timeout_set == 1) {
apr_socket_timeout_set(newsock, worker-timeout);
}
+else if (conf-timeout_set == 1) {
+apr_socket_timeout_set(newsock, conf-timeout);
+}
else {
apr_socket_timeout_set(newsock, s-timeout);
}
Re: ProxyTimeout does not work as documented
-Ursprüngliche Nachricht- Von: Jean-Frederic Gesendet: Donnerstag, 21. Juni 2007 09:56 An: dev@httpd.apache.org Betreff: Re: ProxyTimeout does not work as documented On Mon, 2007-06-18 at 13:57 +0200, Plüm, Rüdiger, VF-Group wrote: -Ursprüngliche Nachricht- Von: Jean-Frederic Gesendet: Montag, 18. Juni 2007 12:06 An: dev@httpd.apache.org Betreff: Re: ProxyTimeout does not work as documented On Sat, 2007-06-16 at 17:40 +0200, Ruediger Pluem wrote: Digging somewhat deeper to actually do this I found the following obstacle (hopefully I am wrong): 1. The right place to fix this is in ap_proxy_connect_backend (around line 2215 in proxy_util.c). The Timeout is the Default value already See c-base_server-timeout in core_pre_connection(). This happens afterwards in ap_proxy_connection_create and not for mod_proxy_ajp as mod_proxy_ajp does not call ap_proxy_connection_create. This was fixed by your patch in r546128. 2. We do not have a proxy_server_conf parameter in ap_proxy_connect_backend like in ap_proxy_connect_to_backend. 3. ap_proxy_connect_backend is part of a public API (mod_proxy.h). 4. Fixing the prototype of ap_proxy_connect_backend to add a proxy_server_conf parameter thus requires a major bump. Thus this would not be backportable :-(. I am not quite sure if we can do void *sconf = s-module_config; proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, proxy_module); See modules/proxy/mod_proxy_balancer.c Thanks for the pointer, but I stick to my opinion that this is ugly there. But possibly we can go for the ugly way in 2.2.x and for the API change in trunk. The ugly hack works. Should I apply the attached patch? As said for trunk I would prefer the cleaner way by changing the API (including the needed major bump). I think the patch is a good solution as a backport for solving the issue in 2.2.x. I hoped to get more feedback on this approach, but it seems that currently only you and I are working on this. So once we agree on the solution we should move forward. Lazy consensus :-). So opinions on the above? Regards Rüdiger
Re: ProxyTimeout does not work as documented
On Mon, 2007-06-18 at 13:57 +0200, Plüm, Rüdiger, VF-Group wrote: -Ursprüngliche Nachricht- Von: Jean-Frederic Gesendet: Montag, 18. Juni 2007 12:06 An: dev@httpd.apache.org Betreff: Re: ProxyTimeout does not work as documented On Sat, 2007-06-16 at 17:40 +0200, Ruediger Pluem wrote: Digging somewhat deeper to actually do this I found the following obstacle (hopefully I am wrong): 1. The right place to fix this is in ap_proxy_connect_backend (around line 2215 in proxy_util.c). The Timeout is the Default value already See c-base_server-timeout in core_pre_connection(). This happens afterwards in ap_proxy_connection_create and not for mod_proxy_ajp as mod_proxy_ajp does not call ap_proxy_connection_create. This was fixed by your patch in r546128. Ok in AJP we really need to check the 3 timeouts. 2. We do not have a proxy_server_conf parameter in ap_proxy_connect_backend like in ap_proxy_connect_to_backend. 3. ap_proxy_connect_backend is part of a public API (mod_proxy.h). 4. Fixing the prototype of ap_proxy_connect_backend to add a proxy_server_conf parameter thus requires a major bump. Thus this would not be backportable :-(. I am not quite sure if we can do void *sconf = s-module_config; proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, proxy_module); See modules/proxy/mod_proxy_balancer.c Thanks for the pointer, but I stick to my opinion that this is ugly there. But possibly we can go for the ugly way in 2.2.x and for the API change in trunk. BTW: ping is not documented in the 2.2 doc, why? Nobody backported the docs so far ;-). Seriously, currently I see no other reason than this. I have committed the changes in manual/mod/mod_proxy.xml Cheers Jean-Frederic Regards Rüdiger
Re: ProxyTimeout does not work as documented
On Sat, 2007-06-16 at 17:40 +0200, Ruediger Pluem wrote: On 05/21/2007 02:44 PM, Jim Jagielski wrote: 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 :) Digging somewhat deeper to actually do this I found the following obstacle (hopefully I am wrong): 1. The right place to fix this is in ap_proxy_connect_backend (around line 2215 in proxy_util.c). The Timeout is the Default value already See c-base_server-timeout in core_pre_connection(). 2. We do not have a proxy_server_conf parameter in ap_proxy_connect_backend like in ap_proxy_connect_to_backend. 3. ap_proxy_connect_backend is part of a public API (mod_proxy.h). 4. Fixing the prototype of ap_proxy_connect_backend to add a proxy_server_conf parameter thus requires a major bump. Thus this would not be backportable :-(. I am not quite sure if we can do void *sconf = s-module_config; proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, proxy_module); See modules/proxy/mod_proxy_balancer.c to get the proxy_server_conf in ap_proxy_connect_backend as we are calling ap_proxy_connect_backend from mod_proxy_ajp / mod_proxy_http and so the symbol proxy_module might not be present there. At the very least it looks like an uncool way of obtaining the proxy_server_conf here. Suggestions? BTW: ping is not documented in the 2.2 doc, why? Cheers Jean-Frederic Regards Rüdiger
Re: ProxyTimeout does not work as documented
-Ursprüngliche Nachricht- Von: Jean-Frederic Gesendet: Montag, 18. Juni 2007 12:06 An: dev@httpd.apache.org Betreff: Re: ProxyTimeout does not work as documented On Sat, 2007-06-16 at 17:40 +0200, Ruediger Pluem wrote: Digging somewhat deeper to actually do this I found the following obstacle (hopefully I am wrong): 1. The right place to fix this is in ap_proxy_connect_backend (around line 2215 in proxy_util.c). The Timeout is the Default value already See c-base_server-timeout in core_pre_connection(). This happens afterwards in ap_proxy_connection_create and not for mod_proxy_ajp as mod_proxy_ajp does not call ap_proxy_connection_create. This was fixed by your patch in r546128. 2. We do not have a proxy_server_conf parameter in ap_proxy_connect_backend like in ap_proxy_connect_to_backend. 3. ap_proxy_connect_backend is part of a public API (mod_proxy.h). 4. Fixing the prototype of ap_proxy_connect_backend to add a proxy_server_conf parameter thus requires a major bump. Thus this would not be backportable :-(. I am not quite sure if we can do void *sconf = s-module_config; proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, proxy_module); See modules/proxy/mod_proxy_balancer.c Thanks for the pointer, but I stick to my opinion that this is ugly there. But possibly we can go for the ugly way in 2.2.x and for the API change in trunk. BTW: ping is not documented in the 2.2 doc, why? Nobody backported the docs so far ;-). Seriously, currently I see no other reason than this. Regards Rüdiger
Re: ProxyTimeout does not work as documented
On 05/21/2007 02:44 PM, Jim Jagielski wrote: 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 :) Digging somewhat deeper to actually do this I found the following obstacle (hopefully I am wrong): 1. The right place to fix this is in ap_proxy_connect_backend (around line 2215 in proxy_util.c). 2. We do not have a proxy_server_conf parameter in ap_proxy_connect_backend like in ap_proxy_connect_to_backend. 3. ap_proxy_connect_backend is part of a public API (mod_proxy.h). 4. Fixing the prototype of ap_proxy_connect_backend to add a proxy_server_conf parameter thus requires a major bump. Thus this would not be backportable :-(. I am not quite sure if we can do void *sconf = s-module_config; proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, proxy_module); to get the proxy_server_conf in ap_proxy_connect_backend as we are calling ap_proxy_connect_backend from mod_proxy_ajp / mod_proxy_http and so the symbol proxy_module might not be present there. At the very least it looks like an uncool way of obtaining the proxy_server_conf here. Suggestions? Regards Rüdiger
Re: ProxyTimeout does not work as documented
On Tue, 2007-05-22 at 10:07 -0400, Jim Jagielski wrote: On May 21, 2007, at 5:30 PM, Ruediger Pluem wrote: On 05/21/2007 02:44 PM, Jim Jagielski wrote: 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 :) This sounds sane and I plan to do this, but what about the original question? Do I get you right that you propose to adjust the documentation for ProxyTimeout? The current behaviour of ProxyTimeout is to fall back to Timeout if no ProxyTimeout is set. The documented behaviour is to have a default value of 300 secs if there is no ProxyTimeout set (regardless of the setting of Timeout, which also defaults to 300). I think that the above logic makes the most sense and that the code and the docs should be adjusted to match the logic :) :) The timeout is set to c-base_server-timeout in core_pre_connection() called by ap_proxy_connection_create via ap_run_pre_connection. Quick patch is apr_socket_timeout_get() before ap_run_pre_connection and apr_socket_timeout_set() after if needed. Comments? Cheers Jean-Frederic
Re: ProxyTimeout does not work as documented
Dear customer, Thank you for your message. Due to the extraordinarily large number of e-mails that we are currently receiving, it might take us up to several days to reply to your request. We thank you for your patience and understanding, and will get back to you as soon as possible. With kind regards, Your NATIVE INSTRUMENTS Registration Team
Re: ProxyTimeout does not work as documented
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 :)
Re: ProxyTimeout does not work as documented
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.0 -0400
+++ ../../trunk/./modules/proxy/proxy_util.c 2007-03-16 13:26:05.0 -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 /
Re: ProxyTimeout does not work as documented
Brian Rectanus wrote: Comments on the idea of this? I was just going to point out that it's definitely useful being able to specify separate connection and actual request timeouts. From a quick look at your diff, you already have this in mind. :) An example: with a reverse proxy, you might expect to be able to connect to a backend server on the same network within milliseconds, so a connection timeout of a couple of seconds is reasonable (give up, try the next server); but the actual response could take much longer to generate. It would be nice to have the connection timeout as a proper directive - rather than only as a parameter to ProxyPass - so that people enabling mod_proxy via other mechanisms can set it. Also so that you can set a default in one place, rather than having to repeat it in multiple ProxyPass statements. Also, the option setting stuff (from http://issues.apache.org/bugzilla/show_bug.cgi?id=11540) is already fixed in trunk (and 2.2.x branch), so you might want to base your work off that. HTH -- Stuart
Re: ProxyTimeout does not work as documented
On 05/21/2007 11:29 PM, Stuart Children wrote: It would be nice to have the connection timeout as a proper directive - rather than only as a parameter to ProxyPass - so that people enabling mod_proxy via other mechanisms can set it. Also so that you can set a This issue is addressed on trunk in r427920 / r427928, which allows you to set all worker parameters (not only timeout) via a Proxy section for this worker, either by specifying them directly in Proxy or by using ProxySet inside the Proxy section for this worker. Very useful if you use the proxy via mod_rewrite. default in one place, rather than having to repeat it in multiple ProxyPass statements. Should be fixed as soon as ProxyTimeout is the fallback for a specific worker timeout. Currently you have the server Timeout working as this. Regards Rüdiger
Re: ProxyTimeout does not work as documented
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 -- Eric Covener [EMAIL PROTECTED]
Re: ProxyTimeout does not work as documented
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. Regards Rüdiger
