Re: should curl_multi_timeout() account for CURLOPT_LOW_SPEED_TIME?

[Daniel Stenberg]

On Mon, 10 Apr 2017, Rainer Canavan wrote:

A patch that turns docs/examples/multi-app.c into a reproducer, as well as 
the trivial PHP program used as a source is attached. When i run it ( > 
/dev/null) it shows that curl_multi_timeout returns values >> 1000ms:


Thanks a lot for that. Using that code it was easy to reproduce your issue!

I propose the fix I've submitted as PR #1407 [1]. It would be lovely if you 
could try it out and see if it fixes the symtoms for you as well!


[1] = https://github.com/curl/curl/pull/1407

--

 / daniel.haxx.se
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: should curl_multi_timeout() account for CURLOPT_LOW_SPEED_TIME?

[Daniel Stenberg via curl-library]

On Mon, 10 Apr 2017, Rainer Canavan via curl-library wrote:

I'm not entirely sure why that is so, but I'd like to mention that the 
CURLOPT_LOW_SPEED_TIME handling and its timeouts were just now changed, in 
commit 29147ade0456a which landed just hours ago.


I can't find that commit anywhere, is that 2d5711dc11 in the github 
repository?


Wow, I don't know how I got that weird hash (probably from a different/wrong 
branch) but yes I meant 2d5711dc11.


When CURLOPT_LOW_SPEED_TIME is set, libcurl should now use no longer than 
1000ms timeouts.


I've just merged 2d5711dc11 into 7.53.1, and I don't really see an 
improvement.


Then we clearly need to debug this more. Can you share code with us that 
reproduces this behavior easily ?


The other thing is that I would have expected CURLOPT_LOW_SPEED_TIME to be 
the actual interval over which the speed is measured, i.e. if a server does 
not send any data for 2 * CURLOPT_LOW_SPEED_TIME seconds, the transfer 
should always fail. The actual implementation uses progress.current_speed, 
which is averaged over longer periods with the result that bursty transfers 
are kept going.


It is an average speed over the last 5 seconds, yes (or less if the transfer 
is younger). Transfer speeds with its bursts and what not is not that easy to 
get "right" by everyone's view, but I think that this is a decent approach.


Well, that is *after* this recent fix because previously it was actually even 
worse! =)


That's perfectly fine, and probably more useful in reality than how I 
thought it would work, but I would say the documentation could be clearer.


Feel free to suggest improvements to the documentation!

--

 / daniel.haxx.se
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: should curl_multi_timeout() account for CURLOPT_LOW_SPEED_TIME?

[Rainer Canavan via curl-library]

On Thu, Apr 6, 2017 at 9:04 PM, Daniel Stenberg  wrote:
> On Thu, 6 Apr 2017, Rainer Canavan wrote:
>
>> it looks like curl_multi_timeout() doesn't always respect
>> CURLOPT_LOW_SPEED_TIME.
>
> I'm not entirely sure why that is so, but I'd like to mention that the
> CURLOPT_LOW_SPEED_TIME handling and its timeouts were just now changed, in
> commit 29147ade0456a which landed just hours ago.

I can't find that commit anywhere, is that 2d5711dc11 in the github repository?

> When CURLOPT_LOW_SPEED_TIME is set, libcurl should now use no longer than
> 1000ms timeouts.

I've just merged 2d5711dc11 into 7.53.1, and I don't really see an improvement.
curl_multi_timeout() still returns the same erratic values, but
docs/examples/multi-app.c
protects itself by limiting the select timeout to 1s, so I suppose I'll have
to do the same.

The other thing is that I would have expected CURLOPT_LOW_SPEED_TIME to
be the actual interval over which the speed is measured, i.e. if a
server does not
send any data for 2 * CURLOPT_LOW_SPEED_TIME seconds, the transfer should
always fail. The actual implementation uses progress.current_speed, which is
averaged over longer periods with the result that bursty transfers are
kept going.
That's perfectly fine, and probably more useful in reality than how I thought it
would work, but I would say the documentation could be clearer.


Rainer
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: should curl_multi_timeout() account for CURLOPT_LOW_SPEED_TIME?

[Daniel Stenberg]

On Thu, 6 Apr 2017, Rainer Canavan wrote:

it looks like curl_multi_timeout() doesn't always respect 
CURLOPT_LOW_SPEED_TIME.


I'm not entirely sure why that is so, but I'd like to mention that the 
CURLOPT_LOW_SPEED_TIME handling and its timeouts were just now changed, in 
commit 29147ade0456a which landed just hours ago.


When CURLOPT_LOW_SPEED_TIME is set, libcurl should now use no longer than 
1000ms timeouts.


--

 / daniel.haxx.se
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

should curl_multi_timeout() account for CURLOPT_LOW_SPEED_TIME?

[Rainer Canavan]

Hi,

it looks like curl_multi_timeout() doesn't always respect
CURLOPT_LOW_SPEED_TIME.
In therory, I'd expect all timeouts "suggested" by
curl_multi_timeout() to be less than or
equal to the lowest of CURLOPT_TIMEOUT and CURLOPT_LOW_SPEED_TIME that
is set in any curl handle it examines, and -1 if none is set. However,
if I alter
docs/examples/multi-app.c to set

 curl_easy_setopt(handles[HTTP_HANDLE], CURLOPT_TIMEOUT, 50);
 curl_easy_setopt(handles[HTTP_HANDLE], CURLOPT_LOW_SPEED_TIME, 2);
 curl_easy_setopt(handles[HTTP_HANDLE], CURLOPT_LOW_SPEED_LIMIT, 10);

and point it to a script, that prints more than SPEED_LIMIT bytes and
only a trickle
every second after that, and check the timeouts computed by
curl_multi_timeout(),
the results are somewhat surprising:

0, 199, 195, [...] 188, 188, 1, 49798, 2000, 999, 46795, 2000, 999,
43794, 195, 194,

If a server happens to stop transmitting when the select() waits until
the TIMEOUT, the
LOW_SPEED_LIMIT basically does not apply. Tested with curl 7.53.1.

A trivial workaround, namely to ignore the result of curl_multi_timeout() if
it is greater than LOW_SPEED_TIME seems to work for me.

rainer
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html