On Sat, 3 Sep 2016 09:40:52 -0400 Dave Reisner <d@falconindy.com> wrote:
On Sat, Sep 03, 2016 at 10:45:42PM +1000, Allan McRae wrote:
On 30/08/16 22:48, Christian Hesse wrote:
Dave Reisner <d@falconindy.com> on Tue, 2016/08/30 08:46:
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse <mail@eworm.de>
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely).
To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective.
That is what my stupid-proxy patch does... Now it is up to Allan to decide. ;)
Crap... Why do I need to make decisions?
OK - lets think on the go here... The two options we "want" to support based on submitted patches are: 1) Disabling the low speed timeout 2) Setting maximum download speed
In the future, we might also add a parallel download option.
I'm guessing these want both global config values and command line options. Anyone want to suggest what these could be. I'll choose the colour of the bikeshed.
Allan
1) DetectDeadConnections 2) MaxDownloadKBps future) MaxConcurrentDownloads
For #2, I'll suggest that you can already do this with a program like netbrake. Curl's internal rate limiting is not good and only uses a flat average, rather than a rolling window of recent samples. It's
FWIW I've actually submitted a patch[1] recently to improve curl's limiting algo that should improve this. (Of course that'll only apply to users with a quite recent libcurl.) In my patch[2] (for pacman this time) I used MaxDlSpeed as name so one can specify a unit, whereas MaxDownloadKBps implies one (though likely the most common one I suppose). Using "Dl" instead of "Download" in the option name seemed like still pretty common/understandable, all the while making things a bit shorter, especially nice for the command line option. With similar intent, maybe a shorter one for #1 might be better... e.g. NoLowSpeedTimeout, or simply NoSlowTimeout ? (Also I guess yours should be NoDetectDeadConnections or similar, as the option is actually meant to *disable* the low speed timeout.)
idea of "limiting" bandwidth is inserting an artificial delay before the next recv call, which might just be draining a buffer of already received data from the kernel. This is particularly true if you're behind a router and not the actual gateway device (and I suspect this is the common case).
d
[1] https://github.com/curl/curl/pull/971 [2] https://lists.archlinux.org/pipermail/pacman-dev/2016-August/021265.html