You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2023/06/30 09:08:24 UTC
svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Author: icing
Date: Fri Jun 30 09:08:23 2023
New Revision: 1910704
URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
Log:
proxy: in proxy tunnels, use the smaller timeout value of
client and origin as timeout for polling the tunnel.
Modified:
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
@@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
- /* Defaults to the biggest timeout of both connections */
- tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
- origin_timeout : client_timeout;
+ /* Defaults to the smallest timeout of both connections */
+ tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
+ client_timeout : origin_timeout);
/* Bidirectional non-HTTP stream will confuse mod_reqtimeoout */
ap_remove_input_filter_byhandle(c_i->input_filters, "reqtimeout");
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Stefan Eissing via dev <de...@httpd.apache.org>.
> Am 03.07.2023 um 11:09 schrieb Ruediger Pluem <rp...@apache.org>:
>
>
>
> On 7/3/23 10:17 AM, Stefan Eissing via dev wrote:
>>
>>
>>> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev <de...@httpd.apache.org>:
>>>
>>>
>>>
>>>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem <rp...@apache.org>:
>>>>
>>>>
>>>>
>>>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
>>>>>
>>>>>
>>>>>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>
>>>>>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>>>>>>>
>>>>>>> On 6/30/23 11:08 AM, icing@apache.org wrote:
>>>>>>>> Author: icing
>>>>>>>> Date: Fri Jun 30 09:08:23 2023
>>>>>>>> New Revision: 1910704
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
>>>>>>>> Log:
>>>>>>>> proxy: in proxy tunnels, use the smaller timeout value of
>>>>>>>> client and origin as timeout for polling the tunnel.
>>>>>>>>
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>>>>
>>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>>>>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
>>>>>>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
>>>>>>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
>>>>>>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>>>>>>>>
>>>>>>>> - /* Defaults to the biggest timeout of both connections */
>>>>>>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
>>>>>>>> - origin_timeout : client_timeout;
>>>>>>>> + /* Defaults to the smallest timeout of both connections */
>>>>>>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
>>>>>>>> + client_timeout : origin_timeout);
>>>>>>>
>>>>>>> Why?
>>>>>>
>>>>>> We discussed this (quickly) with Stefan on
>>>>>> https://github.com/apache/httpd/pull/366, but hey the commit is here
>>>>>> for review finally :)
>>>>>>
>>>>>>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
>>>>>>> e.g. 600 to keep Websockets open for 10 minutes.
>>>>>>
>>>>>> It seems to me that using Timeout (5s) here is a valid case too if
>>>>>> Timeout < ProxyTimeout (as in your example) is a way to limit how long
>>>>>> a client can consume httpd resources.
>>>>>> So maybe we should only use the backend timeout which is an easy(er)
>>>>>> way for the user to control this?
>>>>>
>>>>> So, the goal is to allow someone keeping the websocket open for longer
>>>>> than we usually allow for HTTP requests, to set a long ProxyTimeout or
>>>>> timeout parameter to ProxyPass.
>>>>>
>>>>> For HTTP/1.1 that would override the connection Timeout, since the tunnel
>>>>> poll would only use the largest value.
>>>>>
>>>>> For HTTP/2 I have to check how that how to accomplish that. The working
>>>>> there is different.
>>>>
>>>> Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case
>>>> for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and
>>>> automatically. Any pointers for your use case such that I can have a look and be more constructive :-).
>>>>
>>>
>>> Thanks that you spoke up! I heard no grumpiness. ;)
>>
>> Ok, testing how things *REALLY* work here, I stumbled upon:
>>
>> mod_proxy_http.c:1570
>> /* Let proxy tunnel forward everything within this thread */
>> req->tunnel->timeout = req->idle_timeout;
>> status = ap_proxy_tunnel_run(req->tunnel);
>>
>> which overrides everything we discussed. So, how do we want this to work and who has the final say?
>
> Very good catch. If you look at
> https://github.com/apache/httpd/blob/cc0735d1cde406c8d15016e4b2afcebb4d11fc87/modules/proxy/mod_proxy_http.c#L2009-L2023
>
> then in the proxy case the backend timeout seems to win. Hence my crying was wrong and in the http proxy case things still work as
> before. The above was for trunk. In case of 2.4.x we also override the setting from ap_proxy_tunnel_create here:
> https://github.com/apache/httpd/blob/aefc30086aa71f24f77290deb88f949f36575c05/modules/proxy/mod_proxy_http.c#L1538-L1549 in the
> same fashion as we set it in ap_proxy_tunnel_create: The longest one wins.
>
> What is actually your use case to use the shortest one? The new WebSocket over HTTP/2 feature? Do we get hit by the fact that on
> HTTP/2 we have multiple conn_recs / streams over one TCP connection?
No actual use case. I am just looking to make it work with h2 exactly as with HTTP/1.1. The discussion just came up as Yann was pondering if the old implementation was really the correct one.
I'll revert the timeout selection during tunnel creation to the old behaviour (select max value).
Cheers,
Stefan
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jul 3, 2023 at 11:10 AM Ruediger Pluem <rp...@apache.org> wrote:
>
>
>
> On 7/3/23 10:17 AM, Stefan Eissing via dev wrote:
> >
> >
> >> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev <de...@httpd.apache.org>:
> >>
> >>
> >>
> >>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem <rp...@apache.org>:
> >>>
> >>>
> >>>
> >>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
> >>>>
> >>>>
> >>>>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
> >>>>>
> >>>>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
> >>>>>>
> >>>>>> On 6/30/23 11:08 AM, icing@apache.org wrote:
> >>>>>>> Author: icing
> >>>>>>> Date: Fri Jun 30 09:08:23 2023
> >>>>>>> New Revision: 1910704
> >>>>>>>
> >>>>>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
> >>>>>>> Log:
> >>>>>>> proxy: in proxy tunnels, use the smaller timeout value of
> >>>>>>> client and origin as timeout for polling the tunnel.
> >>>>>>>
> >>>>>>>
> >>>>>>> Modified:
> >>>>>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
> >>>>>>>
> >>>>>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> >>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
> >>>>>>> ==============================================================================
> >>>>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> >>>>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
> >>>>>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
> >>>>>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
> >>>>>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
> >>>>>>>
> >>>>>>> - /* Defaults to the biggest timeout of both connections */
> >>>>>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
> >>>>>>> - origin_timeout : client_timeout;
> >>>>>>> + /* Defaults to the smallest timeout of both connections */
> >>>>>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
> >>>>>>> + client_timeout : origin_timeout);
> >>>>>>
> >>>>>> Why?
> >>>>>
> >>>>> We discussed this (quickly) with Stefan on
> >>>>> https://github.com/apache/httpd/pull/366, but hey the commit is here
> >>>>> for review finally :)
> >>>>>
> >>>>>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
> >>>>>> e.g. 600 to keep Websockets open for 10 minutes.
> >>>>>
> >>>>> It seems to me that using Timeout (5s) here is a valid case too if
> >>>>> Timeout < ProxyTimeout (as in your example) is a way to limit how long
> >>>>> a client can consume httpd resources.
> >>>>> So maybe we should only use the backend timeout which is an easy(er)
> >>>>> way for the user to control this?
> >>>>
> >>>> So, the goal is to allow someone keeping the websocket open for longer
> >>>> than we usually allow for HTTP requests, to set a long ProxyTimeout or
> >>>> timeout parameter to ProxyPass.
> >>>>
> >>>> For HTTP/1.1 that would override the connection Timeout, since the tunnel
> >>>> poll would only use the largest value.
> >>>>
> >>>> For HTTP/2 I have to check how that how to accomplish that. The working
> >>>> there is different.
> >>>
> >>> Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case
> >>> for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and
> >>> automatically. Any pointers for your use case such that I can have a look and be more constructive :-).
> >>>
> >>
> >> Thanks that you spoke up! I heard no grumpiness. ;)
> >
> > Ok, testing how things *REALLY* work here, I stumbled upon:
> >
> > mod_proxy_http.c:1570
> > /* Let proxy tunnel forward everything within this thread */
> > req->tunnel->timeout = req->idle_timeout;
> > status = ap_proxy_tunnel_run(req->tunnel);
> >
> > which overrides everything we discussed. So, how do we want this to work and who has the final say?
>
> Very good catch. If you look at
> https://github.com/apache/httpd/blob/cc0735d1cde406c8d15016e4b2afcebb4d11fc87/modules/proxy/mod_proxy_http.c#L2009-L2023
>
> then in the proxy case the backend timeout seems to win. Hence my crying was wrong and in the http proxy case things still work as
> before. The above was for trunk. In case of 2.4.x we also override the setting from ap_proxy_tunnel_create here:
> https://github.com/apache/httpd/blob/aefc30086aa71f24f77290deb88f949f36575c05/modules/proxy/mod_proxy_http.c#L1538-L1549 in the
> same fashion as we set it in ap_proxy_tunnel_create: The longest one wins.
>
> What is actually your use case to use the shortest one? The new WebSocket over HTTP/2 feature? Do we get hit by the fact that on
> HTTP/2 we have multiple conn_recs / streams over one TCP connection?
The issue with using the bigger or smaller of the client and backend
timeouts is that the use cases (reasons) for having different values
for them in the first place are multiple, and it relates more to the
HTTP communication than websocket/tunneling. That's why I added
ProxyAsyncIdleTimeout in trunk (not in 2.4.x yet since modules async
helpers are not there yet).
For now I agree that using the max is the best option finally :)
Regards;
Yann.
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 7/3/23 10:17 AM, Stefan Eissing via dev wrote:
>
>
>> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev <de...@httpd.apache.org>:
>>
>>
>>
>>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem <rp...@apache.org>:
>>>
>>>
>>>
>>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
>>>>
>>>>
>>>>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>
>>>>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>>>>>>
>>>>>> On 6/30/23 11:08 AM, icing@apache.org wrote:
>>>>>>> Author: icing
>>>>>>> Date: Fri Jun 30 09:08:23 2023
>>>>>>> New Revision: 1910704
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
>>>>>>> Log:
>>>>>>> proxy: in proxy tunnels, use the smaller timeout value of
>>>>>>> client and origin as timeout for polling the tunnel.
>>>>>>>
>>>>>>>
>>>>>>> Modified:
>>>>>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>>>
>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>>>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
>>>>>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
>>>>>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
>>>>>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>>>>>>>
>>>>>>> - /* Defaults to the biggest timeout of both connections */
>>>>>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
>>>>>>> - origin_timeout : client_timeout;
>>>>>>> + /* Defaults to the smallest timeout of both connections */
>>>>>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
>>>>>>> + client_timeout : origin_timeout);
>>>>>>
>>>>>> Why?
>>>>>
>>>>> We discussed this (quickly) with Stefan on
>>>>> https://github.com/apache/httpd/pull/366, but hey the commit is here
>>>>> for review finally :)
>>>>>
>>>>>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
>>>>>> e.g. 600 to keep Websockets open for 10 minutes.
>>>>>
>>>>> It seems to me that using Timeout (5s) here is a valid case too if
>>>>> Timeout < ProxyTimeout (as in your example) is a way to limit how long
>>>>> a client can consume httpd resources.
>>>>> So maybe we should only use the backend timeout which is an easy(er)
>>>>> way for the user to control this?
>>>>
>>>> So, the goal is to allow someone keeping the websocket open for longer
>>>> than we usually allow for HTTP requests, to set a long ProxyTimeout or
>>>> timeout parameter to ProxyPass.
>>>>
>>>> For HTTP/1.1 that would override the connection Timeout, since the tunnel
>>>> poll would only use the largest value.
>>>>
>>>> For HTTP/2 I have to check how that how to accomplish that. The working
>>>> there is different.
>>>
>>> Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case
>>> for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and
>>> automatically. Any pointers for your use case such that I can have a look and be more constructive :-).
>>>
>>
>> Thanks that you spoke up! I heard no grumpiness. ;)
>
> Ok, testing how things *REALLY* work here, I stumbled upon:
>
> mod_proxy_http.c:1570
> /* Let proxy tunnel forward everything within this thread */
> req->tunnel->timeout = req->idle_timeout;
> status = ap_proxy_tunnel_run(req->tunnel);
>
> which overrides everything we discussed. So, how do we want this to work and who has the final say?
Very good catch. If you look at
https://github.com/apache/httpd/blob/cc0735d1cde406c8d15016e4b2afcebb4d11fc87/modules/proxy/mod_proxy_http.c#L2009-L2023
then in the proxy case the backend timeout seems to win. Hence my crying was wrong and in the http proxy case things still work as
before. The above was for trunk. In case of 2.4.x we also override the setting from ap_proxy_tunnel_create here:
https://github.com/apache/httpd/blob/aefc30086aa71f24f77290deb88f949f36575c05/modules/proxy/mod_proxy_http.c#L1538-L1549 in the
same fashion as we set it in ap_proxy_tunnel_create: The longest one wins.
What is actually your use case to use the shortest one? The new WebSocket over HTTP/2 feature? Do we get hit by the fact that on
HTTP/2 we have multiple conn_recs / streams over one TCP connection?
Regards
Rüdiger
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Stefan Eissing via dev <de...@httpd.apache.org>.
> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev <de...@httpd.apache.org>:
>
>
>
>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem <rp...@apache.org>:
>>
>>
>>
>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
>>>
>>>
>>>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>
>>>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>>>>>
>>>>> On 6/30/23 11:08 AM, icing@apache.org wrote:
>>>>>> Author: icing
>>>>>> Date: Fri Jun 30 09:08:23 2023
>>>>>> New Revision: 1910704
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
>>>>>> Log:
>>>>>> proxy: in proxy tunnels, use the smaller timeout value of
>>>>>> client and origin as timeout for polling the tunnel.
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
>>>>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
>>>>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
>>>>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>>>>>>
>>>>>> - /* Defaults to the biggest timeout of both connections */
>>>>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
>>>>>> - origin_timeout : client_timeout;
>>>>>> + /* Defaults to the smallest timeout of both connections */
>>>>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
>>>>>> + client_timeout : origin_timeout);
>>>>>
>>>>> Why?
>>>>
>>>> We discussed this (quickly) with Stefan on
>>>> https://github.com/apache/httpd/pull/366, but hey the commit is here
>>>> for review finally :)
>>>>
>>>>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
>>>>> e.g. 600 to keep Websockets open for 10 minutes.
>>>>
>>>> It seems to me that using Timeout (5s) here is a valid case too if
>>>> Timeout < ProxyTimeout (as in your example) is a way to limit how long
>>>> a client can consume httpd resources.
>>>> So maybe we should only use the backend timeout which is an easy(er)
>>>> way for the user to control this?
>>>
>>> So, the goal is to allow someone keeping the websocket open for longer
>>> than we usually allow for HTTP requests, to set a long ProxyTimeout or
>>> timeout parameter to ProxyPass.
>>>
>>> For HTTP/1.1 that would override the connection Timeout, since the tunnel
>>> poll would only use the largest value.
>>>
>>> For HTTP/2 I have to check how that how to accomplish that. The working
>>> there is different.
>>
>> Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case
>> for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and
>> automatically. Any pointers for your use case such that I can have a look and be more constructive :-).
>>
>
> Thanks that you spoke up! I heard no grumpiness. ;)
Ok, testing how things *REALLY* work here, I stumbled upon:
mod_proxy_http.c:1570
/* Let proxy tunnel forward everything within this thread */
req->tunnel->timeout = req->idle_timeout;
status = ap_proxy_tunnel_run(req->tunnel);
which overrides everything we discussed. So, how do we want this to work and who has the final say?
- Stefan
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Stefan Eissing via dev <de...@httpd.apache.org>.
> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem <rp...@apache.org>:
>
>
>
> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
>>
>>
>>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>>>>
>>>> On 6/30/23 11:08 AM, icing@apache.org wrote:
>>>>> Author: icing
>>>>> Date: Fri Jun 30 09:08:23 2023
>>>>> New Revision: 1910704
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
>>>>> Log:
>>>>> proxy: in proxy tunnels, use the smaller timeout value of
>>>>> client and origin as timeout for polling the tunnel.
>>>>>
>>>>>
>>>>> Modified:
>>>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>>
>>>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
>>>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
>>>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
>>>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>>>>>
>>>>> - /* Defaults to the biggest timeout of both connections */
>>>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
>>>>> - origin_timeout : client_timeout;
>>>>> + /* Defaults to the smallest timeout of both connections */
>>>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
>>>>> + client_timeout : origin_timeout);
>>>>
>>>> Why?
>>>
>>> We discussed this (quickly) with Stefan on
>>> https://github.com/apache/httpd/pull/366, but hey the commit is here
>>> for review finally :)
>>>
>>>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
>>>> e.g. 600 to keep Websockets open for 10 minutes.
>>>
>>> It seems to me that using Timeout (5s) here is a valid case too if
>>> Timeout < ProxyTimeout (as in your example) is a way to limit how long
>>> a client can consume httpd resources.
>>> So maybe we should only use the backend timeout which is an easy(er)
>>> way for the user to control this?
>>
>> So, the goal is to allow someone keeping the websocket open for longer
>> than we usually allow for HTTP requests, to set a long ProxyTimeout or
>> timeout parameter to ProxyPass.
>>
>> For HTTP/1.1 that would override the connection Timeout, since the tunnel
>> poll would only use the largest value.
>>
>> For HTTP/2 I have to check how that how to accomplish that. The working
>> there is different.
>
> Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case
> for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and
> automatically. Any pointers for your use case such that I can have a look and be more constructive :-).
>
Thanks that you spoke up! I heard no grumpiness. ;)
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
>
>
>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>>>
>>> On 6/30/23 11:08 AM, icing@apache.org wrote:
>>>> Author: icing
>>>> Date: Fri Jun 30 09:08:23 2023
>>>> New Revision: 1910704
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
>>>> Log:
>>>> proxy: in proxy tunnels, use the smaller timeout value of
>>>> client and origin as timeout for polling the tunnel.
>>>>
>>>>
>>>> Modified:
>>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>>
>>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
>>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
>>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
>>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>>>>
>>>> - /* Defaults to the biggest timeout of both connections */
>>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
>>>> - origin_timeout : client_timeout;
>>>> + /* Defaults to the smallest timeout of both connections */
>>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
>>>> + client_timeout : origin_timeout);
>>>
>>> Why?
>>
>> We discussed this (quickly) with Stefan on
>> https://github.com/apache/httpd/pull/366, but hey the commit is here
>> for review finally :)
>>
>>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
>>> e.g. 600 to keep Websockets open for 10 minutes.
>>
>> It seems to me that using Timeout (5s) here is a valid case too if
>> Timeout < ProxyTimeout (as in your example) is a way to limit how long
>> a client can consume httpd resources.
>> So maybe we should only use the backend timeout which is an easy(er)
>> way for the user to control this?
>
> So, the goal is to allow someone keeping the websocket open for longer
> than we usually allow for HTTP requests, to set a long ProxyTimeout or
> timeout parameter to ProxyPass.
>
> For HTTP/1.1 that would override the connection Timeout, since the tunnel
> poll would only use the largest value.
>
> For HTTP/2 I have to check how that how to accomplish that. The working
> there is different.
Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case
for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and
automatically. Any pointers for your use case such that I can have a look and be more constructive :-).
Regards
Rüdiger
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Stefan Eissing via dev <de...@httpd.apache.org>.
> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <yl...@gmail.com>:
>
> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 6/30/23 11:08 AM, icing@apache.org wrote:
>>> Author: icing
>>> Date: Fri Jun 30 09:08:23 2023
>>> New Revision: 1910704
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
>>> Log:
>>> proxy: in proxy tunnels, use the smaller timeout value of
>>> client and origin as timeout for polling the tunnel.
>>>
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>
>>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
>>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
>>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
>>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>>>
>>> - /* Defaults to the biggest timeout of both connections */
>>> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
>>> - origin_timeout : client_timeout;
>>> + /* Defaults to the smallest timeout of both connections */
>>> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
>>> + client_timeout : origin_timeout);
>>
>> Why?
>
> We discussed this (quickly) with Stefan on
> https://github.com/apache/httpd/pull/366, but hey the commit is here
> for review finally :)
>
>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
>> e.g. 600 to keep Websockets open for 10 minutes.
>
> It seems to me that using Timeout (5s) here is a valid case too if
> Timeout < ProxyTimeout (as in your example) is a way to limit how long
> a client can consume httpd resources.
> So maybe we should only use the backend timeout which is an easy(er)
> way for the user to control this?
So, the goal is to allow someone keeping the websocket open for longer
than we usually allow for HTTP requests, to set a long ProxyTimeout or
timeout parameter to ProxyPass.
For HTTP/1.1 that would override the connection Timeout, since the tunnel
poll would only use the largest value.
For HTTP/2 I have to check how that how to accomplish that. The working
there is different.
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 6/30/23 11:08 AM, icing@apache.org wrote:
> > Author: icing
> > Date: Fri Jun 30 09:08:23 2023
> > New Revision: 1910704
> >
> > URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
> > Log:
> > proxy: in proxy tunnels, use the smaller timeout value of
> > client and origin as timeout for polling the tunnel.
> >
> >
> > Modified:
> > httpd/httpd/trunk/modules/proxy/proxy_util.c
> >
> > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
> > @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
> > apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
> > apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
> >
> > - /* Defaults to the biggest timeout of both connections */
> > - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
> > - origin_timeout : client_timeout;
> > + /* Defaults to the smallest timeout of both connections */
> > + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
> > + client_timeout : origin_timeout);
>
> Why?
We discussed this (quickly) with Stefan on
https://github.com/apache/httpd/pull/366, but hey the commit is here
for review finally :)
> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
> e.g. 600 to keep Websockets open for 10 minutes.
It seems to me that using Timeout (5s) here is a valid case too if
Timeout < ProxyTimeout (as in your example) is a way to limit how long
a client can consume httpd resources.
So maybe we should only use the backend timeout which is an easy(er)
way for the user to control this?
Regards;
Yann.
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 6/30/23 11:08 AM, icing@apache.org wrote:
> Author: icing
> Date: Fri Jun 30 09:08:23 2023
> New Revision: 1910704
>
> URL: http://svn.apache.org/viewvc?rev=1910704&view=rev
> Log:
> proxy: in proxy tunnels, use the smaller timeout value of
> client and origin as timeout for polling the tunnel.
>
>
> Modified:
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704&r1=1910703&r2=1910704&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023
> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun
> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, &origin_timeout);
> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1);
>
> - /* Defaults to the biggest timeout of both connections */
> - tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)?
> - origin_timeout : client_timeout;
> + /* Defaults to the smallest timeout of both connections */
> + tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ?
> + client_timeout : origin_timeout);
Why? It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to
e.g. 600 to keep Websockets open for 10 minutes.
Regards
Rüdiger