> Am 03.07.2023 um 11:09 schrieb Ruediger Pluem <rpl...@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 
>>> <dev@httpd.apache.org>:
>>> 
>>> 
>>> 
>>>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem <rpl...@apache.org>:
>>>> 
>>>> 
>>>> 
>>>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote:
>>>>> 
>>>>> 
>>>>>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic <ylavic....@gmail.com>:
>>>>>> 
>>>>>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>>>>>> 
>>>>>>> On 6/30/23 11:08 AM, ic...@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

Reply via email to